Add skipped sample as a separate track #3255
Conversation
haixiaosugoogle
commented
Oct 10, 2025
haixiaosugoogle
commented
| private static showSkippedPerfSamples: Setting<boolean>; | ||
|
|
||
| static onActivate(app: App): void { | ||
| const boolSettingDesc: SettingDescriptor<boolean> = { |
There was a problem hiding this comment.
When we discussed flagging, I meant feature_flags.ts, not settings.
The settings are toggles for end users that we expect to keep around, while flags are for things that are still in development or where the UX is not finalised (which is the case here).
However I'm also no longer insisting on this being flagged. We can always roll back if the tracks prove surprising.
| .getGroupForProcess(upid); | ||
| const summaryTrack = new TrackNode({ | ||
| uri, | ||
| name: `Process callstacks (skipped)`, |
There was a problem hiding this comment.
Still not a fan of "skipped" because:
- it's not clear to users what is meant by a skipped sample (is it an error?)
- it's completely valid to do sampling of counters without callstacks, in which case all samples will appear "skipped".
I'm struggling to suggest a better name. How about "Unclassified perf samples"?
| sessionId?: number, | ||
| ) { | ||
| const constraints = []; | ||
| if (upid !== undefined) { |
There was a problem hiding this comment.
There's quite a bit of copypasted code that is never used. I've started a more minimal implementation here (including a different visual styling to avoid confusion with callstacks). Can we similarly cut down the unnecessary code in this patch?
