Move symbol table demangling out of SymbolStore into SymbolProvider#5746
Move symbol table demangling out of SymbolStore into SymbolProvider#5746mstange merged 7 commits intofirefox-devtools:mainfrom
Conversation
a1e4db0 to
568be0a
Compare
Codecov Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
568be0a to
a66fd2c
Compare
| // 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', |
There was a problem hiding this comment.
Ah I was looking at the previous commits and was going to suggest moving the name here, but looks like you already did it :)
src/actions/receive-profile.ts
Outdated
| if (typeof dbNamePrefixOrDB === 'string') { | ||
| this._symbolDb = new SymbolStoreDB(`${dbNamePrefixOrDB}-symbol-tables`); | ||
| } else { | ||
| this._symbolDb = dbNamePrefixOrDB; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Oh, no, we are definitely not intending to use this in the future. I'll add a commit to make it a string.
src/actions/receive-profile.ts
Outdated
| async closeDb() { | ||
| await this._symbolDb.close(); | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
…ses to the SymbolStore it creates.
…'t in the database.
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.
a66fd2c to
3f43d12
Compare
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.
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
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:
After: