Give the frameTable a lib column by mstange · Pull Request #6258 · firefox-devtools/profiler

mstange · 2026-08-12T21:20:33Z

In the past, the way to get the library for a frame would be to go frame -> func -> resource -> lib. For native frames, the resource is of type Library and points at the lib.

This had the following implications:

Native frames cannot ever have a resource of a non-Library type.
If two native frames want to share a func, they must also share the same library - otherwise they'll be in a different resource, and different resource means different func.

This makes it hard to represent a few cases:

JS JIT frames with assembly. To have assembly, we must have a native library, but we also want JIT frames to have a JS file as their resource, and we want JITted frames for a JS function to share the func with non-JITted frames (interpreter frames) of the same JS function.
Comparison profiles / the "diff" thread for different builds, with assembly code. For example if you have two Firefox builds, one with and one without a patch, they'll have different libxul.so libraries, but you still want to combine C++ functions of the same name in the diffed tree.
Resources for Rust crates (not implemented here): We might want to assign different Rust frames from the same native binary to different resources, with one resource per Rust crate.

This commit makes it so that frames now point at their library directly, and removes the lib column from the resourceTable. A resource of type Library only carries the library's name; several libs can share one.

mergeLibs used to key on name + debugName, which collapsed two builds of the same library into one lib (and one resource). Now it keeps the libs separate but still collapses the resources by name.

For func-only contexts there is no longer a func -> lib edge. formatFunctionNameWithLibrary and the new getLibNameForFunc read the name off the func's resource, which is all they ever needed. profile-query's functionInfo, which wants the full Lib record, scans the frame table once per query.

codecov · 2026-08-12T21:22:47Z

Codecov Report

❌ Patch coverage is 89.47368% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.79%. Comparing base (1198e50) to head (9203f11).
⚠️ Report is 50 commits behind head on main.

Files with missing lines	Patch %	Lines
src/profile-query/function-list.ts	42.85%	8 Missing ⚠️
src/profile-query/index.ts	0.00%	3 Missing ⚠️
src/profile-logic/symbolication.ts	96.22%	2 Missing ⚠️
src/profile-logic/bottom-box.ts	50.00%	1 Missing ⚠️
src/profile-query/formatters/marker-info.ts	80.00%	1 Missing ⚠️
src/profile-query/formatters/thread-info.ts	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6258      +/-   ##
==========================================
+ Coverage   83.73%   83.79%   +0.05%     
==========================================
  Files         350      350              
  Lines       37523    37583      +60     
  Branches    10543    10459      -84     
==========================================
+ Hits        31420    31492      +72     
+ Misses       5676     5664      -12     
  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.

canova

Thanks, looks pretty good to me. I added some comments, it looks like the Int32Array conversion doesn't happen properly, r+ with them fixed.

canova · 2026-08-18T13:14:10Z

+  // The library is stored per frame rather than being reached via the frame's
+  // resource (frame -> func -> resource -> lib) so that resources and libraries
+  // can vary independently. Multiple libraries can share one resource: a
+  // comparison profile of two libxul.so builds has one resource named
+  // "libxul.so" but a separate lib for each build, and a profile which combines
+  // several Firefox runs has one webhost resource per origin but a separate
+  // jitdump lib per run. Keeping them separate also leaves room for resources
+  // that describe something other than a library, such as a Rust crate, without
+  // breaking the frame-to-library association.


Hmm, is it useful to give the historical context here? I think we can trim it down a bit.

Oh dear. Trimmed down.

canova · 2026-08-18T13:21:54Z

+  // The library of this function, if any.
+  // Note that, these days, funcs can be associated with multiple
+  // libraries, so this isn't the best representation anymore. In those
+  // cases this will be set to one of them, but it's arbitrary which one.


Could you file an issue for this?

Filed #6270

canova · 2026-08-18T13:29:22Z

