Skip to content

Avoid stack overflow on large marker threads - #6264

Merged
fqueze merged 2 commits into
firefox-devtools:mainfrom
fqueze:profiler-cli-large-thread-crash-pr
Aug 18, 2026
Merged

Avoid stack overflow on large marker threads#6264
fqueze merged 2 commits into
firefox-devtools:mainfrom
fqueze:profiler-cli-large-thread-crash-pr

Conversation

@fqueze

@fquezefqueze commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Main | Deploy preview

thread markers on a thread with over ~1M markers failed with "Maximum call stack size exceeded". The cause was computeRateStats spreading its per-name gap array into Math.min/Math.max: an argument spread pushes one argument per element and V8 overflows around 100k. aggregateMarkersByName calls it once per marker name, so the trigger is the largest name group, not the thread size.

Reduce the gaps to min/avg/max in a single pass with no intermediate array, and wrap the whole-thread aggregation so that any remaining failure points at --search / --min-duration / --category, which narrow the same query.

@fqueze
fqueze requested a review from canovaAugust 18, 2026 08:13
@codecov

codecovBot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.76%. Comparing base (1198e50) to head (eba427b).
⚠️ Report is 46 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #6264 +/- ##
==========================================
+ Coverage 83.73% 83.76% +0.03% 
==========================================
Files 350 350 Lines 37523 37546 +23 Branches 10543 10563 +20 ==========================================
+ Hits 31420 31451 +31 + Misses 5676 5668 -8 
Partials 427 427 

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

@canovacanova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The Math.min/max fix looks correct, but I'm not so sure about aggregateWithLargeThreadHint. Currently it shows the same error message on any error which might actually mislead more.

Comment on lines +711 to +721
if (isNarrowed) {
throw error;
}
const message = error instanceof Error ? error.message : String(error);
throw new Error(
`${message}\nFailed while aggregating all ${markerCount} markers of this ` +
`thread. Narrowing the query to a subset of the same thread works: ` +
`rerun with --search <substring>, or with --min-duration / --category, ` +
`which filter the markers before they are aggregated.`
);
}

@canovacanovaAug 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this still necessary after your fix? If so, what is triggering this issue?

Also currently this aggregateWithLargeThreadHint catches everything which doesn't feel right. It might be another issue that's throwing error.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

aggregateWithLargeThreadHint removed in the update.

Unfiltered thread markers failed with "Maximum call stack size exceeded" on
threads above roughly 1M markers: computeRateStats spread the whole gap array
into Math.min/Math.max, once per marker name group. Compute the min, max and
mean in a single pass instead.
@fqueze
fquezeforce-pushed the profiler-cli-large-thread-crash-pr branch from eda57cc to a58bb85CompareAugust 18, 2026 15:13
@fqueze
fqueze requested a review from canovaAugust 18, 2026 15:17
@fqueze
fqueze enabled auto-merge (squash) August 18, 2026 15:36
@fqueze
fqueze merged commit ef473a1 into firefox-devtools:mainAug 18, 2026
21 checks passed
@fatadelfatadel mentioned this pull request Sep 3, 2026
fatadel added a commit that referenced this pull request Sep 3, 2026
Changes:
[Nazım Can Altınova] Update oxfmt 0.59.0 → 0.63.0 (major) (#6262)
[Andrew Creskey] Show which network requests were prefetched (#6259)
[Florian Quèze] profiler-cli: document the marker field:value search
syntax (#6265)
[Florian Quèze] profiler-cli: report one time base for text and JSON
output (#6266)
[Florian Quèze] profiler-cli: avoid a stack overflow on large marker
threads (#6264)
[Markus Stange] Give the frameTable a lib column (#6258)
[Florian Quèze] profiler-cli: accept --limit 0 as unlimited, and make
truncation loud (#6267)
[Florian Quèze] profiler-cli: report the network request count the
filters ran against (#6274)
[Nazım Can Altınova] Fix dark mode contrast of the warning icons in the
publish panel (#6280)
[Nazım Can Altınova] Extract the publish panel warning indicator into a
small component (#6282)
[Markus Stange] More typed arrays in the FrameTable (and a flags column)
(#6173)
[Nazım Can Altınova] Improve the profiler-cli publish script and
document the whole deployment in a better way (#6260)
[fatadel] 🔃 Sync: l10n -> main (September 3, 2026) (#6301)
[fatadel] Bump profiler-cli version to 0.9.0 (#6302)
And special thanks to our localizers:
es-CL: ravmn
nl: Mark Heijl
sv-SE: Andreas Pettersson
sv-SE: Luna Jernberg
sv-SE: Peter Kihlstedt
tr: Selim Şumlu
tr: giray
zh-CN: 高乐喆
Sign up for freeto 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

@fqueze@canova