1522497 - The mainthreadio feature doesn't work with startup profiling.

Status: RESOLVED FIXED

(Core :: Gecko Profiler, enhancement)

Description

[Florian Quèze [:florian]]

When starting Firefox with these environment variables:
MOZ_PROFILER_STARTUP_FEATURES="mainthreadio"
MOZ_PROFILER_STARTUP=1
the resulting profile doesn't contain any IO marker.

If after the end of startup I restart the profiler (with the "Main Thread IO" checkbox checked in the add-on), then the next profile I capture contains IO markers.

Debugging a bit, I saw that we return early at https://searchfox.org/mozilla-central/rev/39265dd58298c40bed029617ad408bf806cce761/xpcom/build/IOInterposer.cpp#475

Calling IOInterposer::Init() in the profiler code before calling IOInterposer::Register fixes it.

Comment 1

[Florian Quèze [:florian]]

Attachment: Initialize the IOInterposer before registering the profiler's IO interposer observer

This worked for me locally. I don't know if it's correct, especially as I don't understand why the profiler's mainthreadio feature only cares about IO happening on the main thread, but apparently the code can be triggered off main-thread.

Try builds at: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea350fceccc2cc4169fa7ad6316b37877972f25b

Comment 2

[Florian Quèze [:florian]]

Comment on attachment 9038849 [details] [diff] [review]
Initialize the IOInterposer before registering the profiler's IO interposer observer

I used this successfully locally on Mac, and on Windows using the try build from comment 1, so I'm requesting review. Hopefully Markus knows what to do when this is called off main thread (and/or why that would ever happen).

Comment 3

[Markus Stange [:mstange]]

Comment on attachment 9038849 [details] [diff] [review]
Initialize the IOInterposer before registering the profiler's IO interposer observer

Review of attachment 9038849 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah the non-main-thread thing seems like a problem: In content processes, this function can be called off the main thread. However, in that case, that call to IOInterposer::Init() is probably not the *first* call to IOInterposer::Init()... but we shouldn't rely on that. I think you need to follow what we do for IOInterposer::Register just below: call directly if on main thread, otherwise go through a runnable.

Oh, and IOInterposer::Init() contains the code "bool isMainThread = true; RegisterCurrentThread(isMainThread);", so it definitely expects to only be called on the main thread.

Comment 4

[Florian Quèze [:florian]]

Attachment: Patch v2

Comment 5

[Markus Stange [:mstange]]

Comment on attachment 9038911 [details] [diff] [review]
Patch v2

Review of attachment 9038911 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me! Thanks!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/249e88d11b9e
Initialize the IOInterposer before registering the profiler's IO interposer observer, r=mstange.

Comment 7

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/249e88d11b9e