From c0cf10d84c0d41b5c02af3a9f3268e438b8e1e77 Mon Sep 17 00:00:00 2001 From: Maxime Ollivier Date: Thu, 21 Jan 2021 19:55:59 -0800 Subject: [PATCH] ship IGListExperimentArrayAndSetOptimization Summary: This experiment shows a slight decrease in frame-drops on features that have more expensive hashing, so lets ship! Reviewed By: lorixx Differential Revision: D25884785 fbshipit-source-id: a5e33abe6f129166ab9b75de80636db801599b74 --- CHANGELOG.md | 2 ++ Source/IGListDiffKit/IGListExperiments.h | 8 +++---- Source/IGListKit/IGListAdapter.m | 24 ++++--------------- .../Internal/IGListUpdatedObjectContainer.h | 23 ------------------ .../Internal/IGListUpdatedObjectContainer.m | 14 ----------- 5 files changed, 9 insertions(+), 62 deletions(-) delete mode 100644 Source/IGListKit/Internal/IGListUpdatedObjectContainer.h delete mode 100644 Source/IGListKit/Internal/IGListUpdatedObjectContainer.m diff --git a/CHANGELOG.md b/CHANGELOG.md index e5a1c28c6..65ceb1bbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag - Improved performance by using `reloadData` when there are too many diffing updates. Shipped with experiment `IGListExperimentReloadDataFallback` from Ryan Nystrom. [Maxime Ollivier](https://github.com/maxolls) (tbd) +- Small performance improvement by replacing `NSSet` with `NSArray` during the data update to avoid unnecessary hashing, especially when dealing with lots of large objects with non trivial hashes. [Maxime Ollivier](https://github.com/maxolls) (tbd) + ### Fixes - `IGListCollectionViewLayout` should get the section/index counts via `UICollectionView` to stay in sync, instead of the `dataSource` [Maxime Ollivier](https://github.com/maxolls) (tbd) diff --git a/Source/IGListDiffKit/IGListExperiments.h b/Source/IGListDiffKit/IGListExperiments.h index d87c8f646..f25b83b69 100644 --- a/Source/IGListDiffKit/IGListExperiments.h +++ b/Source/IGListDiffKit/IGListExperiments.h @@ -20,14 +20,12 @@ typedef NS_OPTIONS (NSInteger, IGListExperiment) { IGListExperimentBackgroundDiffing = 1 << 2, /// Test invalidating layout when cell reloads/updates in IGListBindingSectionController. IGListExperimentInvalidateLayoutForUpdates = 1 << 3, - /// Test array and set optimization on update - IGListExperimentArrayAndSetOptimization = 1 << 4, /// Test validating the collectionView count just before performBatchUpdate. `IGListExperimentalAdapterUpdater` only. - IGListExperimentSectionCountValidation = 1 << 5, + IGListExperimentSectionCountValidation = 1 << 4, /// Test skipping performBatchUpdate if we don't have any updates. `IGListExperimentalAdapterUpdater` only. - IGListExperimentSkipPerformUpdateIfPossible = 1 << 6, + IGListExperimentSkipPerformUpdateIfPossible = 1 << 5, /// Test skipping creating {view : section controller} map, which has inconsistency issue. - IGListExperimentSkipViewSectionControllerMap = 1 << 7 + IGListExperimentSkipViewSectionControllerMap = 1 << 6 }; /** diff --git a/Source/IGListKit/IGListAdapter.m b/Source/IGListKit/IGListAdapter.m index f3ba9b8c1..3e8655120 100644 --- a/Source/IGListKit/IGListAdapter.m +++ b/Source/IGListKit/IGListAdapter.m @@ -15,7 +15,6 @@ #import "IGListDebugger.h" #import "IGListSectionControllerInternal.h" #import "IGListTransitionData.h" -#import "IGListUpdatedObjectContainer.h" #import "IGListUpdatingDelegateExperimental.h" #import "UICollectionViewLayout+InteractiveReordering.h" #import "UIScrollView+IGListKit.h" @@ -710,17 +709,8 @@ - (IGListTransitionData *)_generateTransitionDataWithObjects:(NSArray *)objects } #endif - NSMutableArray *sectionControllers; - NSMutableArray *validObjects; - - if (IGListExperimentEnabled(_experiments, IGListExperimentArrayAndSetOptimization)) { - // Experiment: Pass the capacity count, so that arrays don't have to re-size. - sectionControllers = [[NSMutableArray alloc] initWithCapacity:objects.count]; - validObjects = [[NSMutableArray alloc] initWithCapacity:objects.count]; - } else { - sectionControllers = [NSMutableArray new]; - validObjects = [NSMutableArray new]; - } + NSMutableArray *sectionControllers = [[NSMutableArray alloc] initWithCapacity:objects.count]; + NSMutableArray *validObjects = [[NSMutableArray alloc] initWithCapacity:objects.count]; // push the view controller and collection context into a local thread container so they are available on init // for IGListSectionController subclasses after calling [super init] @@ -776,14 +766,8 @@ - (void)_updateWithData:(IGListTransitionData *)data { IGListSectionMap *map = self.sectionMap; - id updatedObjects; - if (IGListExperimentEnabled(_experiments, IGListExperimentArrayAndSetOptimization)) { - // Experiment: Avoid using a set, so that we don't need to deal with hashes and equality. The updater - // should have dealt with duplicates already. - updatedObjects = [NSMutableArray new]; - } else { - updatedObjects = [NSMutableSet new]; - } + // Note: We use an array, instead of a set, because the updater should have dealt with duplicates already. + NSMutableArray *updatedObjects = [NSMutableArray new]; for (id object in data.toObjects) { // check if the item has changed instances or is new diff --git a/Source/IGListKit/Internal/IGListUpdatedObjectContainer.h b/Source/IGListKit/Internal/IGListUpdatedObjectContainer.h deleted file mode 100644 index 38e268fd7..000000000 --- a/Source/IGListKit/Internal/IGListUpdatedObjectContainer.h +++ /dev/null @@ -1,23 +0,0 @@ -/* -* Copyright (c) Facebook, Inc. and its affiliates. -* -* This source code is licensed under the MIT license found in the -* LICENSE file in the root directory of this source tree. -*/ - -#import - -NS_ASSUME_NONNULL_BEGIN - -/// Temporary protocol to make IGListExperimentArrayAndSetOptimization easier -@protocol IGListUpdatedObjectContainer -- (void)addObject:(id)object; -@end - -@interface NSMutableArray (IGListUpdatedObjectContainer) -@end - -@interface NSMutableSet (IGListUpdatedObjectContainer) -@end - -NS_ASSUME_NONNULL_END diff --git a/Source/IGListKit/Internal/IGListUpdatedObjectContainer.m b/Source/IGListKit/Internal/IGListUpdatedObjectContainer.m deleted file mode 100644 index afc67b5cb..000000000 --- a/Source/IGListKit/Internal/IGListUpdatedObjectContainer.m +++ /dev/null @@ -1,14 +0,0 @@ -/* -* Copyright (c) Facebook, Inc. and its affiliates. -* -* This source code is licensed under the MIT license found in the -* LICENSE file in the root directory of this source tree. -*/ - -#import "IGListUpdatedObjectContainer.h" - -@implementation NSMutableArray (IGListUpdatedObjectContainer) -@end - -@implementation NSMutableSet (IGListUpdatedObjectContainer) -@end