Implement memory-limiting IPCs
Categories
(Core :: Gecko Profiler, task, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox78 | --- | fixed |
People
(Reporter: mozbugz, Assigned: mozbugz)
References
(Blocks 2 open bugs)
Details
Attachments
(13 files)
(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 | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
After bug 1630872, the profiler now uses a chunk-based storage.
Chunks are created, released (after they're full of data), and destroyed by "chunk managers". The main profile storage uses ProfileBufferChunkManagerWithLocalLimit
, which has a memory limit in its process (just like with the previous storage strategies).
Now we want to limit the total memory used by all processes in the Firefox application.
This can be done by instrumenting the chunk manager to inform a central controller about chunks (created, released, locally destroyed), and this controller can then decide to destroy the oldest chunk (from all processes!) when the memory limit is reached.
A beneficial side effect is that at the end, all processes should have a similar time range of data.
One potentially surprising side effect is that the profiler will now really use the given limit for the whole Firefox app, instead of for each process, meaning that for the same limit we will record less data.
We may need to change some defaults to try and keep the experience the same for most users?
Assignee | ||
Comment 1•5 years ago
|
||
Interface class for a chunk manager that can be controlled: It will provide updates about chunks, and release chunks on command.
Assignee | ||
Comment 2•5 years ago
|
||
Depends on D72362
Assignee | ||
Comment 3•5 years ago
|
||
The Gecko Profiler can provide its current controlled chunk manager.
It is the responsibility of the caller to keep track of the state of the profiler, to avoid using the chunk manager after it's discarded.
Depends on D72363
Assignee | ||
Comment 4•5 years ago
|
||
The Gecko Profiler can notify the ProfilerParent when it's about to stop (if it was started, but the notification will happen even when it's not started).
Depends on D72364
Assignee | ||
Comment 5•5 years ago
|
||
Depends on D72365
Assignee | ||
Comment 6•5 years ago
|
||
When there is at least one ProfilerParent (i.e., we are interacting with at least one child process) AND the parent profiler is running, the ProfilerParentTracker sets up the ProfileBufferGlobalController that will manage all chunks.
As a first step, it connects with the local chunk manager (to receive chunk updates), and sends update requests to all children.
(The actual controller logic is not implemented in this patch, nor is the ProfilerChild side, see following patches.)
Depends on D72366
Assignee | ||
Comment 7•5 years ago
|
||
The logic part of the controller receives all updates, and makes decisions to destroy old chunks when the memory limit is reached.
Depends on D72367
Assignee | ||
Comment 8•5 years ago
|
||
This implement the child side:
When the first request for update arrives, it connects to the local chunk manager, to receive its updates.
If multiple updates are received, they are folded into one.
If there are both an update and a pending request, the request is fulfilled with the update and local data is reset.
And ProfilerChild handles "destroy" instructions to destroy local chunks.
At this point, the whole machinery is in place, and all combined profile buffers used in all processes should use around the maximum amount allowed.
A bit more memory may still be used, e.g., due to IPC delays, and because of recycling which keeps some unused chunks alive for later reuse. But overall that should be a small amount compared to the usual user-requested limit.
Depends on D72368
Assignee | ||
Comment 9•5 years ago
|
||
Whenever chunks are about to be destroyed, we try to keep one or two alive, to hopefully fulfill the next request, thereby avoiding a deallocation+allocation pair.
Depends on D72369
Assignee | ||
Comment 10•5 years ago
|
||
Class diagram showing the important existing classes, and new classes and function in bold.
Assignee | ||
Comment 11•5 years ago
|
||
The chunk metadata size is tiny (less than 100 bytes) compared to the buffer size (1MB by default), so it's fine to ignore it while dealing with cross-Firefox limits.
Depends on D72362
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 12•5 years ago
|
||
These defaults were per process and there are usually around 8 processes.
Now these sizes apply to all processes, so they can be 8 times as big (but less on Android where memory may be more limited.)
Not changing Base Profiler defaults, as its buffer is not cross-process controlled.
Depends on D72370
Assignee | ||
Comment 13•5 years ago
|
||
Presets used 16Mi entries (128MiB) by default, but these numbers were a limit per process.
Now that the limit is for the whole Firefox app, we would record a lot less information, because there are typically around 8 or more processes.
So the default are now 8 times larger to roughly record a similar amount of profiling data.
Also the custom buffer maximum size has been increased from 1GiB to 2GiB.
Depends on D73215
Comment 14•5 years ago
|
||
Comment 15•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ecfdb624443a
https://hg.mozilla.org/mozilla-central/rev/4f71e3dfbef9
https://hg.mozilla.org/mozilla-central/rev/1c5a0c331c78
https://hg.mozilla.org/mozilla-central/rev/6c08ebdecab1
https://hg.mozilla.org/mozilla-central/rev/f735a32f6683
https://hg.mozilla.org/mozilla-central/rev/913272e74602
https://hg.mozilla.org/mozilla-central/rev/a44300bb4c83
https://hg.mozilla.org/mozilla-central/rev/ae9b81e42327
https://hg.mozilla.org/mozilla-central/rev/8debffa7a857
https://hg.mozilla.org/mozilla-central/rev/986db0e8266e
https://hg.mozilla.org/mozilla-central/rev/b0a6620396aa
https://hg.mozilla.org/mozilla-central/rev/037361148517
Comment 16•4 years ago
|
||
I am testing windows10 cppunit tests on hardware (instead of aws cloud) and I get some failures:
https://treeherder.mozilla.org/#/jobs?repo=try&selectedTaskRun=LOoWN3ReTrajPBMZ5gkTyw.0&revision=36d5d16e8501531d164cf5faef467da1280b35a7&searchStr=cppunit
specifically I hit this assertion (added in this bug):
-
MOZ_ASSERT(
-
!chunk->GetNext() || (chunk->ChunkHeader().mDoneTimeStamp <
-
chunk->GetNext()->ChunkHeader().mDoneTimeStamp),
-
"Released chunk groups must have increasing timestamps");
Is there a way to get more information about this?
Assignee | ||
Comment 17•4 years ago
|
||
I've filed bug 1651863 to look into it...
Description
•