Reduce Traffic to Table/Collection View When Updating (#2496)

* Improve the call pattern when loading table/collection data * When deleting sections, no need to delete rows first * Clean up a couple things
ArchimboldiMao · Oct 28, 2016 · 8add461 · 8add461
1 parent 54cda5f
commit 8add461
Show file tree

Hide file tree

Showing 3 changed files with 138 additions and 9 deletions.
diff --git a/AsyncDisplayKit/Details/ASDataController.mm b/AsyncDisplayKit/Details/ASDataController.mm
@@ -427,12 +427,16 @@ - (void)_reloadDataWithAnimationOptions:(ASDataControllerAnimationOptions)animat
     LOG(@"Edit Transaction - reloadData");
 
     // Remove everything that existed before the reload, now that we're ready to insert replacements
-    NSMutableArray *editingNodes = _editingNodes[ASDataControllerRowNodeKind];
-    NSUInteger editingNodesSectionCount = editingNodes.count;
-
-    if (editingNodesSectionCount) {
-      NSIndexSet *indexSet = [[NSIndexSet alloc] initWithIndexesInRange:NSMakeRange(0, editingNodesSectionCount)];
-      [self _deleteNodesAtIndexPaths:ASIndexPathsForTwoDimensionalArray(editingNodes) withAnimationOptions:animationOptions];
+    NSUInteger oldSectionCount = [_editingNodes[ASDataControllerRowNodeKind] count];
+
+    // If we have old sections, we should delete them inside beginUpdates/endUpdates with inserting the new ones.
+    if (oldSectionCount) {
+      // -beginUpdates
+      [_mainSerialQueue performBlockOnMainThread:^{
+        [_delegate dataControllerBeginUpdates:self];
+      }];
+
+      NSIndexSet *indexSet = [[NSIndexSet alloc] initWithIndexesInRange:NSMakeRange(0, oldSectionCount)];
       [self _deleteSectionsAtIndexSet:indexSet withAnimationOptions:animationOptions];
     }
 
@@ -445,6 +449,13 @@ - (void)_reloadDataWithAnimationOptions:(ASDataControllerAnimationOptions)animat
     }
     [self _insertSections:sections atIndexSet:sectionIndexes withAnimationOptions:animationOptions];
 
+    if (oldSectionCount) {
+      // -endUpdates
+      [_mainSerialQueue performBlockOnMainThread:^{
+        [_delegate dataController:self endUpdatesAnimated:NO completion:nil];
+      }];
+    }
+
     [self _batchLayoutAndInsertNodesFromContexts:newContexts withAnimationOptions:animationOptions];
 
     if (completion) {
@@ -623,9 +634,6 @@ - (void)deleteSections:(NSIndexSet *)sections withAnimationOptions:(ASDataContro
 
     // remove elements
     LOG(@"Edit Transaction - deleteSections: %@", sections);
-    NSArray *indexPaths = ASIndexPathsForMultidimensionalArrayAtIndexSet(_editingNodes[ASDataControllerRowNodeKind], sections);
-
-    [self _deleteNodesAtIndexPaths:indexPaths withAnimationOptions:animationOptions];
     [self _deleteSectionsAtIndexSet:sections withAnimationOptions:animationOptions];
   });
 }

diff --git a/AsyncDisplayKitTests/ASTableViewTests.m b/AsyncDisplayKitTests/ASTableViewTests.m
@@ -17,6 +17,7 @@
 #import "ASCellNode.h"
 #import "ASTableNode.h"
 #import "ASTableView+Undeprecated.h"
+#import <JGMethodSwizzler/JGMethodSwizzler.h>
 
 #define NumberOfSections 10
 #define NumberOfRowsPerSection 20
@@ -36,6 +37,13 @@ - (void)relayoutAllNodes
 
 @end
 
+@interface UITableView (Testing)
+// This will start recording all editing calls to UITableView
+// into the provided array.
+// Make sure to call [UITableView deswizzleInstanceMethods] to reset this.
++ (void)as_recordEditingCallsIntoArray:(NSMutableArray<NSString *> *)selectors;
+@end
+
 @interface ASTestTableView : ASTableView
 @property (nonatomic, copy) void (^willDeallocBlock)(ASTableView *tableView);
 @end
@@ -544,4 +552,116 @@ - (void)testThatTableNodeConformsToExpectedProtocols
   XCTAssert([node conformsToProtocol:@protocol(ASRangeControllerUpdateRangeProtocol)]);
 }
 
