Skip to content

Commit

Permalink
Add experiments to skip waiting for updates of collection and table v…
Browse files Browse the repository at this point in the history
…iews under some circumstances (#1311)

* Add experiment to skip waiting until all updates of collection/table view are committed in -accessibilityElements

The wait was introduced in #1217 which blocks the main thread until updates are proccessed. We suspect this causes perf regressions accross the app and need to confirm this via an experiment.

* Add option to skip default behavior of ASCellLayoutMode

* Fix unit test

* Fix unit test in another way

* Remove import

* Minor change

* Add ASCellLayoutModeSyncForSmallContent

* Update unit tests

* Remove unnecessary change

* Remove unnecessary changes
  • Loading branch information
nguyenhuy authored Jan 24, 2019
1 parent 231ba3d commit f180138
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 40 deletions.
7 changes: 5 additions & 2 deletions Schemas/configuration.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@
"exp_unfair_lock",
"exp_infer_layer_defaults",
"exp_network_image_queue",
"exp_dealloc_queue_v2",
"exp_collection_teardown",
"exp_framesetter_cache",
"exp_skip_clear_data"
"exp_skip_clear_data",
"exp_did_enter_preload_skip_asm_layout",
"exp_disable_a11y_cache",
"exp_skip_a11y_wait",
"exp_new_default_cell_layout_mode"
]
}
}
Expand Down
35 changes: 24 additions & 11 deletions Source/ASCollectionView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,12 @@ - (instancetype)_initWithFrame:(CGRect)frame collectionViewLayout:(UICollectionV

[self _configureCollectionViewLayout:layout];

if (ASActivateExperimentalFeature(ASExperimentalNewDefaultCellLayoutMode)) {
_cellLayoutMode = ASCellLayoutModeSyncForSmallContent;
} else {
_cellLayoutMode = ASCellLayoutModeNone;
}

return self;
}

Expand Down Expand Up @@ -1877,17 +1883,22 @@ - (BOOL)dataController:(ASDataController *)dataController shouldSynchronouslyPro
if (ASCellLayoutModeIncludes(ASCellLayoutModeAlwaysAsync)) {
return NO;
}
// The heuristics below apply to the ASCellLayoutModeNone.
// If we have very few ASCellNodes (besides UIKit passthrough ones), match UIKit by blocking.
if (changeSet.countForAsyncLayout < 2) {
return YES;
}
CGSize contentSize = self.contentSize;
CGSize boundsSize = self.bounds.size;
if (contentSize.height <= boundsSize.height && contentSize.width <= boundsSize.width) {
return YES;
if (ASCellLayoutModeIncludes(ASCellLayoutModeSyncForSmallContent)) {
// Reload data is expensive, don't block main while doing so.
if (changeSet.includesReloadData) {
return NO;
}
// If we have very few ASCellNodes (besides UIKit passthrough ones), match UIKit by blocking.
if (changeSet.countForAsyncLayout < 2) {
return YES;
}
CGSize contentSize = self.contentSize;
CGSize boundsSize = self.bounds.size;
if (contentSize.height <= boundsSize.height && contentSize.width <= boundsSize.width) {
return YES;
}
}
return NO;
return NO; // ASCellLayoutModeNone
}

- (BOOL)dataControllerShouldSerializeNodeCreation:(ASDataController *)dataController
Expand Down Expand Up @@ -2474,7 +2485,9 @@ - (void)setPrefetchingEnabled:(BOOL)prefetchingEnabled

- (NSArray *)accessibilityElements
{
[self waitUntilAllUpdatesAreCommitted];
if (!ASActivateExperimentalFeature(ASExperimentalSkipAccessibilityWait)) {
[self waitUntilAllUpdatesAreCommitted];
}
return [super accessibilityElements];
}

