1730985 - Consider putting information into the profile which annotates which addresses are return addresses

Status: NEW

(Core :: Gecko Profiler, enhancement, P3)

Description

[Markus Stange [:mstange]]

The Gecko profiler records raw stack frame addresses in the profile JSON. These addresses can come from two different sources:

• The instruction pointer, and
• Stack walking, which records return addresses.

The instruction pointer is how we get the topmost stack frame, and the rest of the frames are return addresses.
However, once these addresses are stored in the profile JSON, we have forgotten where we got them from.

The profiler front-end needs to treat return addresses different from "instruction pointer" values when looking up symbols and when calculating per-instruction costs. See bug 1642516 comment 0 for background.

How can it know which addresses are return addresses?

It now makes the following assumption:

• Only the top stack frame of "sample" stacks came from the instruction pointer.
• All other addresses are return addresses. This includes the top stack frame from synchronous backtrace stacks (marker cause stacks, allocation stacks).

Julien suggested that this assumption may be fragile and that we may want to encode it into the profile JSON:

I mean: this is probably right in the current code, but how do we ensure this stays right in the future? Should we add comment in some strategic places in the cpp profiler code? Should we have something in the meta properties that says that the stacks in that table have that property?

meta: {
  ...
  stacksNudging: {
    // properties with the same name than the tables
    samples: "all-but-last", // or "all-but-top"
    markers: "all",
    jsAllocations: "all",
    nativeAllocations: "all",
  },
}

Comment 1

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

First, I think the assumption is fine, at least currently:

• Only "samples" contain stacks that were collected by another thread (the Sampler thread), which suspends the target thread, reads its current registers including PC and SP, and then does the stack sampling from there.
• Other stacks (in markers, etc.) must have been collected by functions like profiler_capture_backtrace_into in the thread itself, in which case the leaf frames (including the stack-walker call) are removed, so we only have return addresses -- and hopefully the leaf PC should be just after that stack capture function was requested (e.g., after the add-marker function/macro), but I don't know if it's that useful?

So I'm setting this bug at P3.

With that said, I suppose it would be good to add explicit information about this.

And I think it would be perfect to combine this with bug 1465869, in which we wanted to add per-frame type information.
So instead of the "stackNudging" suggestion, each frame would have one extra field indicating its exact type, one of (I think) : native leaf, native return, js, jit, label. Also Java? And importers could add languages they handle, e.g. Python.

And eventually this could also help tackle bug 1683040: I'm thinking that if sampling goes wrong, we could insert frames of a new type, with information about the sampling issue. For example, if a stack is too big to fit in our storage, the root frame would be something like "Incomplete backtrace", which I think would be less misleading than just a stack starting in a random function.

This feels simpler to me, and more flexible. What do you both think?

Comment 2

[Markus Stange [:mstange]]

That's a great suggestion.