Tab unloading should precede other memory pressure observers
Categories
(Firefox :: Tabbed Browser, enhancement, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox91 | --- | fixed |
People
(Reporter: haik, Assigned: toshi)
References
Details
Attachments
(7 files, 1 obsolete file)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
Tab unloading (not enabled at this time) is implemented as a memory-pressure
observer. Given that tab unloading may reduce memory use, we should experiment with unloading tabs first and then notifying memory-pressure
observers if we are still in a low memory state.
Comment 1•4 years ago
|
||
Note that the first time we detect a low-memory situation we send a memory-pressure
event with a low-memory
payload. If the low memory situation persists further events have the low-memory-ongoing
payload instead.
Currently all listeners tend to respond to the latter but we might switch this around: we make tab unloader respond to low-memory
and everything else to low-memory-ongoing
. This however does not guarantee that the low-memory-ongoing
listeners will ever run.
An alternative would be to augment our nsIObserver implementation so that it's possible to prepend listeners (we currently only support appending them). Since listeners are called in the order they appear in the list prepending the tab unloader to all other listeners would guarantee it is always called first. Arguable this second solution would be simpler but maybe slightly more brittle.
Comment 2•4 years ago
|
||
Tracking for Fission M8 milestone because Toshi says this bug is a blocker for enabling tab unloading in bug 1587762 (which we are also tracking for Fission M8).
Updated•3 years ago
|
Assignee | ||
Comment 3•3 years ago
|
||
This patch splits nsAvailableMemoryWatcher
into 1) an nsISupports-derived class
nsAvailableMemoryWatcherBase
and 2) a platform-specific class nsAvailableMemoryWatcher
,
taking out the 2) part as a new file AvailableMemoryWatcherWin.cpp without any change.
Test cases for nsAvailableMemoryWatcher
will be added by a subsequent patch.
Assignee | ||
Comment 4•3 years ago
|
||
- Use
nsAutoHandle
instead of a rawHANDLE
- Add a dtor with
MOZ_ASSERT
- Prevent double init
- Cache
nsAvailableMemoryWatcher::mObserverSvc
Depends on D117669
Assignee | ||
Comment 5•3 years ago
|
||
This enables JS code to post a memory-pressure event.
Depends on D117670
Assignee | ||
Comment 6•3 years ago
|
||
With this patch, the first OnLowMemory
is triggered by the memory resource
notification which is based on the available physical memory, but subsequent
OnLowMemory
or OnHighMemory
is triggered by the timer based on the
available commit space.
The key part of this change is the if
block in nsAvailableMemoryWatcher::Notify
,
where we use a single condition IsCommitSpaceLow()
to declare either Low or High.
With this, OnLowMemory
is called not only in the main thread but also in a worker
thread, StartPollingIfUserInteracting
also needs a lock to protect mPolling
.
We don't need QueryMemoryResourceNotification
anymore.
Depends on D117671
Assignee | ||
Comment 7•3 years ago
|
||
This is the main part to address bug 1701368.
Before this patch, nsAvailableMemoryWatcher
directly broadcasted
a memory-pressure event when we enter into a low-memory situation and
TabUnloader
unloaded a tab in response to the memory-pressure message.
This patch introduces a new message "memory-watcher-notification" for
nsAvailableMemoryWatcher
to send to TabUnloader
without involving
other memory-pressure listeners.
With this, nsAvailableMemoryWatcher
sends "memory-watcher-notification"
with a subtopic "OnLowMemory" or "OnHighMemory" accordingly. TabUnloader
is responsible for an actual action: unload a tab or send a memory-pressure.
To achieve that, this patch implements a state machine in TabUnloader
,
consisting of three states: Normal, LowMemory, and MemoryPressured.
State transition happens in response to "memory-watcher-notification" from
nsAvailableMemoryWatcher
. Currently we define seven transitions:
- Normal --(OnLowMemory)--> LowMemory
- Normal --(OnLowMemory)--> MemoryPressured
- LowMemory --(OnLowMemory)--> LowMemory
- LowMemory --(OnLowMemory)--> MemoryPressured
- LowMemory --(OnHighMemory)--> Normal
- MemoryPressured --(OnLowMemory)--> MemoryPressured
- MemoryPressured --(OnHighMemory)--> Normal
Depends on D117672
Assignee | ||
Comment 8•3 years ago
|
||
Depends on D117673
Comment 9•3 years ago
|
||
Tab unloading is a nice-to-have for our Fission Release experiment, but doesn't need to block it. I will move this bug from Fission Milestone M8 to MVP.
Assignee | ||
Comment 10•3 years ago
|
||
This patch introduces an XPCOM object which is represented by the single instance of
nsAvailableMemoryWatcherBase
so that nsAvailableMemoryWatcher
can synchronously
access TabUnloader
.
We currently implement a watcher class for Windows only. For other platforms, what
we need to do is to define a class inherinting nsAvailableMemoryWatcherBase
and
a simple factory method CreateAvailableMemoryWatcher()
returning an instance of
that class.
Depends on D117672
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Comment 11•3 years ago
|
||
We had NS_DispatchMemoryPressure
and NS_DispatchEventualMemoryPressure
to dispatch a memory-pressure event which took MemPressure_New
and
MemPressure_Ongoing
to translate into "low-memory" and "low-memory-ongoing"
message respectively.
With that model, we could end up sending a wrong message if somebody
called the API with MemPressure_Ongoing
without sending MemPressure_New
.
To avoid that, this patch removes MemPressure_Ongoing
and makes
the API decide whether it should dispatch a "new" event or "ongoing" event.
Depends on D117670
Updated•3 years ago
|
Updated•3 years ago
|
Comment 12•3 years ago
|
||
Comment 13•3 years ago
|
||
Backed out for causing build bustages on TestMemoryPressure.cpp.
Backout link: https://hg.mozilla.org/integration/autoland/rev/75dcd0588558a4e3723db93151d8f15c5d6a215b
Failure log: https://treeherder.mozilla.org/logviewer?job_id=344504482&repo=autoland&lineNumber=68241
Assignee | ||
Comment 14•3 years ago
|
||
Updated the part3 to fix the compile warning in TestMemoryPressure.cpp and updated the part6 to fix the error in browser_TabUnloader.js during test-windows10-64-qr/debug-test-verify-fis-e10s TV-fis.
Comment 15•3 years ago
|
||
Comment 16•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1251a4fedbd0
https://hg.mozilla.org/mozilla-central/rev/857bb33999bd
https://hg.mozilla.org/mozilla-central/rev/278e8d0bd4dc
https://hg.mozilla.org/mozilla-central/rev/a2c139554e9e
https://hg.mozilla.org/mozilla-central/rev/9c9913e934af
https://hg.mozilla.org/mozilla-central/rev/b88c3bce751e
https://hg.mozilla.org/mozilla-central/rev/f987c607501f
Description
•