Repository navigation
profiler-cli: include per-marker fields and data in --list --json - #6275
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6275 +/- ##
==========================================
+ Coverage 84.03% 84.08% +0.04%
==========================================
Files 354 354
Lines 38100 38287 +187
Branches 10662 10744 +82
==========================================
+ Hits 32018 32194 +176
- Misses 5654 5665 +11
Partials 428 428 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| export type MarkerFieldValue = { | ||
| key: string; | ||
| label: string; | ||
| value: any; // Raw, with string-table indexes already resolved |
There was a problem hiding this comment.
This is a slightly incorrect claim because collectMarkerFields pushes the unresolved payload value, so on the same --list row fields[].value is a numeric index while data[key] is the resolved string for unique-string/flow-id/terminating-flow-id fields.
We either need to resolve once (via isStringIndexMarkerField) and derive both fields and data from that, or change the comment.
| FlatMarkerItem: | ||
| { | ||
| handle, name, label, duration?, hasStack, category, | ||
| start, // absolute ms ("startTime" in thread-network) |
There was a problem hiding this comment.
start here is marker.start - zeroAt (profile-relative), and thread network's startTime is the unshifted marker.start.
Better: "ms since profile start"?
| markerType: marker.data?.type, | ||
| fields: collectMarkerFields(marker, markerSchemaByName, stringTable), | ||
| data, | ||
| innerWindowID: |
There was a problem hiding this comment.
Top-level innerWindowID duplicates data.innerWindowID on the same row. Is there a consumer that needs it hoisted? There is also getInnerWindowID(data) helper in page-load.ts.
| continue; | ||
| } | ||
| const resolved = | ||
| stringIndexFields?.includes(key) && typeof value === 'number' |
There was a problem hiding this comment.
Could we reuse isStringIndexMarkerField from marker-schema.ts here? (already imported)
| const m = result.flatMarkers![0]; | ||
| // Guard against the fixture silently storing a string: if `url` were not an | ||
| // index, resolution would be a no-op and this test would prove nothing. | ||
| expect(typeof urlIdx).toBe('number'); |
There was a problem hiding this comment.
This can't fail since indexForString always returns a number. Maybe worth correcting the comment too.
| * Build a profile with one CompositorScreenshot marker whose `url` is a real | ||
| * index into the string table, rather than an already-resolved string. | ||
| * | ||
| * `getProfileWithMarkers` stores payload values verbatim, so passing a string |
There was a problem hiding this comment.
The claim isn't accurate - it converts unique-string fields to string indexes for any schema in markerSchemaForTests. The URL survives only because there's no CompositorScreenshot schema in the fixtures. Worth rewording.
baa4446 to
a063f5e
Compare
fatadel
left a comment
There was a problem hiding this comment.
Thanks for addressing the issues! Two minor things left:
collectMarkerFieldsis also used by the existingmarker info --jsoncommand but we are changing its output here (number becoming string). We should at least document it, if it's intentional.- Stall PR description: it still says rows include top-level
innerWindowID, but that field was removed and is now available only underdata.
`thread markers --list --json` could filter on a payload field
(`--search eventType:keydown`) without returning it, so reading the values back
meant one `marker info` call per marker. Rows now carry `markerType`, `fields`
and `data`.
`fields[].value` reports unique-string, flow-id and terminating-flow-id fields
as the resolved string rather than the string table index, so it agrees with
`data` for the same key. `collectMarkerFields` is shared, so this changes those
values in `marker info --json` too; schemas.txt documents it.
`data` elides a CompositorScreenshot's base64 JPEG to
`{ elided: true, length, preview }`, which keeps a 100-row response at 27KB
instead of 940KB. Elision is keyed by field identity rather than size: a
screenshot URL and a real `Preference Read.prefValue` measured 2855 and 2939
chars, so no byte cap separates them.
`profile markers` rows are typed as `FlatMarkerItem` plus thread identity, so
they gain the same three keys.
a063f5e to
7956aa5
Compare
Changes: [fatadel] Show a category breakdown in profiler-cli (#6256) [fatadel] Show allocation data in the cli (#6246) [Florian Quèze] profiler-cli: name the process in the thread banner, and follow the queried thread (#6269) [fatadel] Drive PII sanitization from marker schemas (#6291) [fatadel] Move FileIO table labels into marker schemas (#6296) [Florian Quèze] profiler-cli: add a thread list command (#6273) [Nazım Can Altınova] Update oxfmt 0.63.0 → 0.68.0 (major) (#6316) [Markus Stange] Add missing finishRawMarkerTableBuilder. (#6318) [Markus Stange] Fix the upload error button contrast (#6315) [Markus Stange] Allow typed arrays in the nativeSymbols table (#6300) [Markus Stange] Remove brittle test. (#6322) [Markus Stange] Fix file extension typo. (#6325) [Nazım Can Altınova] Add a permalink command to profiler-cli for already published profiles (#6331) [Markus Stange] Typed arrays + flags for the FuncTable (#6323) [Florian Quèze] profiler-cli: add profile markers for cross-thread marker search (#6276) [fatadel] Convert extension text markers to structured payloads (#6314) [Florian Quèze] profiler-cli: include per-marker fields and data in --list --json (#6275) [Nazım Can Altınova] Add `--with-samply` to "profiler-cli load" (#6339) [Markus Stange] Speed up call tree sidebar by sharing work with the activity graph (#6329) [Nazım Can Altınova] 🔃 Sync: l10n -> main (Sept 22, 2026) (#6341) [Nazım Can Altınova] Bump profiler-cli version to 0.10.0 (#6342) And special thanks to our localizers: el: Jim Spentzos ro: robbp
Main | Deploy preview
thread markers --list --jsoncould filter on a payload field (--search eventType:keydown) without returning it, so reading the values back meant onemarker infocall per marker. Rows now carrymarkerType,fields,dataandinnerWindowID; no existing key changes value.dataelides a CompositorScreenshot's base64 JPEG to{ elided: true, length, preview }, which keeps a 100-row response at 27KB instead of 940KB. Elision is keyed by field identity rather than size: a screenshot URL and a realPreference Read.prefValuemeasured 2855 and 2939 chars, so no byte cap separates them.