1834629 - Notify the background main thread of QoS changes from another thread

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Kris Wright :KrisWright]

Bug 1832883 shows a problem with QoS changes on main threads where they can have their warm-up process deprioritized by the CPU and result in noticeably low tab change performance at high CPU load. While the change in the bug temporarily raises priority to prevent the issue, a more elegant solution is needed. Something like:

• Store data about the main thread accessible offthread for QoS changes
• Receive the IPC message to change the priority from some higher-priority offthread
• Create a temporary override to the main thread to change its priority, "warming up" the main thread
• Have the main thread set its actual priority (can only be set by itself on MacOS)
• Remove the priority override

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: WIP: Bug 1834629 - Set main thread QoS priority from off-main-thread on macOS

This is a rough patch which uses the existing ProcessHangMonitor
background actor to manage the QoS priority of the main thread on macOS,
using overrides to ensure that it is scheduled when raising from a lower
priority.

This does not minimize thread hops, as 2 threads are required for the
IPC (the I/O thread and the ProcessHangMonitor thread), however it does
avoid the main thread needing to be scheduled.

Comment 2

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

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

Comment 3

[Donal Meehan [:dmeehan]]

:nika FYI soft code freeze is this week, and 115 merge day is on 2023-06-05.
Could you set the Priority/Severity on this?
Wondering if you plan on landing a fix before merge day

Comment 4

[Andrew McCreight [:mccr8]]

Bug 1832365 flips a pref to true in Nightly only, so I don't think the regression matters for beta.

Comment 5

[Donal Meehan [:dmeehan]]

(In reply to Andrew McCreight [:mccr8] from comment #4)

Bug 1832365 flips a pref to true in Nightly only, so I don't think the regression matters for beta.

Good point!

Comment 6

[Nika Layzell [:nika] (ni? for response)]

(In reply to Donal Meehan [:dmeehan] from comment #3)

:nika FYI soft code freeze is this week, and 115 merge day is on 2023-06-05.
Could you set the Priority/Severity on this?
Wondering if you plan on landing a fix before merge day

I put this patch together as a draft for :kriswright to iterate on, so redirecting the ni? there.

Comment 7

[Kris Wright :KrisWright]

Was unavailable for much of last week; will be landing this into 116 and uplifting for experimenting once we confirm its stability. It'll all be restricted to Nightly only regardless. The regression / fixes are all in nightly only until experiment launch in july.

Comment 8

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

The severity field is not set for this bug.
:nika, could you have a look please?

For more information, please visit BugBot documentation.

Comment 9

[Kris Wright :KrisWright]

Attachment: Bug 1834629 - Set main thread QoS Priority from another thread. r?mccr8

This bug uses the existing BHM messaging infrastructure to raise priorities on main threads. To ensure reduced latency, we also increase the priority of ProcessHangMon threads. We also address an edge-case bug where flipping the QoS prefs during use might lead to a tab getting stuck at the wrong priority.

Due to increasing the priority of the hang monitor thread, we may see some increase in its utilization on MacOS during high CPU load. I'm not sure the extent of how this may impact the browser, but as it makes the thread "faster" it may be more responsive than some other threads during this case.

I tested thread responsiveness by using the stress tool and dispatching various numbers of worker threads, up to 250. During these tests, even when other parts of firefox were less responsive under stress, tab switching appeared to remain snappy and responsive.

I captured some updated power profiles using the change. Profile where pref flipped off during use: https://share.firefox.dev/46BksO3 (Note that we start with the prefs on, then we flip off the prefs half way and repeat the same behavior to observe the fix to the previous bug that left tabs trapped in the background)

Profile with the pref fully enabled: https://share.firefox.dev/46EBIC7

In regards to the edge case, to avoid spurious tab wakeups, we won't reinstate normal thread priority when pref is disabled until the tab is interacted with again.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70e3969301b9
Set main thread QoS Priority from another thread. r=mccr8

Comment 11

[Narcis Beleuzu [:NarcisB]]

Backed out for bustages on ProcessHangMonitor.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/6f03f47a55846f371b7b480f283fefae28897f76
Log link: https://treeherder.mozilla.org/logviewer?job_id=422562132&repo=autoland&lineNumber=14972

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/66381e9aba98
Set main thread QoS Priority from another thread. r=mccr8

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/66381e9aba98