Deploy Dec 5, 2023#4832
Merged
Merged
Conversation
Compute the inverted selected call node in the action creator, not in the reducer. This means we don't have to pass the call tree as far down. Also move the computation into the CallTree class. And fix it to stop at nodes with heaviest self time, even if they're not leaf nodes.
…anded call paths in the redux state.
…nverted and non-inverted views.
This makes it faster to switch back and forth between inverted and non-inverted mode. It also makes it faster to switch tabs between inverted call tree and the chart views which are always non-inverted.
Pass it just the sample index, and have it return the depth. The yPixelsPerHeight multiplication can happen in the caller.
This is a minor simplification and removes some raw callNodeTable usage.
Move the fillStyle setting outside of a loop in the SampleGraph implementation. This was likely moved by mistake in f8631ae . And don't set the fillStyle at all if there's nothing to draw with this color.
This will be used to make various uses of the call node table more efficient.
This is a bit faster because we don't need to create children arrays and sort them. It also fixes a bug with diff profiles. (Test coming up in the next commit.)
In the past, the indexes would be incorrect for the diffed thread because we were using the dict for one of the input threads, which has a different funcTable.
Before: https://share.firefox.dev/47ydcmp After: https://share.firefox.dev/40Y48og Fixes #844. The old code was saving time by sorting siblings only for the nodes that were displayed based on the current (preview) range selection. This made it cheaper to compute the flame graph for a small range, but it meant that we had to re-do the sort every time the selection changed. Now we do the sorting once, based on the entire call node table. This is expensive but it is a one-time cost (and it's still cheaper than if you were looking at the entire call tree with the old implementation). Then we cache the "ordered rows" and don't have to sort again on each range selection change.
…astChild instead of a Map to find existing call nodes with the same prefix and func.
Rather than talking about "0-based" and "1-based" depths, I think it would be less confusing to always treat depths as zero-based and rename these variables. When I was reading the stack chart code, I was initially quite confused because we were creating an array with length maxDepth, and that seemed too short.
…r-row arrays in computeFlameGraphRows. We don't use computeCallNodeMaxDepthPlusOne here because that one would only include the depth of call nodes that are used by any of the filtered samples. But computeFlameGraphRows looks at the entire call node table, and needs to reserve enough rows to have space even for call nodes which aren't used by any samples.
…able ordering. Before: https://share.firefox.dev/47CvQto (1471ms) After: https://share.firefox.dev/47z8Ewb (429ms, 3.4x faster)
Bumps [@adobe/css-tools](https://github.com/adobe/css-tools) from 4.3.1 to 4.3.2. - [Changelog](https://github.com/adobe/css-tools/blob/main/History.md) - [Commits](https://github.com/adobe/css-tools/commits) --- updated-dependencies: - dependency-name: "@adobe/css-tools" dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
We no longer need the child count, because the call tree's getChildren() method now uses nextSibling to find the children, so it doesn't need the child count any more for ending the search early. And the new flame graph implementation has dropped the other use of the child count. Having just one byte per call node rather than four bytes might improve the performance of computeCallTreeCountsAndSummary slightly, because it needs to touch fewer bytes in memory.
This avoids calls to getChildren for nodes which are visible but not expanded. The tree view calls hasChildren in order to determine whether to show the disclosure arrow. Avoiding the call to getChildren means that we don't have to allocate the array and sort it. This is especially useful in the inverted tree, which has lots of non-expanded nodes at the root level.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Important performance changes:
[Markus Stange] Replace child count with "has children" (Merge PR #4829)
[Markus Stange] Reduce the time of switching to the stack chart tab for the first time (Merge PR #4825)
[Markus Stange] Speed up getCallNodeInfo (#4826)
[Markus Stange] Improve flame graph performance. (Merge PR #4820)
[Markus Stange] Improve findHeavyPathToSameFunctionAfterInversion. (Merge PR #4813)
[Markus Stange] Optimize call node table traversals (Merge PR #4807)
[Markus Stange] Make traced durations honor the preview selection. (Merge PR #4817)
[Markus Stange] Use a call node path to make the stack copy string. (Merge PR #4811)
[Markus Stange] Simplify heightFunction. (Merge PR #4810)
[Markus Stange] Stop using sampleCallNodes in ThreadSampleGraph. (Merge PR #4809)
[Markus Stange] Only set fillStyle once in drawSamples. (Merge PR #4816)
[Markus Stange] Make the flame graph and stack chart always non-inverted (Merge PR #4804)