Skip to content

feat: Taskdump viewer & expand inline frames in flamegraphs#377

Merged
rcoh merged 3 commits into
taskdumps-integratinfrom
taskdump-viewer
May 8, 2026
Merged

feat: Taskdump viewer & expand inline frames in flamegraphs#377
rcoh merged 3 commits into
taskdumps-integratinfrom
taskdump-viewer

Conversation

@rcoh
Copy link
Copy Markdown
Contributor

@rcoh rcoh commented May 8, 2026

I am pretty stoked about this it came out really nice.

when the idle time has a crosshatch, it means we have a task dump for that period
Screenshot 2026-05-08 at 2 07 29 PM

when you click it, we render it as a flamegraph (there can be multiple futures that you are waiting on)

Screenshot 2026-05-08 at 2 07 39 PM

You can also see a task-flamegraph for the entire task

rcoh added 2 commits May 8, 2026 17:35
tokio's `taskdump` feature only compiles on Linux (aarch64/x86/x86_64).
The unconditional [dependencies] entry was causing macOS CI builds to fail.
Move it to the linux-only target section where it already existed.
@rcoh rcoh force-pushed the taskdump-viewer branch from 85e1e89 to a24b141 Compare May 8, 2026 18:28
Copy link
Copy Markdown
Member

@jlizen jlizen left a comment

Choose a reason for hiding this comment

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

New interface looks great, just some small things.

Consider adding unit tests for TaskDumpEvent parsing and the frame expansion logic.

callframeSymbols: mapToEntries(trace.callframeSymbols),
threadNames: mapToEntries(trace.threadNames),
runtimeWorkers: mapToEntries(trace.runtimeWorkers),
taskDumps: mapToEntries(trace.taskDumps),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Bit surprised not to see any changes to steering, i would have expected the schema to change at least?

Np if planned as a follow up.

self: 0,
});
// Expand inlined frames: an array entry means multiple frames at one address
const frames = Array.isArray(entry) ? entry : [entry];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it just me or is this reversing the ordering of the flamegraphs, to put the outermost deepest? That seems unintuitive if so.

for (let fi = frames.length - 1; fi >= 0; fi--) {
const resolved = frames[fi];
const key = resolved ? resolved.symbol : addr || "??";
const formatted = resolved ? formatFrame(resolved) : formatFrame(addr, callframeSymbols);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems more fragile than the old way since frames might be [null].

We could do something like:

Array.isArray(entry) ? entry[0] : entry.

Comment thread dial9-viewer/ui/viewer.html Outdated
// Check if clicking an idle region with task dumps
for (const r of taskDetailHitRegions) {
if (mx >= r.x1 && mx <= r.x2 && my >= r.y1 && my <= r.y2 && r.taskDumps) {
showTaskDumpStack(r.taskDumps);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

probably should be in a try/catch similar to showIdleTimeFlamegraph?

Comment thread dial9-viewer/ui/viewer.html Outdated
const fgContainer = document.getElementById("fg-container");
const fgInstance = FlamegraphRenderer.createFlamegraph(fgContainer);

function showTaskDumpStack(dumps) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

there is a lot of redundant sidebar code between showTaskDumpStack and showIdleTimeFlamegraph, should we consolidate?

Also consider extracting the dump UI logic into a separate module.

ctx.strokeStyle = "#444";

// Cross-hatch pattern for idle periods with task dumps
if (hasDump) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

feels like we'll want this in a helper sooner or later

@rcoh rcoh changed the title Taskdump viewer feat: Taskdump viewer & expand inline frames in flamegraphs May 8, 2026
1. Fix inlined-frame iteration order in buildFlamegraphTree. Per blazesym,
   an array entry in callframeSymbols is [outermost, ..., innermost] — the
   real function at the address is at [0] and [i>0] are inlined callees. The
   previous implementation iterated N→0, which inverted the call graph in the
   flamegraph (inner inlined functions appeared as parents of the outer
   function they were inlined into). Now iterates 0→N, matching caller→callee.
   Skip nullish slots so sparse arrays (from out-of-order SymbolTableEntry
   events) no longer produce phantom (unknown) tree levels.

2. Wrap showTaskDumpStack click in try/catch matching the
   showIdleTimeFlamegraph pattern — errors now surface as a toast instead of
   silently failing.

3. Extract renderFlamegraphInSidebar({title, subtitle, samples}) helper.
   Both showTaskDumpStack and showIdleTimeFlamegraph now delegate to it,
   removing ~40 lines of duplicated sidebar setup.

4. Extract drawCrossHatch(x, y, w, h) helper for the diagonal stripe pattern
   used on idle periods that have a task dump.

5. Document taskDumps in the ParsedTrace schema
   (dial9-trace-loading/SKILL.md) so agent skills can find it. Extend the
   schema validator to understand Map<K, [{obj}]> (array-of-objects map
   values).

6. Add unit tests in test_trace_analysis.js covering:
   - Flamegraph inline frame ordering (catches the bug fixed in #1)
   - Sparse inline arrays with undefined slots
   - Unresolved address handling
   - TaskDumpEvent parsing shape, sort order, task-id integrity
@rcoh rcoh merged commit 38ad4df into taskdumps-integratin May 8, 2026
17 checks passed
rcoh added a commit that referenced this pull request May 8, 2026
1. Fix inlined-frame iteration order in buildFlamegraphTree. Per blazesym,
   an array entry in callframeSymbols is [outermost, ..., innermost] — the
   real function at the address is at [0] and [i>0] are inlined callees. The
   previous implementation iterated N→0, which inverted the call graph in the
   flamegraph (inner inlined functions appeared as parents of the outer
   function they were inlined into). Now iterates 0→N, matching caller→callee.
   Skip nullish slots so sparse arrays (from out-of-order SymbolTableEntry
   events) no longer produce phantom (unknown) tree levels.

2. Wrap showTaskDumpStack click in try/catch matching the
   showIdleTimeFlamegraph pattern — errors now surface as a toast instead of
   silently failing.

3. Extract renderFlamegraphInSidebar({title, subtitle, samples}) helper.
   Both showTaskDumpStack and showIdleTimeFlamegraph now delegate to it,
   removing ~40 lines of duplicated sidebar setup.

4. Extract drawCrossHatch(x, y, w, h) helper for the diagonal stripe pattern
   used on idle periods that have a task dump.

5. Document taskDumps in the ParsedTrace schema
   (dial9-trace-loading/SKILL.md) so agent skills can find it. Extend the
   schema validator to understand Map<K, [{obj}]> (array-of-objects map
   values).

6. Add unit tests in test_trace_analysis.js covering:
   - Flamegraph inline frame ordering (catches the bug fixed in #1)
   - Sparse inline arrays with undefined slots
   - Unresolved address handling
   - TaskDumpEvent parsing shape, sort order, task-id integrity
noxware pushed a commit to noxware/dial9-tokio-telemetry that referenced this pull request May 9, 2026
…#378)

* add: TaskDumps UI to viewer

* Address PR dial9-rs#377 review feedback

1. Fix inlined-frame iteration order in buildFlamegraphTree. Per blazesym,
   an array entry in callframeSymbols is [outermost, ..., innermost] — the
   real function at the address is at [0] and [i>0] are inlined callees. The
   previous implementation iterated N→0, which inverted the call graph in the
   flamegraph (inner inlined functions appeared as parents of the outer
   function they were inlined into). Now iterates 0→N, matching caller→callee.
   Skip nullish slots so sparse arrays (from out-of-order SymbolTableEntry
   events) no longer produce phantom (unknown) tree levels.

2. Wrap showTaskDumpStack click in try/catch matching the
   showIdleTimeFlamegraph pattern — errors now surface as a toast instead of
   silently failing.

3. Extract renderFlamegraphInSidebar({title, subtitle, samples}) helper.
   Both showTaskDumpStack and showIdleTimeFlamegraph now delegate to it,
   removing ~40 lines of duplicated sidebar setup.

4. Extract drawCrossHatch(x, y, w, h) helper for the diagonal stripe pattern
   used on idle periods that have a task dump.

5. Document taskDumps in the ParsedTrace schema
   (dial9-trace-loading/SKILL.md) so agent skills can find it. Extend the
   schema validator to understand Map<K, [{obj}]> (array-of-objects map
   values).

6. Add unit tests in test_trace_analysis.js covering:
   - Flamegraph inline frame ordering (catches the bug fixed in dial9-rs#1)
   - Sparse inline arrays with undefined slots
   - Unresolved address handling
   - TaskDumpEvent parsing shape, sort order, task-id integrity
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