This patch introduces QOS_PRIORITY_SHUTDOWN, which exists exclusively to signal to QoS managing code that we want to raise the priority and accept no other changes. I decided to make this modification directly in `ContentParent` since all QoS changes come through here, and then we can use it while sending a shutdown signal. That way when we actually reach SendShutdown(), we've already warmed up the main thread.
This patch also introduces an assertion in case we somehow don't get a QoS override to catch any potential edge cases where this might be happening.
Differential Revision: https://phabricator.services.mozilla.com/D199497
This bug uses the existing process hang 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.
Differential Revision: https://phabricator.services.mozilla.com/D182787
This bug uses the existing process hang 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.
Differential Revision: https://phabricator.services.mozilla.com/D182787
For desktop this should basically have no impact (maybe impacts tab
warming, but if we wanted we could set the priority hint from the tab
switcher the same way we set renderLayers), but Fenix always has
renderLayers as true, effectively, so we were never de-prioritizing the
background tab processes.
Differential Revision: https://phabricator.services.mozilla.com/D145351
For desktop this should basically have no impact (maybe impacts tab
warming, but if we wanted we could set the priority hint from the tab
switcher the same way we set renderLayers), but Fenix always has
renderLayers as true, effectively, so we were never de-prioritizing the
background tab processes.
Differential Revision: https://phabricator.services.mozilla.com/D145351
Prior behavior for active tabs in an inactive app would have the process priority oom score of 15. Now it's set to a higher priority oom score of 11. This will increase priority for recently used tabs on apps and prevent Android from unloading them.
Differential Revision: https://phabricator.services.mozilla.com/D142386
Prior behavior for active tabs in an inactive app would have the process priority oom score of 15. Now it's set to a higher priority oom score of 11. This will increase priority for recently used tabs on apps and prevent Android from unloading them.
Differential Revision: https://phabricator.services.mozilla.com/D142386
Prior behavior for active tabs in an inactive app would have the process priority oom score of 15. Now it's set to a higher priority oom score of 11. This will increase priority for recently used tabs on apps and prevent Android from unloading them.
Differential Revision: https://phabricator.services.mozilla.com/D142386
In the parent process, all sub-process priority changes are recorded, with the stack.
In child processes, the new priority is recorded when notified.
Also record priorities during profiler state changes. This is to ensure that we always have at least one record of the priority at the end of the profile, when old priority changes may have already been discarded.
Differential Revision: https://phabricator.services.mozilla.com/D118129
As far as I can see, all this does is protect the user from
running some useless code if they manually enable the priority
manager using a pref on an OS that doesn't support it. The
upside of allowing this is that it makes it possible to debug
the priority manager on OSX and Linux with just a pref flip.
Differential Revision: https://phabricator.services.mozilla.com/D114767
With Fission, there can be multiple BrowserParents in a single tab, so
this patch moves the tracking of active tabs onto the top BrowsingContext
in a tab. If the priority of a top BC is changed, then the activity
of all of the BPs of the BCs in the tree are all adjusted. The flag
that tracks this state gets carried forward to the new BC in the case
of a cross-group navigation by the changes in ReplacedBy().
The other change here covers the case where we do a process-switching
navigation on an iframe. If we create a new BrowserParent with an active
top BC, then the BP gets marked as active in the priority manager. Doing
this tracking on the BP instead of the BC (both here in and in the part
that landed previously in BrowserParent::Deactivated()) means that we
don't need to track down every place that a BC switches processes.
I left the tracking of activity in ParticularProcessPriorityManager
centered around BrowserParents, instead of changing it to BCs, to
minimize the changes required. There are some tricky interactions there
with wakelocks that I didn't want to have to figure out.
browser_ProcessPriorityManager.js was set up to track a mapping of
browsers to priorities, but for the purposes of testing Fission support
I changed it to track a mapping of child IDs to priorities.
In the test, I also removed an assignment (this.window = null) that
didn't seem to be doing anything.
Differential Revision: https://phabricator.services.mozilla.com/D112213
With Fission, there can be multiple BrowserParents in a single tab, so
this patch moves the tracking of active tabs onto the top BrowsingContext
in a tab. If the priority of a top BC is changed, then the activity
of all of the BPs of the BCs in the tree are all adjusted. The flag
that tracks this state gets carried forward to the new BC in the case
of a cross-group navigation by the changes in ReplacedBy().
The other change here covers the case where we do a process-switching
navigation on an iframe. If we create a new BrowserParent with an active
top BC, then the BP gets marked as active in the priority manager. Doing
this tracking on the BP instead of the BC (both here in and in the part
that landed previously in BrowserParent::Deactivated()) means that we
don't need to track down every place that a BC switches processes.
I left the tracking of activity in ParticularProcessPriorityManager
centered around BrowserParents, instead of changing it to BCs, to
minimize the changes required. There are some tricky interactions there
with wakelocks that I didn't want to have to figure out.
browser_ProcessPriorityManager.js was set up to track a mapping of
browsers to priorities, but for the purposes of testing Fission support
I changed it to track a mapping of child IDs to priorities.
In the test, I also removed an assignment (this.window = null) that
didn't seem to be doing anything.
Differential Revision: https://phabricator.services.mozilla.com/D112213
The priority manager tracks which browser parents are active in each process,
so it needs to be told when a browser parent is no longer active. This
is currently implemented by observing ipc:browser-destroyed, which happens
in BrowserHost::DestroyComplete().
However, with Fission, not all BrowserParents are held by BrowserHosts. I
don't think this is an issue right now, but if the priority manager is going
to properly prioritize processes due to the presence of active non-top-level
frames, then it needs to also deprioritize them when they go away.
This patch deals with this situation by directly telling the priority
manager that the browser parent is becoming inactive via the existing
ActivityChanged method, in the BrowserParent::Deactivated() method,
which is called when a browser parent is being destroyed or entering
the BF cache. I think it makes sense in both cases that we no longer
want to prioritize the process that the page is in.
This does mean that we are telling the priority manager about more
ContentParents which is potentially more expensive, but the old code
also ended up doing a hashtable remove in every single
ParticularProcessPriorityManager, whereas the new code only does it
for one, so hopefully it is a net win overall.
(The reason for this is that in ParticularProcessPriorityManager's
OnBrowserParentDestroyed() method the browserHost->GetContentParent()
check will always return null because the browser host nulls out
mRoot immediately before it does ipc:browser-destroyed, so the hash
table remove is never skipped.)
This patch also removes the last thing that ParticularProcessPriorityManager
was observing, so it no longer needs to be an nsIObserver.
Differential Revision: https://phabricator.services.mozilla.com/D111654
As of the next patch, it won't be used just for indicating the
activity of an entire tab, so let's rename it.
Also alphabetize the order of some forward declarations.
Differential Revision: https://phabricator.services.mozilla.com/D111701
ParticularProcessPriorityManager observes remote-browser-shown and recomputes
the priority for the given browser. This patch eliminates the observer call
and instead makes the frame loader do a direct call.
I think doing it this way is a little clearer, and also the next patch will
eliminate the other reason ParticularProcessPriorityManager is an observer,
which will let us not not register something as an observer for every content
process, which I think is also good.
There should be no change in behavior.
Differential Revision: https://phabricator.services.mozilla.com/D111653
Also remove the default arguments. This function is only called in a single
place, so drop some of the unnecessary generality.
Differential Revision: https://phabricator.services.mozilla.com/D110709
The only reason to immediate create a ParticularProcessPriorityManager
when a ContentParent is created is to ensure that the test only
observer notification for process-created is accurate, but no test
actually listens for it, so we can delete the test notification
and then stop observing ipc:content-created.
Differential Revision: https://phabricator.services.mozilla.com/D110708
This patch was generated by running:
```
perl -p -i \
-e 's/^(\s+)([a-zA-Z0-9.]+) = NS_ConvertUTF8toUTF16\((.*)\);/\1CopyUTF8toUTF16(\3, \2);/;' \
-e 's/^(\s+)([a-zA-Z0-9.]+) = NS_ConvertUTF16toUTF8\((.*)\);/\1CopyUTF16toUTF8(\3, \2);/;' \
$FILE
```
against every .cpp and .h in mozilla-central, and then fixing up the
inevitable errors that happen as a result of matching C++ expressions with
regexes. The errors fell into three categories:
1. Calling the convert functions with `std::string::c_str()`; these were
changed to simply pass the string instead, relying on implicit conversion
to `mozilla::Span`.
2. Calling the convert functions with raw pointers, which is not permitted
with the copy functions; these were changed to invoke `MakeStringSpan` first.
3. Other miscellaneous errors resulting from over-eager regexes and/or the
replacement not being type-aware. These changes were reverted.
Differential Revision: https://phabricator.services.mozilla.com/D88903
