Skip to content

Commit

Permalink
[ASCollectionNode] Add -isProcessingUpdates and -onDidFinishProcessin…
Browse files Browse the repository at this point in the history
…gUpdates: APIs.

Over time, there have actually been a lot of legitimate uses for an API like this.
In fact, I'm not quite sure what has held us back from adding one!

I believe that at least some portion of -wait calls (even if less than 50%) could be
replaced with -onDidFinishProcessingUpdates:, which could potentially improve the
performance of applications using -wait by a significant amount.

Please take a close look at implementation correctness. Although I'm in a bit of a
rush, I'm aiming to make this properly documented and added a basic test -- but it
could certainly use some more detailed testing as a followup.
  • Loading branch information
appleguy committed Aug 20, 2017
1 parent cae9517 commit 6d687a8
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## master

* Add your own contributions to the next release on the line below this with your name.
- [ASCollectionNode] Add -isProcessingUpdates and -onDidFinishProcessingUpdates: APIs. [#522](https://github.com/TextureGroup/Texture/pull/522) [Scott Goodson](https://github.com/appleguy)
- Fix an issue in layout transition that causes it to unexpectedly use the old layout [Huy Nguyen](https://github.com/nguyenhuy) [#464](https://github.com/TextureGroup/Texture/pull/464)
- Add -[ASDisplayNode detailedLayoutDescription] property to aid debugging. [Adlai Holler](https://github.com/Adlai-Holler) [#476](https://github.com/TextureGroup/Texture/pull/476)
- Fix an issue that causes calculatedLayoutDidChange being called needlessly. [Huy Nguyen](https://github.com/nguyenhuy) [#490](https://github.com/TextureGroup/Texture/pull/490)
Expand Down
39 changes: 36 additions & 3 deletions Source/ASCollectionNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,36 @@ NS_ASSUME_NONNULL_BEGIN
/**
* Blocks execution of the main thread until all section and item updates are committed to the view. This method must be called from the main thread.
*/
- (void)waitUntilAllUpdatesAreCommitted;
- (void)waitUntilAllUpdatesAreProcessed;

/**
* Returns YES if the ASCollectionNode is still processing changes from performBatchUpdates:.
* This is typically the concurrent allocation (calling nodeBlocks) and layout of newly inserted
* ASCellNodes. If YES is returned, then calling -waitUntilAllUpdatesAreProcessed may take tens of
* milliseconds to return as it blocks on these concurrent operations.
*
* Returns NO if ASCollectionNode is fully synchronized with the underlying UICollectionView. This
* means that until the next performBatchUpdates: is called, it is safe to compare UIKit values
* (such as from UICollectionViewLayout) with your app's data source.
*
* This method will always return NO if called immediately after -waitUntilAllUpdatesAreProcessed.
*/
- (BOOL)isProcessingUpdates;

/**
* Schedules a block to be performed (on the main thread) after processing of performBatchUpdates:
* is finished (completely synchronized to UIKit). The blocks will be run at the moment that
* -isProcessingUpdates changes from YES to NO;
*
* When isProcessingUpdates == NO, the block is run block immediately (before the method returns).
*
* Blocks scheduled by this mechanism are NOT guaranteed to run in the order they are scheduled.
* They may also be delayed if performBatchUpdates continues to be called; the blocks will wait until
* all running updates are finished.
*
* Calling -waitUntilAllUpdatesAreProcessed is one way to flush any pending update completion blocks.
*/
- (void)onDidFinishProcessingUpdates:(nullable void (^)())didFinishProcessingUpdates;

/**
* Inserts one or more sections.
Expand Down Expand Up @@ -501,9 +530,13 @@ NS_ASSUME_NONNULL_BEGIN
* @warning This method is substantially more expensive than UICollectionView's version.
*
* @deprecated This method is deprecated in 2.0. Use @c reloadDataWithCompletion: and
* then @c waitUntilAllUpdatesAreCommitted instead.
* then @c waitUntilAllUpdatesAreProcessed instead.
*/
- (void)reloadDataImmediately ASDISPLAYNODE_DEPRECATED_MSG("Use -reloadData / -reloadDataWithCompletion: followed by -waitUntilAllUpdatesAreCommitted instead.");
- (void)reloadDataImmediately ASDISPLAYNODE_DEPRECATED_MSG("Use -reloadData / -reloadDataWithCompletion: followed by -waitUntilAllUpdatesAreProcessed instead.");

// TODO: Rename framework uses of this method and add deprecation message.
// ASDISPLAYNODE_DEPRECATED_MSG("This method has been renamed to -waitUntilAllUpdatesAreProcessed.");
- (void)waitUntilAllUpdatesAreCommitted;

@end

Expand Down
19 changes: 19 additions & 0 deletions Source/ASCollectionNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,25 @@ - (void)waitUntilAllUpdatesAreCommitted
}
}

- (void)waitUntilAllUpdatesAreProcessed
{
[self waitUntilAllUpdatesAreCommitted];
}

- (BOOL)isProcessingUpdates
{
return (self.nodeLoaded ? [self.view isProcessingUpdates] : NO);
}

- (void)onDidFinishProcessingUpdates:(nullable void (^)())completion
{
if (!self.nodeLoaded) {
completion();
} else {
[self.view onDidFinishProcessingUpdates:completion];
}
}

- (void)reloadDataWithCompletion:(void (^)())completion
{
ASDisplayNodeAssertMainThread();
Expand Down
4 changes: 3 additions & 1 deletion Source/ASCollectionView.h
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,11 @@ NS_ASSUME_NONNULL_BEGIN
- (void)relayoutItems ASDISPLAYNODE_DEPRECATED_MSG("Use ASCollectionNode method instead.");

/**
* Blocks execution of the main thread until all section and row updates are committed. This method must be called from the main thread.
* See ASCollectionNode.h for full documentation of these methods.
*/
- (void)waitUntilAllUpdatesAreCommitted ASDISPLAYNODE_DEPRECATED_MSG("Use ASCollectionNode method instead.");
- (BOOL)isProcessingUpdates;
- (void)onDidFinishProcessingUpdates:(nullable void (^)())completion;

/**
* Registers the given kind of supplementary node for use in creating node-backed supplementary views.
Expand Down
10 changes: 10 additions & 0 deletions Source/ASCollectionView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,16 @@ - (void)waitUntilAllUpdatesAreCommitted
[_dataController waitUntilAllUpdatesAreCommitted];
}

- (BOOL)isProcessingUpdates
{
return [_dataController isProcessingUpdates];
}

- (void)onDidFinishProcessingUpdates:(nullable void (^)())completion
{
[_dataController onDidFinishProcessingUpdates:completion];
}

- (void)setDataSource:(id<UICollectionViewDataSource>)dataSource
{
// UIKit can internally generate a call to this method upon changing the asyncDataSource; only assert for non-nil. We also allow this when we're doing interop.
Expand Down
5 changes: 5 additions & 0 deletions Source/Details/ASDataController.h
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,12 @@ extern NSString * const ASCollectionInvalidUpdateException;
*/
- (void)relayoutNodes:(id<NSFastEnumeration>)nodes nodesSizeChanged:(NSMutableArray * _Nonnull)nodesSizesChanged;

/**
* See ASCollectionNode.h for full documentation of these methods.
*/
- (void)waitUntilAllUpdatesAreCommitted;
- (BOOL)isProcessingUpdates;
- (void)onDidFinishProcessingUpdates:(nullable void (^)())completion;

/**
* Notifies the data controller object that its environment has changed. The object will request its environment delegate for new information
Expand Down
23 changes: 23 additions & 0 deletions Source/Details/ASDataController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,29 @@ - (void)waitUntilAllUpdatesAreCommitted
[self _scheduleBlockOnMainSerialQueue:^{ }];
}

- (BOOL)isProcessingUpdates
{
ASDisplayNodeAssertMainThread();
long waitResult = dispatch_group_wait(_editingTransactionGroup, DISPATCH_TIME_NOW);
return (waitResult == 0 ? NO : YES);
}

- (void)onDidFinishProcessingUpdates:(nullable void (^)())completion
{
ASDisplayNodeAssertMainThread();
if ([self isProcessingUpdates] == NO) {
ASPerformBlockOnMainThread(completion);
} else {
dispatch_async(_editingTransactionQueue, ^{
// Retry the block. If we're done processing updates, it'll run immediately, otherwise
// wait again for updates to quiesce completely.
dispatch_async(dispatch_get_main_queue(), ^{
[self onDidFinishProcessingUpdates:completion];
});
});
}
}

- (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet
{
ASDisplayNodeAssertMainThread();
Expand Down
9 changes: 9 additions & 0 deletions Tests/ASCollectionViewTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1050,8 +1050,17 @@ - (void)testInitialRangeBounds
// Trigger the initial reload to start
[window layoutIfNeeded];

// Test the APIs that monitor ASCollectionNode update handling
XCTAssertTrue(cn.isProcessingUpdates, @"ASCollectionNode should still be processing updates after initial layoutIfNeeded call (reloadData)");
[cn onDidFinishProcessingUpdates:^{
XCTAssertTrue(!cn.isProcessingUpdates, @"ASCollectionNode should no longer be processing updates inside -onDidFinishProcessingUpdates: block");
}];

// Wait for ASDK reload to finish
[cn waitUntilAllUpdatesAreCommitted];

XCTAssertTrue(!cn.isProcessingUpdates, @"ASCollectionNode should no longer be processing updates after -wait call");

// Force UIKit to read updated data & range controller to update and account for it
[cn.view layoutIfNeeded];

Expand Down

0 comments on commit 6d687a8

Please sign in to comment.