Conversation
📏 Size AnalysisTotal install size 9.4 MB | This change: ⬆️ +26.6 kB (+0.283%)🗂 See size breakdown
🔎 See the full size analysis (b126fe1) merging into develop (8a01f51)
|
Generated by 🚫 Danger |
nuno-vieira
left a comment
There was a problem hiding this comment.
LGTM overall 👍 Just added some minor comments
| import CoreData | ||
| import Foundation | ||
|
|
||
| class ListDatabaseObserverWrapper<Item, DTO: NSManagedObject> { |
There was a problem hiding this comment.
Since it looks like both ListDatabaseObserver's have the same API, instead of having the isBackground spread all over this wrapper, can't we export the API to a protocol, and set the ListDatabaseObserver implementation depending on the isBackground flag?
There was a problem hiding this comment.
I tried the protocol approach but because of the generics it was not possible to then maintain certain APIs
| /// When called, release the notification observers | ||
| private var releaseNotificationObservers: (() -> Void)? | ||
|
|
||
| private let queue = DispatchQueue.global() |
There was a problem hiding this comment.
Better to create a queue from scratch instead of using the global one
There was a problem hiding this comment.
Yes, and might be good to specify qos in this case.
martinmitrevski
left a comment
There was a problem hiding this comment.
As we've discussed, we need to see what happens with the diffing solution, as well as the SwiftUI SDK (there are some glitches there).
|
Closed in benefit of #2237 with uses the new messages list |

🔗 Issue Links
https://stream-io.atlassian.net/browse/CIS-1854
🎯 Goal
The goal of this PR is to investigate the viability of moving the mapping of DTOs to Model to a background thread, reducing the pressure we put in the view/main thread.
📝 Summary
This PR introduces a new ListDatabaseObserver, matching the same API for simplicity purposes. This new object, named BackgroundListDatabaseObserver does exactly the same as its predecessor, but in a background and asynchronous way, and only notifies about results when those are already mapped.
🛠 Implementation
As of now:
_isLazyMappingEnabledIn order for background mapping to work, we cannot use the diffing mechanism on the messages list (for now)
There are several deficiencies that were found during this process and should be addressed before iterating this POC even further.
All this implementation is guarded now behind the following mechanisms:
_isBackgroundMappingEnabledwhich enables the usage of BackgroundListDatabaseObserver_isLazyMappingEnabledwhen enabling_isBackgroundMappingEnabledWhen using DemoApp-StreamDevelopers, it uses Background Mapping by default
☑️ Contributor Checklist
🎁 Meme