Improve the visibleObjects performance

Summary:
Trying to improve my previous QE regression on all the perf: https://www.internalfb.com/intern/qe2/ig_ios_listkit_improvements_universe/ig_ios_listkit_improvements_skip_view_section_controller_map/review

My theory is that it's incresing the load in the -visibleObjects call, then we call -[UICollectionView cellForIndexPath:] which is expensive in the `_sectionControllerForCell:`

Thus my new approach is to optimize this -visibleObjects entirely and instead of dealing with visible cells, we can use the `indexPathsForVisibleItems` which can give us the indexpath instead and make it faster. It would be O(N) instead of O(N^2) compared to before.

Reviewed By: calimarkus

Differential Revision: D24663555

fbshipit-source-id: 69fcf2d0b44f8bc06351c92f96c3cbe227c22479
This commit is contained in:
Zhisheng Huang 2020-11-02 11:17:02 -08:00 committed by Facebook GitHub Bot
parent 6311bbcfb4
commit 2c519b0b10
2 changed files with 51 additions and 12 deletions

View file

@ -606,21 +606,40 @@
- (NSArray *)visibleObjects {
IGAssertMainThread();
NSArray<UICollectionViewCell *> *visibleCells = [self.collectionView visibleCells];
NSMutableSet *visibleObjects = [NSMutableSet new];
for (UICollectionViewCell *cell in visibleCells) {
IGListSectionController *sectionController = [self _sectionControllerForCell:cell];
IGAssert(sectionController != nil, @"Section controller nil for cell %@", cell);
if (sectionController != nil) {
const NSInteger section = [self sectionForSectionController:sectionController];
if (IGListExperimentEnabled(_experiments, IGListExperimentSkipViewSectionControllerMap)) {
NSArray<NSIndexPath *> *visibleIndexPaths = [self.collectionView indexPathsForVisibleItems];
NSMutableIndexSet *visibleSections = [NSMutableIndexSet new];
[visibleIndexPaths enumerateObjectsUsingBlock:^(NSIndexPath * _Nonnull indexPath, NSUInteger idx, BOOL * _Nonnull stop) {
[visibleSections addIndex:indexPath.section];
}];
NSMutableArray *visibleObjects = [NSMutableArray new];
[visibleSections enumerateIndexesUsingBlock:^(NSUInteger section, BOOL * _Nonnull stop) {
id object = [self objectAtSection:section];
IGAssert(object != nil, @"Object not found for section controller %@ at section %li", sectionController, (long)section);
IGAssert(object != nil, @"Object not found at section %li", (long)section);
if (object != nil) {
[visibleObjects addObject:object];
}
}];
return visibleObjects;
} else {
NSArray<UICollectionViewCell *> *visibleCells = [self.collectionView visibleCells];
NSMutableSet *visibleObjects = [NSMutableSet new];
for (UICollectionViewCell *cell in visibleCells) {
IGListSectionController *sectionController = [self _sectionControllerForCell:cell];
IGAssert(sectionController != nil, @"Section controller nil for cell %@", cell);
if (sectionController != nil) {
const NSInteger section = [self sectionForSectionController:sectionController];
id object = [self objectAtSection:section];
IGAssert(object != nil, @"Object not found for section controller %@ at section %li", sectionController, (long)section);
if (object != nil) {
[visibleObjects addObject:object];
}
}
}
return [visibleObjects allObjects];
}
return [visibleObjects allObjects];
}
- (NSArray<UICollectionViewCell *> *)visibleCellsForObject:(id)object {
@ -1055,8 +1074,15 @@
// only return a cell if it belongs to the section controller
// this association is created in -collectionView:cellForItemAtIndexPath:
UICollectionViewCell *cell = [self.collectionView cellForItemAtIndexPath:indexPath];
if ([self _sectionControllerForCell:cell] == sectionController) {
return cell;
if (IGListExperimentEnabled(_experiments, IGListExperimentSkipViewSectionControllerMap)) {
if ([self sectionControllerForSection:indexPath.section] == sectionController) {
return cell;
}
} else {
if ([self _sectionControllerForCell:cell] == sectionController) {
return cell;
}
}
}
return nil;

View file

@ -614,6 +614,19 @@
XCTAssertEqualObjects(visibleObjects, expectedObjects);
}
- (void)test_whenAdapterUpdated_withSkipViewSectionControllerMap_withObjectsOverflow_thatVisibleObjectsIsSubsetOfAllObjects {
self.adapter.experiments |= IGListExperimentSkipViewSectionControllerMap;
// each section controller returns n items sized 100x10
self.dataSource.objects = @[@1, @2, @3, @4, @5, @6];
[self.adapter reloadDataWithCompletion:nil];
self.collectionView.contentOffset = CGPointMake(0, 30);
[self.collectionView layoutIfNeeded];
NSArray *visibleObjects = [[self.adapter visibleObjects] sortedArrayUsingSelector:@selector(compare:)];
NSArray *expectedObjects = @[@3, @4, @5];
XCTAssertEqualObjects(visibleObjects, expectedObjects);
}
- (void)test_whenAdapterUpdated_thatVisibleCellsForObjectAreFound {
// each section controller returns n items sized 100x10
self.dataSource.objects = @[@2, @10, @5];
@ -933,7 +946,7 @@
- (void)test_whenQueryingSectionControllerForSection_thatControllerReturned {
self.dataSource.objects = @[@0, @1, @2];
[self.adapter reloadDataWithCompletion:nil];
XCTAssertEqual([self.adapter sectionControllerForSection:0], [self.adapter sectionControllerForObject:@0]);
XCTAssertEqual([self.adapter sectionControllerForSection:1], [self.adapter sectionControllerForObject:@1]);
XCTAssertEqual([self.adapter sectionControllerForSection:2], [self.adapter sectionControllerForObject:@2]);