Skip to content

Add source map symbolication and source view support#6018

Open
canova wants to merge 16 commits into
firefox-devtools:mainfrom
canova:sourcemaps
Open

Add source map symbolication and source view support#6018
canova wants to merge 16 commits into
firefox-devtools:mainfrom
canova:sourcemaps

Conversation

@canova
Copy link
Copy Markdown
Member

@canova canova commented May 12, 2026

The bugzilla part of this PR: https://bugzilla.mozilla.org/show_bug.cgi?id=2035493

This requires a Firefox that has patches applied in bug 2035493. And also it requires the "JavaScript Sources" features to be on.

It adds a pipeline that maps compiled/bundled JS frame positions back to their original source files using source maps fetched from the browser after profile load, similarly to our native symbolication step. And it adds a way to see the source mapped source contents.

After this PR, the call tree, flame graph tooltip, source view, and line timings show original TS/JS positions and function names for frames recorded in minified bundles.

I split the work in multiple patches so it's easier to review, but admittedly it's a lot of changes. The initial commits in the PR don't change the behavior until the commit that wires everything up and updates the visualization.

Example STR:

  • Use a Firefox that includes my patches. And enable the "JavaScript Sources" feature in about:profiling.
  • Start the profiler
  • Load an example website that has JS source maps, for example the Firefox Profiler frontend
  • Capture a profile
  • Wait until symbolication and source map resolution is complete

