1310011 - Allow thread hang annotations to be set for in-progress hang

Status: NEW

(Toolkit :: Telemetry, defect, P3)

Description

[Mike Conley (:mconley) (:⚙️)]

I landed the ability for us to annotate hangs that occur during an e10s tab switch in bug 1303077. Unfortunately, this will only allow us to capture stacks if the hang occurs _after_ the force paint is requested and before the paint completes.

I suspect that the tab switch spinner occurs most often when the main thread in the content process is _already in the midst of a hang_, and the user switches tab.

Unfortunately, our annotation system for hangs only gathers annotations at the start of the hang.

I want to change this so that the hang monitor thread polls for annotations, and accumulates them during a hang.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1310011 - Store HangAnnotations in a map instead of a vector to avoid duplicates.

Review commit: https://reviewboard.mozilla.org/r/85700/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/85700/

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1310011 - Allow annotations on a hang to accumulate.

Review commit: https://reviewboard.mozilla.org/r/85702/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/85702/

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8800882 [details]
Bug 1310011 - Allow annotations on a hang to accumulate.

The HangAnnotations stuff is written using a more modern (and probably better) style than I'm used to. In particular, it's using things like UniquePtr and Move semantics.

What this patch is attempting to do is take some annotations, and add them to a possibly pre-existing set of annotations. I'm not handling nullptr cases very well just yet.

But this won't even compile, and I guess I have some studying to do with ::Move and stuff.

aklotz - the urgency of this is high enough that I think waiting for me to learn how to write modern C++ is likely too long a wait. Do you have any recommendations on how I can accomplish the accumulating behaviour correctly?

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8800882 [details]
Bug 1310011 - Allow annotations on a hang to accumulate.

I've had a night to sleep on this, and I think I see what I'm doing wrong. I'll hopefully have a working patch up today.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8800881 [details]
Bug 1310011 - Store HangAnnotations in a map instead of a vector to avoid duplicates.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/85700/diff/1-2/

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8800882 [details]
Bug 1310011 - Allow annotations on a hang to accumulate.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/85702/diff/1-2/

Comment 7

[Mike Conley (:mconley) (:⚙️)]

The patch that switches HangAnnotations to use a std::map has moved to bug 1310809 instead.