Change profile.shared.stackTable.prefix to prefixOffset (delta from current index) by mstange · Pull Request #6089 · firefox-devtools/profiler

mstange · 2026-06-05T00:32:28Z

Another new version on top of #6087:

v66: Change stackTable.prefix to prefixOffset (with 0 for root nodes and i - prefix[i] offsets for non-roots)

Here's how this change impacts profile sizes and loading times on this profile: https://storage.googleapis.com/profiler-get-symbols-fixtures/large-speedometer3-profile.json.gz (with this corresponding size profile)

Version	.jslb.gz size	.jslb size	Load time	Profile of it loading
64	122 MB	605 MB	7.6 seconds	https://share.firefox.dev/4ogUKba
65	125 MB	544 MB	6.0 seconds	https://share.firefox.dev/3Qopiem
66	89 MB	468 MB	5.3 seconds	https://share.firefox.dev/49JetKx

125 MB -> 89 MB on the compressed size! That's a 29% reduction!

codecov · 2026-06-05T00:35:00Z

Codecov Report

❌ Patch coverage is 98.69281% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.50%. Comparing base (de0953b) to head (2880a96).
⚠️ Report is 23 commits behind head on main.

Files with missing lines	Patch %	Lines
src/profile-logic/transforms.ts	95.91%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6089      +/-   ##
==========================================
+ Coverage   83.47%   83.50%   +0.03%     
==========================================
  Files         342      342              
  Lines       36097    36183      +86     
  Branches    10098    10133      +35     
==========================================
+ Hits        30131    30216      +85     
- Misses       5538     5539       +1     
  Partials      428      428

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

mstange · 2026-06-18T18:02:19Z

This is now ready for review.

@canova, how do you feel about this? People will have to change any code that generates profiles, but I actually think the changes would be pretty straightforward and non-controversial.

Also, I can fold the two versions into one if you'd prefer that, so that we don't have an intermediate version that uses -1 values.

mstange · 2026-06-18T18:07:43Z

When we compute the derived stack table, we turn prefixOffset back into prefix. We could shave off another few milliseconds if the derived stack table also switched to the prefixOffset format, then we wouldn't need to do the conversion. We can do that as a follow-up if we think it's worth doing.

canova

Thanks for the PR! I mostly added a few questions, they don't necessarily have to be changed if the existing version makes sense, but wanted to understand them first.

@canova, how do you feel about this? People will have to change any code that generates profiles, but I actually think the changes would be pretty straightforward and non-controversial.

I think prefixOffset sounds like a great idea! The size improvements looks really good!

Also, I can fold the two versions into one if you'd prefer that, so that we don't have an intermediate version that uses -1 values.

Yeah, I think I would prefer to have a single upgrader for it instead of having 2.

One thing I don't fully understand is the new normalization step. Why didn't we have that for the frame column? I wanna r+ it after I fully understand. I also added a comment below.

