[ASCollectionNode] Add -isProcessingUpdates and -onDidFinishProcessin…

…gUpdates: APIs. (#522) * [ASCollectionNode] Add -isProcessingUpdates and -onDidFinishProcessingUpdates: 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. * [ASCollectionNode] Improvements to the implementation of -isProcessingUpdates and -onDidFinishProcessingUpdates: * Add lock to ASMainSerialQueue count method. * [ASTableNode] Implement -isProcessingUpdates and -onDidFinishProcessingUpdates:. Rename -waitUntil to consistent naming.
TextureGroup · Aug 23, 2017 · ccc5786 · ccc5786
1 parent 16ce3c9
commit ccc5786
Show file tree

Hide file tree

Showing 16 changed files with 220 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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)
 - [Accessibility] Add .isAccessibilityContainer property, allowing automatic aggregation of children's a11y labels. [#468][Scott Goodson](https://github.com/appleguy)
 - [ASImageNode] Enabled .clipsToBounds by default, fixing the use of .cornerRadius and clipping of GIFs. [Scott Goodson](https://github.com/appleguy) [#466](https://github.com/TextureGroup/Texture/pull/466)
 - 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)

diff --git a/Source/ASCollectionNode.h b/Source/ASCollectionNode.h
@@ -252,10 +252,39 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (void)performBatchUpdates:(nullable AS_NOESCAPE void (^)())updates completion:(nullable void (^)(BOOL finished))completion;
 
+/**
+ *  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.
+ */
+@property (nonatomic, readonly) 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;
+
 /**
  *  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;
 
 /**
  * Inserts one or more sections.
@@ -501,9 +530,11 @@ 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.");
+
+- (void)waitUntilAllUpdatesAreCommitted ASDISPLAYNODE_DEPRECATED_MSG("This method has been renamed to -waitUntilAllUpdatesAreProcessed.");
 
 @end
 

diff --git a/Source/ASCollectionNode.mm b/Source/ASCollectionNode.mm
@@ -705,14 +705,33 @@ - (void)performBatchUpdates:(void (^)())updates completion:(void (^)(BOOL))compl
   [self performBatchAnimated:UIView.areAnimationsEnabled updates:updates completion:completion];
 }
 
-- (void)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)waitUntilAllUpdatesAreProcessed
 {
   ASDisplayNodeAssertMainThread();
   if (self.nodeLoaded) {
     [self.view waitUntilAllUpdatesAreCommitted];
   }
 }
 
+- (void)waitUntilAllUpdatesAreCommitted
+{
+  [self waitUntilAllUpdatesAreProcessed];
+}
+
 - (void)reloadDataWithCompletion:(void (^)())completion
 {
   ASDisplayNodeAssertMainThread();
@@ -738,7 +757,7 @@ - (void)reloadDataImmediately
 {
   ASDisplayNodeAssertMainThread();
   [self reloadData];
-  [self waitUntilAllUpdatesAreCommitted];
+  [self waitUntilAllUpdatesAreProcessed];
 }
 
 - (void)relayoutItems

diff --git a/Source/ASCollectionView.h b/Source/ASCollectionView.h
@@ -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.");
+@property (nonatomic, readonly) BOOL isProcessingUpdates;
+- (void)onDidFinishProcessingUpdates:(nullable void (^)())completion;
+- (void)waitUntilAllUpdatesAreCommitted ASDISPLAYNODE_DEPRECATED_MSG("Use -[ASCollectionNode waitUntilAllUpdatesAreProcessed] instead.");
 
 /**
  * Registers the given kind of supplementary node for use in creating node-backed supplementary views.

diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm
@@ -359,6 +359,16 @@ - (void)relayoutItems
   [_dataController relayoutAllNodes];
 }
 
+- (BOOL)isProcessingUpdates
+{
+  return [_dataController isProcessingUpdates];
+}
+
+- (void)onDidFinishProcessingUpdates:(nullable void (^)())completion
+{
+  [_dataController onDidFinishProcessingUpdates:completion];
+}
+
 - (void)waitUntilAllUpdatesAreCommitted
 {
   ASDisplayNodeAssertMainThread();
@@ -367,8 +377,8 @@ - (void)waitUntilAllUpdatesAreCommitted
     //    ASDisplayNodeFailAssert(@"Should not call %@ during batch update", NSStringFromSelector(_cmd));
     return;
   }
-  
-  [_dataController waitUntilAllUpdatesAreCommitted];
+
+  [_dataController waitUntilAllUpdatesAreProcessed];
 }
 
 - (void)setDataSource:(id<UICollectionViewDataSource>)dataSource
@@ -2193,7 +2203,7 @@ - (void)layer:(CALayer *)layer didChangeBoundsWithOldValue:(CGRect)oldBounds new
 
   if (changedInNonScrollingDirection) {
     [_dataController relayoutAllNodes];
-    [_dataController waitUntilAllUpdatesAreCommitted];
+    [_dataController waitUntilAllUpdatesAreProcessed];
     // We need to ensure the size requery is done before we update our layout.
     [self.collectionViewLayout invalidateLayout];
   }

diff --git a/Source/ASTableNode.h b/Source/ASTableNode.h
@@ -207,9 +207,38 @@ NS_ASSUME_NONNULL_BEGIN
 - (void)performBatchUpdates:(nullable AS_NOESCAPE void (^)())updates completion:(nullable void (^)(BOOL finished))completion;
 
 /**
- *  Blocks execution of the main thread until all section and row updates are committed. This method must be called from the main thread.
+ *  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.
+ */
+@property (nonatomic, readonly) 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)waitUntilAllUpdatesAreCommitted;
+- (void)onDidFinishProcessingUpdates:(nullable void (^)())didFinishProcessingUpdates;
+
+/**
+ *  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)waitUntilAllUpdatesAreProcessed;
 
 /**
  * Inserts one or more sections, with an option to animate the insertion.
@@ -699,4 +728,10 @@ NS_ASSUME_NONNULL_BEGIN
 
 @end
 
+@interface ASTableNode (Deprecated)
+
+- (void)waitUntilAllUpdatesAreCommitted ASDISPLAYNODE_DEPRECATED_MSG("This method has been renamed to -waitUntilAllUpdatesAreProcessed.");
+
+@end
+
 NS_ASSUME_NONNULL_END
diff --git a/Source/ASTableNode.mm b/Source/ASTableNode.mm
@@ -733,14 +733,33 @@ - (void)moveRowAtIndexPath:(NSIndexPath *)indexPath toIndexPath:(NSIndexPath *)n
   }
 }
 
-- (void)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)waitUntilAllUpdatesAreProcessed
 {
   ASDisplayNodeAssertMainThread();
   if (self.nodeLoaded) {
     [self.view waitUntilAllUpdatesAreCommitted];
   }
 }
 
+- (void)waitUntilAllUpdatesAreCommitted
+{
+  [self waitUntilAllUpdatesAreProcessed];
+}
+
 #pragma mark - Debugging (Private)
 
 - (NSMutableArray<NSDictionary *> *)propertiesForDebugDescription

diff --git a/Source/ASTableView.h b/Source/ASTableView.h
@@ -219,9 +219,11 @@ NS_ASSUME_NONNULL_BEGIN
 - (void)endUpdatesAnimated:(BOOL)animated completion:(void (^ _Nullable)(BOOL completed))completion ASDISPLAYNODE_DEPRECATED_MSG("Use ASTableNode's -performBatchUpdates:completion: 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 ASTableNode.h for full documentation of these methods.
  */
