puffin icon indicating copy to clipboard operation
puffin copied to clipboard

Is it sensible to sum overlapping thread scopes?

Open MarijnS95 opened this issue 4 years ago • 1 comments

Is your feature request related to a problem? Please describe.

While visualizing delays from an identical starting point - effectively overlapping each other - through scopes manually added to puffin::Stream, the resulting scopes get summed and show a prolonged track. This is an example registering the same 500ms child workload - starting at 0 - three times:

image

Likewise we have GPU workloads where the next command buffer starts running ahead of the previous one completing. Here too - albeit with different names - their entire track gets prolonged to fit every item on the line, even when it exceeds the parent Context `frame 0` Command buffer `2` parent scope despite setting explicit start and end timings for a scope.

image

(I take no responsibility for three different pipelines in the same frame having both a space, hyphen, and underscore :rofl:)

That's done by: https://github.com/EmbarkStudios/puffin/blob/17d0429bcca5f8b398378918b38d109a4c1caf9d/puffin/src/merge.rs#L128-L133

This is somewhat related to GPU profiling in #59.

Describe the solution you'd like I expected either a panic/Err() because of submitting invalid data through the puffin::global_reporter, and not initially knowing that - presumably - profiling submitted for "threads" (a CPU thread in the literal sense) is assumed to run serially. Ie. if the start of the next sibling scope lies before the end of the current, that should be an error?

Describe alternatives you've considered

It'd be great if puffin could somehow visualize these overlapping scopes, maybe a color or pattern to display overdraw? Displaying on multiple tracks is bound to be tricky, hard to see, and pretty much breaks the "flamegraph" concept. Perhaps a different waterfall view like Radeon Graphics Profiler could be considered? This may need a different kind of "profiling mode" to allow such kind of overlaps though.

MarijnS95 avatar Jan 24 '22 14:01 MarijnS95

I agree that returning an error here makes sense. I don't think puffin should return early though but instead let the user know the data is invalid after the fact.

A radeon gpu profiler like view makes sense as well. Definitely open for contributions for that even if its just for egui and not puffin_viewer!

VZout avatar Jan 25 '22 09:01 VZout