Skip to content

Give flame graph siblings with the same function name a defined order - #6419

Open
mstange wants to merge 1 commit into
firefox-devtools:mainfrom
mstange:push-kypzpsmwrtom
Open

mstange wants to merge 1 commit into
firefox-devtools:mainfrom
mstange:push-kypzpsmwrtom

Conversation

@mstange

@mstange mstange commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Main | Deploy preview

Compare these flame graphs: https://share.firefox.dev/4jy20z8 vs https://share.firefox.dev/4k1tYTW
There are two view call nodes with the same parent which swap positions in the two views. That's because they currently do not have a defined order. This PR fixes that.

@mstange
mstange requested a review from fatadel October 26, 2026 19:53
@mstange mstange self-assigned this Oct 26, 2026
@codecov

codecov Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.82759% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.96%. Comparing base (e5ea81f) to head (3801343).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/profile-logic/flame-graph.ts 94.11% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6419   +/-   ##
=======================================
  Coverage   84.95%   84.96%           
=======================================
  Files         359      359           
  Lines       39137    39170   +33     
  Branches    11091    11105   +14     
=======================================
+ Hits        33249    33280   +31     
- Misses       5451     5453    +2     
  Partials      437      437           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fatadel fatadel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the inverted flame graph planned for a follow-up or was it supposed to be handled here? (same-name siblings are still ordered by func index there)

Comment thread src/profile-logic/flame-graph.ts Resolved
@mstange

mstange commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Is the inverted flame graph planned for a follow-up or was it supposed to be handled here? (same-name siblings are still ordered by func index there)

Oh I forgot about the inverted flame graph. I'll fix that in this PR.

The flame graph ordered siblings by function name only. Siblings with the
same name kept the order in which they appear in the call node table, which
depends on the stack table. Two similar profiles could therefore show the
same functions in opposite orders.

Example: in the JS-only flame graph of https://share.firefox.dev/4jy20z8
vs https://share.firefox.dev/4k1tYTW, the call node for callHook in the
stack below has two children named "view":

    view [../../src/components/Controls.js:9:5]
    callHook [../../node_modules/mithril/render/render.js:34:11]
    updateComponent [../../node_modules/mithril/render/render.js:462:11]
    updateNode [../../node_modules/mithril/render/render.js:396:11]
    updateNodes [../../node_modules/mithril/render/render.js:270:11]
    updateElement [../../node_modules/mithril/render/render.js:451:11]

Both view functions come from the same bundle, so they have the same name
and the same file name. The two profiles showed them in opposite orders.

Siblings with the same function name are now compared by:

1. The generated file name.
2. The original (source-mapped) file name.
3. The line, then the column, where the function starts. This uses the
   original location if there is one, and the generated location otherwise.
4. The resource name, for example the library of a native function.
5. The function index, so that two different functions never compare
   equal.

The inverted flame graph uses the same order. It previously ordered
siblings with the same name by function index only.

The new makeFlameGraphFuncComparator function creates this comparison
function. computeFlameGraphRows and getInvertedFlameGraphTiming now take
the comparison function instead of the FuncTable and the StringTable.
bisectionRightByStrKey is replaced with bisectionRightBy, which takes a
comparison function.
@mstange
mstange requested a review from fatadel November 6, 2026 19:46

@fatadel fatadel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good - thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants