Fix logic for doing incremental marking steps on tenured allocation.

Fix logic for doing incremental marking steps on tenured allocation.

R=hpayer@chromium.org
BUG=

Committed: https://crrev.com/9716468ae63500adb74f5188c47de847e195d71b
Cr-Commit-Position: refs/heads/master@{#27883}

[Erik Corry Chromium.org, 2015-03-31 13:11:33]

This makes more progress on incremental marking when there is a lot of
pretenured allocation going on, as in the JSON parser.  It reduces the peak
memory use on a recent example:

erikcorry-macbookpro2:v8 erikcorry$ ./out/x64.release/d8 --test
--random-seed=181912476  --trace-gc  regress-3976.js | grep biggest
[90088]      108 ms: Mark-sweep 19.3 (52.1) -> 18.5 (55.1) MB, 14.3 ms (+ 9.2 ms
in 170 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC in
old space requested].
[90088]      490 ms: Mark-sweep 130.5 (156.1) -> 121.2 (158.1) MB, 61.3 ms (+
57.1 ms in 701 steps since start of marking, biggest step 0.3 ms) [GC interrupt]
[GC in old space requested].
[90088]     1135 ms: Mark-sweep 508.8 (542.9) -> 51.2 (103.9) MB, 33.8 ms (+ 1.3
ms in 13 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
in old space requested].
[90088]     1338 ms: Mark-sweep 228.5 (265.9) -> 51.2 (102.9) MB, 12.4 ms (+ 1.1
ms in 12 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
in old space requested].
[90088]     1538 ms: Mark-sweep 228.4 (264.9) -> 51.2 (102.9) MB, 12.5 ms (+ 1.1
ms in 12 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
in old space requested].
[90088]     1738 ms: Mark-sweep 229.5 (266.9) -> 51.2 (103.9) MB, 12.9 ms (+ 1.1
ms in 13 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
in old space requested].
[90088]     1961 ms: Mark-sweep 228.5 (265.9) -> 51.2 (102.9) MB, 12.1 ms (+ 1.3
ms in 12 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
in old space requested].
erikcorry-macbookpro2:v8 erikcorry$ ./out/x64.release/d8-oldspacestep --test
--random-seed=181912476  --trace-gc  regress-3976.js | grep biggest
[90095]       91 ms: Mark-sweep 19.3 (52.1) -> 18.5 (55.1) MB, 14.1 ms (+ 8.5 ms
in 170 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC in
old space requested].
[90095]      364 ms: Mark-sweep 103.6 (129.1) -> 94.7 (131.1) MB, 34.8 ms (+
59.2 ms in 1042 steps since start of marking, biggest step 0.2 ms) [GC
interrupt] [GC in old space requested].
[90095]      885 ms: Mark-sweep 380.6 (414.9) -> 51.2 (102.9) MB, 25.2 ms (+ 1.0
ms in 15 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
in old space requested].
[90095]     1064 ms: Mark-sweep 209.1 (245.9) -> 51.2 (100.9) MB, 16.6 ms (+ 1.3
ms in 15 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
in old space requested].
[90095]     1232 ms: Mark-sweep 209.1 (245.9) -> 51.1 (101.9) MB, 10.9 ms (+ 0.9
ms in 15 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC
in old space requested].
[90095]     1396 ms: Mark-sweep 209.1 (245.9) -> 51.1 (100.9) MB, 10.7 ms (+ 0.9
ms in 15 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC
in old space requested].
[90095]     1562 ms: Mark-sweep 209.1 (245.9) -> 51.1 (101.9) MB, 10.8 ms (+ 0.9
ms in 15 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC
in old space requested].
[90095]     1726 ms: Mark-sweep 209.1 (245.9) -> 51.1 (100.9) MB, 10.8 ms (+ 0.9
ms in 15 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC
in old space requested].
[90095]     1890 ms: Mark-sweep 209.1 (245.9) -> 51.1 (101.9) MB, 10.8 ms (+ 0.9
ms in 15 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC
in old space requested].

Incremental marking pauses caused by allocation are slightly lower at 0.2ms on a
Mac which as a rule of thumb will be 2ms on a phone.

https://codereview.chromium.org/1040233003/diff/1/src/heap/incremental-markin...
File src/heap/incremental-marking.cc (right):

https://codereview.chromium.org/1040233003/diff/1/src/heap/incremental-markin...
src/heap/incremental-marking.cc:839: Start(PREVENT_COMPACTION);
Time to revisit this TODO if we didn't already today?

https://codereview.chromium.org/1040233003/diff/1/src/heap/spaces.cc
File src/heap/spaces.cc (left):

https://codereview.chromium.org/1040233003/diff/1/src/heap/spaces.cc#oldcode2379
src/heap/spaces.cc:2379: old_linear_size);
This was basically wrong, basing the step on the size of one allocation plus
what was left of the old linear allocation area, but ignoring the rest of the
new linear allocation area.

