Reland of "[heap] Switch to 500k pages"

Reland of "[heap] Switch to 500k pages"

Decrease regular heap object size to 400k. In a follow up, we can now get rid of
the new space border page while keeping the 1M minimum new space size.

BUG=chromium:636331
This reverts commit 555c961990088d545e2b475da3872bd63fa27569.

Committed: https://crrev.com/20e2ea80e169e85c5b8231adc02901fb6c989609
Cr-Commit-Position: refs/heads/master@{#38608}

[Michael Lippautz, 2016-08-10 17:27:52]

Description was changed from

==========
Reland of "[heap] Switch to 500k pages"

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.

BUG=chromium:636331
==========

to

==========
Reland of "[heap] Switch to 500k pages"

- Decrease regular heap object size to 300k, keeping the same ration (60%)
  between this limit and page size.

In a follow up, we can now get rid of the new space border page while
keeping the 1M minimum new space size.

BUG=chromium:636331

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.
==========

[Michael Lippautz, 2016-08-10 17:28:07]

Description was changed from

==========
Reland of "[heap] Switch to 500k pages"

- Decrease regular heap object size to 300k, keeping the same ration (60%)
  between this limit and page size.

In a follow up, we can now get rid of the new space border page while
keeping the 1M minimum new space size.

BUG=chromium:636331

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.
==========

to

==========
Reland of "[heap] Switch to 500k pages"

Decrease regular heap object size to 300k, keeping the same ration (60%)
between this limit and page size. In a follow up, we can now get rid of 
the new space border page while keeping the 1M minimum new space size.

BUG=chromium:636331

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.
==========

[Michael Lippautz, 2016-08-10 17:30:10]

Description was changed from

==========
Reland of "[heap] Switch to 500k pages"

Decrease regular heap object size to 300k, keeping the same ration (60%)
between this limit and page size. In a follow up, we can now get rid of 
the new space border page while keeping the 1M minimum new space size.

BUG=chromium:636331

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.
==========

to

==========
Reland of "[heap] Switch to 500k pages"

Decrease regular heap object size to 400k. In a follow up, we can now get rid of

the new space border page while keeping the 1M minimum new space size.

BUG=chromium:636331

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.
==========

[Michael Lippautz, 2016-08-10 17:30:18]

Description was changed from

==========
Reland of "[heap] Switch to 500k pages"

Decrease regular heap object size to 400k. In a follow up, we can now get rid of

the new space border page while keeping the 1M minimum new space size.

BUG=chromium:636331

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.
==========

to

==========
Reland of "[heap] Switch to 500k pages"

Decrease regular heap object size to 400k. In a follow up, we can now get rid of
the new space border page while keeping the 1M minimum new space size.

BUG=chromium:636331

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.
==========

[Michael Lippautz, 2016-08-10 17:30:29]

Description was changed from

==========
Reland of "[heap] Switch to 500k pages"

Decrease regular heap object size to 400k. In a follow up, we can now get rid of
the new space border page while keeping the 1M minimum new space size.

BUG=chromium:636331

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.
==========

to

==========
Reland of "[heap] Switch to 500k pages"

Decrease regular heap object size to 400k. In a follow up, we can now get rid of
the new space border page while keeping the 1M minimum new space size.

BUG=chromium:636331

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.
==========

[Michael Lippautz, 2016-08-10 17:30:46]

The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 17:30:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Michael Lippautz, 2016-08-10 17:35:05]

Patchset #3 (id:40001) has been deleted

[Michael Lippautz, 2016-08-10 17:43:00]

The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 17:43:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 18:31:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-10 18:31:32]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2016-08-10 21:17:34]

hpayer@chromium.org changed reviewers:
+ hpayer@chromium.org

[Hannes Payer (out of office), 2016-08-10 21:17:35]

