Open Bug 1364091 Opened 4 years ago Updated 2 months ago

Defer mozilla::widget::AudioSession::Start() until later in startup

Categories

(Core :: Widget: Win32, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: florian, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: perf, Whiteboard: [fxperf:p2][qf:p2:responsiveness]tpi:+)

Attachments

(1 file)

4.53 MB, application/x-7z-compressed
Details
See this profile: https://perfht.ml/2qXGs4Q where a CoCreateInstance call in mozilla::widget::AudioSession::Start() takes 35ms and CEndpointDevice::Activate takes another 22ms.

This is on startup on the quantum reference device. The cost of both of these calls is due to them causing DLL files to load.

Could we either run this on a different thread, or after we are done with startup? (this is currently happening before the first window is shown)
Priority: -- → P3
Whiteboard: [qf] → [qf], tpi:+
Hey jimm, how possible is it to move this until after first paint? Or is it possible to somehow do this off of the main thread?
Flags: needinfo?(jmathies)
Whiteboard: [qf], tpi:+ → [qf:p1], tpi:+
Tricky since this has to be set up before a content process or plugin process makes similar calls to sync up audio indicators. What we really want to be able to do is delay this until audio is played. Chrome does this quite well. However Flash may present an issue there since we aren't latched into it. Some experimentation might yield positive but this isn't what I'd consider low hanging fruit.
Flags: needinfo?(jmathies)
Whiteboard: [qf:p1], tpi:+ → [qf:p2], tpi:+
Keywords: perf
Whiteboard: [qf:p2], tpi:+ → [qf:p1], tpi:+
Whiteboard: [qf:p1], tpi:+ → [qf:i60][qf:p1,]tpi:+
Whiteboard: [qf:i60][qf:p1,]tpi:+ → [qf:f60][qf:p1,]tpi:+
Here is a startup profile showing slow main thread I/O for audioses.dll https://perfht.ml/2CSaCNJ Is this the same bug?
(In reply to Florian Quèze [:florian] from comment #3)
> Here is a startup profile showing slow main thread I/O for audioses.dll
> https://perfht.ml/2CSaCNJ Is this the same bug?

Answering my own question: yes, mozilla::widget::AudioSession::Start is what comes immediately after the audioses.dll main thread I/O in this profile, making this bug block startup before first paint for ~190ms.
Whiteboard: [qf:f60][qf:p1,]tpi:+ → [qf:f61][qf:p1,]tpi:+
Whiteboard: [qf:f61][qf:p1,]tpi:+ → [qf:f64][qf:p1]tpi:+
Whiteboard: [qf:f64][qf:p1]tpi:+ → [qf:p1:f64]tpi:+
Whiteboard: [qf:p1:f64]tpi:+ → [fxperf][qf:p2:responsiveness]tpi:+
Whiteboard: [fxperf][qf:p2:responsiveness]tpi:+ → [fxperf:p2][qf:p2:responsiveness]tpi:+
See Also: → 1548590
See Also: → 1570616

AudioSession::Start() is responsible for several DLL loads during startup. We should investigate the suggestion in comment #2 to see if it is possible to delay AudioSession::Start().

Summary: mozilla::widget::AudioSession::Start() causes main thread IO during startup → Defer mozilla::widget::AudioSession::Start() until later in startup
Blocks: 1627071

(In reply to Jim Mathies [:jimm] from comment #2)

Tricky since this has to be set up before a content process or plugin
process makes similar calls to sync up audio indicators. What we really want
to be able to do is delay this until audio is played. Chrome does this quite
well. However Flash may present an issue there since we aren't latched into
it. Some experimentation might yield positive but this isn't what I'd
consider low hanging fruit.

Can you help us understand what makes this tricky (other than Flash), and where an entry point into taking a look at this might be? Do you think it would be feasible for someone unfamiliar with this code to dive in and start solving it, or would it be so hairy that someone very familiar with our audio code would need to tackle it?

Flags: needinfo?(jmathies)

We remoted audio in https://bugzilla.mozilla.org/show_bug.cgi?id=1432303 so this might be simpler to do now. David, you did some work in audio init, any thoughts how easy it would be to delay initialization until audio is played? I wonder if we even need the init call down in widget anymore.

Flags: needinfo?(jmathies) → needinfo?(davidp99)

I removed the AudioSession from the content process when audio was remoted -- in bug 1430907. That leaves it in use for XPCShell (I don't know what that's about but I bet its unused) and the plugin process, which still needs it. OTOH, it does more than just consolidate the processes' volume controls -- it also sets the name/icon for Firefox that is used in sndvol.exe (the Windows volume mixer) [1]. We can probably delay until we either play a sound or load a plugin. The only thing I can imagine that breaking is if there is a volume slider created before we use sound -- which shouldn't be the case.

I'm certain I could find a place to ensure initialization before plugin activation but I'm not as well versed in the cubeb stuff so I'm not sure about that part.

[1] https://searchfox.org/mozilla-central/rev/598e50d2c3cd81cd616654f16af811adceb08f9f/widget/windows/AudioSession.cpp#361

Flags: needinfo?(davidp99)

For Reasons, I would love it if we could make it async by returning a MozPromise too!

You need to log in before you can comment on or make changes to this bug.