Skip to content

Commit

Permalink
Add experiment to defer requesting objects from data source until jus…
Browse files Browse the repository at this point in the history
…t before diffing

Summary:
Inspired by recent performance investigations in Instagram, adding an experimental feature to defer requesting objects from the `IGListAdapterDataSource` until //just before// diffing is executed. If //n// updates are coalesced into one, this results in just a single request for objects from the data source.

This is a **breaking change** to the public API, but since writing custom `IGListUpdatingDelegate`s is discouraged, I'm less worried about saving this for a major version launch.

Reviewed By: manicakes

Differential Revision: D7744518

fbshipit-source-id: f0001263cddda383e3dedd1d350a83d2cfb8362a
  • Loading branch information
Ryan Nystrom authored and facebook-github-bot committed Apr 26, 2018
1 parent c189462 commit 3059c5e
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 89 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag
4.0.0 (upcoming release)
-----

### Enhancements

- Experimental performance improvement from deferring `-[IGListAdapterDataSource objectsForListAdapter:]` calls until just before diffing. [Ryan Nystrom](https://github.com/rnystrom) (tbd)

3.3.0
-----

Expand Down
2 changes: 2 additions & 0 deletions Source/Common/IGListExperiments.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ typedef NS_OPTIONS (NSInteger, IGListExperiment) {
IGListExperimentFasterVisibleSectionController = 1 << 4,
/// Test deduping item-level updates.
IGListExperimentDedupeItemUpdates = 1 << 5,
/// Test deferring object creation until just before diffing.
IGListExperimentDeferredToObjectCreation = 1 << 6,
};

/**
Expand Down
22 changes: 18 additions & 4 deletions Source/IGListAdapter.m
Original file line number Diff line number Diff line change
Expand Up @@ -324,14 +324,28 @@ - (void)performUpdatesAnimated:(BOOL)animated completion:(IGListUpdaterCompletio
}

NSArray *fromObjects = self.sectionMap.objects;
NSArray *newObjects = [dataSource objectsForListAdapter:self];

[self _enterBatchUpdates];

IGListToObjectBlock toObjectsBlock;
__weak __typeof__(self) weakSelf = self;
if (IGListExperimentEnabled(self.experiments, IGListExperimentDeferredToObjectCreation)) {
toObjectsBlock = ^NSArray *{
__typeof__(self) strongSelf = weakSelf;
if (strongSelf == nil) {
return nil;
}
return [dataSource objectsForListAdapter:strongSelf];
};
} else {
NSArray *newObjects = [dataSource objectsForListAdapter:self];
toObjectsBlock = ^NSArray *{
return newObjects;
};
}

[self _enterBatchUpdates];
[self.updater performUpdateWithCollectionView:collectionView
fromObjects:fromObjects
toObjects:newObjects
toObjectsBlock:toObjectsBlock
animated:animated
objectTransitionBlock:^(NSArray *toObjects) {
// temporarily capture the item map that we are transitioning from in case
Expand Down
38 changes: 21 additions & 17 deletions Source/IGListAdapterUpdater.m
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ - (BOOL)hasChanges {
return self.hasQueuedReloadData
|| [self.batchUpdates hasChanges]
|| self.fromObjects != nil
|| self.toObjects != nil;
|| self.toObjectsBlock != nil;
}

- (void)performReloadDataWithCollectionView:(UICollectionView *)collectionView {
Expand Down Expand Up @@ -106,12 +106,25 @@ - (void)performBatchUpdatesWithCollectionView:(UICollectionView *)collectionView
// create local variables so we can immediately clean our state but pass these items into the batch update block
id<IGListAdapterUpdaterDelegate> delegate = self.delegate;
NSArray *fromObjects = [self.fromObjects copy];
NSArray *toObjects = objectsWithDuplicateIdentifiersRemoved(self.toObjects);
IGListToObjectBlock toObjectsBlock = [self.toObjectsBlock copy];
NSMutableArray *completionBlocks = [self.completionBlocks mutableCopy];
void (^objectTransitionBlock)(NSArray *) = [self.objectTransitionBlock copy];
const BOOL animated = self.queuedUpdateIsAnimated;
IGListBatchUpdates *batchUpdates = self.batchUpdates;

NSArray *toObjects = nil;
if (toObjectsBlock != nil) {
toObjects = objectsWithDuplicateIdentifiersRemoved(toObjectsBlock());
}
#ifdef DEBUG
for (id obj in toObjects) {
IGAssert([obj conformsToProtocol:@protocol(IGListDiffable)],
@"In order to use IGListAdapterUpdater, object %@ must conform to IGListDiffable", obj);
IGAssert([obj diffIdentifier] != nil,
@"Cannot have a nil diffIdentifier for object %@", obj);
}
#endif

// clean up all state so that new updates can be coalesced while the current update is in flight
[self cleanStateBeforeUpdates];

Expand Down Expand Up @@ -340,7 +353,7 @@ - (void)cleanStateBeforeUpdates {

// destroy to/from transition items
self.fromObjects = nil;
self.toObjects = nil;
self.toObjectsBlock = nil;

// destroy reloadData state
self.reloadUpdates = nil;
Expand Down Expand Up @@ -408,11 +421,11 @@ - (NSPointerFunctions *)objectLookupPointerFunctions {
}

- (void)performUpdateWithCollectionView:(UICollectionView *)collectionView
fromObjects:(nullable NSArray *)fromObjects
toObjects:(nullable NSArray *)toObjects
fromObjects:(NSArray *)fromObjects
toObjectsBlock:(IGListToObjectBlock)toObjectsBlock
animated:(BOOL)animated
objectTransitionBlock:(void (^)(NSArray *))objectTransitionBlock
completion:(nullable void (^)(BOOL))completion {
objectTransitionBlock:(IGListObjectTransitionBlock)objectTransitionBlock
completion:(IGListUpdatingCompletion)completion {
IGAssertMainThread();
IGParameterAssert(collectionView != nil);
IGParameterAssert(objectTransitionBlock != nil);
Expand All @@ -423,21 +436,12 @@ - (void)performUpdateWithCollectionView:(UICollectionView *)collectionView
// if performBatchUpdates: hasn't applied the update block, then data source hasn't transitioned its state. if an
// update is queued in between then we must use the pending toObjects
self.fromObjects = self.fromObjects ?: self.pendingTransitionToObjects ?: fromObjects;
self.toObjects = toObjects;
self.toObjectsBlock = toObjectsBlock;

// disabled animations will always take priority
// reset to YES in -cleanupState
self.queuedUpdateIsAnimated = self.queuedUpdateIsAnimated && animated;

#ifdef DEBUG
for (id obj in toObjects) {
IGAssert([obj conformsToProtocol:@protocol(IGListDiffable)],
@"In order to use IGListAdapterUpdater, object %@ must conform to IGListDiffable", obj);
IGAssert([obj diffIdentifier] != nil,
@"Cannot have a nil diffIdentifier for object %@", obj);
}
#endif

// always use the last update block, even though this should always do the exact same thing
self.objectTransitionBlock = objectTransitionBlock;

Expand Down
7 changes: 5 additions & 2 deletions Source/IGListReloadDataUpdater.m
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ - (NSPointerFunctions *)objectLookupPointerFunctions {

- (void)performUpdateWithCollectionView:(UICollectionView *)collectionView
fromObjects:(NSArray *)fromObjects
toObjects:(NSArray *)toObjects
toObjectsBlock:(IGListToObjectBlock)toObjectsBlock
animated:(BOOL)animated
objectTransitionBlock:(IGListObjectTransitionBlock)objectTransitionBlock
completion:(IGListUpdatingCompletion)completion {
objectTransitionBlock(toObjects);
if (toObjectsBlock != nil) {
NSArray *toObjects = toObjectsBlock() ?: @[];
objectTransitionBlock(toObjects);
}
[self _synchronousReloadDataWithCollectionView:collectionView];
if (completion) {
completion(YES);
Expand Down
8 changes: 6 additions & 2 deletions Source/IGListUpdatingDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ typedef void (^IGListItemUpdateBlock)(void);
NS_SWIFT_NAME(ListReloadUpdateBlock)
typedef void (^IGListReloadUpdateBlock)(void);

/// A block that returns an array of objects to transition to.
NS_SWIFT_NAME(ListToObjectBlock)
typedef NSArray * _Nullable (^IGListToObjectBlock)(void);

/**
Implement this protocol in order to handle both section and row based update events. Implementation should forward or
coalesce these events to a backing store or collection.
Expand All @@ -63,7 +67,7 @@ NS_SWIFT_NAME(ListUpdatingDelegate)
@param collectionView The collection view to perform the transition on.
@param fromObjects The previous objects in the collection view. Objects must conform to `IGListDiffable`.
@param toObjects The new objects in collection view. Objects must conform to `IGListDiffable`.
@param toObjectsBlock A block returning the new objects in the collection view. Objects must conform to `IGListDiffable`.
@param animated A flag indicating if the transition should be animated.
@param objectTransitionBlock A block that must be called when the adapter applies changes to the collection view.
@param completion A completion block to execute when the update is finished.
Expand All @@ -77,7 +81,7 @@ NS_SWIFT_NAME(ListUpdatingDelegate)
*/
- (void)performUpdateWithCollectionView:(UICollectionView *)collectionView
fromObjects:(nullable NSArray<id <IGListDiffable>> *)fromObjects
toObjects:(nullable NSArray<id <IGListDiffable>> *)toObjects
toObjectsBlock:(nullable IGListToObjectBlock)toObjectsBlock
animated:(BOOL)animated
objectTransitionBlock:(IGListObjectTransitionBlock)objectTransitionBlock
completion:(nullable IGListUpdatingCompletion)completion;
Expand Down
4 changes: 2 additions & 2 deletions Source/Internal/IGListAdapterUpdater+DebugDescription.m
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ @implementation IGListAdapterUpdater (DebugDescription)
[debug addObjectsFromArray:IGListDebugIndentedLines(linesFromObjects(self.fromObjects))];
}

if (self.toObjects != nil) {
if (self.toObjectsBlock != nil) {
[debug addObject:@"To objects:"];
[debug addObjectsFromArray:IGListDebugIndentedLines(linesFromObjects(self.toObjects))];
[debug addObjectsFromArray:IGListDebugIndentedLines(linesFromObjects(self.toObjectsBlock()))];
}

if (self.pendingTransitionToObjects != nil) {
Expand Down
2 changes: 1 addition & 1 deletion Source/Internal/IGListAdapterUpdaterInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ FOUNDATION_EXTERN void convertReloadToDeleteInsert(NSMutableIndexSet *reloads,
@interface IGListAdapterUpdater ()

@property (nonatomic, copy, nullable) NSArray *fromObjects;
@property (nonatomic, copy, nullable) NSArray *toObjects;
@property (nonatomic, copy, nullable) IGListToObjectBlock toObjectsBlock;
@property (nonatomic, copy, nullable) NSArray *pendingTransitionToObjects;
@property (nonatomic, strong) NSMutableArray<IGListUpdatingCompletion> *completionBlocks;

Expand Down
Loading

0 comments on commit 3059c5e

Please sign in to comment.