+- (void)testThatInitialDataLoadHappensInOneShot
+{
+  NSMutableArray *selectors = [NSMutableArray array];
+  ASTableNode *node = [[ASTableNode alloc] initWithStyle:UITableViewStylePlain];
+
+  ASTableViewFilledDataSource *dataSource = [ASTableViewFilledDataSource new];
+  node.frame = CGRectMake(0, 0, 100, 100);
+
+  node.dataSource = dataSource;
+  node.delegate = dataSource;
+
+  [UITableView as_recordEditingCallsIntoArray:selectors];
+  XCTAssertGreaterThan(node.numberOfSections, 0);
+  [node waitUntilAllUpdatesAreCommitted];
+  XCTAssertGreaterThan(node.view.numberOfSections, 0);
+
+  // Assert that the beginning of the call pattern is correct.
+  // There is currently noise that comes after that we will allow for this test.
+  NSArray *expectedSelectors = @[ NSStringFromSelector(@selector(reloadData)),
+                                  NSStringFromSelector(@selector(insertSections:withRowAnimation:)),
+                                  NSStringFromSelector(@selector(insertRowsAtIndexPaths:withRowAnimation:))];
+  NSArray *firstSelectors = [selectors subarrayWithRange:NSMakeRange(0, expectedSelectors.count)];
+  XCTAssertEqualObjects(firstSelectors, expectedSelectors);
+
+  [UITableView deswizzleAllInstanceMethods];
+}
+
+- (void)testThatReloadDataHappensInOneShot
+{
+  NSMutableArray *selectors = [NSMutableArray array];
+  ASTableNode *node = [[ASTableNode alloc] initWithStyle:UITableViewStylePlain];
+
+  ASTableViewFilledDataSource *dataSource = [ASTableViewFilledDataSource new];
+  node.frame = CGRectMake(0, 0, 100, 100);
+
+  node.dataSource = dataSource;
+  node.delegate = dataSource;
+
+  // Load initial data.
+  XCTAssertGreaterThan(node.numberOfSections, 0);
+  [node waitUntilAllUpdatesAreCommitted];
+  XCTAssertGreaterThan(node.view.numberOfSections, 0);
+
+  // Reload data.
+  [UITableView as_recordEditingCallsIntoArray:selectors];
+  [node reloadData];
+  [node waitUntilAllUpdatesAreCommitted];
+
+  // Assert that the beginning of the call pattern is correct.
+  // There is currently noise that comes after that we will allow for this test.
+  NSArray *expectedSelectors = @[NSStringFromSelector(@selector(reloadData)),
+                                 NSStringFromSelector(@selector(beginUpdates)),
+                                 NSStringFromSelector(@selector(deleteSections:withRowAnimation:)),
+                                 NSStringFromSelector(@selector(insertSections:withRowAnimation:)),
+                                 NSStringFromSelector(@selector(endUpdates)),
+                                 NSStringFromSelector(@selector(insertRowsAtIndexPaths:withRowAnimation:))];
+  NSArray *firstSelectors = [selectors subarrayWithRange:NSMakeRange(0, expectedSelectors.count)];
+  XCTAssertEqualObjects(firstSelectors, expectedSelectors);
+
+  [UITableView deswizzleAllInstanceMethods];
+}
+
+@end
+
+@implementation UITableView (Testing)
+
++ (void)as_recordEditingCallsIntoArray:(NSMutableArray<NSString *> *)selectors
+{
+  [UITableView swizzleInstanceMethod:@selector(reloadData) withReplacement:JGMethodReplacementProviderBlock {
+    return JGMethodReplacement(void, UITableView *) {
+      JGOriginalImplementation(void);
+      [selectors addObject:NSStringFromSelector(_cmd)];
+    };
+  }];
+  [UITableView swizzleInstanceMethod:@selector(beginUpdates) withReplacement:JGMethodReplacementProviderBlock {
+    return JGMethodReplacement(void, UITableView *) {
+      JGOriginalImplementation(void);
+      [selectors addObject:NSStringFromSelector(_cmd)];
+    };
+  }];
+  [UITableView swizzleInstanceMethod:@selector(endUpdates) withReplacement:JGMethodReplacementProviderBlock {
+    return JGMethodReplacement(void, UITableView *) {
+      JGOriginalImplementation(void);
+      [selectors addObject:NSStringFromSelector(_cmd)];
+    };
+  }];
+  [UITableView swizzleInstanceMethod:@selector(insertRowsAtIndexPaths:withRowAnimation:) withReplacement:JGMethodReplacementProviderBlock {
+    return JGMethodReplacement(void, UITableView *, NSArray *indexPaths, UITableViewRowAnimation anim) {
+      JGOriginalImplementation(void, indexPaths, anim);
+      [selectors addObject:NSStringFromSelector(_cmd)];
+    };
+  }];
+  [UITableView swizzleInstanceMethod:@selector(deleteRowsAtIndexPaths:withRowAnimation:) withReplacement:JGMethodReplacementProviderBlock {
+    return JGMethodReplacement(void, UITableView *, NSArray *indexPaths, UITableViewRowAnimation anim) {
+      JGOriginalImplementation(void, indexPaths, anim);
+      [selectors addObject:NSStringFromSelector(_cmd)];
+    };
+  }];
+  [UITableView swizzleInstanceMethod:@selector(insertSections:withRowAnimation:) withReplacement:JGMethodReplacementProviderBlock {
+    return JGMethodReplacement(void, UITableView *, NSIndexSet *indexes, UITableViewRowAnimation anim) {
+      JGOriginalImplementation(void, indexes, anim);
+      [selectors addObject:NSStringFromSelector(_cmd)];
+    };
+  }];
+  [UITableView swizzleInstanceMethod:@selector(deleteSections:withRowAnimation:) withReplacement:JGMethodReplacementProviderBlock {
+    return JGMethodReplacement(void, UITableView *, NSIndexSet *indexes, UITableViewRowAnimation anim) {
+      JGOriginalImplementation(void, indexes, anim);
+      [selectors addObject:NSStringFromSelector(_cmd)];
+    };
+  }];
+}
+
 @end
diff --git a/Podfile b/Podfile
@@ -5,4 +5,5 @@ platform :ios, '7.0'
 target :'AsyncDisplayKitTests' do
   pod 'OCMock', '~> 2.2'
   pod 'FBSnapshotTestCase/Core', '~> 2.1'
+  pod 'JGMethodSwizzler', :git => 'https://github.com/JonasGessner/JGMethodSwizzler', :branch => 'master'
 end