1529922 - Trailing guard pages for huge allocations

Status: RESOLVED FIXED

(Core :: Memory Allocator, enhancement)

Description

[Gian-Carlo Pascutto [:gcp]]

Split off from bug 1446040. Trailing guard pages for normal allocations are relatively easier, and should be able to land ahead of the rest of the work.

Comment 1

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b16f785ee0e7783b15a8fb1ae46555453e948b73

Comment 2

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f0fa8a77d9e17a32f5babb2e22ba6456bf2fb9df

Comment 3

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a391630719fd3ae8a8ecd838de23199b6f58ceb

Comment 4

[Gian-Carlo Pascutto [:gcp]]

Attachment: Bug 1529922 - Part 1: Add guard pages following huge allocations.

Comment 5

[Gian-Carlo Pascutto [:gcp]]

Changes compared to Matt's original patch:

1. Make sure guard pages are always applied on non-Windows systems on realloc.
2. Deal with a nasty edge case where we realloc and now there's no more room for a guard.
3. Simplify code to remove separate Windows (DECOMMIT) and non-Windows paths.
4. apply/remove guard calls the (de)commit function so memory gets tagged.
5. Poison zone between the end of the allocation and the start of the guard.
6. Fixes for the tests on non-Windows, add test to check for guards when enlarging.
7. Tiny optimization to zero only the part of the chunk that will be used.

Comment 6

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1afb6cdca6fec15b9a71fa40e1cf7e29f3046f5a

Comment 7

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=de657d4c84b651ec6c85cd55e091ff81c379003b

Comment 8

[Gian-Carlo Pascutto [:gcp]]

Attachment: Bug 1529922 - Part 1: Add guard pages following huge allocations. r=mhowell!,glandium!

Comment 9

[Gian-Carlo Pascutto [:gcp]]

Attachment: Bug 1529922 - Part 2: Trailing guard regions for normal allocations. r=mhowell!,glandium!

Depends on D21563

Comment 10

[Gian-Carlo Pascutto [:gcp]]

Attachment: Bug 1537781 - Add a test for guard regions in runs. r=glandium

Depends on D21565

Comment 11

[Gian-Carlo Pascutto [:gcp]]

Attachment: Bug 1529922 - Part 4: Allow madvise(.., MADV_NORMAL) in the GMP sandbox. r=jld!

Depends on D21566

Comment 12

[Gian-Carlo Pascutto [:gcp]]

Attachment: Bug 1529922 - Part 5: Poison area in between allocations and guards. r=glandium!

Depends on D21567

Comment 13

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=702ec3f7887bad157f464d33d0df134de8a8bf85

Comment 14

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=924dc99e1cc23c5e30a5613193bf37fb9ac06591

Comment 15

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=06fe48c084b065460c0c860d329b64c0cd1dc7b0

Comment 16

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=da2ab3b942d9a847e1261c1dfa8e8e894155e962

Comment 17

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f89c74fd2843b0bb97b11675f253792d78ad529b

Comment 18

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=af93be76a47fd08c161b2362cad9450fd1498656

Comment 19

[Gian-Carlo Pascutto [:gcp]]

Attachment: Bug 1529922 - Add guard pages following huge allocations. r=glandium!

Comment 20

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ead783df3c5891f25221eae6ead8c892c5d9f10

Comment 21

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bcaf91de0e7231d3df5a80cc8e526a458915511c

Comment 22

[Gian-Carlo Pascutto [:gcp]]

Hopefully the last try push, to check the final performance and memory impact.

Comment 23

[Gian-Carlo Pascutto [:gcp]]

I have an implementation for normal pages done in the way glandium suggested, i.e. by making the DECOMMIT logic the general case and then not allocating the final page during the initial run, together with some fixes to prevent that page being included in coalescing/extending. Need to do some more testing first though.

Comment 24

[Gian-Carlo Pascutto [:gcp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ced39480d6f36ca941e2e783034276a6e67ab6ed

Comment 25

[Gian-Carlo Pascutto [:gcp]]

Attachment: Bug 1537781 - Guard pages for normal allocations. r=glandium

Depends on D23292

Comment 26

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/becf3a27c2bb

Comment 27

[Gian-Carlo Pascutto [:gcp]]

Ouch, looks like moz-phab is clueless that the bug has been moved.

Comment 28

[Phabricator Automation]

Comment on attachment 9052180 [details]
Bug 1537781 - Guard pages for normal allocations. r=glandium

Revision D24135 was moved to bug 1537781. Setting attachment 9052180 [details] to obsolete.

Comment 29

[Phabricator Automation]

Comment on attachment 9047448 [details]
Bug 1537781 - Add a test for guard regions in runs. r=glandium

Revision D21566 was moved to bug 1537781. Setting attachment 9047448 [details] to obsolete.

Comment 30

[Eric Rahm [:erahm]]

glandium, any idea why we saw such a big memory win for this on macOS?

Comment 31

[Mike Hommey [:glandium]]

I think what's happening is that pages_decommit, which now just maps fresh new pages, is better at reducing RSS than when it was using madvise(MADV_FREE).