Make PHC more aggressive on Nightly.
Categories
(Core :: Memory Allocator, enhancement)
Tracking
()
Tracking | Status | |
---|---|---|
firefox113 | --- | fixed |
People
(Reporter: jesup, Assigned: pbone)
References
(Blocks 2 open bugs)
Details
Attachments
(2 files)
It would be interesting to be able to target specific sizes or classes or jemalloc bucket in the field with PHC.
We could cycle through different sizes (via normandy?), or have browsers randomly select a size to target at startup. If it's currently stochastic and only guards a small number of allocations, then it may not hit certain allocations often enough to catch low-frequency trashing.
Another variant might be to only mark as inaccessible freed data, but not allocate guard pages (to reduce the total memory hit mostly). This would help catch UAFs, but not help catch overruns. The reasoning is that the memory hit would be lower (though if guard pages don't take up real ram, this isn't true and this idea should be dropped).
We could vary the amount of storage that's guarded dependent on the ram available.
We could target specific (random) content processes, perhaps one per browser instance, now that we have fission. Content processes generally are less long-lived, and the overhead would only be in that process/origin. Also we have better OOM handling now, so a content process OOM is less likely to provoke a main process OOM.
We may also want to revisit the constants used for tuning PHC (see bug 1614875)
Reporter | ||
Comment 1•2 years ago
|
||
Updated•2 years ago
|
Reporter | ||
Updated•2 years ago
|
Assignee | ||
Comment 2•2 years ago
|
||
I want to organise the PHC work a little bit. comment 0 has a few different ideas, however I'd like to focus on one idea per bug.
It would be interesting to be able to target specific sizes or classes or jemalloc bucket in the field with PHC.
I'd like to learn more so I can prioritise this with other PHC work. Randell, was this your first idea because you were debugging something specific and thought it would be useful?
Thanks.
Assignee | ||
Comment 3•2 years ago
|
||
decoder, I hope you don't mind if I take this bug.
Assignee | ||
Updated•2 years ago
|
Updated•2 years ago
|
Assignee | ||
Comment 4•2 years ago
|
||
Depends on D161934
Assignee | ||
Comment 5•2 years ago
|
||
I've started try jobs to compare the performance with and without these patches. I started talos, awsy, browsertime-essential and speedometer tests:
Assignee | ||
Comment 6•2 years ago
|
||
(In reply to Paul Bone [:pbone] from comment #5)
The big change is AWSY. it's reporting a LOT more base content explicit memory, but see how resident memory hasn't changed. There's a bug here, PHC's guard pages are reported in explicit when they shouldn't be because they're not resident. I filed Bug 1822451.
The performance is mostly comparable. There are changes in page load, visual metrics and talos tests. Maybe PHC gives different alignment for some allocations causing different performance.
I'd like to fix Bug 1822451 first and then re-test AWSY.
Assignee | ||
Comment 7•2 years ago
|
||
I added an about:memory
entry in Bug 1822451. It says that this change adds 1MB of extra memory usage on x86_64 Linux. @Jesup, is that what you expected? Is 1MB good value for more PHC coverage?
Assignee | ||
Updated•2 years ago
|
Reporter | ||
Comment 8•2 years ago
|
||
IMHO yes, especially since the current patch doesn't change what versions PHC is turned on for (i.e. not release). Even for release, I think the win will be acceptable. I do think when we expand usage of PHC we should loop in mccr8, perf team, etc.
Assignee | ||
Comment 9•2 years ago
|
||
Here's a perfherder compare for this change, the memory increase is what we were expecting.
Updated•2 years ago
|
Updated•2 years ago
|
Comment 10•2 years ago
|
||
Comment 11•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4086829b990a
https://hg.mozilla.org/mozilla-central/rev/39c07b603ef5
Assignee | ||
Comment 12•2 years ago
|
||
Hi perfherder people. This is going to regress some resident memory numbers, adding about 1MB per-process. that's okay, that's the trade-off we're making.
(I don't know who to NI so I put a comment in your channel on Matrix).
Comment 13•2 years ago
|
||
(In reply to Paul Bone [:pbone] from comment #12)
Hi perfherder people. This is going to regress some resident memory numbers, adding about 1MB per-process. that's okay, that's the trade-off we're making.
(I don't know who to NI so I put a comment in your channel on Matrix).
The changes generated the following alert on awsy:
== Change summary for alert #38036 (as of Mon, 10 Apr 2023 03:56:51 GMT) ==
Regressions:
Ratio | Test | Platform | Options | Absolute values (old vs new) |
---|---|---|---|---|
36% | Base Content Explicit | linux1804-64-shippable-qr | fission | 7,358,122.67 -> 10,026,325.33 |
33% | Base Content Explicit | windows11-64-2009-shippable-qr | fission | 8,061,098.67 -> 10,696,533.33 |
30% | Base Content Explicit | macosx1015-64-shippable-qr | fission | 8,145,237.33 -> 10,578,773.33 |
30% | Base Content Explicit | macosx1015-64-shippable-qr | fission | 8,183,466.67 -> 10,615,466.67 |
15% | Base Content Resident Unique Memory | windows11-64-2009-shippable-qr | fission | 8,799,744.00 -> 10,138,112.00 |
10% | Base Content Resident Unique Memory | linux1804-64-shippable-qr | fission | 14,204,776.30 -> 15,640,405.33 |
9% | Explicit Memory | windows11-64-2009-shippable-qr | fission tp6 | 547,815,725.06 -> 595,818,959.90 |
For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=38036
Description
•