1465926 - Register every thread with the GeckoProfiler, and make it really hard to add new threads without registering

Status: NEW

(Core :: Gecko Profiler, defect, P2)

Description

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

In bug 1458375, I was dealing with a rather stubborn Talos regression, and ultimately what ended up being the critical clue was noticing that some JS decoding was happening off of the main thread in the "good" case, and not in the "bad" case.

Unfortunately, those threads doing the decoding off of the main thread were js::HelperThread's that aren't currently registered with the Gecko Profiler, so they were completely invisible in Gecko Profiles.

The only reason I knew that was because I ended up using XCode Instruments' System Trace tool to sample every thread on the system while the Talos tests were running and doing manual inspection.

I'm certain this is not the last time that these "invisible" threads will cause more profiling headaches in the future. We should try to get every thread registered - especially as we start doing more and more in parallel - to gain more visibility into the cross-thread scheduling stuff that's going on.

Once we've done that, we should make it really hard to add _new_ threads without registering them. Maybe as something deep within our cross-platform threading library, or maybe as a static analysis or linting rule.

Comment 1

[Greg Tatum [:gregtatum]]

Tentatively marking as a P2, as it seems like something we should prioritize.

Comment 2

[Paul Adenot (:padenot)]

This is really important I feel and it is the very reason that I can't use our profiling tool.

Speaking about real-time audio programming, we (Firefox) don't create the important threads where all the performance critical work is happening (all the DSP, so to speak). We also don't create a number of threads where some important callbacks are called on, such as device changes callback, or under-run alert callbacks: a callback is registered using an OS API, and sometime, some thread created by the underlying library or framework calls it.

This means that ensuring all threads we create are registered is not enough. It is necessary to introspect the Firefox processes to find what it is doing. It scales better and removes the human factor from the equation. In fact I think there is value in having an hybrid approach: allowing the registration of thread, allowing to attach profiling metadata (name, thread type, whatnot), while ensuring all of them are registered (with a synthesized name), to ensure exploratory profiling is possible.

After a quick look online, it is possible to list the threads of a program from itself. It seems slightly harder to get notified of a new thread creation (short of forking + attaching a process that acts as a debugger).

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

This may be a dupe of bug 1341811, but I'll just link them for now.

Comment 4

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

I've just filed bug 1742522, which will discover unregistered threads the same way about:processes does.
It should help with parts of the requested features in this bug, but maybe not fully (in particular "make it really hard to add new threads without registering"), so I will keep this bug here open in case there are more relevant solutions possible in the future...