[ASDataController] Add event logging for transaction queue flush duration #trivial #334
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hannahmbanana
changed the title
hannahmbanana
changed the title
appleguy
reviewed
Jun 6, 2017
Source/Details/ASDataController.mm
Outdated
| @@ -485,7 +486,11 @@ - (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet | |||
| _initialReloadDataHasBeenCalled = YES; | |||
| } | |||
|
|
|||
| dispatch_group_wait(_editingTransactionGroup, DISPATCH_TIME_FOREVER); | |||
| NSTimeInterval transactionQueueFlushDuration; | |||
There was a problem hiding this comment.
Add an initialization, = 0.0f; This would be initialized to 0 automatically if an instance variable, but in this case it's technically needed.
appleguy
reviewed
Jun 6, 2017
Source/Details/ASDataController.mm
Outdated
| @@ -498,7 +503,7 @@ - (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet | |||
| [self invalidateDataSourceItemCounts]; | |||
|
|
|||
| // Log events | |||
| ASDataControllerLogEvent(self, @"triggeredUpdate: %@", changeSet); | |||
| ASDataControllerLogEvent(self, @"triggeredUpdate (waited on editing queue for %@ms): %@", transactionQueueFlushDuration * 1000, changeSet); | |||
|
@appleguy: The event log shows fractions of a ms, while the console log timestamp shows ~100+ms. Any ideas? |
|
@nguyenhuy - FYI, this logging may be incorrect - I suspect it's understating the number of ms. @appleguy - did you have a chance to take a look at some test values? |
bernieperez
pushed a commit
to AtomTickets/Texture
that referenced
this pull request
Apr 25, 2018
…tion #trivial (TextureGroup#334) * Add event logging for transaction queue flush duration * address @appleguy's comments * Improve formatting of Event Log messages * Small indentation fix in ASDataController
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One of the most common performance problems in a Texture app occurs when a developer makes multiple edit transactions to a collection / table right after each other, before the processing of the previous operation is finished. In this case, Texture blocks on the completion of the current edit operation, even though it would have been asynchronous if the second / subsequent operation had not been started.
The ASEventLog provides the ability to capture the information on this subtle perf issue. In the -ASDataController updateWithChangeSet: method, we can time the duration of the dispatch group wait of the editing transaction queue and log both the time as well as the stack trace so the developer can see which edit operations are triggering performance issues.