1681043 - mozilla::PDMFactory::Supported blocks the main thread when creating the first content process during startup

Status: VERIFIED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[Florian Quèze [:florian]]

Here is a startup profile captured on the Quantum 2018 reference hardware. The view is focused on the 2s when the main thread is blocked while creating the first content process: https://share.firefox.dev/39MADNB

This was seen on the Nightly from 2020 12 02.

From looking at bit at the code and bugzilla, this may be related to changes made in bug 1674043, but I'm really not sure, so I'm not marking this bug as a regression.

Comment 1

[Jean-Yves Avenard [:jya]]

All I can see in that profile is that the main thread is waiting on the EnsureMTA method; could you re-capture this profile and include the capture "COM MTA" ; please also include all the media threads (including media thread pool).

Thanks

Comment 2

[Florian Quèze [:florian]]

Here's another profile showing the COM MTA thread: https://share.firefox.dev/3lT5af7

Comment 3

[Florian Quèze [:florian]]

Here's another one after updating to today's nightly: https://share.firefox.dev/3lXe37B

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1674043

Comment 5

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

Are the profiles in comment 2 or comment 3 illuminating?

Comment 6

[Jean-Yves Avenard [:jya]]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #5)

Are the profiles in comment 2 or comment 3 illuminating?

Well, we can see a 1.7s jank with the parent process blocking in nsFrameLoader::LoadContext. However, the PDMFactory initialisation introduced in bug 1674043, only takes 600ms of that.
In that blockage , WebRender initialization takes even more time

Initialising the PDMFactory and testing the capability of the system is something we must do early unfortunately.
What we could do for the time being is only run this code on first launch and cache it.

We can use the same logic as what gfx does which is to only test again once we have a new firefox version.
We could force the test if we didn't find the H264 decoder only; as on windows it would indicate we haven't installed the media package yet and we want to find it once it runs.

Comment 7

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1681043 - P1. Create method to launch RDD process early. r=alwu!,mattwoodrow!

We can launch the RDD process now that it is a fully asynchronous process and doesn't block anything.

This will allow to retrieve the decoding capabilities of the RDD process right away.

Comment 8

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1681043 - P2. Don't reject outright support for a GPU decoder if compositor isn't known. r=mattwoodrow!,alwu!,bryce!

When calling PDM::SupportsMimeType not enough information is provided for the PDM to categorically state that a codec isn't supported.

Only creating the decoder successfully matters and the PDMFactory only uses the value returned by Support as a hint to further query the PDM.

Without this change WMFDecoderModule::SupportsMimeType in the GPU process would always return false as it doesn't know yet the KnownCompositor.

Depends on D100304

Comment 9

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1681043 - P3. Retrieve decoding capabilities in GPU or RDD process and pass it to content. r=mattwoodrow!,alwu!,bryce!

Upon starting the RDD and GPU process; we check their decoding capabilities and send it to the parent process. It will then broadcast this information to all content children so that RemodeDecoderModule::Supports can return if a codec is supported or not.

Depends on D100305

Comment 10

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1681043 - P4. Stop checking on gfxVars::CanUseHardwareVideoDecoding on mac. r=mattwoodrow!,alwu!

gfxVars::CanUseHardwareVideoDecoding is often set after the first use of the PDMFactory; in this case this would disable HW VP9 decoding for the entire session of the browser.

This value will always end up being true on macOS, so ignore its value in the static Init method.

Depends on D100306

Comment 11

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1681043 - P5. Recalculate decoder capabilities if CanUseHardwareVideoDecoding changes. r=mattwoodrow!,alwu!,bryce!

Not the most elegant, but reworking the PDMFactory to be fully re-initialized would be a significant change, as only the WMFDecoderModule requires it we take some shortcuts.

Depends on D100307

Comment 12

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1681043 - P6. Remove no longer necessary workaround. r=mattwoodrow!,alwu!,bryce!

The WMFDecoderModule now uses a crash guard around the loading of the VP9/VP8 MFT. We don't want to always assume that we have a VPx decoder in the GPU process in particular, as it will only be true if a HW decoder is available.

Depends on D100308

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8fac91d4012
P1. Launch RDD process early. r=alwu
https://hg.mozilla.org/integration/autoland/rev/25a015e84cc4
P2. Don't reject outright support for a GPU decoder if compositor isn't known. r=alwu,bryce
https://hg.mozilla.org/integration/autoland/rev/9319fdd0389c
P3. Retrieve decoding capabilities in GPU or RDD process and pass it to content. r=alwu,bryce
https://hg.mozilla.org/integration/autoland/rev/5ecb208a85d3
P4. Stop checking on gfxVars::CanUseHardwareVideoDecoding on mac. r=alwu
https://hg.mozilla.org/integration/autoland/rev/64d8abe2104d
P5. Recalculate decoder capabilities if CanUseHardwareVideoDecoding changes. r=alwu,bryce
https://hg.mozilla.org/integration/autoland/rev/ea075101dc94
P6. Remove no longer necessary workaround. r=alwu,bryce