You should then see the symbolicated functions on the frames that are coming from Firefox Profiler source code. (Note that the react code will still be minified as it doesn't have source maps). You should also be able to double click on a JS function function to be able to see the source mapped JS source.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2026

Codecov Report

❌ Patch coverage is 79.48140% with 182 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.65%. Comparing base (3182970) to head (f77359e).
⚠️ Report is 33 commits behind head on main.

Files with missing lines Patch % Lines
src/profile-logic/source-map-symbolication.ts 60.55% 114 Missing ⚠️
src/actions/source-map-symbolication.ts 63.04% 17 Missing ⚠️
src/profile-logic/source-map-scope-tree.ts 95.12% 12 Missing ⚠️
src/app-logic/browser-connection.ts 20.00% 8 Missing ⚠️
src/test/fixtures/mocks/web-channel.ts 0.00% 6 Missing ⚠️
src/utils/query-api.ts 0.00% 6 Missing ⚠️
src/app-logic/web-channel.ts 0.00% 4 Missing ⚠️
src/profile-logic/line-timings.ts 85.18% 4 Missing ⚠️
src/profile-query/function-annotate.ts 82.35% 3 Missing ⚠️
src/utils/line-offsets.ts 80.00% 3 Missing ⚠️
... and 4 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6018      +/-   ##
==========================================
- Coverage   83.78%   83.65%   -0.13%     
==========================================
  Files         329      335       +6     
  Lines       34528    35388     +860     
  Branches     9659     9823     +164     
==========================================
+ Hits        28930    29605     +675     
- Misses       5169     5354     +185     
  Partials      429      429              

☔ View full report in Codecov by Sentry.
📢 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 canova force-pushed the sourcemaps branch 3 times, most recently from c372d65 to 5fe5ca4 Compare May 26, 2026 14:06
@canova canova marked this pull request as ready for review May 26, 2026 14:10
@canova canova requested a review from fatadel as a code owner May 26, 2026 14:10
@canova canova requested a review from mstange May 26, 2026 14:10
@canova
Copy link
Copy Markdown
Member Author

canova commented May 26, 2026

This PR is now ready for review!

Copy link
Copy Markdown
Contributor

@mstange mstange left a comment

Choose a reason for hiding this comment

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

Submitting what I have for now

Comment thread src/app-logic/web-channel.ts
Comment thread src/app-logic/browser-connection.ts Outdated
Comment on lines +96 to +98
// The negotiated WebChannel version. Callers use this to gate features that
// require a minimum version.
readonly webChannelVersion: number;
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.

Do we need this? Could we have a supportsGetSourceMap flag instead so that we don't have to propagate knowledge about versions outwards?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hehe, I initially did the same thing, and then thought that it's too specific for the browser connection and changed to this one. I reverted again.

Comment thread package.json
"redux-thunk": "^3.1.0",
"reselect": "^4.1.8",
"source-map": "^0.7.6",
"url": "^0.11.4",
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.

I'm confused - does the source-map dependency just straight up not work in the browser if you don't also depend on url?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, it's a bit weird and it's more like a workaround. source-map has this at lib/url.js: module.exports = typeof URL === "function" ? URL : require("url").URL;. At runtime in the browser the first branch wins (since window.URL is a global), so the require("url") is dead code. But esbuild statically follows every require() it sees during bundling, even if the branch is not reachable, so without a real "url" module in node_modules the build fails. This is the error message:

$ yarn start
yarn run v1.22.22
$ cross-env NODE_ENV=development node scripts/run-dev-server.mjs
------------------------------------------------------------------------------------------
> Firefox Profiler is listening at: http://localhost:4242

> You can change this default port with the environment variable FX_PROFILER_PORT.

> esbuild development server enabled
------------------------------------------------------------------------------------------
✘ [ERROR] Could not resolve "url"

    node_modules/source-map/lib/url.js:13:59:
      13 │ module.exports = typeof URL === "function" ? URL : require("url").URL;
         │                                                            ~~~~~
         ╵                                                            "./url"

  The package "url" wasn't found on the file system but is built into node. Are you trying to bundle
  for node? You can use "platform: 'node'" to do that, which will remove this error.

Comment thread yarn.lock Outdated
Comment on lines +1829 to +1844
dependencies:
"@lezer/common" "^1.2.0"
"@lezer/highlight" "^1.1.3"
"@lezer/lr" "^1.3.0"
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 like we're not sharing the same library as we're using for source view syntax highlighting? Do we need to update codemirror?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah right, I missed this one. codemirror declares ^1.0.0", which permits 1.5.4 as well. But unfortunately yarn doesn't do deduplication automatically. You have to execute npx yarn-deduplicate to do that. Looks like I forgot to execute it. Updated it now.

Comment thread src/profile-logic/source-map-scope-tree.ts Outdated
Comment thread src/profile-logic/source-map-scope-tree.ts
Comment on lines +9 to +23
### Version 63

A new `SourceMapInfoTable` has been added to `profile.shared.sourceMapInfo`. Each entry maps a generated (compiled) position to its original source position as determined by source map symbolication.

- `originalSource: IndexIntoSourceTable[]`: original source file index. Set independently for both func entries (the function's definition file) and frame entries (the execution point's file).
- `originalLine: number[]`: 1-based original line number
- `originalColumn: number[]`: 1-based original column number

Two new columns were added that index into this table:

- `FrameTable.sourceMapInfo: Array<IndexIntoSourceMapInfoTable | null>`: points to the original execution point for the frame
- `FuncTable.sourceMapInfo: Array<IndexIntoSourceMapInfoTable | null>`: points to the original definition site for the function

A new `content: Array<string | null>` column was added to `SourceTable`.

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.

Not a fan of the names. Here are some ideas:

sourceMappedLocation
SourceMappedLocationTable
SourceLocationTable
originalLocation
sourceLocationFromSourceMap

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm, I think I like SourceLocationTable and originalLocation. Changed it.

Comment thread src/profile-logic/bottom-box.ts Outdated
const funcLine = funcTable.lineNumber[funcIndex];
// source and assembly view to scroll those into view. funcLine and the per-sample
// frame lines come from getOriginalPositionForFrame, so the scroll target lines
// up with the (original) source view's coordinate system when symbolicated.
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.

coordinate system? what does this mean?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh lol, yeah it's weird. I wanted to mean "source view's line numbering when symbolicated." but "coordinate system" was a slop output.

Comment thread src/profile-logic/line-timings.ts Outdated
Comment on lines +65 to +66
const { source: sourceIndexOfThisStack, line: selfLineFromMapping } =
getOriginalPositionForFrame(
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.

This scares me from a performance perspective because it runs for every stack in the stack table. How does this impact e.g. the profile linked in #5761?

Copy link
Copy Markdown
Member Author

@canova canova May 27, 2026

Choose a reason for hiding this comment

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

Hmm you're right. I profiled it and saw that getStackLineInfo went from 144ms to 373ms (with 209ms is inside this function). I think the easiest option is to inline this into this function so we don't allocate though. I tried that and it went to 186ms. Still worse than 144ms but definitely better than before. What do you think?

Profiles:
main
non-inlined
inlined

Comment on lines +3356 to +3360
): {
source: IndexIntoSourceTable | null;
line: number | null;
column: number | null;
} {
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.

Ideally we'd be able to return an IndexIntoSourceLocationTable here. I wonder if this justifies using the SourceLocationTable for both source-mapped and non-source-mapped locations. For example, the FrameTable could have both a location and a locationFromSourceMap column.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I thought about that one as well but decided not do it eventually. Here was my reasoning:

  • If we move the source, line, and column table values in the frame table to another map, it means that we will have 4 table columns that we allocate for each frame instead of 3 that we have currently.
  • We will dedup some of the things, so the above issue is a bit better, but then we will have de dedup cost during the profile loading etc.
  • The above issue is mostly okay for the source mapped locations because the data is a lot less dense, for example most native frames are going to have only a single null as oppose to 4 nulls.
  • It's a big migration. Not a huge concern considering that we can just add an upgrader, but it touches a lot of places, I'm not so sure about it considering the above issues.

What do you think? I think the first item was the biggest thing that made me decide not to go with this option.

Comment thread src/selectors/profile.ts

const fileNameStrIndex = sources.filename[sourceIndex];
return fileNameStrIndex !== null
return fileNameStrIndex !== null && fileNameStrIndex !== undefined
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.

So we're looking up a sourceIndex >= sources.length? I wonder if we should block such a sourceIndex from getting into the state at an earlier part of the pipeline.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, but I wanted this STR to work:

  • Let's say that I captured a profile and I didn't publish it yet.
  • I opened up the JS source view that has some source mapped source content.
  • I reload the profile.

After this STR, I want the profile to load the source content as expected (after the source map resolution).
This is happening because first the source table is short, and then we append a bunch of new sources later.

Without this commit it crashes. If we move this to an earlier part of the pipeline, it means that we'll just clear out the sourceIndex. If we do that, we lose that state. It's kind of a niche use case, but I found it very useful especially when I was working on the feature.

Comment on lines +21 to +24
* Names come from two sources, in priority order. The compiled source is
* always available (it's the bundle text we already have). The original
* source is whatever `sourcesContent` carried: often everything, sometimes
* nothing.
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.

The compiled source is always available? Does this assume we only hit this code if the user is in Firefox and capturing a profile with the jssources feature enabled?

Also what is the meaning of the second sentence?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, currently this requires JS Sources feature, because we need the CST parser to have accurate names. But I was thinking about the same thing actually. Maybe we can still do the source map resolution with the information that we have and accept that it will not be fully correct.

For the second sentence. source maps have this sourcesContent array that contains the original source contents. I wanted to emphasize that sometimes they can be removed by the bundlers. But it's not the perfect phrasing. Updated it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Edit: Also updated it to always attempt to do source map symbolication if there is no compiled source. It'll be imperfect, but probably still better.

Comment on lines +51 to +53
* var foo = arrow // -> foo (bare named, no chain)
* var foo = wrap(arrow) // -> outer/foo< (contributes-to: chain + `<`)
* obj[key] = arrow // -> outer/obj[key] (member-style: chain)
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.

var foo = arrow? What does this mean?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, I wanted to mean an arrow function like var foo = () => {} or wrap(() => {}). Updated them to () => {}.


let eligibility = funcEligibility[funcIndex];
if (eligibility === 0) {
eligibility = _isFuncSymbolicable(funcIndex, funcTable, sources) ? 1 : 2;
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.

Might be a good idea to use const enum here, rather than magic numbers.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I thought the same! Then I implemented it and thought that it's an overkill since it's only used here and nowhere else. But I can update it still if it makes things clearer.

* Approximated by: GLB(pos) resolves differently from GLB(predecessor of pos).
*
* For pos.col > 0 the predecessor is (line, col - 1). For pos.col === 0 the
* predecessor is the end of the previous line: source-map's GLB will clip to
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.

What's GLB?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

source-map's originalPositionFor is a greatest-lower-bound (GLB) lookup, so it returns the largest mapping whose generated position is <= the query rather than requiring an exact match.
The _isExactSourceMapEntry helper detects "is this position the start of a new mapping?" by comparing the lookup result at pos with the result at its predecessor: if they differ, pos must be an exact entry.

Updated some of the comments to make it clearer.

Comment on lines +239 to +243
* TODO: This approximation produces false negatives when two consecutive
* source-map mappings resolve to the same original position (GLB(pos) ==
* GLB(predecessor) even though pos is an exact entry). If this causes
* incorrect function names in practice, replace with a direct mapping-entry
* lookup once the source-map library exposes that API.
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.

Huh? Do you have an example?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

When I was looking at the profiles that I captured from the profiler.firefox.com source maps, I was hitting an issue frequently that it was getting the first argument name instead of an anonymous function (since it doesn't have any name itself. That's why I had to add this _isExactSourceMapEntry. When I was comparing it with Chrome's devtools, I was seeing the same issue on them as well, it was frequently getting the wrong name.

I added this function to make sure that it's an exact entry. I tested it heavily and I can say that so far all the frames I see are correct. But claude was telling me that it could produce false negatives, but I haven't found any so far. So this TODO is more of a theoretical edge case that could happen, I added here to be more defensive. I can remove it if you prefer as well.

Comment thread src/profile-logic/source-map-symbolication.ts Outdated
Comment thread src/actions/receive-profile.ts Outdated
Comment on lines +264 to +277
const tracksWithOrder = {
globalTracks: getGlobalTracks(getState()),
globalTrackOrder: getGlobalTrackOrder(getState()),
localTracksByPid: getLocalTracksByPid(getState()),
localTrackOrderByPid: getLocalTrackOrderByPid(getState()),
};
const hiddenTracks = {
hiddenGlobalTracks: getHiddenGlobalTracks(getState()),
hiddenLocalTracksByPid: getHiddenLocalTracksByPid(getState()),
};
const visibleThreadIndexes = getVisibleThreads(
tracksWithOrder,
hiddenTracks
);
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.

Doing this only for the visible threads is really weird. Is it necessary?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Probably not fully necessary but I wanted to be a bit more defensive at first. I was thinking the cases where we have hundreds of web contents with source maps on all of them. But probably that's not super realistic.

Edit: Removed that, now we do it for all.

Comment on lines +49 to +59
const result = await _runSourceMapWorker(input);
switch (result.type) {
case 'success':
dispatch({
type: 'BULK_SOURCE_MAP_SYMBOLICATION',
newFuncTable: result.newFuncTable,
newFrameTable: result.newFrameTable,
newSourceMapInfo: result.newSourceMapInfo,
newSources: result.newSources,
newStringArray: result.newStringArray,
});
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.

I think it would be better to treat the work in the worker as something like a symbolication API request, where the response doesn't know about our profile tables but still has the necessary information. And then when the result from the worker comes in, we can compute the new tables with main thread JS, like we do for native symbolication. This would address the "stomp over each other" concern.

canova added 14 commits May 27, 2026 17:44
…SOURCES

Version 7 of the WebChannel renames the GET_JS_SOURCES request field
from `sourceUuids` to `sourceIds`. The sender picks the field name based
on the negotiated WebChannel version so both older Firefox (v6) and
newer (v7+) builds are supported.
Lets the browser fetch source maps from URLs the frontend cannot reach.
The frontend identifies the bundle source via its sourceId, and the
browser returns the parsed source map.

Currently it's plumbing only. No caller dispatches GET_SOURCE_MAP yet.

Adds source-map and url as direct dependencies; `source-map` provides
the RawSourceMap type and runtime parser, and `url` is required by
source-map's URL resolution in browser bundles.
Extends the processed profile with a per-thread-shared SourceLocationTable
(stored at profile.shared.originalLocation) and a content column on the
SourceTable. Each frame and func now has a nullable originalLocation index,
set to null by all existing initializers and upgraders. Source content
stays null until JS symbolication populates it.

The new tables are plumbed through data-structures, profile-compacting,
merge-compare, and every importer.

It adds a v63 upgrader and a CHANGELOG entry. No call site reads the new
columns yet, behavior is unchanged.
The nonymous algorithm (johnjbarton.github.io/nonymous) is how
SpiderMonkey labels anonymous JS functions inside its
NameFunctions.cpp pass. Adds a parser and serializer for the
`/`-separated, `<`-marked name format Gecko emits, so later
symbolication can produce names that match what the browser already
shows.

Pure utility, no callers yet.
Parses compiled JS with @lezer/javascript and walks the CST to build a
tree of function scopes with name-mapping locations. The character
offsets later probed against the source map to recover original
function names. `@lezer/javascript` is already a transitive dep via
`@codemirror/lang-javascript`.

Pure utility, no callers yet. Tested via the new
source-map-scope-tree.test.ts.
Implements the off-main-thread part of JS source map symbolication.
SourceMapStore wraps the source-map library's WASM-backed
SourceMapConsumer, source-map-symbolication.ts walks every func and
frame and maps its compiled position back to the original source via
exact-match source-map lookups (with scope-tree probes to recover
function names), and source-map.worker.ts wires those pieces up as a
Web Worker entry point.

The action thunk doSourceMapSymbolication spawns the worker on demand
and dispatches BULK_SOURCE_MAP_SYMBOLICATION on success.

No caller dispatches the thunk yet.
The Web Worker added in the previous patch needs its own bundle so the
@lezer/javascript and source-map dependencies (plus the source-map
WASM mappings) ship to the browser.

Still no caller invokes the worker, so this patch only changes how
`yarn build` builds the dist directory.
Activates JS source map symbolication end-to-end. After native
symbolication settles, the receive-profile flow now fetches source
maps and compiled JS for the bundle sources visible in any track,
hands them to doSourceMapSymbolication, and lets the worker rewrite
funcTable / frameTable / sourceMapInfo / sources / stringArray
in-place via BULK_SOURCE_MAP_SYMBOLICATION.

After this patch, sourceMapInfo is populated but UI code still reads
the compiled positions. The next patch makes it look at sourceMapInfo.
The sourceMapInfo table is populated after the previous patch's
wire-up, but no UI code read it yet. This patch teaches the source
view, call tree, tooltip, line timings, and getOriginAnnotationForFunc
to prefer source-mapped positions when present and fall back to the
compiled position otherwise. Frame-level entries override func-level
entries (so inlined code lands in the right original file).

Adds the inline-content fallback in selectors/code.tsx: when the
profile already carries sources.content (from a shared profile or from
JS symbolication), the source view uses it directly instead of hitting
the browser.
The source map pipeline (fetching + worker) previously ran invisibly
after profile load. This surfaces two generic status messages in the
existing SymbolicationStatusOverlay: "Fetching source maps..." while
doResolveSourceMaps runs, and "Symbolicating JS source maps..." while
the worker runs. Native symbolication takes priority. The source map
messages only appear when the native overlay is not active.

It's tracked via a new sourceMapSymbolicationStatus on viewOptions.

Note that it looks like this component was never localized. We should do
it! But I think this is a task for a follow-up
I noticed this bug while testing the source map PR locally. I was doing
this:
- Capture a profile with the JS sources feature enabled that includes
  source maps.
- After the symbolication and source fetching, double click on a JS
  function that is source mapped.
- Refresh the page.

I was expecting it to work, but it was throwing errors because the
source that I already opened wasn't in the profile and string table
anymore (since it's added after the source map symbolication). So this
fixes this issue by checking if the string table index is undefined
…debugging

It proved somewhat difficult when I had the debug the source map
symbolication worker on the worker thread. It makes things a lot easier
when it runs on the main thread instead. I tried adding a const flag for
this with its proper function, but esbuild wasn't properly removing the
dead code, so it was inflating the bundle a lot when this function was
there even if it was unused.

I tried also converting that into an async import, which extracted the
symbolication into a different bundle, but it was still outputting the
extra bundle even if it was false constant value. So, I decided to
include it as a commented out documentation in case we need it in the
future. It's not the best, but it should still help us when we need to
debug the worker in the future.
`fetchSourceAnnotation` was always reading from
funcTable.source[funcIndex], which points at the compiled bundle. For
JS-symbolicated functions the original file already lives behind
funcTable.sourceMapInfo, and its text is typically embedded on the
shared source table either by sourcesContent during symbolication or by
the publish-with-sources path.

Resolve the effective source through getOriginalPositionForFrame so the
filename, line hit attribution, and content lookup all target the
original source when sourceMapInfo is present. When sources.content is
populated, use it directly and skip the network fetch entirely so the
source view works offline for published-with-sources profiles.
canova added 2 commits May 27, 2026 17:45
…h native

Refactor the source-map worker to return position-keyed resolutions instead
of full table snapshots. The main thread applies the response against the
current shared state via applySourceMapSymbolicationResponse, allocating
sourceMapInfo rows, interning names, and deduping URLs against whatever
native symbolication has committed by then.

With per-funcIndex/per-frameIndex idempotency (skip rows already populated)
and JS funcs/frames being insulated from native symbolication, the worker
no longer needs to wait for native symbolication to finish. finalizeProfileView
now runs source-map fetch, native symbolication, and the source-map worker
all in parallel.
getStackLineInfo runs once per stack in the stack table, which can
mean hundreds of thousands of iterations on large profiles. Calling
getOriginalPositionForFrame in that loop allocated a fresh
{source, line, column} object per stack and always resolved the line
(and column) even though the line is only used when the stack's
source matches the source view, and the column is never used here.

Inline the 3-tier source-map fallback so we drop the per-stack
allocation, skip the column entirely, and defer the line lookup
until matchesSource is true. Apply the same inlining to
getTotalLineTimingsForCallNode, which has the same shape and only
needs the line.

Measured on a large profile: getStackLineInfo went from ~373ms back
to ~199ms.
Copy link
Copy Markdown
Member Author

@canova canova left a comment

Choose a reason for hiding this comment

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

Thanks for the review @mstange! I addressed and added comments. I already squashed some of the things that are not tricky to their own commits, but created their own commits for some things that we might want to discuss. Let me know what you think!

Comment thread src/app-logic/browser-connection.ts Outdated
Comment on lines +96 to +98
// The negotiated WebChannel version. Callers use this to gate features that
// require a minimum version.
readonly webChannelVersion: number;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hehe, I initially did the same thing, and then thought that it's too specific for the browser connection and changed to this one. I reverted again.

Comment thread src/actions/receive-profile.ts Outdated
Comment on lines +264 to +277
const tracksWithOrder = {
globalTracks: getGlobalTracks(getState()),
globalTrackOrder: getGlobalTrackOrder(getState()),
localTracksByPid: getLocalTracksByPid(getState()),
localTrackOrderByPid: getLocalTrackOrderByPid(getState()),
};
const hiddenTracks = {
hiddenGlobalTracks: getHiddenGlobalTracks(getState()),
hiddenLocalTracksByPid: getHiddenLocalTracksByPid(getState()),
};
const visibleThreadIndexes = getVisibleThreads(
tracksWithOrder,
hiddenTracks
);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Probably not fully necessary but I wanted to be a bit more defensive at first. I was thinking the cases where we have hundreds of web contents with source maps on all of them. But probably that's not super realistic.

Edit: Removed that, now we do it for all.

Comment thread src/app-logic/web-channel.ts
Comment thread src/profile-logic/source-map-scope-tree.ts
Comment thread package.json
"redux-thunk": "^3.1.0",
"reselect": "^4.1.8",
"source-map": "^0.7.6",
"url": "^0.11.4",
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, it's a bit weird and it's more like a workaround. source-map has this at lib/url.js: module.exports = typeof URL === "function" ? URL : require("url").URL;. At runtime in the browser the first branch wins (since window.URL is a global), so the require("url") is dead code. But esbuild statically follows every require() it sees during bundling, even if the branch is not reachable, so without a real "url" module in node_modules the build fails. This is the error message:

$ yarn start
yarn run v1.22.22
$ cross-env NODE_ENV=development node scripts/run-dev-server.mjs
------------------------------------------------------------------------------------------
> Firefox Profiler is listening at: http://localhost:4242

> You can change this default port with the environment variable FX_PROFILER_PORT.

> esbuild development server enabled
------------------------------------------------------------------------------------------
✘ [ERROR] Could not resolve "url"

    node_modules/source-map/lib/url.js:13:59:
      13 │ module.exports = typeof URL === "function" ? URL : require("url").URL;
         │                                                            ~~~~~
         ╵                                                            "./url"

  The package "url" wasn't found on the file system but is built into node. Are you trying to bundle
  for node? You can use "platform: 'node'" to do that, which will remove this error.

Comment thread src/selectors/profile.ts

const fileNameStrIndex = sources.filename[sourceIndex];
return fileNameStrIndex !== null
return fileNameStrIndex !== null && fileNameStrIndex !== undefined
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, but I wanted this STR to work:

  • Let's say that I captured a profile and I didn't publish it yet.
  • I opened up the JS source view that has some source mapped source content.
  • I reload the profile.

After this STR, I want the profile to load the source content as expected (after the source map resolution).
This is happening because first the source table is short, and then we append a bunch of new sources later.

Without this commit it crashes. If we move this to an earlier part of the pipeline, it means that we'll just clear out the sourceIndex. If we do that, we lose that state. It's kind of a niche use case, but I found it very useful especially when I was working on the feature.

Comment on lines +3356 to +3360
): {
source: IndexIntoSourceTable | null;
line: number | null;
column: number | null;
} {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I thought about that one as well but decided not do it eventually. Here was my reasoning:

  • If we move the source, line, and column table values in the frame table to another map, it means that we will have 4 table columns that we allocate for each frame instead of 3 that we have currently.
  • We will dedup some of the things, so the above issue is a bit better, but then we will have de dedup cost during the profile loading etc.
  • The above issue is mostly okay for the source mapped locations because the data is a lot less dense, for example most native frames are going to have only a single null as oppose to 4 nulls.
  • It's a big migration. Not a huge concern considering that we can just add an upgrader, but it touches a lot of places, I'm not so sure about it considering the above issues.

What do you think? I think the first item was the biggest thing that made me decide not to go with this option.

Comment on lines +9 to +23
### Version 63

A new `SourceMapInfoTable` has been added to `profile.shared.sourceMapInfo`. Each entry maps a generated (compiled) position to its original source position as determined by source map symbolication.

- `originalSource: IndexIntoSourceTable[]`: original source file index. Set independently for both func entries (the function's definition file) and frame entries (the execution point's file).
- `originalLine: number[]`: 1-based original line number
- `originalColumn: number[]`: 1-based original column number

Two new columns were added that index into this table:

- `FrameTable.sourceMapInfo: Array<IndexIntoSourceMapInfoTable | null>`: points to the original execution point for the frame
- `FuncTable.sourceMapInfo: Array<IndexIntoSourceMapInfoTable | null>`: points to the original definition site for the function

A new `content: Array<string | null>` column was added to `SourceTable`.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm, I think I like SourceLocationTable and originalLocation. Changed it.

Comment on lines +21 to +24
* Names come from two sources, in priority order. The compiled source is
* always available (it's the bundle text we already have). The original
* source is whatever `sourcesContent` carried: often everything, sometimes
* nothing.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Edit: Also updated it to always attempt to do source map symbolication if there is no compiled source. It'll be imperfect, but probably still better.

Comment thread src/profile-logic/line-timings.ts Outdated
Comment on lines +65 to +66
const { source: sourceIndexOfThisStack, line: selfLineFromMapping } =
getOriginalPositionForFrame(
Copy link
Copy Markdown
Member Author

@canova canova May 27, 2026

Choose a reason for hiding this comment

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

Hmm you're right. I profiled it and saw that getStackLineInfo went from 144ms to 373ms (with 209ms is inside this function). I think the easiest option is to inline this into this function so we don't allocate though. I tried that and it went to 186ms. Still worse than 144ms but definitely better than before. What do you think?

Profiles:
main
non-inlined
inlined

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