Repository navigation
Some FlameGraph changes, for the function list flame graphs - #6402
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6402 +/- ##
========================================
Coverage 84.78% 84.79%
========================================
Files 358 358
Lines 38946 38957 +11
Branches 10917 11020 +103
========================================
+ Hits 33022 33032 +10
- Misses 5488 5489 +1
Partials 436 436 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mstange
force-pushed
the
push-rwkpsqpwxqvt
branch
from
October 22, 2026 14:32
f727a8d to
76c9da5
Compare
canova
approved these changes
Oct 28, 2026
| // This is used for computing the percentages displayed in the call tree. | ||
| rootTotalSummary: number; | ||
| // The total used as the 100% reference for flame graph box widths. | ||
| // Usually equals rootTotalSummary, but for the upper wing it is set to the |
Member
There was a problem hiding this comment.
What's "upper wing"? Is it a function list thing?
Contributor
Author
There was a problem hiding this comment.
ah yes, it's the "callee view" in the function list. I'll just remove that part from the comment.
…ming. The Canvas computed the tooltip percentage itself, by subtracting the box's start from its end. That only works because the non-inverted flame graph happens to lay boxes out relative to rootTotalSummary. Move the computation behind a getRatioOfRootTotalSummary() method on FlameGraphTiming so that each implementation can decide how its box widths relate to the percentage it wants to display.
The flame graph currently derives box widths from rootTotalSummary, which also serves as the 100% reference for the displayed percentages. The upcoming function list wings need those to differ: a wing's root should fill the full width, while percentages should stay relative to all filtered samples. Add a separate flameGraphWidthTotal to CallTreeTimings for the width reference. It equals rootTotalSummary everywhere so far, so nothing changes yet.
mstange
force-pushed
the
push-rwkpsqpwxqvt
branch
from
October 28, 2026 21:45
76c9da5 to
1a67b19
Compare
mstange
enabled auto-merge
October 28, 2026 21:46
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
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.
Main | Deploy preview
In #5233, I'm making it so that there's a flame graph for the selected function. If the selected function only has 20% of the samples, we want the flame graph for those 20% to still take up the entire width (100%) of the flame graph canvas, but we want to correctly state 20% in the tooltip.
At the moment the percentages in the tooltip and the percentages of the box widths in the canvas come from the same source.
This PR adds a
flameGraphWidthTotalproperty which decouples those two.