1538061 - Stack sampling sometimes overshoots the PROFILER_INIT block

Status: NEW

(Core :: Gecko Profiler, enhancement, P3)

Description

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

Seen on Windows, haven't checked others OSes.

In the Flame Graph, I would expect the first node above "(root)" to always be the same, as it's the first AUTO_PROFILER_LABEL created right after AUTO_PROFILER_INIT.
However, lots of stacks also include parent nodes all the way to RtlUserThreadStart.
E.g.: https://perfht.ml/2Ydq7qI

Glancing at the code, stack walkers seem to compare pointers to the stack pointer recorded during INIT, maybe that's not enough to guarantee proper walk termination?

In any case, MergeStacks() should be able to always cut stacks at the correct spot, when it hits the known initial label. (Though as this label is not currently guaranteed to be there and with the same name for different processes, we should instead force a guaranteed root label that's always added by INIT, assuming that's possible).

It would still be nice to fix the walkers as well if possible, to save a bit of overhead wasted in exploring and storing unwanted frames. This could be a follow-up bug if we only patch MergeStacks() first.

Comment 1

[Florian Quèze [:florian]]

(In reply to Gerald Squelart [:gerald] from comment #0)

In the Flame Graph, I would expect the first node above "(root)" to always be the same, as it's the first AUTO_PROFILER_LABEL created right after AUTO_PROFILER_INIT.
However, lots of stacks also include parent nodes all the way to RtlUserThreadStart.
E.g.: https://perfht.ml/2Ydq7qI

My impression when looking at this profile is that the samples that don't contain RtlUserThreadStart are samples where we only have labels and JS frames, so maybe the stack walking failed completely in them?

Comment 2

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

(In reply to Florian Quèze [:florian] from comment #1)

My impression when looking at this profile is that the samples that don't contain RtlUserThreadStart are samples where we only have labels and JS frames, so maybe the stack walking failed completely in them?

Hmm, interesting, so we actually have two issues here:

1. Stack walking can fail, so we only see labels and JS frames. Could this be bug 1434402?
2. Stack walking overshoots the PROFILER_INIT call. (Or am I wrong in my assumption?) I would like to focus on this in this bug.

One thought:
If we had a strict "root label" that the profiler could assume was always there, it could be used to ensure that profiles always have the exact same common root, which would produce more consistent stacks (in cases where stack walking goes wrong).
What do you think?

Comment 3

[Florian Quèze [:florian]]

(In reply to Gerald Squelart [:gerald] from comment #2)

1. Stack walking can fail, so we only see labels and JS frames. Could this be bug 1434402?

Could it just be the impact of https://searchfox.org/mozilla-central/rev/6dab6dad9cc852011a14275a8b2c2c03ed7600a7/mozglue/misc/StackWalk.cpp#101-116 ?