@@ -4466,12 +4467,8 @@ export function findAddressProofForFile(
  if (address === null) {


Oh noticed while looking at this function, but I think we missed this check while changing address. I think this check should be if (address === -1) too now. But it's unrelated to this PR.

Good catch, fixed in a separate commit.

canova · 2026-08-18T13:33:19Z

+  // comparison profile). Use the library of the first frame that has one.
+  let libIndex: IndexIntoLibs | null = null;
+  for (const frameIndex of callNodeFramePerStack.values()) {
+    const frameLib = frameTable.lib[frameIndex];


frameIndex can be -1 for the stacks outside the call node's subtree. We should check that too otherwise frameLib will be undefined for those

I realized that this libIndex was completely unused. I added a commit to remove it.

canova · 2026-08-18T13:36:00Z

+
+This decouples resources from libraries.
+
+The `lib` column can optionally be stored as an `Int32Array`, for profiles loaded from [JsonSlabs](https://github.com/mstange/json-slabs/) files (.jslb, .jslb.gz). Regular JS / JSON arrays are still accepted - but note that `-1` (not `null`) must be used regardless of format.


I was checking where we do this lib array to Int32Array conversion and couldn't find it. I think this needs to happen in finishRawFrameTableBuilder and convertSharedTablesEligibleColumns.

Oops. Added.

In the past, the way to get the library for a frame would be to go frame -> func -> resource -> lib. For native frames, the resource is of type Library and points at the lib. This had the following implications: - Native frames cannot ever have a resource of a non-Library type. - If two native frames want to share a func, they must also share the same library - otherwise they'll be in a different resource, and different resource means different func. This makes it hard to represent a few cases: - JS JIT frames with assembly. To have assembly, we must have a native library, but we also want JIT frames to have a JS file as their resource, and we want JITted frames for a JS function to share the func with non-JITted frames (interpreter frames) of the same JS function. - Comparison profiles / the "diff" thread for different builds, with assembly code. For example if you have two Firefox builds, one with and one without a patch, they'll have different libxul.so libraries, but you still want to combine C++ functions of the same name in the diffed tree. - Resources for Rust crates (not implemented here): We might want to assign different Rust frames from the same native binary to different resources, with one resource per Rust crate. This commit makes it so that frames now point at their library directly, and removes the lib column from the resourceTable. A resource of type Library only carries the library's name; several libs can share one. mergeLibs used to key on name + debugName, which collapsed two builds of the same library into one lib (and one resource). Now it keeps the libs separate but still collapses the resources by name. For func-only contexts there is no longer a func -> lib edge. formatFunctionNameWithLibrary and the new getLibNameForFunc read the name off the func's resource, which is all they ever needed. profile-query's functionInfo, which wants the full Lib record, scans the frame table once per query.

frameTable.address uses -1 rather than null for "no address", so the existing null check was always useless.

mstange force-pushed the push-punossrmnotm branch 4 times, most recently from a2d6fdc to ea3fdb5 Compare August 12, 2026 21:48

mstange marked this pull request as ready for review August 12, 2026 21:48

mstange requested a review from canova August 12, 2026 21:48

canova approved these changes Aug 18, 2026

View reviewed changes

mstange mentioned this pull request Aug 18, 2026

Revisit FunctionInfoResult type in profile-query library #6270

Open

mstange added 3 commits August 18, 2026 14:18

Remove unused libIndex from SourceViewState.

787907d

Fix address check in findAddressProofForFile.

9203f11

frameTable.address uses -1 rather than null for "no address", so the existing null check was always useless.

mstange force-pushed the push-punossrmnotm branch from ea3fdb5 to 9203f11 Compare August 18, 2026 18:28

mstange enabled auto-merge August 18, 2026 18:29

mstange merged commit e156f70 into firefox-devtools:main Aug 18, 2026
21 checks passed

Navigation Menu

Uh oh!

Give the frameTable a lib column - #6258

Give the frameTable a lib column#6258
mstange merged 3 commits into
firefox-devtools:mainfrom
mstange:push-punossrmnotm

mstange commented Aug 12, 2026 •

edited by github-actions Bot

Loading

Uh oh!

codecov Bot commented Aug 12, 2026 •

edited

Loading

Uh oh!

canova left a comment

Uh oh!

canova Aug 18, 2026

Uh oh!

mstange Aug 18, 2026

Uh oh!

canova Aug 18, 2026

Uh oh!

mstange Aug 18, 2026

Uh oh!

canova Aug 18, 2026

Uh oh!

mstange Aug 18, 2026

Uh oh!

canova Aug 18, 2026

Uh oh!

mstange Aug 18, 2026

Uh oh!

canova Aug 18, 2026

Uh oh!

mstange Aug 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -4466,12 +4467,8 @@ export function findAddressProofForFile(
		if (address === null) {


		This decouples resources from libraries.

		The `lib` column can optionally be stored as an `Int32Array`, for profiles loaded from [JsonSlabs](https://github.com/mstange/json-slabs/) files (.jslb, .jslb.gz). Regular JS / JSON arrays are still accepted - but note that `-1` (not `null`) must be used regardless of format.

Uh oh!

Conversation

mstange commented Aug 12, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

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!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mstange commented Aug 12, 2026 •

edited by github-actions Bot

Loading

codecov Bot commented Aug 12, 2026 •

edited

Loading