Comment 14

[Alexandru Michis [:malexandru]]

Backed out 6 changesets (Bug 1681043) for causing multiple failures.

Backout link: https://hg.mozilla.org/integration/autoland/rev/3cba4d895b8e8d11fc2b36b4c15cc1d80af73543

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&tochange=3cba4d895b8e8d11fc2b36b4c15cc1d80af73543&fromchange=5d453db46486ec3dbcdd68fe0268af4bbbefd201&selectedTaskRun=bXIJfwPmSLW9C6exeNprvQ.0

Failure logs:

• xpc: https://treeherder.mozilla.org/logviewer?job_id=325722734&repo=autoland&lineNumber=1500
• mochitest without e10s: https://treeherder.mozilla.org/logviewer?job_id=325722972&repo=autoland&lineNumber=112611

Comment 15

[Jean-Yves Avenard [:jya]]

I've split the P1 bit into bug 1684991 so that this series can land.

We would need to fix the tests and make the sandbox initialization lazy

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/21ea78b9dc84
P1. Create method to launch RDD process early. r=alwu
https://hg.mozilla.org/integration/autoland/rev/44cbe16c5d9d
P2. Don't reject outright support for a GPU decoder if compositor isn't known. r=alwu,bryce
https://hg.mozilla.org/integration/autoland/rev/e7e3b8065eb7
P3. Retrieve decoding capabilities in GPU or RDD process and pass it to content. r=alwu,bryce
https://hg.mozilla.org/integration/autoland/rev/195d164c5618
P4. Stop checking on gfxVars::CanUseHardwareVideoDecoding on mac. r=alwu
https://hg.mozilla.org/integration/autoland/rev/487a070f4989
P5. Recalculate decoder capabilities if CanUseHardwareVideoDecoding changes. r=alwu,bryce
https://hg.mozilla.org/integration/autoland/rev/50fb5b9343f6
P6. Remove no longer necessary workaround. r=alwu,bryce

Comment 17

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/21ea78b9dc84
https://hg.mozilla.org/mozilla-central/rev/44cbe16c5d9d
https://hg.mozilla.org/mozilla-central/rev/e7e3b8065eb7
https://hg.mozilla.org/mozilla-central/rev/195d164c5618
https://hg.mozilla.org/mozilla-central/rev/487a070f4989
https://hg.mozilla.org/mozilla-central/rev/50fb5b9343f6

Comment 18

[Florian Quèze [:florian]]

In an attempt to verify the fix I've just done 2 startup profiles on the same hardware I used in comment 0.

On Nightly 20210106093742 (I think that's the last one before the patches landed), cold startup, where PDMFactory blocks the main thread for almost 14s, during which the COM MTA thread (started before first paint) is busy - https://share.firefox.dev/35mEAFU

On the current Nightly - https://share.firefox.dev/2LxHqkp I don't see the bug anymore. The COM MTA thread starts much later, and doesn't seem to do much during startup.

The second profile isn't captured in the exact same conditions; this is a first start after an update, not a cold startup, so the times are not comparable between the two profiles. But I think it's good enough to verify the behavior change.

Comment 19

[Alexandru Ionescu (needinfo me) [:alexandrui]]

== Change summary for alert #28352 (as of Thu, 07 Jan 2021 17:21:33 GMT) ==

Improvements:

Ratio	Suite	Test	Platform	Options	Absolute values (old vs new)
36%	glvideo	Mean tick time across 100 ticks:	windows10-64-shippable-qr	e10s stylo webrender-sw	32.04 -> 20.63
3%	ts_paint	ts_paint	windows10-64-shippable-qr	e10s stylo webrender-sw	368.75 -> 358.67

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=28352

Comment 20

[Florian Quèze [:florian]]

(In reply to Florian Quèze [:florian] from comment #18)

On the current Nightly - https://share.firefox.dev/2LxHqkp I don't see the bug anymore. The COM MTA thread starts much later, and doesn't seem to do much during startup.

The second profile isn't captured in the exact same conditions; this is a first start after an update, not a cold startup, so the times are not comparable between the two profiles. But I think it's good enough to verify the behavior change.

Here's a cold startup profile that's comparable to my first profile in comment 18: https://share.firefox.dev/2Xk6Ggu
My observations in comment 18 still stand, I don't see the bug anymore. Thanks for fixing this Jean-Yves!