Also, the first commit message says: "so the processed profile version is bumped to 63 ..." but it adds 66 (we've updated a lot lately! 😄) Could you update that as well?

canova · 2026-06-30T20:38:16Z

+function _normalizeAfterUpgrade(profile: any): void {
+  const stackTable = profile.shared?.stackTable ?? null;
+  if (stackTable && !(stackTable.prefix instanceof Int32Array)) {
+    stackTable.prefix = new Int32Array(stackTable.prefix);


Hm, why do we add this normalize step now? We had converted the frame column before but we didn't need to do it for it?

Huh oops no I don't want this either. Thanks for catching it!

canova · 2026-06-30T20:48:39Z

+      // Mirror the normalization that unserializeProfileOfArbitraryFormat
+      // applies when the store loads the JSON profile, so `toEqual` matches
+      // the in-memory profile shape after loading.
+      profile.shared.stackTable.prefix = new Int32Array(
+        profile.shared.stackTable.prefix
+      );


This part looks a bit fragile, especially if we are going to add more to the normalization step later. Could we maybe use unserializeProfileOfArbitraryFormat directly here?

Huh, what the heck. No I'm just deleting this added code entirely.

canova · 2026-06-30T20:56:18Z

-  // Maps each stack to its parent stack, or -1 if it is a root.
-  prefix: Array<IndexIntoStackTable | -1> | Int32Array<ArrayBuffer>;
-
+  prefixOffset: Int32Array<ArrayBuffer>;


Why not Uint32Array for the prefixOffset?

Because accessing an Int32Array is ever-so-slightly faster than accessing a Uint32Array, in Spidermonkey. see this report from Claude

Oh that's very interesting. Thanks for the report!

Nit: It would be nice to have a code comment also mentioning that Int32Array is a conscious choice with the reason why, so we won't forget and try to refactor later.

Ah, yes. Added this comment:

// We use Int32Array instead of Uint32Array so that Spidermonkey // doesn't need to check for i32 overflow on element access. prefixOffset: number[] | Int32Array<ArrayBuffer>;

I also noticed that I forgot to allow plain arrays in the type; I've added number[] | now. I could make a OffsetIntoStackTableFromCurrentIndex typedef if you prefer.

Now we use `-1` (instead of `null`) to indicate that a stack node is a root. This only changes the derived `StackTable`; the `RawStackTable` stored in the files still uses a regular array with nulls.

mstange · 2026-07-02T20:13:26Z

Also, I can fold the two versions into one if you'd prefer that, so that we don't have an intermediate version that uses -1 values.

Yeah, I think I would prefer to have a single upgrader for it instead of having 2.

Sounds good! I've squashed and re-split; the first commit now only affects the derived stack table and not the raw format. The second commit makes the format change and introduces the offset representation.

One thing I don't fully understand is the new normalization step. Why didn't we have that for the frame column? I wanna r+ it after I fully understand. I also added a comment below.

Sorry, looks like I hadn't reviewed it properly myself. I've removed the normalization.

Also, the first commit message says: "so the processed profile version is bumped to 63 ..." but it adds 66 (we've updated a lot lately! 😄) Could you update that as well?

Hah! Removed from the commit message.

canova

Looks good to me, thank you!

canova · 2026-07-03T07:43:05Z

-  // Maps each stack to its parent stack, or -1 if it is a root.
-  prefix: Array<IndexIntoStackTable | -1> | Int32Array<ArrayBuffer>;
-
+  prefixOffset: Int32Array<ArrayBuffer>;


Oh that's very interesting. Thanks for the report!

Nit: It would be nice to have a code comment also mentioning that Int32Array is a conscious choice with the reason why, so we won't forget and try to refactor later.

Replace `RawStackTable.prefix` with a `prefixOffset` column. For each stack i, `prefixOffset[i] === 0` means i is a root, and `prefixOffset[i] === k` (with k > 0) means i's parent is at index i - k. All values are non-negative because the stack table is stored in topological order. This results in large size savings after gzipping, because chains of `1,1,1,1` compress much better than chains of `123,124,125,126`. It's common to have these "immediate precedent is the prefix" chains because of how the stack table is usually created: You have a list of frames and you want to make a stack node for it. You find an existing stack node for a prefix that already exists, and then you create the other stack nodes for the remainder. Those new stack nodes form such a chain. The `prefixOffset` change is only made for the `RawStackTable`, i.e. for what we store in the file. The derived `StackTable` still uses `prefix`, with `-1` indicating roots.

mstange force-pushed the stacktable-prefix-offset branch from 372c5dd to b581752 Compare June 6, 2026 18:12

mstange force-pushed the stacktable-prefix-offset branch 2 times, most recently from 2b6a379 to c8f1a69 Compare June 18, 2026 17:49

mstange marked this pull request as ready for review June 18, 2026 17:50

mstange requested a review from canova June 18, 2026 17:50

mstange force-pushed the stacktable-prefix-offset branch from c8f1a69 to 74b6c2e Compare June 18, 2026 17:56

mstange force-pushed the stacktable-prefix-offset branch from 74b6c2e to 610f1a9 Compare June 18, 2026 18:04

canova requested changes Jun 30, 2026

View reviewed changes

mstange force-pushed the stacktable-prefix-offset branch from 610f1a9 to f576e2f Compare July 2, 2026 19:18

Change the derived StackTable's prefix column to Int32Array.

7bf57d6

Now we use `-1` (instead of `null`) to indicate that a stack node is a root. This only changes the derived `StackTable`; the `RawStackTable` stored in the files still uses a regular array with nulls.

mstange force-pushed the stacktable-prefix-offset branch 3 times, most recently from 3078888 to 179c6aa Compare July 2, 2026 20:07

mstange requested a review from canova July 2, 2026 20:11

canova approved these changes Jul 3, 2026

View reviewed changes

mstange force-pushed the stacktable-prefix-offset branch from 179c6aa to 2880a96 Compare July 3, 2026 12:30

mstange merged commit e3608a4 into firefox-devtools:main Jul 3, 2026
23 checks passed

Uh oh!

Conversation

mstange commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mstange commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mstange commented Jun 18, 2026

Uh oh!

canova left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstange Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstange commented Jul 2, 2026

Uh oh!

canova left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mstange commented Jun 5, 2026 •

edited

Loading

codecov Bot commented Jun 5, 2026 •

edited

Loading

mstange commented Jun 18, 2026 •

edited

Loading

mstange Jul 2, 2026 •

edited

Loading