-- (void)waitUntilAllUpdatesAreCommitted ASDISPLAYNODE_DEPRECATED_MSG("Use ASTableNode method instead.");
+@property (nonatomic, readonly) BOOL isProcessingUpdates;
+- (void)onDidFinishProcessingUpdates:(nullable void (^)())completion;
+- (void)waitUntilAllUpdatesAreCommitted ASDISPLAYNODE_DEPRECATED_MSG("Use -[ASTableNode waitUntilAllUpdatesAreProcessed] instead.");
 
 - (void)insertSections:(NSIndexSet *)sections withRowAnimation:(UITableViewRowAnimation)animation ASDISPLAYNODE_DEPRECATED_MSG("Use ASTableNode method instead.");
 

diff --git a/Source/ASTableView.mm b/Source/ASTableView.mm
@@ -548,7 +548,7 @@ - (void)reloadDataImmediately
 {
   ASDisplayNodeAssertMainThread();
   [self reloadData];
-  [_dataController waitUntilAllUpdatesAreCommitted];
+  [_dataController waitUntilAllUpdatesAreProcessed];
 }
 
 - (void)scrollToRowAtIndexPath:(NSIndexPath *)indexPath atScrollPosition:(UITableViewScrollPosition)scrollPosition animated:(BOOL)animated
@@ -735,6 +735,16 @@ - (void)endUpdatesAnimated:(BOOL)animated completion:(void (^)(BOOL completed))c
   } 
 }
 