[Hannes Payer (out of office), 2015-04-02 12:59:20]

On 2015/03/31 13:11:33, Erik Corry Chromium.org wrote:
> This makes more progress on incremental marking when there is a lot of
> pretenured allocation going on, as in the JSON parser.  It reduces the peak
> memory use on a recent example:
> 
> erikcorry-macbookpro2:v8 erikcorry$ ./out/x64.release/d8 --test
> --random-seed=181912476  --trace-gc  regress-3976.js | grep biggest
> [90088]      108 ms: Mark-sweep 19.3 (52.1) -> 18.5 (55.1) MB, 14.3 ms (+ 9.2
ms
> in 170 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC
in
> old space requested].
> [90088]      490 ms: Mark-sweep 130.5 (156.1) -> 121.2 (158.1) MB, 61.3 ms (+
> 57.1 ms in 701 steps since start of marking, biggest step 0.3 ms) [GC
interrupt]
> [GC in old space requested].
> [90088]     1135 ms: Mark-sweep 508.8 (542.9) -> 51.2 (103.9) MB, 33.8 ms (+
1.3
> ms in 13 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
> in old space requested].
> [90088]     1338 ms: Mark-sweep 228.5 (265.9) -> 51.2 (102.9) MB, 12.4 ms (+
1.1
> ms in 12 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
> in old space requested].
> [90088]     1538 ms: Mark-sweep 228.4 (264.9) -> 51.2 (102.9) MB, 12.5 ms (+
1.1
> ms in 12 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
> in old space requested].
> [90088]     1738 ms: Mark-sweep 229.5 (266.9) -> 51.2 (103.9) MB, 12.9 ms (+
1.1
> ms in 13 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
> in old space requested].
> [90088]     1961 ms: Mark-sweep 228.5 (265.9) -> 51.2 (102.9) MB, 12.1 ms (+
1.3
> ms in 12 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
> in old space requested].
> erikcorry-macbookpro2:v8 erikcorry$ ./out/x64.release/d8-oldspacestep --test
> --random-seed=181912476  --trace-gc  regress-3976.js | grep biggest
> [90095]       91 ms: Mark-sweep 19.3 (52.1) -> 18.5 (55.1) MB, 14.1 ms (+ 8.5
ms
> in 170 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC
in
> old space requested].
> [90095]      364 ms: Mark-sweep 103.6 (129.1) -> 94.7 (131.1) MB, 34.8 ms (+
> 59.2 ms in 1042 steps since start of marking, biggest step 0.2 ms) [GC
> interrupt] [GC in old space requested].
> [90095]      885 ms: Mark-sweep 380.6 (414.9) -> 51.2 (102.9) MB, 25.2 ms (+
1.0
> ms in 15 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
> in old space requested].
> [90095]     1064 ms: Mark-sweep 209.1 (245.9) -> 51.2 (100.9) MB, 16.6 ms (+
1.3
> ms in 15 steps since start of marking, biggest step 0.2 ms) [GC interrupt] [GC
> in old space requested].
> [90095]     1232 ms: Mark-sweep 209.1 (245.9) -> 51.1 (101.9) MB, 10.9 ms (+
0.9
> ms in 15 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC
> in old space requested].
> [90095]     1396 ms: Mark-sweep 209.1 (245.9) -> 51.1 (100.9) MB, 10.7 ms (+
0.9
> ms in 15 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC
> in old space requested].
> [90095]     1562 ms: Mark-sweep 209.1 (245.9) -> 51.1 (101.9) MB, 10.8 ms (+
0.9
> ms in 15 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC
> in old space requested].
> [90095]     1726 ms: Mark-sweep 209.1 (245.9) -> 51.1 (100.9) MB, 10.8 ms (+
0.9
> ms in 15 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC
> in old space requested].
> [90095]     1890 ms: Mark-sweep 209.1 (245.9) -> 51.1 (101.9) MB, 10.8 ms (+
0.9
> ms in 15 steps since start of marking, biggest step 0.1 ms) [GC interrupt] [GC
> in old space requested].
> 
> Incremental marking pauses caused by allocation are slightly lower at 0.2ms on
a
> Mac which as a rule of thumb will be 2ms on a phone.
> 
>
https://codereview.chromium.org/1040233003/diff/1/src/heap/incremental-markin...
> File src/heap/incremental-marking.cc (right):
> 
>
https://codereview.chromium.org/1040233003/diff/1/src/heap/incremental-markin...
> src/heap/incremental-marking.cc:839: Start(PREVENT_COMPACTION);
> Time to revisit this TODO if we didn't already today?
> 
> https://codereview.chromium.org/1040233003/diff/1/src/heap/spaces.cc
> File src/heap/spaces.cc (left):
> 
>
https://codereview.chromium.org/1040233003/diff/1/src/heap/spaces.cc#oldcode2379
> src/heap/spaces.cc:2379: old_linear_size);
> This was basically wrong, basing the step on the size of one allocation plus
> what was left of the old linear allocation area, but ignoring the rest of the
> new linear allocation area.