https://codereview.chromium.org/2232653003/diff/80001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2232653003/diff/80001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:1245: for (HeapObject* obj = it.Next(); obj != nullptr; obj =
it.Next()) {
This seems to be pretty costly. Let's discuss this offline.

https://codereview.chromium.org/2232653003/diff/80001/src/heap/spaces.h
File src/heap/spaces.h (right):

https://codereview.chromium.org/2232653003/diff/80001/src/heap/spaces.h#newco...
src/heap/spaces.h:2147: // high water mark.
We only do that for immortal immovable pages.

https://codereview.chromium.org/2232653003/diff/80001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2232653003/diff/80001/src/objects.h#newcode10470
src/objects.h:10470: // 400 * KB is the Page::kMaxRegularHeapObjectSize defined
in spaces.h which
Why not 300?

https://codereview.chromium.org/2232653003/diff/80001/test/cctest/heap/test-h...
File test/cctest/heap/test-heap.cc (right):

https://codereview.chromium.org/2232653003/diff/80001/test/cctest/heap/test-h...
test/cctest/heap/test-heap.cc:2386: i::FLAG_predictable = true;
Why?
And to avoid shrinking, just set the min_semi_space_size.

[Michael Lippautz, 2016-08-11 09:48:13]

See delta PS4-PS5

https://codereview.chromium.org/2232653003/diff/80001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2232653003/diff/80001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:1245: for (HeapObject* obj = it.Next(); obj != nullptr; obj =
it.Next()) {
On 2016/08/10 21:17:35, Hannes Payer wrote:
> This seems to be pretty costly. Let's discuss this offline.

As discussed offline: Switch back to using the high water mark.

Since immortal immovable pages are not swept with this patch, we can
safely use the water mark even with intermediate GCs happening. The
only difficulty is accounting, which is different depending on whether
a GC happened, or not.

https://codereview.chromium.org/2232653003/diff/80001/src/heap/spaces.h
File src/heap/spaces.h (right):

https://codereview.chromium.org/2232653003/diff/80001/src/heap/spaces.h#newco...
src/heap/spaces.h:2147: // high water mark.
On 2016/08/10 21:17:35, Hannes Payer wrote:
> We only do that for immortal immovable pages.

Done.

https://codereview.chromium.org/2232653003/diff/80001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2232653003/diff/80001/src/objects.h#newcode10470
src/objects.h:10470: // 400 * KB is the Page::kMaxRegularHeapObjectSize defined
in spaces.h which
On 2016/08/10 21:17:35, Hannes Payer wrote:
> Why not 300?

Crankshaft (and esp hydrogen stubs) don't check for kMaxRegularHeapObjectSize at
all places. The problem is more involved, as the stubs often require objects to
be allocated in new space to avoid write barriers.

Since those stubs are already deprecated and in the process of being move to
turbo fan we just mitigate it by staying at 400 (which will not trigger a case).
This is merely a problem with our verification, as allocating a large object in
new space works like expected (except that it should not go there ;)).

Turbofan already suffers from similar issues: tracking bug: chromium:636391

https://codereview.chromium.org/2232653003/diff/80001/test/cctest/heap/test-h...
File test/cctest/heap/test-heap.cc (right):

https://codereview.chromium.org/2232653003/diff/80001/test/cctest/heap/test-h...
test/cctest/heap/test-heap.cc:2386: i::FLAG_predictable = true;
On 2016/08/10 21:17:35, Hannes Payer wrote:
> Why?
> And to avoid shrinking, just set the min_semi_space_size.

This test checks shrinking. With predictable I merely disabled the shrinking
during the GC. This is already a flake on ToT.

https://codereview.chromium.org/2232653003/diff/100001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2232653003/diff/100001/src/heap/spaces.cc#new...
src/heap/spaces.cc:1249: #ifdef DEBUG
Debug verification useful?

[Michael Lippautz, 2016-08-11 11:25:44]

The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-11 11:25:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 11:41:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-11 11:41:37]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/10804)
  v8_linux_dbg_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/b...)

[Michael Lippautz, 2016-08-11 11:43:29]

The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-11 11:43:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 11:59:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-11 11:59:30]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/6832)
  v8_mac_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng_triggered/bui...)

[Michael Lippautz, 2016-08-11 13:49:46]

The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-11 13:49:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 14:08:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-11 14:08:30]

Dry run: Try jobs failed on following builders:
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
  v8_win_nosnap_shared_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)

[Michael Lippautz, 2016-08-11 15:38:00]

The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-11 15:38:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 16:11:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-11 16:12:00]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2016-08-11 20:12:24]

https://codereview.chromium.org/2232653003/diff/180001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/2232653003/diff/180001/src/heap/heap.cc#newco...
src/heap/heap.cc:5431: deserialization_complete_ = true;
Can you move deserialization_complete_ = true; to the end of this function? With
that we can DCHECK in ShrinkImmortalImmovablePages that we are currently
deserializing.

https://codereview.chromium.org/2232653003/diff/180001/src/heap/heap.cc#newco...
src/heap/heap.cc:5432: CHECK_EQ(0, gc_count());
DCHECK_EQ