+- (BOOL)isProcessingUpdates
+{
+  return [_dataController isProcessingUpdates];
+}
+
+- (void)onDidFinishProcessingUpdates:(nullable void (^)())completion
+{
+  [_dataController onDidFinishProcessingUpdates:completion];
+}
+
 - (void)waitUntilAllUpdatesAreCommitted
 {
   ASDisplayNodeAssertMainThread();
@@ -743,8 +753,8 @@ - (void)waitUntilAllUpdatesAreCommitted
     //    ASDisplayNodeFailAssert(@"Should not call %@ during batch update", NSStringFromSelector(_cmd));
     return;
   }
-  
-  [_dataController waitUntilAllUpdatesAreCommitted];
+
+  [_dataController waitUntilAllUpdatesAreProcessed];
 }
 
 - (void)layoutSubviews

diff --git a/Source/Details/ASDataController.h b/Source/Details/ASDataController.h
@@ -254,7 +254,12 @@ extern NSString * const ASCollectionInvalidUpdateException;
  */
 - (void)relayoutNodes:(id<NSFastEnumeration>)nodes nodesSizeChanged:(NSMutableArray * _Nonnull)nodesSizesChanged;
 
-- (void)waitUntilAllUpdatesAreCommitted;
+/**
+ * See ASCollectionNode.h for full documentation of these methods.
+ */
+@property (nonatomic, readonly) BOOL isProcessingUpdates;
+- (void)onDidFinishProcessingUpdates:(nullable void (^)())completion;
+- (void)waitUntilAllUpdatesAreProcessed;
 
 /**
  * Notifies the data controller object that its environment has changed. The object will request its environment delegate for new information

diff --git a/Source/Details/ASDataController.mm b/Source/Details/ASDataController.mm
@@ -430,13 +430,42 @@ - (ASSizeRange)constrainedSizeForNodeOfKind:(NSString *)kind atIndexPath:(NSInde
 
 #pragma mark - Batching (External API)
 
-- (void)waitUntilAllUpdatesAreCommitted
+- (void)waitUntilAllUpdatesAreProcessed
 {
   // Schedule block in main serial queue to wait until all operations are finished that are
   // where scheduled while waiting for the _editingTransactionQueue to finish
   [self _scheduleBlockOnMainSerialQueue:^{ }];
 }
 
+- (BOOL)isProcessingUpdates
+{
+  ASDisplayNodeAssertMainThread();
+  if (_mainSerialQueue.numberOfScheduledBlocks > 0) {
+    return YES;
+  } else if (dispatch_group_wait(_editingTransactionGroup, DISPATCH_TIME_NOW) != 0) {
+    // After waiting for zero duration, a nonzero value is returned if blocks are still running.
+    return YES;
+  }
+  // Both the _mainSerialQueue and _editingTransactionQueue are drained; we are fully quiesced.
+  return NO;
+}
+
+- (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.
+      [_mainSerialQueue performBlockOnMainThread:^{
+        [self onDidFinishProcessingUpdates:completion];
+      }];
+    });
+  }
+}
+
 - (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet
 {
   ASDisplayNodeAssertMainThread();
@@ -563,7 +592,7 @@ - (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet
   });
 
   if (_usesSynchronousDataLoading) {
-    [self waitUntilAllUpdatesAreCommitted];
+    [self waitUntilAllUpdatesAreProcessed];
   }
 }
 

diff --git a/Source/Details/ASMainSerialQueue.h b/Source/Details/ASMainSerialQueue.h
@@ -21,6 +21,7 @@
 AS_SUBCLASSING_RESTRICTED
 @interface ASMainSerialQueue : NSObject
 
+@property (nonatomic, readonly) NSUInteger numberOfScheduledBlocks;
 - (void)performBlockOnMainThread:(dispatch_block_t)block;
 
 @end
diff --git a/Source/Details/ASMainSerialQueue.mm b/Source/Details/ASMainSerialQueue.mm
@@ -40,6 +40,12 @@ - (instancetype)init
   return self;
 }
 
+- (NSUInteger)numberOfScheduledBlocks
+{
+  ASDN::MutexLocker l(_serialQueueLock);
+  return _blocks.count;
+}
+
 - (void)performBlockOnMainThread:(dispatch_block_t)block
 {
   ASDN::MutexLocker l(_serialQueueLock);

diff --git a/Tests/ASCollectionModernDataSourceTests.m b/Tests/ASCollectionModernDataSourceTests.m
@@ -71,7 +71,7 @@ - (void)setUp {
 
 - (void)tearDown
 {
-  [collectionNode waitUntilAllUpdatesAreCommitted];
+  [collectionNode waitUntilAllUpdatesAreProcessed];
   [super tearDown];
 }