improve performance when has data conflicts #84
Conversation
|
Sorry for the slow response on this one. I was camping over the weekend. Taking a look now. :D |
RocketData/DataHolder.swift
Outdated
| */ | ||
| mutating func setData(_ data: T, changeTime: ChangeTime) { | ||
| mutating func setData(_ data: T, changeTime: ChangeTime) -> Bool { |
There was a problem hiding this comment.
could you add @discardableResult here so the _ = isn't needed everywhere?
There was a problem hiding this comment.
yeah, i have finished this work，check it please。
| @@ -178,7 +178,7 @@ extension CollectionDataProvider: SharedCollection { | |||
| listenForUpdates(model: batchModel) | |||
|
|
|||
| if !isPaused { | |||
| dataHolder.setData(newData, changeTime: ChangeTime()) | |||
| _ = dataHolder.setData(newData, changeTime: ChangeTime()) | |||
There was a problem hiding this comment.
Remove these once you've added @discardableResult
|
This change looks great. Thanks a lot. It'd be awesome if you also wrote a test for the new performance optimization (which verified that the cache is not hit if you make a change which is out of date). But, if you're short on time, don't worry about it. |
Codecov Report
@@ Coverage Diff @@
## master #84 +/- ##
==========================================
- Coverage 93.81% 93.69% -0.13%
==========================================
Files 47 47
Lines 4784 4803 +19
==========================================
+ Hits 4488 4500 +12
- Misses 296 303 +7
Continue to review full report at Codecov.
|
|
I'm going to do the same to the Consistency Manager and then update cocoapods today |
|
Sadly, I can't update the consistency manager because of a cocoapods issue: CocoaPods/CocoaPods#8117 (unless I'm doing something wrong here). Going to wait on that issue to see what happens. |
|
🚀 RocketData (7.0.0) successfully published Sorry for the delay...ran into several issues with cocoapods and travis. But all fixed now. |
|
hi,
I am writing this email and would like to ask you two of the questions I found using this library.
The first one，collection data provider does not support data comparison when calling methods such as setData\insert\append… For example, collection data provider has an array of 10 elements. I call the setData method to set a new array of 5 elements, so it does not trigger the callback of the delegate method. Callbacks are only triggered when a member of the array changes. I think this does not meet the business scenarios of most apps. Calling tableview's reloadData method after setData is not an elegant approach like in demo.
The second one, the fetchDataFromCache method just reads the data and sets it to cdp. I think the function of this method needs to be extended. Take the instant messaging app as an example. It is very common to get the data from the cache and insert it into the front or the end of the existing data.
If you agree with me, I will send you a PR as soon as possible, but I am sorry that I don't have time to do enough testing on my code, I need your code review and testing.
… 在 2018年9月22日，上午4:18，Peter Livesey ***@***.***> 写道：
🚀 RocketData (7.0.0) successfully published
Sorry for the delay...ran into several issues with cocoapods and travis. But all fixed now.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#84 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AK9wwsKnaVcoWnjylPHTB8jg3U0WbSQtks5udUmBgaJpZM4WrM49>.
|
|
Regarding the first comment, this was a difficult API problem to solve when writing the library. Originally, the library actually behaved how you wanted it to, but it actually caused a lot of complaints in practice. People wanted a block API for updating from the cache. Originally, this just called the delegate method, but this was annoying because you'd lose context. So, we decided to only call the delegate method when the change came from another source. It also makes it very hard to write code like this: If you want, you could easily fix this for your own app by subclassing CollectionDataProvider and adding your own delegate callbacks. So something like: It's also too much of a breaking change to change the API for all users since most people are relying on the old behavior. For the second example, paging is a very complex thing to do with a cache. It tends to expire very quickly as rows are added or deleted. For instance, say you save page 1 and page 2 with items 0-9 and 10-19. Then you reload from the network and get a new set of items A-C and 0-6 (because we have 3 new items). Now, loading page 2 is incorrect because it'll return 10-19 and be incorrect! So, we advise that you cache all pages under one key. So, your network request may be paged, but your cache shouldn't be. Simply keep adding to the same collection provider. So:
Does that make sense? Maybe if you gave me some more information on exactly how your server pages data, I can be of more help. |
In this pr, i do 2 things. The first one, I adapte the swift4 syntax to eliminate the warnings. The second one is performance improvement. Performance improvement as follow, when called dataHolder.setData(data, changeTime: ChangeTime()) method, if method return directly because of the conflict resolution mechanism，and code logic will continue to write data to the cache. In this case, it is unnecessary.