Skip to content

Move symbol table demangling out of SymbolStore into SymbolProvider#5746

Merged
mstange merged 7 commits intofirefox-devtools:mainfrom
mstange:move-db-access
Jan 9, 2026
Merged

Move symbol table demangling out of SymbolStore into SymbolProvider#5746
mstange merged 7 commits intofirefox-devtools:mainfrom
mstange:move-db-access

Conversation

@mstange
Copy link
Contributor

@mstange mstange commented Jan 6, 2026

We have a WASM module for symbol demangling. However, usually we don't actually make any use of it: Usually, we get symbols via the symbolication API, either from the server or from the browser. And in the JSON API response, the symbols are already demangled.

The only time when we need to do the demangling in the front-end is if we fail to get symbols via the API and instead get a full symbol table from the browser. This is a rare case. With modern versions of Firefox (anything newer than Firefox 95), we only hit this case if we capture a profile from Firefox for Android (via about:debugging) and get symbols for native Android libraries. (There's one other case: Sometimes the symbolication API query runs out of memory when you have a local Firefox build with a big symbol table, so then the browser symbolication API fails but getting the symbol table succeeds.)

This PR is an attempt to make it so that we only load the demangling module when we run the symbolication code in a browser context.

Today, symbolicator-cli bundles the unused demangling wasm module. This PR moves the wasm dependency into receive-profile.ts which isn't used by symbolicator-cli, so the symbolicator-cli build will no longer bundle it.


Before:

% yarn build-symbolicator-cli && ls -l dist
[...]
built modules 1.17 MiB (javascript) 511 KiB (webassembly) 1.72 KiB (asset) [built]
[...]
-rw-r--r--@ 1 mstange  staff  522947 Jan  6 10:42 158fdaedb0d7e61a0391.module.wasm
-rw-r--r--@ 1 mstange  staff     842 Jan  6 10:42 1ddbc3ce40af8c7a648b.svg
-rw-r--r--@ 1 mstange  staff    1555 Jan  6 10:42 307.symbolicator-cli.js
-rw-r--r--@ 1 mstange  staff   45110 Jan  6 10:42 461.symbolicator-cli.js
-rw-r--r--@ 1 mstange  staff   31575 Jan  6 10:42 957.symbolicator-cli.js
-rw-r--r--@ 1 mstange  staff     720 Jan  6 10:42 957.symbolicator-cli.js.LICENSE.txt
-rw-r--r--@ 1 mstange  staff     916 Jan  6 10:42 d6df0017c0241dfe86ff.svg
-rw-r--r--@ 1 mstange  staff  142051 Jan  6 10:42 symbolicator-cli.js

After:

% yarn build-symbolicator-cli && ls -l dist
[...]
built modules 1.15 MiB (javascript) 1.72 KiB (asset) [built]
[...]
-rw-r--r--@ 1 mstange  staff     842 Jan  6 10:41 1ddbc3ce40af8c7a648b.svg
-rw-r--r--@ 1 mstange  staff     916 Jan  6 10:41 d6df0017c0241dfe86ff.svg
-rw-r--r--@ 1 mstange  staff  213276 Jan  6 10:41 symbolicator-cli.js

@mstange mstange requested a review from canova January 6, 2026 09:59
@mstange mstange self-assigned this Jan 6, 2026
@codecov
Copy link

codecov bot commented Jan 6, 2026

Codecov Report

❌ Patch coverage is 84.81013% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.67%. Comparing base (1a808fe) to head (3f43d12).
⚠️ Report is 50 commits behind head on main.

Files with missing lines Patch % Lines
src/actions/receive-profile.ts 80.35% 11 Missing ⚠️
src/symbolicator-cli/index.ts 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5746      +/-   ##
==========================================
+ Coverage   85.64%   85.67%   +0.02%     
==========================================
  Files         313      314       +1     
  Lines       31013    31106      +93     
  Branches     8535     8558      +23     
==========================================
+ Hits        26562    26649      +87     
- Misses       4021     4027       +6     
  Partials      430      430              

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

Copy link
Member

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

Looks good to me, thanks!

// Note, the database name still references the old project name, "perf.html". It was
// left the same as to not invalidate user's information.
const symbolProvider = new RegularSymbolProvider(
'perf-html-async-storage',
Copy link
Member

Choose a reason for hiding this comment

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

Ah I was looking at the previous commits and was going to suggest moving the name here, but looks like you already did it :)

if (typeof dbNamePrefixOrDB === 'string') {
this._symbolDb = new SymbolStoreDB(`${dbNamePrefixOrDB}-symbol-tables`);
} else {
this._symbolDb = dbNamePrefixOrDB;
Copy link
Member

Choose a reason for hiding this comment

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

Do we ever use this path where we pass a ISymbolStoreDB (or intend to use in the future)? I can't find any use cases for it. So maybe we can convert dbNamePrefixOrDB to a string only and simplify this logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, no, we are definitely not intending to use this in the future. I'll add a commit to make it a string.

Comment on lines 625 to 627
async closeDb() {
await this._symbolDb.close();
}
Copy link
Member

Choose a reason for hiding this comment

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

I can't find any calls to closeDb. Is this expected? Where do we close the DB or do we keep it open all the time?

Hmm, looking at the old code, I can't find any callers to it either. So it's not new at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, this was used by tests, but I removed all those tests. Our tests don't really exercise the caching logic any more because I moved it into the RegularSymbolProvider which isn't used in the tests. I'll remove closeDb as well then.

…k cached symbol tables only after API requests come back with no info.
This removes the demangling dependency from symbol-store.ts.
It also lets us remove the unused "database" from symbolicator-cli.

The cache only exists so that we don't give the browser unnecessary
work to recompute the symbol table. So we only need to access the cache
if we're about to ask the browser for a full symbol table.
@mstange mstange enabled auto-merge January 9, 2026 16:26
@mstange mstange merged commit 45bf912 into firefox-devtools:main Jan 9, 2026
19 checks passed
mstange added a commit that referenced this pull request Jan 9, 2026
With this and #5746 combined, `yarn build-symbolicator-cli` will only
produce a single output file. Without this PR, it also produced two SVG
files.
@canova canova mentioned this pull request Jan 27, 2026
canova added a commit that referenced this pull request Jan 27, 2026
Lots of exciting changes 🎉:

[arai-a] Put radio buttons into labels (#5738)
[DaniPopes] Update comment for "unique-string" (#5741)
[Karan Pradhan] Hide tooltip filter button in non-sticky tooltips and
add hideFilterButton tests (#5718)
[arai-a] Add a menu to copy the Marker Table as text (#5732)
[arai-a] Make the entire list item clickable for the "Full Range"
(#5742)
[Markus Stange] Move symbol table demangling out of SymbolStore into
SymbolProvider (#5746)
[Markus Stange] Remove SVG asset imports from profile-data.ts (#5747)
[arai-a] Do not apply sticky tooltip on double click (#5754)
[arai-a] Skip the ChartCanvas redraw on the Viewport's internal default
state usage (#5744)
[Markus Stange] Stop blindly extracting uint8array.buffer after calling
compress() (#5753)
[Markus Stange] In the assembly view state, refer to the current symbol
by index (#5755)
[Markus Stange] Fix "scroll to hotspot" functionality in the source view
+ assembly view (#5759)
[Markus Stange] Keep the colorField markerSchema field when processing
profiles in the gecko format (#5760)
[Markus Stange] Implement dark mode (#5740)
[Markus Stange] Fix light-mode colors (#5765)
[Markus Stange] Tweak dark mode colours. (#5767)
[Nazım Can Altınova] Enable some basic type-aware lints (#5775)
[Markus Stange] Allow seeing different assembly code for the same
function (#5349)
[fatadel] Refine tree view a11y (#5779)
[fatadel] Align double-click behavior of stack chart with flame graph
(#5782)
[Markus Stange] Split gz.ts properly into node and browser variants
(#5764)
[Markus Stange] Simplify and optimize the computation of per-call-node
line and address timings (#5770)
[Nazım Can Altınova] Move the dark mode toggle to devtools console
(#5783)
[Nazım Can Altınova] 🔃 Sync: l10n -> main (Jan 27, 2026) (#5785)
[Nazım Can Altınova] Improve Chrome importer marker payload logic
(#5717)
[Markus Stange] Add a Focus Self transform (#5774)
[Nazım Can Altınova] Enable the Turkish locale in production (#5786)


And huge thanks to our localizers 🎉 :

be: Mikalai Udodau
de: Ger
de: Michael Köhler
el: Jim Spentzos
en-CA: chutten
en-CA: Saurabh
en-GB: Ian Neal
en-GB: Saurabh
es-CL: ravmn
fy-NL, nl: Fjoerfoks
fr: Skywarp
fr: Théo Chevalier
fur: Fabio Tomat
fy-NL: Fjoerfoks
ia: Melo46
it: Francesco Lodolo [:flod]
nl: Fjoerfoks
nl: Mark Heijl
pt-BR: Marcelo Ghelman
ru: berry
ru: Valery Ledovskoy
sv-SE: Andreas Pettersson
tr: Grk
zh-CN: Olvcpr423
zh-CN: wxie
zh-TW: Pin-guang Chen
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