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.

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.

To support this, bisectionRightByStrKey is replaced with bisectionRightBy,
which takes a comparison function. computeFlameGraphRows now also takes the
SourceTable and the SourceLocationTable.
@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 84.09091% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.78%. Comparing base (7269a79) to head (e4e3447).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/profile-logic/flame-graph.ts 82.92% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6419      +/-   ##
==========================================
- Coverage   84.78%   84.78%   -0.01%     
==========================================
  Files         358      358              
  Lines       38946    38987      +41     
  Branches    10917    11038     +121     
==========================================
+ Hits        33022    33056      +34     
- Misses       5488     5495       +7     
  Partials      436      436              

☔ 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.

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.

1 participant