Add a --symbolicate-wasm arg to profiler-edit.#6008
Conversation
52ba22e to
ab1e4b6
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6008 +/- ##
==========================================
- Coverage 83.82% 83.79% -0.03%
==========================================
Files 328 329 +1
Lines 34255 34395 +140
Branches 9572 9618 +46
==========================================
+ Hits 28713 28821 +108
- Misses 5114 5145 +31
- Partials 428 429 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0202adf to
be15b28
Compare
a70577c to
a8608c4
Compare
canova
left a comment
There was a problem hiding this comment.
Nice, thanks for adding some tests! I added a bunch of nits to make things a bit clearer, but otherwise I don't see any issues.
This also made me look into the wasm spec a bit and learn more about it, so I think it was a good use of time 😄
This also made me think more about the source map support that I've been working on. These 2 things seem like solving it for 2 different use cases, so it seems very interesting. I didn't know about the name section in the wasm spec. I wonder how devtools handles it (or if it does). It looks like it's mostly looking at the dwarf info for now, but need to look deeper.
| const symbolicateWasm: WasmSymbolicationCliSpec[] = []; | ||
| const rawWasmArg = argv['symbolicate-wasm']; | ||
| let wasmArgs: unknown[]; | ||
| if (rawWasmArg === undefined) { | ||
| wasmArgs = []; | ||
| } else if (Array.isArray(rawWasmArg)) { | ||
| wasmArgs = rawWasmArg; | ||
| } else { | ||
| wasmArgs = [rawWasmArg]; | ||
| } | ||
| for (const arg of wasmArgs) { | ||
| if (typeof arg !== 'string' || arg === '') { | ||
| throw new Error('--symbolicate-wasm requires a value'); | ||
| } | ||
| // Accept "<url>=<path>" if the LHS looks like a URL, otherwise treat the | ||
| // whole string as a path and infer the URL from the profile. Split on | ||
| // the last `=` so URLs containing `=` (e.g. in query strings) survive | ||
| // intact; this assumes file paths don't contain `=`. | ||
| const eqIndex = arg.lastIndexOf('='); | ||
| if (eqIndex !== -1 && /^[a-z]+:\/\//i.test(arg.slice(0, eqIndex))) { | ||
| symbolicateWasm.push({ | ||
| url: arg.slice(0, eqIndex), | ||
| path: arg.slice(eqIndex + 1), | ||
| }); | ||
| } else { | ||
| symbolicateWasm.push({ path: arg }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Not for this PR, but now that we have commander in the package.json, we can possibly simplify these things by using it.
| // Parses the function-name subsection of a wasm "name" custom section and | ||
| // returns a map from function index to name. Returns an empty map if the | ||
| // module has no name section. The function index space includes imports | ||
| // (imports come first) — same numbering used in `wasm-function[N]` strings. |
There was a problem hiding this comment.
Oh can we add a link to the Firefox source code where we generate this N number?
There was a problem hiding this comment.
a8608c4 to
f845b5b
Compare
This allows applying wasm symbols to existing profiles that were captured with a stripped wasm bundle. The script looks for functions with names of the shape `wasm-function[123]`, which is what Firefox uses when the wasm file doesn't have a names section. Usage: ``` yarn build-node-tools && \ node node-tools-dist/profiler-edit.js -i input.json.gz \ --symbolicate-wasm http://host/a.wasm=./a-unstripped.wasm \ --symbolicate-wasm http://host/b.wasm=./b-unstripped.wasm \ -o out.json.gz ```
f845b5b to
b4054d5
Compare
This allows applying wasm symbols to existing profiles that were captured with a stripped wasm bundle.
The script looks for functions with names of the shape
wasm-function[123], which is what Firefox uses when the wasm file doesn't have a names section.Usage:
I've successfully used it to turn https://share.firefox.dev/4f9O7oh into https://share.firefox.dev/4nd6GKk .