1724777 - a tab with an alert open and lots of pending microtasks may slow down other tabs using the same process

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Olli Pettay [:smaug][[e-mail]]]

Testcase http://mozilla.pettay.fi/moztests/suppressed_microtasks.html

But this has happened to people with real web sites, for example Google calendar.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

For example - bad jank trying to type into a gmail tab while in a WebRTC call:
https://share.firefox.dev/2VsoJDN

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Looks like Fission changes have regressed the behavior making this way more worse than I thought. See bug 1724899.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1724777, optimize suppressed MicroTask handling, r=mccr8

The test is in theory racy, but trying to limit the cases when it might behave badly
by running it on opt desktop builds only. Without the patch the 'period' check takes over 400ms locally and with the
patch 1-3ms.

The changes are just trying to optimize execution, not change the behavior.
Use of SuppressedMicroTasks is perhaps a bit odd, but it helps keeping
SavedMicroTaskQueue simple.

Comment 4

[Chris Peterson [:cpeterson]]

Tentatively tracking this bug as a soft blocker for Fission MVP. Olli said we might want to uplift the fix to Beta.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f14f5616dca
optimize suppressed MicroTask handling, r=mccr8

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/2f14f5616dca

Comment 7

[Chris Peterson [:cpeterson]]

Olli says we should uplift this fix (and bug 1724899) to Beta 92 and ESR 91 because this bug can cause performance problems with Google Docs and Calendar.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9235694 [details]
Bug 1724777, optimize suppressed MicroTask handling, r=mccr8

Beta/Release Uplift Approval Request

• User impact if declined: Slower performance (seen at least on some google websites)
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: Bug 1724899
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): The patch should not change the behavior, but obviously it isn't super trivial
• String changes made/needed: NA

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined:
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String or UUID changes made by this patch:

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9235694 [details]
Bug 1724777, optimize suppressed MicroTask handling, r=mccr8

Approved for 92.0b5. We'll revisit the ESR request when this has baked a bit more.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ee309b5c73ed

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9235694 [details]
Bug 1724777, optimize suppressed MicroTask handling, r=mccr8

Approved for 91.1esr.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/877d9fcafe67