1857841 - Try to reduce VirtualAlloc calls in mozjemalloc

Status: RESOLVED FIXED

(Core :: Memory Allocator, task)

Description

[Paul Bone [:pbone]]

Jeff Muizelaar found that mozjemalloc's allocation patterns can cause us to make very frequent calls to VirtualAlloc, at least more frequent that Chrome does.

He wrote:
pbone: another related thing that I saw today: During a run of 10 iterations of TodoMVC-> jquery we're doing 20x the number VirtualAllocs/VirtualFrees that Chrome is
[they're comming from] splitRun [see] https://share.firefox.dev/45scxBe

Comment 1

[Mike Hommey [:glandium]]

Off the top of my head, the only way to reduce the number of these VirtualAllocs would be to have something akin to the double purge we have on mac, and in any case, that will have an impact on RSS, since these VirtualAllocs are there to reduce memory usage (they decommit/recommit memory).

Comment 2

[Paul Bone [:pbone]]

Attachment: Bug 1857841 - pt 1. Modernise variable declarations in SplitRun r=glandium

Comment 3

[Paul Bone [:pbone]]

Attachment: Bug 1857841 - pt 3. InitChunk refactoring r=glandium

Depends on D195511

Comment 4

[Paul Bone [:pbone]]

Attachment: Bug 1857841 - pt 4. Don't decommit then recommit pages in InitChunk r=glandium

Depends on D195512

Comment 5

[Paul Bone [:pbone]]

Attachment: Bug 1857841 - pt 6. Commit more pages than necessary to amortise costs r=glandium

This breaks the old invariant that there are no committed pages that aren't
either allocated or dirty. There is now also new code in DeallocChunk that
counts the number of "fresh" committed pages and adjusts mStats.committed.

Depends on D195513

Comment 6

[Paul Bone [:pbone]]

Attachment: Bug 1857841 - pt 7. Also amortise VirtualAlloc in SplitRun() r=glandium

Depends on D195514

Comment 7

[Paul Bone [:pbone]]

https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=733e18745b8bdfe487c9da631453a48c75154f02&newProject=try&newRevision=8773bf14897fb5b0fb3a5c99cba3e3831905f8d7&framework=13&page=1&filter=speedometer&replicates=1

This is a 2-3% performance win if we don't mind a 13% memory increase.

I'll keep working on it.

Comment 8

[Paul Bone [:pbone]]

Actually the difference is in how the page cache is measured as a kind of explicit memory. But these pages aren't paged in and you can see that there's no difference to resident memory. So I think the memory usage difference is actually negligible.

Comment 9

[Paul Bone [:pbone]]

Attachment: Bug 1857841 - pt 2. Remove obsolete comment r=glandium

Comment 10

[Paul Bone [:pbone]]

Attachment: Bug 1857841 - pt 5. Simplify decommit logic in SplitRun() r=glandium

The decommit logic is now only used when MALLOC_DECOMMIT is defined. Now
the CHUNK_MAP_MADVISED bit is cleared when the page is initialised avoiding
leaving the chunk map in an inconsistent state if pages_commit failes and
simplifying some logic.

These changes will make the next patch simpler.

Comment 11

[Paul Bone [:pbone]]

Attachment: Bug 1857841 - pt 9. Count fresh and madvised memory separately r=glandium

This memory was previously measured as part of committed memory. However
since it probably has no physical page allocated it's more correct to count
it as clean. This has the side effect of now including it in the page cache
and therefore decisions like when to purge memory will change.

Comment 12

[Paul Bone [:pbone]]

When recording the memory usage properly the benchmarks are much better: https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=733e18745b8bdfe487c9da631453a48c75154f02&newProject=try&newRevision=88392f200d142403cd0705f57c9a096fdb72c99f&framework=13&page=1 Click through to the awsy results too.

Comment 13

[Paul Bone [:pbone]]

Attachment: Bug 1857841 - pt 8. Rename CHUNK_MAP_MADVISED r=glandium

CHUNK_MAP_MADVISED is also used for never-touched pages. Rename it
CHUNK_MAP_CLEAN since it's the clean page cache.

Comment 14

[Paul Bone [:pbone]]

Attachment: Bug 1857841 - pt 3. Add a new page kind named "fresh" r=glandium

Also re-write the descriptions of the page kinds.

Comment 15

[Paul Bone [:pbone]]

Attachment: Bug 1857841 - pt 8. Use CHUNK_MAP_FRESH for double-purged pages r=glandium

Using CHUNK_MAP_FRESH for double purged pages fits better than overloading
the CHUNK_MAP_DECOMMITTED bit with two different semantics.

Comment 16

[Paul Bone [:pbone]]

Attachment: Bug 1857841 - pt 10. Tighten an assertion r=glandium

Comment 17

[Paul Bone [:pbone]]

I'm waiting for the code freeze to end before landing this.

Comment 18

[Paul Bone [:pbone]]

Here's the latest performance comparison:

https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=cd81556b73b839957e9de7370c2978718e26fbcf&newProject=try&newRevision=7afe1cfe228d6c3cc43b92778425d0edb4d9b422&framework=13&page=1

There are tests in both "browsertime" and "awsy". In the awsy tests the big difference in explicit memory is a change in how we measure memory - not a real memory difference, for that please look at "resident".

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a09b357080f
pt 1. Modernise variable declarations in SplitRun r=glandium
https://hg.mozilla.org/integration/autoland/rev/83e751e19af4
pt 2. Remove obsolete comment r=glandium
https://hg.mozilla.org/integration/autoland/rev/fe61be13535d
pt 3. Add a new page kind named "fresh" r=glandium
https://hg.mozilla.org/integration/autoland/rev/1a70135ddbda
pt 4. Don't decommit then recommit pages in InitChunk r=glandium
https://hg.mozilla.org/integration/autoland/rev/afe7c24ca50e
pt 5. Simplify decommit logic in SplitRun() r=glandium
https://hg.mozilla.org/integration/autoland/rev/52f8506f3bfb
pt 6. Commit more pages than necessary to amortise costs r=glandium
https://hg.mozilla.org/integration/autoland/rev/f335f70a1c14
pt 7. Also amortise VirtualAlloc in SplitRun() r=glandium
https://hg.mozilla.org/integration/autoland/rev/82252f397be4
pt 9. Count fresh and madvised memory separately r=glandium
https://hg.mozilla.org/integration/autoland/rev/86d16de09fc9
pt 8. Use CHUNK_MAP_FRESH for double-purged pages r=glandium
https://hg.mozilla.org/integration/autoland/rev/37720af4970f
pt 10. Tighten an assertion r=glandium

Comment 20

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/7a09b357080f
https://hg.mozilla.org/mozilla-central/rev/83e751e19af4
https://hg.mozilla.org/mozilla-central/rev/fe61be13535d
https://hg.mozilla.org/mozilla-central/rev/1a70135ddbda
https://hg.mozilla.org/mozilla-central/rev/afe7c24ca50e
https://hg.mozilla.org/mozilla-central/rev/52f8506f3bfb
https://hg.mozilla.org/mozilla-central/rev/f335f70a1c14
https://hg.mozilla.org/mozilla-central/rev/82252f397be4
https://hg.mozilla.org/mozilla-central/rev/86d16de09fc9
https://hg.mozilla.org/mozilla-central/rev/37720af4970f

Comment 21

[Mayank Bansal]

Some improvements from AWFY-SP2

5% improvement jqueryTodoMVC
4.2% on Vanilla-ES2015-Babel-Webpack-TodoMVC
3.5% on Vanilla-ES2015-TodoMVC

Comment 22

[Mayank Bansal]

4.6% improvement on SP3-TodoMVC-Javascript-ES5 on mozilla-central