Expand Down
23 changes: 14 additions & 9 deletions Source/ASCollectionViewProtocols.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ typedef NS_OPTIONS(NSUInteger, ASCellLayoutMode) {
* each flag listed below is used.
*/
ASCellLayoutModeNone = 0,
/**
* If ASCellLayoutModeSyncForSmallContent is enabled it will cause ASDataController to wait on the
* background queue if the amount of new content is small.
*/
ASCellLayoutModeSyncForSmallContent = 1 << 1,
/**
* If ASCellLayoutModeAlwaysSync is enabled it will cause the ASDataController to wait on the
* background queue, and this ensures that any new / changed cells are in the hierarchy by the
Expand All @@ -26,26 +31,26 @@ typedef NS_OPTIONS(NSUInteger, ASCellLayoutMode) {
* default behavior is synchronous when there are 0 or 1 ASCellNodes in the data source, and
* asynchronous when there are 2 or more.
*/
ASCellLayoutModeAlwaysSync = 1 << 1, // Default OFF
ASCellLayoutModeAlwaysAsync = 1 << 2, // Default OFF
ASCellLayoutModeForceIfNeeded = 1 << 3, // Deprecated, default OFF.
ASCellLayoutModeAlwaysPassthroughDelegate = 1 << 4, // Deprecated, default ON.
ASCellLayoutModeAlwaysSync = 1 << 2, // Default OFF
ASCellLayoutModeAlwaysAsync = 1 << 3, // Default OFF
ASCellLayoutModeForceIfNeeded = 1 << 4, // Deprecated, default OFF.
ASCellLayoutModeAlwaysPassthroughDelegate = 1 << 5, // Deprecated, default ON.
/** Instead of using performBatchUpdates: prefer using reloadData for changes for collection view */
ASCellLayoutModeAlwaysReloadData = 1 << 5, // Default OFF
ASCellLayoutModeAlwaysReloadData = 1 << 6, // Default OFF
/** If flag is enabled nodes are *not* gonna be range managed. */
ASCellLayoutModeDisableRangeController = 1 << 6, // Default OFF
ASCellLayoutModeAlwaysLazy = 1 << 7, // Deprecated, default OFF.
ASCellLayoutModeDisableRangeController = 1 << 7, // Default OFF
ASCellLayoutModeAlwaysLazy = 1 << 8, // Deprecated, default OFF.
/**
* Defines if the node creation should happen serialized and not in parallel within the
* data controller
*/
ASCellLayoutModeSerializeNodeCreation = 1 << 8, // Default OFF
ASCellLayoutModeSerializeNodeCreation = 1 << 9, // Default OFF
/**
* When set, the performBatchUpdates: API (including animation) is used when handling Section
* Reload operations. This is useful only when ASCellLayoutModeAlwaysReloadData is enabled and
* cell height animations are desired.
*/
ASCellLayoutModeAlwaysBatchUpdateSectionReload = 1 << 9 // Default OFF
ASCellLayoutModeAlwaysBatchUpdateSectionReload = 1 << 10 // Default OFF
};

NS_ASSUME_NONNULL_BEGIN
Expand Down
2 changes: 2 additions & 0 deletions Source/ASExperimentalFeatures.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) {
ASExperimentalSkipClearData = 1 << 8, // exp_skip_clear_data
ASExperimentalDidEnterPreloadSkipASMLayout = 1 << 9, // exp_did_enter_preload_skip_asm_layout
ASExperimentalDisableAccessibilityCache = 1 << 10, // exp_disable_a11y_cache
ASExperimentalSkipAccessibilityWait = 1 << 11, // exp_skip_a11y_wait
ASExperimentalNewDefaultCellLayoutMode = 1 << 12, // exp_new_default_cell_layout_mode
ASExperimentalFeatureAll = 0xFFFFFFFF
};

Expand Down
4 changes: 3 additions & 1 deletion Source/ASExperimentalFeatures.mm
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
@"exp_framesetter_cache",
@"exp_skip_clear_data",
@"exp_did_enter_preload_skip_asm_layout",
@"exp_disable_a11y_cache"]));
@"exp_disable_a11y_cache",
@"exp_skip_a11y_wait",
@"exp_new_default_cell_layout_mode"]));

