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.

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

Add experiments to skip waiting for updates of collection and table views #1311

Merged
merged 10 commits into from
Jan 24, 2019
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 @@ -1868,17 +1874,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) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Added this check because countForAsyncLayout doesn't account for reload data.

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 @@ -2465,7 +2476,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
24 changes: 15 additions & 9 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,19 +1079,17 @@ - (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:^{
XCTAssertTrue(!cn.isProcessingUpdates, @"ASCollectionNode should no longer be processing updates inside -onDidFinishProcessingUpdates: block");
}];

// Wait for ASDK reload to finish
[cn waitUntilAllUpdatesAreProcessed];
}

XCTAssertTrue(!cn.isProcessingUpdates, @"ASCollectionNode should no longer be processing updates after -wait call");

// Force UIKit to read updated data & range controller to update and account for it
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