From 2f76e8ce684bf7cea75ee52f25d4ea0af3e0081b Mon Sep 17 00:00:00 2001 From: Zhisheng Huang Date: Fri, 8 Mar 2019 09:55:33 -0800 Subject: [PATCH] Cleanup the coalescing time experiment Summary: We have observed that keeping the coalescing time here would actually decrease the stability here, it makes the app crash more often. It's probably caused by some race condition during that time and UICV updates become out of sync. Reviewed By: rnystrom Differential Revision: D14379265 fbshipit-source-id: 502c1c14fb8bdfc35969f721687e82888b160110 --- CHANGELOG.md | 2 ++ Source/IGListAdapterUpdater.h | 5 ----- Source/IGListAdapterUpdater.m | 12 ++++++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eefa824..b5c81ab1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag ----- ### Breaking Changes +- Remove `coalescanceTime` from IGListAdapterUpdate, since it increase crash rate. [Zhisheng Huang](https://github.com/lorixx) (tbd) + - All `IGListBindingSectionControllerSelectionDelegate` methods are now required. [Bofei Zhu] (https://github.com/zhubofei) [(#1186)](https://github.com/Instagram/IGListKit/pull/1186) - Renamed `[IGListAdapterUpdatingDelegate listAdapterUpdater:willPerformBatchUpdatesWithCollectionView:]` to `[IGListAdapterUpdatingDelegate listAdapterUpdater:willPerformBatchUpdatesWithCollectionView:fromObjects:toObjects:listIndexSetResult:]` to include more supporting info on updated objects. [Jeremy Cohen] (https://github.com/jeremycohen) (tbd) diff --git a/Source/IGListAdapterUpdater.h b/Source/IGListAdapterUpdater.h index 2d7215dd..234e6257 100644 --- a/Source/IGListAdapterUpdater.h +++ b/Source/IGListAdapterUpdater.h @@ -58,11 +58,6 @@ NS_SWIFT_NAME(ListAdapterUpdater) */ @property (nonatomic, assign) BOOL allowsBackgroundReloading; -/** - Time, in seconds, to wait and coalesce batch updates. Default is 0. - */ -@property (nonatomic, assign) NSTimeInterval coalescanceTime; - /** A bitmask of experiments to conduct on the updater. */ diff --git a/Source/IGListAdapterUpdater.m b/Source/IGListAdapterUpdater.m index 066aff4e..21a8b3f8 100644 --- a/Source/IGListAdapterUpdater.m +++ b/Source/IGListAdapterUpdater.m @@ -427,15 +427,19 @@ static NSArray *convertSectionReloadToItemUpdates(NSIndexSet *sec - (void)_queueUpdateWithCollectionViewBlock:(IGListCollectionViewBlock)collectionViewBlock { IGAssertMainThread(); + __weak __typeof__(self) weakSelf = self; - - // dispatch after a given amount of time to coalesce other updates and execute as one - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, self.coalescanceTime * NSEC_PER_SEC), dispatch_get_main_queue(), ^{ + + // dispatch_async to give the main queue time to collect more batch updates so that a minimum amount of work + // (diffing, etc) is done on main. dispatch_async does not garauntee a full runloop turn will pass though. + // see -performUpdateWithCollectionView:fromObjects:toObjects:animated:objectTransitionBlock:completion: for more + // details on how coalescence is done. + dispatch_async(dispatch_get_main_queue(), ^{ if (weakSelf.state != IGListBatchUpdateStateIdle || ![weakSelf hasChanges]) { return; } - + if (weakSelf.hasQueuedReloadData) { [weakSelf performReloadDataWithCollectionViewBlock:collectionViewBlock]; } else {