1564674 - Enable hard-decommit on windows for partial nursery chunks

Status: RESOLVED FIXED

(Core :: JavaScript: GC, enhancement, P1)

Description

[Paul Bone [:pbone]]

In Bug 1506733 Comment 11 erahm notes that decommit probably isn't working as intended on windows. Let's investigate this or maybe it's a difference with how the platforms report memory usage.

Unsure about macos for now.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a task, but please change it back in case of error.

Comment 2

[Jon Coppeard (:jonco)]

What we call 'decommit' is more a soft decommit, like calling madvise MADV_DONTNEED. It's not intended to actually decommit the memory.

Comment 3

[Paul Bone [:pbone]]

I've been talking with glandium and one idea he suggested was to hard-decommit these pages on Windows (and maybe also Macos) because doing so could avoid us getting into a low memory state, compared with soft-decommit which just allows us to handle low-memory states better.

Comment 4

[Jon Coppeard (:jonco)]

(In reply to Paul Bone [:pbone] from comment #3)
That doesn't work because we want to keep the virtual address space reserved (otherwise something else could get allocated there).

Comment 5

[Paul Bone [:pbone]]

(In reply to Jon Coppeard (:jonco) from comment #4)

(In reply to Paul Bone [:pbone] from comment #3)
That doesn't work because we want to keep the virtual address space reserved (otherwise something else could get allocated there).

Windows has specific calls/flags for explicitly decommitting memory while keeping that part of the virtual address space reserved. So at least there we can do this. VirtualAlloc has flags for reserving but not filling virtual memory.

Linux says that it will usually free MADV_FREE 'd pages immediately if there's no swap space configured. If swap space is configured it'll delay it. MADV_FREE is like MADV_DONTNEED but seems to be more immediate. The address space is still reserved and will be damand-filled with zero pages. Linux already reduces the RSS for MADV_DONTNEED pages so the case for changing this on Linux is weaker anyway.

Macos: gladium said that macos rarely releases MADV_FREE pages. And we might want to use mmap with MAP_FIXED instead. I can't see how that can be used to free pages but keep the address space reserved. Anyway, it looks like (Bug 670596 comment 5) mprotect(PROT_NONE) can be used on macos to do the same thing. And macos also has MADV_FREE_REUSEABLE (https://stackoverflow.com/a/53450077) which does the same thing as MADV_FREE except won't count the pages as part of RSS.

We should do the hard-decommit thing on Windows, MADV_FREE_REUSABLE on macos and keep our existing Linux code. That way on macos and LInux getting pages back that havn't actually been decommitted yet has zero cost.

Comment 6

[Paul Bone [:pbone]]

Attachment: Bug 1564674 - (part 1) Rename MarkPagesUnused r=sfink

The MarkPagesUnused function does a soft-decommit, advising the OS that it
can reclaim the pages. Renaming this function makes this clear and prepares
for introducing an alternative hard-decommit.

This change also comments-out a use of MarkPagesInUse because it's not used
with MarkPagesUnusedSoft.

Comment 7

[Paul Bone [:pbone]]

Attachment: Bug 1564674 - (part 2) Abstract out some common code r=sfink

Depends on D38607

Comment 8

[Paul Bone [:pbone]]

Attachment: Bug 1564674 - (part 3) Implement MarkPagesUnusedHard for Windows r=sfink

Depends on D38608

Comment 9

[Paul Bone [:pbone]]

Attachment: Bug 1564674 - (part 4) Use hard-decommit for partial nursery chunks r=sfink

Depends on D38609

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a67fe5cc0e3e
(part 1) Rename MarkPagesUnused r=sfink
https://hg.mozilla.org/integration/autoland/rev/daf350384772
(part 2) Abstract out some common code r=sfink
https://hg.mozilla.org/integration/autoland/rev/606e8a259d65
(part 3) Implement MarkPagesUnusedHard for Windows r=sfink
https://hg.mozilla.org/integration/autoland/rev/eb2471a4651a
(part 4) Use hard-decommit for partial nursery chunks r=sfink

Comment 11

[Paul Bone [:pbone]]

Hi again perfherders,

I'm expecting a positive (good, less memory) change in resident memory size on Windows.

Comment 12

[Paul Bone [:pbone]]

Attachment: Bug 1564674 - (part 5) Rename parameters in Memory.h r=sfink

I didn't notice that clang-tidy had flagged some issues with the earlier
patches. This patch renames these parameters to to match those in the
definitions of these functions.

Comment 13

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/a67fe5cc0e3e
https://hg.mozilla.org/mozilla-central/rev/daf350384772
https://hg.mozilla.org/mozilla-central/rev/606e8a259d65
https://hg.mozilla.org/mozilla-central/rev/eb2471a4651a

Comment 14

[Phabricator Automation]

Comment on attachment 9079927 [details]
Bug 1564674 - (part 5) Rename parameters in Memory.h r=sfink

Revision D38973 was moved to bug 1568406. Setting attachment 9079927 [details] to obsolete.

Comment 15

[Florin Strugariu [:Bebe]]

== Change summary for alert #22125 (as of Thu, 25 Jul 2019 05:00:58 GMT) ==

Improvements:

5% Base Content Resident Unique Memory windows10-64-shippable opt 15,180,117.33 -> 14,349,312.00
5% Base Content Resident Unique Memory windows10-64-shippable-qr opt 14,980,608.00 -> 14,185,984.00
5% Base Content Resident Unique Memory windows10-64-shippable-qr opt 14,968,405.33 -> 14,187,178.67

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=22125