Skip to content

Commit

Permalink
Add an experiment that makes ASDataController to do everything on mai…
Browse files Browse the repository at this point in the history
…n thread (#1911)

* Add an experiment that makes ASDataController to do everything on main thread

Under this experiment, ASDataController will allocate and layout all nodes on the main thread. This helps to avoid deadlocks that would otherwise occur if some of the node allocations or layouts caused ASDataController's background queue to block on main thread. As a bonus, this experiment also helps to measure how much performance wins we get from doing the work off main.

* Remove ASSERT_ON_EDITING_QUEUE
  • Loading branch information
nguyenhuy committed Sep 18, 2020
1 parent def2762 commit db3e4c4
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 10 deletions.
1 change: 1 addition & 0 deletions Source/ASExperimentalFeatures.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) {
ASExperimentalDrawingGlobal = 1 << 8, // exp_drawing_global
ASExperimentalOptimizeDataControllerPipeline = 1 << 9, // exp_optimize_data_controller_pipeline
ASExperimentalDisableGlobalTextkitLock = 1 << 10, // exp_disable_global_textkit_lock
ASExperimentalMainThreadOnlyDataController = 1 << 11, // exp_main_thread_only_data_controller
ASExperimentalFeatureAll = 0xFFFFFFFF
};

Expand Down
3 changes: 2 additions & 1 deletion Source/ASExperimentalFeatures.mm
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
@"exp_dispatch_apply",
@"exp_drawing_global",
@"exp_optimize_data_controller_pipeline",
@"exp_disable_global_textkit_lock"]));
@"exp_disable_global_textkit_lock",
@"exp_main_thread_only_data_controller"]));
if (flags == ASExperimentalFeatureAll) {
return allNames;
}
Expand Down
34 changes: 25 additions & 9 deletions Source/Details/ASDataController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@
//#define LOG(...) NSLog(__VA_ARGS__)
#define LOG(...)

#define ASSERT_ON_EDITING_QUEUE ASDisplayNodeAssertNotNil(dispatch_get_specific(&kASDataControllerEditingQueueKey), @"%@ must be called on the editing transaction queue.", NSStringFromSelector(_cmd))

const static char * kASDataControllerEditingQueueKey = "kASDataControllerEditingQueueKey";
const static char * kASDataControllerEditingQueueContext = "kASDataControllerEditingQueueContext";

Expand Down Expand Up @@ -133,8 +131,6 @@ - (void)setLayoutDelegate:(id<ASDataControllerLayoutDelegate>)layoutDelegate

- (void)_allocateNodesFromElements:(NSArray<ASCollectionElement *> *)elements
{
ASSERT_ON_EDITING_QUEUE;

NSUInteger nodeCount = elements.count;
__weak id<ASDataControllerSource> weakDataSource = _dataSource;
if (nodeCount == 0 || weakDataSource == nil) {
Expand Down Expand Up @@ -622,12 +618,9 @@ - (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet
os_log_debug(ASCollectionLog(), "New content: %@", newMap.smallDescription);

Class<ASDataControllerLayoutDelegate> layoutDelegateClass = [self.layoutDelegate class];
++_editingTransactionGroupCount;
dispatch_group_async(_editingTransactionGroup, _editingTransactionQueue, ^{
__block __unused os_activity_scope_state_s preparationScope = {}; // unused if deployment target < iOS10
as_activity_scope_enter(as_activity_create("Prepare nodes for collection update", AS_ACTIVITY_CURRENT, OS_ACTIVITY_FLAG_DEFAULT), &preparationScope);

// Step 3: Call the layout delegate if possible. Otherwise, allocate and layout all elements
// Step 3: Call the layout delegate if possible. Otherwise, allocate and layout all elements
dispatch_block_t step3 = ^{
if (canDelegate) {
[layoutDelegateClass calculateLayoutWithContext:layoutContext];
} else {
Expand All @@ -644,6 +637,29 @@ - (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet
}
[self _allocateNodesFromElements:elementsToProcess];
}
};

// Step 3 can be done on the main thread or on _editingTransactionQueue
// depending on an experiment.
BOOL mainThreadOnly = ASActivateExperimentalFeature(ASExperimentalMainThreadOnlyDataController);
if (mainThreadOnly) {
// We'll still dispatch to _editingTransactionQueue only to schedule a block
// to _mainSerialQueue to execute next steps. This is not optimized because
// in theory we can skip _editingTransactionQueue entirely, but it's much safer
// because change sets will still flow through the pipeline in pretty the same way
// (main thread -> _editingTransactionQueue -> _mainSerialQueue) and so
// any methods that block on _editingTransactionQueue will still work.
step3();
}

++_editingTransactionGroupCount;
dispatch_group_async(_editingTransactionGroup, _editingTransactionQueue, ^{
__block __unused os_activity_scope_state_s preparationScope = {}; // unused if deployment target < iOS10
as_activity_scope_enter(as_activity_create("Prepare nodes for collection update", AS_ACTIVITY_CURRENT, OS_ACTIVITY_FLAG_DEFAULT), &preparationScope);

if (!mainThreadOnly) {
step3();
}

// Step 4: Inform the delegate on main thread
[self->_mainSerialQueue performBlockOnMainThread:^{
Expand Down
2 changes: 2 additions & 0 deletions Tests/ASConfigurationTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ + (NSArray *)names {
@"exp_dispatch_apply",
@"exp_drawing_global",
@"exp_optimize_data_controller_pipeline",
@"exp_disable_global_textkit_lock",
@"exp_main_thread_only_data_controller"
];
}

Expand Down

0 comments on commit db3e4c4

Please sign in to comment.