Skip to content

Commit

Permalink
Concurrency Fixes (#283)
Browse files Browse the repository at this point in the history
* Review places where MainActor used

* Adapt to MainActor changes, avoid redundant task detachments

* Move Rust client operations into a dedicated concurrent queue, make init async

* Add changelog

* Fixed small working for awaiting non-async method

* Remove some redundant tasks

* Put back some main actors

* Fix tests

* Fixed timeline updates when the number of items doesn't change. Keeps more previous home screen room data between reloads

Co-authored-by: Stefan Ceriu <stefanc@matrix.org>
  • Loading branch information
ismailgulek and stefanceriu authored Nov 2, 2022
1 parent 965569a commit f6a599a
Show file tree
Hide file tree
Showing 19 changed files with 240 additions and 218 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,13 @@ class StateStoreViewModel<State: BindableState, ViewAction> {

init(initialViewState: State) {
context = Context(initialViewState: initialViewState)
context.viewActions.sink { [weak self] action in
guard let self else { return }

Task { await self.process(viewAction: action) }
}
.store(in: &cancellables)
context.viewActions
.sink { [weak self] action in
guard let self else { return }

Task { await self.process(viewAction: action) }
}
.store(in: &cancellables)
}

/// Override to handles incoming `ViewAction`s from the `ViewModel`.
Expand Down
81 changes: 42 additions & 39 deletions ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
private let userSession: UserSessionProtocol
private let roomSummaryProvider: RoomSummaryProviderProtocol
private let attributedStringBuilder: AttributedStringBuilderProtocol
private var roomsForIdentifiers = [String: HomeScreenRoom]()

var callback: ((HomeScreenViewModelAction) -> Void)?

Expand Down Expand Up @@ -72,9 +73,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
roomSummaryProvider.roomListPublisher
.receive(on: DispatchQueue.main)
.sink { [weak self] _ in
Task {
await self?.updateRooms()
}
self?.updateRooms()
}
.store(in: &cancellables)

Expand All @@ -91,10 +90,8 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
state.userDisplayName = userDisplayName
}
}

Task {
await updateRooms()
}

updateRooms()
}

// MARK: - Public
Expand All @@ -117,52 +114,58 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
// MARK: - Private

private func loadDataForRoomIdentifier(_ identifier: String) {
guard let summary = roomSummaryProvider.roomListPublisher.value.first(where: { $0.asFilled?.id == identifier })?.asFilled,
let homeRoomIndex = state.rooms.firstIndex(where: { $0.id == identifier }) else {
guard let roomSummary = roomSummaryProvider.roomListPublisher.value.first(where: { $0.asFilled?.id == identifier })?.asFilled,
let roomIndex = state.rooms.firstIndex(where: { $0.id == identifier }) else {
return
}

var details = state.rooms[homeRoomIndex]
var room = state.rooms[roomIndex]

guard details.avatar == nil,
let avatarURLString = summary.avatarURLString else {
guard room.avatar == nil,
let avatarURLString = roomSummary.avatarURLString else {
return
}

Task {
if case let .success(image) = await userSession.mediaProvider.loadImageFromURLString(avatarURLString, avatarSize: .room(on: .home)) {
details.avatar = image
state.rooms[homeRoomIndex] = details
room.avatar = image
state.rooms[roomIndex] = room
roomsForIdentifiers[roomSummary.id] = room
}
}
}

private func updateRooms() async {
state.rooms = await Task.detached {
var rooms = [HomeScreenRoom]()

for summary in self.roomSummaryProvider.roomListPublisher.value {
switch summary {
case .empty(let id):
rooms.append(HomeScreenRoom.placeholder(id: id))
case .filled(let summary):
let avatarImage = await self.userSession.mediaProvider.imageFromURLString(summary.avatarURLString, avatarSize: .room(on: .home))

var timestamp: String?
if let lastMessageTimestamp = summary.lastMessageTimestamp {
timestamp = lastMessageTimestamp.formatted(date: .omitted, time: .shortened)
}

rooms.append(HomeScreenRoom(id: summary.id,
name: summary.name,
hasUnreads: summary.unreadNotificationCount > 0,
timestamp: timestamp,
lastMessage: summary.lastMessage,
avatar: avatarImage))
private func updateRooms() {
var rooms = [HomeScreenRoom]()
var newRoomsForIdentifiers = [String: HomeScreenRoom]()

for summary in roomSummaryProvider.roomListPublisher.value {
switch summary {
case .empty(let id):
rooms.append(HomeScreenRoom.placeholder(id: id))
case .filled(let summary):
let oldRoom = roomsForIdentifiers[summary.id]

let avatarImage = userSession.mediaProvider.imageFromURLString(summary.avatarURLString, avatarSize: .room(on: .home))

var timestamp: String?
if let lastMessageTimestamp = summary.lastMessageTimestamp {
timestamp = lastMessageTimestamp.formatted(date: .omitted, time: .shortened)
}

let room = HomeScreenRoom(id: summary.id,
name: summary.name,
hasUnreads: summary.unreadNotificationCount > 0,
timestamp: timestamp ?? oldRoom?.timestamp,
lastMessage: summary.lastMessage ?? oldRoom?.lastMessage,
avatar: avatarImage ?? oldRoom?.avatar)

rooms.append(room)
newRoomsForIdentifiers[summary.id] = room
}

return rooms
}.value
}

state.rooms = rooms
roomsForIdentifiers = newRoomsForIdentifiers
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ struct TimelineItemList: View {
// Otherwise just update the items
timelineItems = context.viewState.items
}
.onChange(of: context.viewState.items, perform: { items in
if timelineItems != items {
timelineItems = items
}
})
.background(GeometryReader { geo in
Color.clear.preference(key: ViewFramePreferenceKey.self, value: [geo.frame(in: .global)])
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ enum AuthenticationServiceError: Error {
case failedLoggingIn
}

@MainActor
protocol AuthenticationServiceProxyProtocol {
var homeserver: LoginHomeserver { get }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import Foundation

@MainActor
protocol BackgroundTaskServiceProtocol {
func startBackgroundTask(withName name: String,
isReusable: Bool,
Expand Down
Loading

0 comments on commit f6a599a

Please sign in to comment.