https://codereview.chromium.org/2232653003/diff/180001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2232653003/diff/180001/src/heap/spaces.cc#new...
src/heap/spaces.cc:1232: void PagedSpace::ShrinkImmortalImmovablePages() {
Add DCHECK that we are currently deserializing.

https://codereview.chromium.org/2232653003/diff/180001/src/heap/spaces.cc#new...
src/heap/spaces.cc:1239: if (!page->IsFlagSet(Page::NEVER_EVACUATE)) continue;
Why are there non NEVER_EVACUATE pages?

https://codereview.chromium.org/2232653003/diff/180001/src/heap/spaces.cc#new...
src/heap/spaces.cc:1244: if (filler->address() == page->area_end()) continue;
Why do you continue here? Shouldn't each page have a filler at the high water
mark?

[Michael Lippautz, 2016-08-12 06:45:59]

Thanks for the comments! All addressed.

https://codereview.chromium.org/2232653003/diff/180001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/2232653003/diff/180001/src/heap/heap.cc#newco...
src/heap/heap.cc:5431: deserialization_complete_ = true;
On 2016/08/11 20:12:23, Hannes Payer wrote:
> Can you move deserialization_complete_ = true; to the end of this function?
With
> that we can DCHECK in ShrinkImmortalImmovablePages that we are currently
> deserializing.

Done.

https://codereview.chromium.org/2232653003/diff/180001/src/heap/heap.cc#newco...
src/heap/heap.cc:5432: CHECK_EQ(0, gc_count());
On 2016/08/11 20:12:23, Hannes Payer wrote:
> DCHECK_EQ

Done.

https://codereview.chromium.org/2232653003/diff/180001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2232653003/diff/180001/src/heap/spaces.cc#new...
src/heap/spaces.cc:1232: void PagedSpace::ShrinkImmortalImmovablePages() {
On 2016/08/11 20:12:23, Hannes Payer wrote:
> Add DCHECK that we are currently deserializing.

Done.

https://codereview.chromium.org/2232653003/diff/180001/src/heap/spaces.cc#new...
src/heap/spaces.cc:1239: if (!page->IsFlagSet(Page::NEVER_EVACUATE)) continue;
On 2016/08/11 20:12:23, Hannes Payer wrote:
> Why are there non NEVER_EVACUATE pages?

Not the case anymore. Transformed into a DCHECK.

https://codereview.chromium.org/2232653003/diff/180001/src/heap/spaces.cc#new...
src/heap/spaces.cc:1244: if (filler->address() == page->area_end()) continue;
On 2016/08/11 20:12:23, Hannes Payer wrote:
> Why do you continue here? Shouldn't each page have a filler at the high water
> mark?

Not if there were exactly page->area_size() bytes allocated on the page. The
high water mark would point to the area_end() and there would be no filler.

[Michael Lippautz, 2016-08-12 06:46:29]

The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-12 06:46:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-12 07:10:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-12 07:10:38]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2016-08-12 07:59:48]

https://codereview.chromium.org/2232653003/diff/200001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2232653003/diff/200001/src/heap/spaces.cc#new...
src/heap/spaces.cc:1232: void PagedSpace::ShrinkImmortalImmovablePages() {
Please write tests for these methods that check the corner cases.

[Michael Lippautz, 2016-08-12 09:29:51]

I added a couple of cctests. No unit tests since we need a proper Heap for
fillers.

https://codereview.chromium.org/2232653003/diff/200001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2232653003/diff/200001/src/heap/spaces.cc#new...
src/heap/spaces.cc:1232: void PagedSpace::ShrinkImmortalImmovablePages() {
On 2016/08/12 07:59:47, Hannes Payer wrote:
> Please write tests for these methods that check the corner cases.

Done.

[Hannes Payer (out of office), 2016-08-12 09:34:01]

lgtm

[Michael Lippautz, 2016-08-12 09:35:34]

The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-12 09:35:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Michael Lippautz, 2016-08-12 09:38:34]

+Yang for snapshot/ 

Nothing actually changed there ;)

[Michael Lippautz, 2016-08-12 09:38:55]

mlippautz@chromium.org changed reviewers:
+ yangguo@chromium.org

[commit-bot: I haz the power, 2016-08-12 10:08:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-12 10:08:36]

Dry run: This issue passed the CQ dry run.

[Yang, 2016-08-12 10:34:59]

lgtm

[Michael Lippautz, 2016-08-12 10:56:50]

The CQ bit was checked by mlippautz@chromium.org

[commit-bot: I haz the power, 2016-08-12 10:56:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-12 11:06:55]

Description was changed from

==========
Reland of "[heap] Switch to 500k pages"

Decrease regular heap object size to 400k. In a follow up, we can now get rid of
the new space border page while keeping the 1M minimum new space size.

BUG=chromium:636331

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.
==========

to

==========
Reland of "[heap] Switch to 500k pages"

Decrease regular heap object size to 400k. In a follow up, we can now get rid of
the new space border page while keeping the 1M minimum new space size.

BUG=chromium:636331

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.
==========

[commit-bot: I haz the power, 2016-08-12 11:06:56]

Committed patchset #11 (id:220001)

[commit-bot: I haz the power, 2016-08-12 11:07:13]

Description was changed from

==========
Reland of "[heap] Switch to 500k pages"

Decrease regular heap object size to 400k. In a follow up, we can now get rid of
the new space border page while keeping the 1M minimum new space size.

BUG=chromium:636331

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.
==========

to

==========
Reland of "[heap] Switch to 500k pages"

Decrease regular heap object size to 400k. In a follow up, we can now get rid of
the new space border page while keeping the 1M minimum new space size.

BUG=chromium:636331

This reverts commit 555c961990088d545e2b475da3872bd63fa27569.

Committed: https://crrev.com/20e2ea80e169e85c5b8231adc02901fb6c989609
Cr-Commit-Position: refs/heads/master@{#38608}
==========

[commit-bot: I haz the power, 2016-08-12 11:07:14]

Patchset 11 (id:??) landed as
https://crrev.com/20e2ea80e169e85c5b8231adc02901fb6c989609
Cr-Commit-Position: refs/heads/master@{#38608}

[Michael Lippautz, 2016-08-12 11:42:13]

A revert of this CL (patchset #11 id:220001) has been created in
https://codereview.chromium.org/2239323002/ by mlippautz@chromium.org.

The reason for reverting is: Breaks benchmark with --turbo on avx2

https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20-%20avx2/build....