Implements experimental diff with optimized moves calculation to prod…

…uce only minimal required moves
Instagram · Apr 12, 2018 · 7042491 · 7042491
1 parent f469c34
commit 7042491
Show file tree

Hide file tree

Showing 5 changed files with 349 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,8 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag
 
 ### Enhancements
 
+- Implemented experimental diff with optimized moves calculation to produce only minimal required moves, enabled by `IGListExperimentOptimizedMoves` option. [Nickolay Tarbayev](https://github.com/tarbayev) [(#1139)](https://github.com/Instagram/IGListKit/pull/1139)
+
 - Add support for UICollectionView's interactive reordering in iOS 9+.  Updates include `-[IGListSectionController canMoveItemAtIndex:]` to enable the behavior, `-[IGListSectionController moveObjectFromIndex:toIndex:]` called when items within a section controller were moved through reordering, `-[IGListAdapterDataSource listAdapter:moveObject:from:to]` called when section controllers themselves were reordered (only possible when all section controllers contain exactly 1 object), and `-[IGListUpdatingDelegate moveSectionInCollectionView:fromIndex:toIndex]` to enable custom updaters to conform to the reordering behavior. The update also includes two new examples `ReorderableSectionController` and `ReorderableStackedViewController` to demonstrate how to enable interactive reordering in your client app. [Jared Verdi](https://github.com/jverdi) [(#976)](https://github.com/Instagram/IGListKit/pull/976)
 
 - 5x improvement to diffing performance when result is only inserts or deletes. [Ryan Nystrom](https://github.com/rnystrom) [(tbd)](tbd)

diff --git a/IGListKit.xcodeproj/project.pbxproj b/IGListKit.xcodeproj/project.pbxproj
@@ -7,6 +7,9 @@
 	objects = {
 
 /* Begin PBXBuildFile section */
+		0179E635207FA8EB0082DFE6 /* IGListDiffExperimentOptimizedMovesTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 0179E634207FA8EB0082DFE6 /* IGListDiffExperimentOptimizedMovesTests.m */; };
+		0179E636207FA8F30082DFE6 /* IGListDiffExperimentOptimizedMovesTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 0179E634207FA8EB0082DFE6 /* IGListDiffExperimentOptimizedMovesTests.m */; };
+		0179E637207FA8F30082DFE6 /* IGListDiffExperimentOptimizedMovesTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 0179E634207FA8EB0082DFE6 /* IGListDiffExperimentOptimizedMovesTests.m */; };
 		0B3B92DA1E08D7F5008390ED /* IGListKit.h in Headers */ = {isa = PBXBuildFile; fileRef = 0B3B928B1E08D7F5008390ED /* IGListKit.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		0B3B92DB1E08D7F5008390ED /* IGListKit.h in Headers */ = {isa = PBXBuildFile; fileRef = 0B3B928B1E08D7F5008390ED /* IGListKit.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		0B3B92F61E08D7F5008390ED /* IGListAdapter.h in Headers */ = {isa = PBXBuildFile; fileRef = 0B3B929A1E08D7F5008390ED /* IGListAdapter.h */; settings = {ATTRIBUTES = (Public, ); }; };
@@ -407,6 +410,7 @@
 /* End PBXContainerItemProxy section */
 
 /* Begin PBXFileReference section */
+		0179E634207FA8EB0082DFE6 /* IGListDiffExperimentOptimizedMovesTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = IGListDiffExperimentOptimizedMovesTests.m; sourceTree = "<group>"; };
 		08F0B0FD0690F4FC46DDF21B /* Pods-IGListKit-tvOSTests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-IGListKit-tvOSTests.release.xcconfig"; path = "Pods/Target Support Files/Pods-IGListKit-tvOSTests/Pods-IGListKit-tvOSTests.release.xcconfig"; sourceTree = "<group>"; };
 		0B3B928B1E08D7F5008390ED /* IGListKit.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = IGListKit.h; sourceTree = "<group>"; };
 		0B3B929A1E08D7F5008390ED /* IGListAdapter.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = IGListAdapter.h; sourceTree = "<group>"; };
@@ -975,6 +979,7 @@
 				294AC6311DDE4C19002FCE5D /* IGListDiffResultTests.m */,
 				88144EE61D870EDC007C7F66 /* IGListDiffSwiftTests.swift */,
 				88144EE81D870EDC007C7F66 /* IGListDiffTests.m */,
+				0179E634207FA8EB0082DFE6 /* IGListDiffExperimentOptimizedMovesTests.m */,
 				88144EE91D870EDC007C7F66 /* IGListDisplayHandlerTests.m */,
 				29DA5CA21EA7C72400113926 /* IGListGenericSectionControllerTests.m */,
 				88144EEB1D870EDC007C7F66 /* IGListKitTests-Bridging-Header.h */,
@@ -1559,6 +1564,7 @@
 			files = (
 				298DDA381E3B168E00F76F50 /* IGLayoutTestItem.m in Sources */,
 				885FE2361DC51B76009CE2B4 /* IGListStackSectionControllerTests.m in Sources */,
+				0179E636207FA8F30082DFE6 /* IGListDiffExperimentOptimizedMovesTests.m in Sources */,
 				885FE2311DC51B76009CE2B4 /* IGListDisplayHandlerTests.m in Sources */,
 				298DDA3B1E3B16F800F76F50 /* IGLayoutTestDataSource.m in Sources */,
 				29C474901DDF460500AE68CE /* IGListSectionMapTests.m in Sources */,
@@ -1681,6 +1687,7 @@
 				294AC6321DDE4C19002FCE5D /* IGListDiffResultTests.m in Sources */,
 				88144F141D870EDC007C7F66 /* IGListTestOffsettingLayout.m in Sources */,
 				8240C7FB1DC2F6CF00B3AAE7 /* IGListTestAdapterStoryboardDataSource.m in Sources */,
+				0179E635207FA8EB0082DFE6 /* IGListDiffExperimentOptimizedMovesTests.m in Sources */,
 				298DDA011E3AE28000F76F50 /* IGTestDiffingObject.m in Sources */,
 				88144F131D870EDC007C7F66 /* IGListTestAdapterDataSource.m in Sources */,
 				88144F071D870EDC007C7F66 /* IGListAdapterE2ETests.m in Sources */,
@@ -1732,6 +1739,7 @@
 			isa = PBXSourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
+				0179E637207FA8F30082DFE6 /* IGListDiffExperimentOptimizedMovesTests.m in Sources */,
 				88DF898A1E010F7000B1B9B4 /* IGListDiffTests.m in Sources */,
 				88DF89891E010F6500B1B9B4 /* IGListDiffSwiftTests.swift in Sources */,
 				882BC1321E0118CB0083B311 /* IGTestObject.m in Sources */,

diff --git a/Source/Common/IGListDiff.mm b/Source/Common/IGListDiff.mm
@@ -94,6 +94,123 @@ static void addIndexToCollection(BOOL useIndexPaths, __unsafe_unretained id coll
     return paths;
 }
 
+// Calculates longest increasing indexes set using O(n log n) complexity algorithm
+static NSIndexSet *longestIncreasingIndexes(const vector<IGListRecord> &newResultsArray)
+{
+    NSUInteger count = newResultsArray.size();
+    vector<NSUInteger> prevIndexes(count);
+    vector<NSUInteger> indexes(count + 1);
+
+    NSUInteger length = 0;
+    for (NSUInteger i = 0; i < count; i++) {
+        // Binary search for the largest positive j ≤ length
+        // such that X[M[j]] < X[i]
+        NSUInteger lo = 1;
+        NSUInteger hi = length;
+        while (lo <= hi) {
+            auto mid = ceil((lo + hi) / 2);
+            if (newResultsArray[indexes[mid]].index < newResultsArray[i].index) {
+                lo = mid + 1;
+            } else {
+                hi = mid - 1;
+            }
+        }
+
+        // After searching, lo is 1 greater than the
+        // length of the longest prefix of X[i]
+        auto newLength = lo;
+
+        // The predecessor of X[i] is the last index of
+        // the subsequence of length newLength-1
+        prevIndexes[i] = indexes[newLength - 1];
+        indexes[newLength] = i;
+
+        if (newLength > length) {
+            // If we found a subsequence longer than any we've
+            // found yet, update L
+            length = newLength;
+        }
+    }
+
+    // Reconstruct the longest increasing indexes
+    NSMutableIndexSet *result = [NSMutableIndexSet new];
+    auto k = indexes[length];
+    for (NSUInteger i = 0; i < length; i++) {
+        NSUInteger index = newResultsArray[k].index;
+
+        // Ignore inserted entries
+        if (index != NSNotFound) {
+            [result addIndex:index];
+        }
+        k = prevIndexes[k];
+    }
+    return result;
+}
+
+static BOOL mapContainsMoves(const unordered_map<NSUInteger, NSUInteger> &movesMap, NSUInteger fromIndex, NSUInteger toIndex)
+{
+    auto iter = movesMap.find(fromIndex);
+    return iter != movesMap.end() && iter->second == toIndex;
+}
+
+
+class IGListMoveChecker {
+public:
+    virtual bool isMove(const NSInteger oldIndex,
+                        const NSInteger newIndex,
+                        const NSInteger insertOffset,
+                        const NSInteger deleteOffset) {
+
+        return (oldIndex - deleteOffset + insertOffset) != newIndex;
+    }
+
+    virtual ~IGListMoveChecker() {}
+};
+
+class IGListOptimalMoveChecker : public IGListMoveChecker {
+
+    NSIndexSet *longestIncreasingSequence;
+
+    // track previously made moves
+    unordered_map<NSUInteger, NSUInteger> moves;
+
+    // track offsets for previously made moves to identify unnecessary moves
+    NSInteger movesOffset = 0;
+public:
+  IGListOptimalMoveChecker(const vector<IGListRecord> &newResultsArray)
+      : longestIncreasingSequence(
+            // Calculate longest increasing indexes, identifying entries which do not require any moves
+            longestIncreasingIndexes(newResultsArray))
+  {}
+
+    virtual bool isMove(const NSInteger oldIndex,
+                        const NSInteger newIndex,
+                        const NSInteger insertOffset,
+                        const NSInteger deleteOffset) override {
+
+        auto oldIndexNoDeletes = oldIndex - deleteOffset;
+        auto oldCorrectedIndex = oldIndexNoDeletes + insertOffset;
+
+        // 1. if the indexes match, ignore the index, else
+        // 2. if the index requires no move and is not swap move, ignore the index, else
+        // 3. if the index matches old index with moves offset, ignore the index, else
+        if (oldCorrectedIndex != newIndex
+            && (![longestIncreasingSequence containsIndex:oldIndex] || mapContainsMoves(moves, newIndex + deleteOffset - insertOffset, oldIndexNoDeletes))
+            && ((oldCorrectedIndex + movesOffset) != newIndex)) {
+
+            // store move for later checks
+            moves[oldIndex] = newIndex;
+
+            // track offset caused by the move
+            movesOffset++;
+
+            return true;
+        }
+
+        return false;
+    }
+};
+
 static id IGListDiffing(BOOL returnIndexPaths,
                         NSInteger fromSection,
                         NSInteger toSection,
@@ -262,6 +379,13 @@ static id IGListDiffing(BOOL returnIndexPaths,
         addIndexToMap(returnIndexPaths, fromSection, i, oldArray[i], oldMap);
     }
 
+
+    aligned_union<0, IGListMoveChecker, IGListOptimalMoveChecker>::type moveCheckerBuf;
+
+    IGListMoveChecker *moveChecker = IGListExperimentEnabled(experiments, IGListExperimentOptimizedMoves)
+                                         ? new (&moveCheckerBuf) IGListOptimalMoveChecker(newResultsArray)
+                                         : new (&moveCheckerBuf) IGListMoveChecker();
+
     // reset and track offsets from inserted items to calculate where items have moved
     runningOffset = 0;
 
@@ -280,10 +404,12 @@ static id IGListDiffing(BOOL returnIndexPaths,
             }
 
             // calculate the offset and determine if there was a move
-            // if the indexes match, ignore the index
             const NSInteger insertOffset = insertOffsets[i];
             const NSInteger deleteOffset = deleteOffsets[oldIndex];
-            if ((oldIndex - deleteOffset + insertOffset) != i) {
+
+            if (moveChecker->isMove(oldIndex, i, insertOffset, deleteOffset)) {
+
+                // add move from old index to new index
                 id move;
                 if (returnIndexPaths) {
                     NSIndexPath *from = [NSIndexPath indexPathForItem:oldIndex inSection:fromSection];
@@ -299,6 +425,8 @@ static id IGListDiffing(BOOL returnIndexPaths,
         addIndexToMap(returnIndexPaths, toSection, i, newArray[i], newMap);
     }
 
+    moveChecker->~IGListMoveChecker();
+
     NSCAssert((oldCount + [mInserts count] - [mDeletes count]) == newCount,
               @"Sanity check failed applying %li inserts and %lu deletes to old count %lu equaling new count %li",
               (long)oldCount, (unsigned long)[mInserts count], (unsigned long)[mDeletes count], (long)newCount);

diff --git a/Source/Common/IGListExperiments.h b/Source/Common/IGListExperiments.h
@@ -26,6 +26,8 @@ typedef NS_OPTIONS (NSInteger, IGListExperiment) {
     IGListExperimentFasterVisibleSectionController = 1 << 4,
     /// Test deduping item-level updates.
     IGListExperimentDedupeItemUpdates = 1 << 5,
+    /// Test optimized moves to mimimal required number
+    IGListExperimentOptimizedMoves = 1 << 6,
 };
 
 /**