1435091 - We are accumulating pending markers for threads that aren't sampled without bound, until the profiler is stopped

Status: RESOLVED FIXED

(Core :: Gecko Profiler, enhancement, P2)

Comment 1

[Markus Stange [:mstange]]

Yes we do. Luckily, the only threads that frequently add markers (GeckoMain and Compositor) are part of the default thread filter. But if I remove Compositor from the thread filter, then I can see thousands of markers pile up in the mPendingMarkers list for that thread. This is only a problem while the profiler is running; once the profiler is stopped, these markers get deleted anyway.

Comment 2

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

Working on it. My basic idea is to add RacyRegisteredThread::ShouldProfile() and only add marker when it's true (i.e., when the thread is in the filter list).

Comment 3

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

Attachment: Bug 1435091 - p1. RacyRegisteredThread::IsBeingProfiled() - r?mstange

Comment 4

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

Attachment: Bug 1435091 - p2. Don't record markers when current thread is not actively being profiled - r?mstange

Depends on D11304

Comment 5

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

Attachment: Bug 1435091 - p3. profiler_thread_is_being_profiled() - r?mstange

profiler_thread_is_actively_profiled() should probably be used instead of
profiler_is_active() when guarding a profiler_add_marker() call.

Depends on D11306

Comment 6

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

Attachment: Bug 1435091 - p4. Use profiler_thread_is_being_profiled() instead of profiler_is_active() around profiler_add_marker()s - r?mstange

(Unless there were other profiler actions, as I'm not sure yet whether it would
be safe to skip them when the profiler is paused; another bug should investigate.)

Depends on D11307

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9096b7d003cf
p1. RacyRegisteredThread::IsBeingProfiled() - r=mstange,njn
https://hg.mozilla.org/integration/autoland/rev/a8d4a9012456
p2. Don't record markers when current thread is not actively being profiled - r=mstange
https://hg.mozilla.org/integration/autoland/rev/46d2f807824a
p3. profiler_thread_is_being_profiled() - r=mstange
https://hg.mozilla.org/integration/autoland/rev/02d02cae92cd
p4. Use profiler_thread_is_being_profiled() instead of profiler_is_active() around profiler_add_marker()s - r=mstange

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/9096b7d003cf
https://hg.mozilla.org/mozilla-central/rev/a8d4a9012456
https://hg.mozilla.org/mozilla-central/rev/46d2f807824a
https://hg.mozilla.org/mozilla-central/rev/02d02cae92cd