1506733 - Try to decommit unused part of nursery chunks

Status: RESOLVED FIXED

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

Description

[Paul Bone [:pbone]]

After Bug 1433007 lands the nursery will be able to use fractions of a chunk.  This allows us to decommit the parts of a chunk that we're not using.

Comment 1

[Andrew McCreight [:mccr8]]

Paul, is this still relevant? I know some work landed already about reducing the size of the nursery.

Comment 2

[Paul Bone [:pbone]]

Yep. it's all related.

I'll summarise but let me know if you need more info.

Bug 1433007 allows the nursery to use less than 1MB, but that memory is not released to the OS, it's just not actively used by JS or anything else.
This bug proposes to decommit that memory, releasing the physical RAM to the OS.
Bug 1499570 landed yesterday and decommits whole chunks as the nursery releases them. It's a separate case of where we can handle memory better, but related because it involved decommitting memory & the JS Nursery.

Comment 3

[Paul Bone [:pbone]]

Attachment: Bug 1506733 - (part 1) Rename NurseryDecommitChunkTask r=jonco

This class will not just decommit whole chunks, so rename it to not imply
that as much.

Comment 4

[Paul Bone [:pbone]]

Attachment: Bug 1506733 - (part 2) Refactor how the NurseryDecomitter finishes r=jonco

This refactoring is required for the next change.

Depends on D35589

Comment 5

[Paul Bone [:pbone]]

Attachment: Bug 1506733 - (part 4) Decommit unused portion of Nursery chunks r=jonco

Depends on D35590

Comment 6

[Paul Bone [:pbone]]

Hrm, here's a try build but it doesn't have the expected change. I'll look for bugs later:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=31d1ed41c604667a902d06fd41a7cfe8fcdb2164&newProject=try&newRevision=6a102b462e29ec71d124bde9791e02f24bdc84df&framework=4

Comment 7

[Paul Bone [:pbone]]

Attachment: Bug 1506733 - (part 3) Re-initalise memory regions when growing the nursery r=jonco

Depends on D35590

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/78a0e74fae92
(part 1) Rename NurseryDecommitChunkTask r=jonco
https://hg.mozilla.org/integration/autoland/rev/2d990d87e35b
(part 2) Refactor how the NurseryDecomitter finishes r=jonco
https://hg.mozilla.org/integration/autoland/rev/f381dc55a8bc
(part 3) Re-initalise memory regions when growing the nursery r=jonco
https://hg.mozilla.org/integration/autoland/rev/fbfcb9fbf6c5
(part 4) Decommit unused portion of Nursery chunks r=jonco

Comment 9

[Paul Bone [:pbone]]

Hi perfherders.

Hopefully this will drop minimum resident memory by 768K.

Thanks.

Comment 10

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/78a0e74fae92
https://hg.mozilla.org/mozilla-central/rev/2d990d87e35b
https://hg.mozilla.org/mozilla-central/rev/f381dc55a8bc
https://hg.mozilla.org/mozilla-central/rev/fbfcb9fbf6c5

Comment 11

[Eric Rahm [:erahm]]

Paul, it looks like this showed an improvement on Linux but not other platforms. Is it possible the decommit logic isn't doing the right thing on other platforms, or are we only expecting this to work on Linux?

There does appear to be a difference in logic of SpiderMonkey's decommit and mozjemalloc's decommit (VirtualAlloc vs VirtualFree). This might be intentional or the term decommit might not even mean the same thing in these contexts.

Comment 12

[Paul Bone [:pbone]]

Thanks erahm,

I was expecting this to be cross platform because I assumed that code "just worked". I've filed Bug 1564674.

Comment 13

[Jon Coppeard (:jonco)]

(In reply to Eric Rahm [:erahm] from comment #11)

the term decommit might not even mean the same thing in these contexts

Indeed, we use the term decommit to mean something else here. This is arguably a bug itself.

Comment 14

[Ionuț Goldan [:igoldan]]

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

Hi perfherders.

Hopefully this will drop minimum resident memory by 768K.

Thanks.

Thanks for the heads up!