Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements experimental diff with optimized moves calculation to produce only minimal required moves #1139

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Implements experimental diff with optimized moves calculation to produce only minimal required moves #1139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 12 additions & 0 deletions IGListKit.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
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 */; };
01A3FEE120FFB30100E91657 /* GameplayKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 01A3FEE020FFB30000E91657 /* GameplayKit.framework */; };
Copy link

@iperry90 iperry90 Dec 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this framework may have been added by mistake?

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, ); }; };
Expand Down Expand Up @@ -407,6 +411,8 @@
/* End PBXContainerItemProxy section */

/* Begin PBXFileReference section */
0179E634207FA8EB0082DFE6 /* IGListDiffExperimentOptimizedMovesTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = IGListDiffExperimentOptimizedMovesTests.m; sourceTree = "<group>"; };
01A3FEE020FFB30000E91657 /* GameplayKit.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = GameplayKit.framework; path = System/Library/Frameworks/GameplayKit.framework; sourceTree = SDKROOT; };
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>"; };
Expand Down Expand Up @@ -661,6 +667,7 @@
isa = PBXFrameworksBuildPhase;
buildActionMask = 2147483647;
files = (
01A3FEE120FFB30100E91657 /* GameplayKit.framework in Frameworks */,
887D0B401D870D7F009E01F7 /* IGListKit.framework in Frameworks */,
DD468D380BBF350ACE7EA28B /* Pods_IGListKitTests.framework in Frameworks */,
);
Expand Down Expand Up @@ -846,6 +853,7 @@
41882EBBC340173A8053E3AF /* Frameworks */ = {
isa = PBXGroup;
children = (
01A3FEE020FFB30000E91657 /* GameplayKit.framework */,
E980179F5E885E417EB20D55 /* Pods_IGListKit_tvOSTests.framework */,
1AB7195278D0BBB5DA88D36F /* Pods_IGListKitTests.framework */,
);
Expand Down Expand Up @@ -975,6 +983,7 @@
294AC6311DDE4C19002FCE5D /* IGListDiffResultTests.m */,
88144EE61D870EDC007C7F66 /* IGListDiffSwiftTests.swift */,
88144EE81D870EDC007C7F66 /* IGListDiffTests.m */,
0179E634207FA8EB0082DFE6 /* IGListDiffExperimentOptimizedMovesTests.m */,
88144EE91D870EDC007C7F66 /* IGListDisplayHandlerTests.m */,
29DA5CA21EA7C72400113926 /* IGListGenericSectionControllerTests.m */,
88144EEB1D870EDC007C7F66 /* IGListKitTests-Bridging-Header.h */,
Expand Down Expand Up @@ -1559,6 +1568,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 */,
Expand Down Expand Up @@ -1681,6 +1691,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 */,
Expand Down Expand Up @@ -1732,6 +1743,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 */,
Expand Down
128 changes: 120 additions & 8 deletions Source/Common/IGListDiff.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,105 @@ static void addIndexToCollection(BOOL useIndexPaths, __unsafe_unretained id coll
return paths;
}

// Calculates indexes which require no explicit moves based on a longest increasing indexes set using O(n log n) complexity algorithm
static NSIndexSet *autoMovedIndexes(const vector<IGListRecord> &newResultsArray, NSIndexSet *untouchedIndexes)
{
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;
NSInteger currentIndex = newResultsArray[i].index;

NSUInteger nextUntouched = [untouchedIndexes indexGreaterThanIndex:i];

if (nextUntouched != NSNotFound && currentIndex > nextUntouched) {
continue;
}

NSUInteger prevUntouched = [untouchedIndexes indexLessThanIndex:i];

if (prevUntouched != NSNotFound && currentIndex < prevUntouched) {
continue;
}

while (lo <= hi) {
auto mid = lo + (hi - lo) / 2;
NSInteger prevIndex = newResultsArray[indexes[mid]].index;
if (prevIndex < currentIndex && (prevUntouched == NSNotFound || prevIndex > prevUntouched)) {
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;
}

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 *_autoMovedIndexes;

public:
IGListOptimalMoveChecker(const vector<IGListRecord> &newResultsArray, NSIndexSet *untouchedIndexes)
: _autoMovedIndexes(autoMovedIndexes(newResultsArray, untouchedIndexes))
{}

virtual bool isMove(const NSInteger oldIndex,
const NSInteger newIndex,
const NSInteger insertOffset,
const NSInteger deleteOffset) {

return (oldIndex != newIndex) && ![_autoMovedIndexes containsIndex:oldIndex];
}
};

static id IGListDiffing(BOOL returnIndexPaths,
NSInteger fromSection,
NSInteger toSection,
Expand Down Expand Up @@ -245,9 +344,11 @@ static id IGListDiffing(BOOL returnIndexPaths,
}

// track offsets from deleted items to calculate where items have moved
vector<NSInteger> deleteOffsets(oldCount), insertOffsets(newCount);
vector<NSInteger> deleteOffsets(oldCount);
NSInteger runningOffset = 0;

auto untouchedIndexes = [NSMutableIndexSet new];

// iterate old array records checking for deletes
// incremement offset for each delete
for (NSInteger i = 0; i < oldCount; i++) {
Expand All @@ -257,33 +358,42 @@ static id IGListDiffing(BOOL returnIndexPaths,
if (record.index == NSNotFound) {
addIndexToCollection(returnIndexPaths, mDeletes, fromSection, i);
runningOffset++;
} else if (record.index == i) {
[untouchedIndexes addIndex:record.index];
}

addIndexToMap(returnIndexPaths, fromSection, i, oldArray[i], oldMap);
}

// reset and track offsets from inserted items to calculate where items have moved
runningOffset = 0;

aligned_union<0, IGListMoveChecker, IGListOptimalMoveChecker>::type moveCheckerBuf;

IGListMoveChecker *moveChecker = IGListExperimentEnabled(experiments, IGListExperimentOptimizedMoves)
? new (&moveCheckerBuf) IGListOptimalMoveChecker(newResultsArray, untouchedIndexes)
: new (&moveCheckerBuf) IGListMoveChecker();

// offset incremented for each insert
NSInteger insertOffset = 0;

for (NSInteger i = 0; i < newCount; i++) {
insertOffsets[i] = runningOffset;
const IGListRecord record = newResultsArray[i];
const NSInteger oldIndex = record.index;
// add to inserts if the opposing index is NSNotFound
if (record.index == NSNotFound) {
addIndexToCollection(returnIndexPaths, mInserts, toSection, i);
runningOffset++;
insertOffset++;
} else {
// note that an entry can be updated /and/ moved
if (record.entry->updated) {
addIndexToCollection(returnIndexPaths, mUpdates, fromSection, oldIndex);
}

// 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];
Expand All @@ -299,6 +409,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);
Expand Down
2 changes: 2 additions & 0 deletions Source/Common/IGListExperiments.h
View file
Open in desktop
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 optimized moves to minimal required number
IGListExperimentOptimizedMoves = 1 << 6,
};

/**
Expand Down
Loading