1627097 - Profiler taking incorrect cpu count and cpu core information

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect, P2)

Description

[Emma Malysz]

After Bug 1579581, we are now gathering the cpu count and cpu core information off the main thread and through the ProcessInfo struct (https://searchfox.org/mozilla-central/source/xpcom/base/nsSystemInfo.h#37,40-41).

This means the current implementation in the profiler is now failing https://searchfox.org/mozilla-central/source/tools/profiler/core/platform.cpp#2160,2269-2279

Comment 1

[Emma Malysz]

:julienw, Since these are now async values, what do you think the best approach would be?

Comment 2

[Markus Stange [:mstange]]

Is the profiler the only place that expects these values to be initialized synchronously as part of do_GetService("@mozilla.org/system-info;1")? Is there as asynchronous replacement API?

Comment 3

[Emma Malysz]

Yeah the profiler seems to be the only place. Almost all of the OS info, which seems to be most of what is being accessed by do_GetService("@mozilla.org/system-info;1"), is still initialized synchronously other than installYear and prefetch values.

Comment 4

[Julien Wajsberg [:julienw]]

Passing on the NI request to Gerald, he'll know better than me :-)
(TBH I have no idea)

Comment 5

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

StreamMetaJSCustomObject() is called synchronously when writing a profile out at the end of a profiling session, so I don't think we can make it asynchronous -- not without a massive rewrite of the whole output code!

So I guess the solution here will be to request this information at the start of a profiling session, and asynchronously store the response, hopefully before the end of the session.

I'll need to learn how to actually do this async call... (Little experience here.)

Possibly an embarrassing question:
Emma, I see the actual code handling this request looks like plain C++ synchronous code ( https://searchfox.org/mozilla-central/rev/c2bc259414706ef4be5d13df1344eebb7507a51b/xpcom/base/nsSystemInfo.cpp#556 ), could we possibly make this function public so I could call it directly instead?

Comment 6

[Emma Malysz]

Yeah I believe that should be fine. And then instead of accessing the properties via the strings, we could instead access the values stored in the struct. It may make sense to also note that the function is intended to be called asynchronously

Comment 7

[Emma Malysz]

I'm going to start working on a fix for Bug 1625553, so there's a chance we may also see some failure from when we access the cpu core. But that should be less frequent than how it's currently implemented

Comment 8

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

Attachment: Bug 1627097 - Profiler synchronously collect processor information - r?emalysz

Comment 9

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

The proposed patch on this bug here focuses on fixing this particular issue, to re-enable cpu counts ASAP.

I've opened separate bug 1627871 to add tests that should catch this kind of disappearing-data issues in the future.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/820e15a086c0
Profiler synchronously collect processor information - r=emalysz

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/820e15a086c0