Did you look into Splay and SplayLatency performance?

[Hannes Payer (out of office), 2015-04-02 13:36:39]

https://codereview.chromium.org/1040233003/diff/1/src/heap/incremental-markin...
File src/heap/incremental-marking.cc (right):

https://codereview.chromium.org/1040233003/diff/1/src/heap/incremental-markin...
src/heap/incremental-marking.cc:839: Start(PREVENT_COMPACTION);
On 2015/03/31 13:11:32, Erik Corry Chromium.org wrote:
> Time to revisit this TODO if we didn't already today?

Done.

[Erik Corry Chromium.org, 2015-04-02 14:38:53]

Splay performance is very positive: +14% on 32 bit, +5% on 64 bit
Splay latency is more mixed: +5% on 32 bit, -8% on 64 bit.

When I run 64 bit on the command line we are getting 6 scavenges, 6
mark-compacts during splay, where before we got 5 of each.  The actual pauses
don't look worse, there are just more of them.

However, the peak memory usage is much improved during Splay.  Before:

Mark-sweep 488.5 (526.1) -> 99.7 (409.1) MB, 14.7 ms

After:

373.6 (415.1) -> 84.2 (379.1) MB, 10.0 ms

This is on a Mac, so freeing up 117 pages will actually cost us dearly due to
munmap.

Strangely the patch makes scavenges slower and old space collections faster.

[Hannes Payer (out of office), 2015-04-02 16:36:02]

On 2015/04/02 14:38:53, Erik Corry Chromium.org wrote:
> Splay performance is very positive: +14% on 32 bit, +5% on 64 bit
> Splay latency is more mixed: +5% on 32 bit, -8% on 64 bit.
> 
> When I run 64 bit on the command line we are getting 6 scavenges, 6
> mark-compacts during splay, where before we got 5 of each.  The actual pauses
> don't look worse, there are just more of them.
> 
> However, the peak memory usage is much improved during Splay.  Before:
> 
> Mark-sweep 488.5 (526.1) -> 99.7 (409.1) MB, 14.7 ms
> 
> After:
> 
> 373.6 (415.1) -> 84.2 (379.1) MB, 10.0 ms
> 
> This is on a Mac, so freeing up 117 pages will actually cost us dearly due to
> munmap.
> 
> Strangely the patch makes scavenges slower and old space collections faster.

I guess we are doing more incremental marking during scavenges with this patch.
Is this tanking the SplayLatency score on 64bit?

[Erik Corry Chromium.org, 2015-04-13 07:38:11]

I fixed a bug that sometimes triggers where it would not start incremental
marking in time and would eventually hit a stop-the-world mark-sweep.

This CL changes all the timings of GCs in Octane after splay, so it shakes up
the bag of benchmarks, but the end result looks neutral in terms of performance,
and the peak memory usage is reduced.  There's a surprising amount of randomness
in the GCs even in the 'predictable' mode of Octane.  See
https://docs.google.com/spreadsheets/d/1eyjWDkCSEdjuU9rmBGIHZ2s6a1wdmrIHgpoXN...
for graphs.

PTAL

[Hannes Payer (out of office), 2015-04-14 12:56:46]

lgtm

[Erik Corry Chromium.org, 2015-04-16 12:21:32]

The CQ bit was checked by erikcorry@chromium.org

[Erik Corry Chromium.org, 2015-04-16 12:21:33]

The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org
Link to the patchset: https://codereview.chromium.org/1040233003/#ps60001
(title: "Don't do huge steps in response to large object allocation")

[commit-bot: I haz the power, 2015-04-16 12:21:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1040233003/60001

[commit-bot: I haz the power, 2015-04-16 13:20:53]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-16 13:21:11]

Patchset 4 (id:??) landed as
https://crrev.com/9716468ae63500adb74f5188c47de847e195d71b
Cr-Commit-Position: refs/heads/master@{#27883}

[Jakob Kummerow, 2015-04-20 15:07:47]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1099633002/ by jkummerow@chromium.org.

The reason for reverting is: Suspected to trigger memory corruption issues, e.g.
crbug.com/478401.

The testcase in https://cluster-fuzz.appspot.com/testcase?key=5781432505466880
contains an object that's promoted from new space onto an evacuation candidate,
which hpayer says shouldn't happen, and bisects to this commit..