1596922 - Consider not stack walking and merging stacks if the sample is idle a second time

Status: NEW

(Core :: Gecko Profiler, enhancement, P3)

Description

[Greg Tatum [:gregtatum]]

A stackwalk for the first idleness sample is somewhat interesting since it shows where code is being executed, and how it got into the idleness state. However, it's not interesting after that, and wastes lots of CPU to collect a stackwalk and merge the stacks. We should consider not continuing to collect a full stackwalk if the frame label's stack length is the same (assuming that's easy to get at), and if the current and previous frame label has an idle category. This could be a very quick check to end the sample early, and reduce wasteful stack collection. Profiling the profiler shows that the stack collection and merging is where a lot of overhead comes from. Another benefit is in memory usage, as the sample could be much smaller when stored in the buffer, and reduce the memory usage.

One risk is that the first idleness sample would roll off the buffer, but maybe that's ok? Another option is to just not record idleness stackwalks.

I'd like to talk this out realtime, but I wanted to capture it before I forgot.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

First, did you know that we already have a system to avoid doing stackwalks...
https://searchfox.org/mozilla-central/rev/492214c05cde6e6db5feff9465ece4920400acc3/tools/profiler/core/platform.cpp#2840-2851
... when the profiler is notified that the code is going to sleep, e.g.:
https://searchfox.org/mozilla-central/rev/492214c05cde6e6db5feff9465ece4920400acc3/xpcom/threads/nsThreadPool.cpp#282

Of course, this may not catch all situations where there CPU is idle, if we don't mark them or they happen outside of our code; are you trying to make this more automatic?

(In reply to Greg Tatum [:gregtatum] from comment #0)

A stackwalk for the first idleness sample is somewhat interesting since it shows where code is being executed, and how it got into the idleness state. However, it's not interesting after that, and wastes lots of CPU to collect a stackwalk and merge the stacks. We should consider not continuing to collect a full stackwalk if the frame label's stack length is the same (assuming that's easy to get at),

During stackwalking we know the address of each stack frame, and also of labels, so I think it could be used as "stack length". However I don't think it would be wise to assume that the stack is still the same!
E.g., if we had the following function calls: A -> B -> X and you're sampling at X (being marked as idle), and the next time we had A -> C -> X with C using exactly as much stack as B, we would wrongly assume nothing changed.

and if the current and previous frame label has an idle category.

Now this is a good idea, we could possibly do something similar to AUTO_PROFILER_THREAD_SLEEP when entering/exiting an IDLE label.
(This doesn't suffer from the ABX/ACX issue, because separate AUTO_PROFILER_THREAD_SLEEPs reset the profiler's "awake" flag as necessary.)

This could be a very quick check to end the sample early, and reduce wasteful stack collection. Profiling the profiler shows that the stack collection and merging is where a lot of overhead comes from. Another benefit is in memory usage, as the sample could be much smaller when stored in the buffer, and reduce the memory usage.

One risk is that the first idleness sample would roll off the buffer, but maybe that's ok? Another option is to just not record idleness stackwalks.

The existing system duplicates the stack if it's still in the buffer, and falls back to a real stackwalk otherwise, so that case is covered.

I'd like to talk this out realtime, but I wanted to capture it before I forgot.

NI:Greg, please let me know if I've been blinded by AUTO_PROFILER_THREAD_SLEEP and you were proposing something completely different!
I'd be happy to chat in any case; and automatically marking IDLE labels would be something to explore anyway.

Comment 2

[Greg Tatum [:gregtatum]]

Ah thanks for the additional info. I'd seen stack copying, but hadn't looked into the AUTO_PROFILER_THREAD_SLEEP mechanism. I'll dig into it a bit more.