if (flags == ASExperimentalFeatureAll) {
return allNames;
Expand Down
26 changes: 17 additions & 9 deletions Source/ASTableView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1677,14 +1677,20 @@ - (BOOL)dataControllerShouldSerializeNodeCreation:(ASDataController *)dataContro

- (BOOL)dataController:(ASDataController *)dataController shouldSynchronouslyProcessChangeSet:(_ASHierarchyChangeSet *)changeSet
{
// For more details on this method, see the comment in the ASCollectionView implementation.
if (changeSet.countForAsyncLayout < 2) {
return YES;
}
CGSize contentSize = self.contentSize;
CGSize boundsSize = self.bounds.size;
if (contentSize.height <= boundsSize.height && contentSize.width <= boundsSize.width) {
return YES;
if (ASActivateExperimentalFeature(ASExperimentalNewDefaultCellLayoutMode)) {
// Reload data is expensive, don't block main while doing so.
if (changeSet.includesReloadData) {
return NO;
}
// For more details on this method, see the comment in the ASCollectionView implementation.
if (changeSet.countForAsyncLayout < 2) {
return YES;
}
CGSize contentSize = self.contentSize;
CGSize boundsSize = self.bounds.size;
if (contentSize.height <= boundsSize.height && contentSize.width <= boundsSize.width) {
return YES;
}
}
return NO;
}
Expand Down Expand Up @@ -2004,7 +2010,9 @@ - (void)didMoveToSuperview

- (NSArray *)accessibilityElements
{
[self waitUntilAllUpdatesAreCommitted];
if (!ASActivateExperimentalFeature(ASExperimentalSkipAccessibilityWait)) {
[self waitUntilAllUpdatesAreCommitted];
}
return [super accessibilityElements];
}

Expand Down
18 changes: 12 additions & 6 deletions Tests/ASCollectionViewTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ - (instancetype)initWithNibName:(NSString *)nibNameOrNil bundle:(NSBundle *)nibB
@interface ASCollectionView (InternalTesting)

- (NSArray<NSString *> *)dataController:(ASDataController *)dataController supplementaryNodeKindsInSections:(NSIndexSet *)sections;
- (BOOL)dataController:(ASDataController *)dataController shouldSynchronouslyProcessChangeSet:(_ASHierarchyChangeSet *)changeSet;

@end

Expand Down Expand Up @@ -1047,15 +1046,24 @@ - (void)_primitiveBatchFetchingFillTestAnimated:(BOOL)animated visible:(BOOL)vis

- (void)testInitialRangeBounds
{
[self testInitialRangeBoundsWithCellLayoutMode:ASCellLayoutModeNone];
[self testInitialRangeBoundsWithCellLayoutMode:ASCellLayoutModeNone
shouldWaitUntilAllUpdatesAreProcessed:YES];
}

- (void)testInitialRangeBoundsCellLayoutModeSyncForSmallContent
{
[self testInitialRangeBoundsWithCellLayoutMode:ASCellLayoutModeSyncForSmallContent
shouldWaitUntilAllUpdatesAreProcessed:YES]; // Need to wait because the first initial data load is always async
}

- (void)testInitialRangeBoundsCellLayoutModeAlwaysAsync
{
[self testInitialRangeBoundsWithCellLayoutMode:ASCellLayoutModeAlwaysAsync];
[self testInitialRangeBoundsWithCellLayoutMode:ASCellLayoutModeAlwaysAsync
shouldWaitUntilAllUpdatesAreProcessed:YES];
}

- (void)testInitialRangeBoundsWithCellLayoutMode:(ASCellLayoutMode)cellLayoutMode
shouldWaitUntilAllUpdatesAreProcessed:(BOOL)shouldWait
{
UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
ASCollectionViewTestController *testController = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil];
Expand All @@ -1071,9 +1079,7 @@ - (void)testInitialRangeBoundsWithCellLayoutMode:(ASCellLayoutMode)cellLayoutMod
// Trigger the initial reload to start
[window layoutIfNeeded];

// Test the APIs that monitor ASCollectionNode update handling if collection node should
// layout asynchronously
if (![cn.view dataController:nil shouldSynchronouslyProcessChangeSet:nil]) {
if (shouldWait) {
XCTAssertTrue(cn.isProcessingUpdates, @"ASCollectionNode should still be processing updates after initial layoutIfNeeded call (reloadData)");

[cn onDidFinishProcessingUpdates:^{
Expand Down
8 changes: 6 additions & 2 deletions Tests/ASConfigurationTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
ASExperimentalFramesetterCache,
ASExperimentalSkipClearData,
ASExperimentalDidEnterPreloadSkipASMLayout,
ASExperimentalDisableAccessibilityCache
ASExperimentalDisableAccessibilityCache,
ASExperimentalSkipAccessibilityWait,
ASExperimentalNewDefaultCellLayoutMode
};

@interface ASConfigurationTests : ASTestCase <ASConfigurationDelegate>
Expand All @@ -51,7 +53,9 @@ + (NSArray *)names {
@"exp_framesetter_cache",
@"exp_skip_clear_data",
@"exp_did_enter_preload_skip_asm_layout",
@"exp_disable_a11y_cache"
@"exp_disable_a11y_cache",
@"exp_skip_a11y_wait",
@"exp_new_default_cell_layout_mode"
];
}

Expand Down

0 comments on commit f180138

Please sign in to comment.