1800010 - Make PHC more aggressive on Nightly.

Status: RESOLVED FIXED

(Core :: Memory Allocator, enhancement)

Description

[Randell Jesup [:jesup] (needinfo me)]

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)

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Bug 1800010 - Make PHC more aggressive r=glandium

Comment 2

[Paul Bone [:pbone]]

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.

Comment 3

[Paul Bone [:pbone]]

decoder, I hope you don't mind if I take this bug.

Comment 4

[Paul Bone [:pbone]]

Attachment: Bug 1800010 - Log lifetimes in PHC r=glandium

Depends on D161934

Comment 5

[Paul Bone [:pbone]]

I've started try jobs to compare the performance with and without these patches. I started talos, awsy, browsertime-essential and speedometer tests:

https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=93a1d48d4059b342ae3e2a2e379c71259b1fc8cb&newProject=try&newRevision=ec0fe13bc712ce871ac8f4302231551edbba7c4d&framework=1&page=1

Comment 6

[Paul Bone [:pbone]]

(In reply to Paul Bone [:pbone] from comment #5)

https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=93a1d48d4059b342ae3e2a2e379c71259b1fc8cb&newProject=try&newRevision=ec0fe13bc712ce871ac8f4302231551edbba7c4d&framework=1&page=1

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.

Comment 7

[Paul Bone [:pbone]]

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?

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

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.

Comment 9

[Paul Bone [:pbone]]

Here's a perfherder compare for this change, the memory increase is what we were expecting.

https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=dcb550adca95248a895548e3380af73c5ec63fd6&newProject=try&newRevision=198328cc01b4d99d3cde1ace73e8dbb5ba9af1cf&framework=4&page=1&pageTitle=Make+PHC+more+agressive

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4086829b990a
Make PHC more aggressive r=glandium
https://hg.mozilla.org/integration/autoland/rev/39c07b603ef5
Log lifetimes in PHC r=glandium

Comment 11

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/4086829b990a
https://hg.mozilla.org/mozilla-central/rev/39c07b603ef5

Comment 12

[Paul Bone [:pbone]]

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

[Acasandrei Beatrice (needinfo me)]

(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