Closed Bug 1632750 Opened 2 years ago Closed 2 years ago

Implement memory-limiting IPCs

Categories

(Core :: Gecko Profiler, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox78 --- fixed

People

(Reporter: gerald, Assigned: gerald)

References

(Blocks 1 open bug)

Details

Attachments

(13 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
128.22 KB, image/png
Details
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

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?

Interface class for a chunk manager that can be controlled: It will provide updates about chunks, and release chunks on command.

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

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

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

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

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

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

Class diagram showing the important existing classes, and new classes and function in bold.

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

Depends on: 1634216
Depends on: 1634234
No longer depends on: 1634216

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

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

Blocks: 1589977
Depends on: 1634216
Depends on: 1571171
Depends on: 1635338
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ecfdb624443a
ProfileBufferControlledChunkManager - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/4f71e3dfbef9
ProfileBufferChunkManagerWithLocalLimit doesn't need to track chunk metadata sizes - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/1c5a0c331c78
Make ProfileBufferChunkManagerWithLocalLimit a ProfileBufferControlledChunkManager - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/6c08ebdecab1
profiler_get_controlled_chunk_manager - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/f735a32f6683
ProfilerParent::ProfilerWillStopIfStarted - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/913272e74602
New profiler IPDL functions to manage chunks across processes - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/a44300bb4c83
ProfileBufferGlobalController - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/ae9b81e42327
ProfileBufferGlobalController::HandleChunkManagerUpdate logic - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/8debffa7a857
ProfilerChild handling update and destruction requests - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/986db0e8266e
Recycle chunks that are destroyed through DestroyChunksAtOrBefore or the local limit - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/b0a6620396aa
Tweak MOZ_PROFILER_STARTUP_ENTRIES default values - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/037361148517
Tweak about:profiling default buffer sizes - r=gregtatum
Regressions: 1636398
Blocks: 1543407

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?

Flags: needinfo?(gsquelart)
Blocks: 1651863

I've filed bug 1651863 to look into it...

Flags: needinfo?(gsquelart)
You need to log in before you can comment on or make changes to this bug.