[heap] Minor MC: Implement page moving

[heap] Minor MC: Implement page moving

BUG=chromium:651354
Review-Url: https://codereview.chromium.org/2855143003
Cr-Commit-Position: refs/heads/master@{#45223}
Committed: https://chromium.googlesource.com/v8/v8/+/cf37556f0e78ff522cc8a14bad3922efab991844

[Michael Lippautz, 2017-05-04 15:30:45]

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

[commit-bot: I haz the power, 2017-05-04 15:30:55]

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, 2017-05-04 15:53:03]

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

[commit-bot: I haz the power, 2017-05-04 15:53:04]

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

[Michael Lippautz, 2017-05-04 17:26:41]

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

[commit-bot: I haz the power, 2017-05-04 17:26:52]

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, 2017-05-04 17:58:43]

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

[commit-bot: I haz the power, 2017-05-04 17:58:44]

Dry run: Try jobs failed on following builders:
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/25481)
  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, 2017-05-04 19:35:59]

Description was changed from

==========
[heap] Minor MC improvements

BUG=
==========

to

==========
[heap] Minor MC: Move pages NEW->NEW

BUG=
==========

[Michael Lippautz, 2017-05-04 20:04:35]

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

[commit-bot: I haz the power, 2017-05-04 20:04:47]

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, 2017-05-04 20:41:01]

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

[commit-bot: I haz the power, 2017-05-04 20:41:02]

Dry run: Try jobs failed on following builders:
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/25498)
  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, 2017-05-05 07:59:06]

Description was changed from

==========
[heap] Minor MC: Move pages NEW->NEW

BUG=
==========

to

==========
[heap] Minor MC: Implement page moving

BUG=
==========

[Michael Lippautz, 2017-05-05 19:25:25]

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

[commit-bot: I haz the power, 2017-05-05 19:25:31]

Dry run: CQ is trying da patch.

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

[Michael Lippautz, 2017-05-09 10:37:14]

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

[commit-bot: I haz the power, 2017-05-09 10:37:19]

Dry run: CQ is trying da patch.

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

[Michael Lippautz, 2017-05-09 11:00:40]

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

[commit-bot: I haz the power, 2017-05-09 11:00:48]

Dry run: CQ is trying da patch.

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

[Michael Lippautz, 2017-05-09 11:01:13]

Description was changed from

==========
[heap] Minor MC: Implement page moving

BUG=
==========

to

==========
[heap] Minor MC: Implement page moving

BUG=chromium:651354
==========

[Michael Lippautz, 2017-05-09 11:01:13]

mlippautz@chromium.org changed reviewers:
+ hpayer@chromium.org, ulan@chromium.org

[Michael Lippautz, 2017-05-09 11:01:22]

ptal

[commit-bot: I haz the power, 2017-05-09 11:28:57]

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

[commit-bot: I haz the power, 2017-05-09 11:28:58]

Dry run: This issue passed the CQ dry run.

[ulan, 2017-05-09 11:31:54]

lgtm

https://codereview.chromium.org/2855143003/diff/220001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (right):

https://codereview.chromium.org/2855143003/diff/220001/src/heap/mark-compact....
src/heap/mark-compact.cc:2657:
full_collector->marking_state(p).bitmap()->ClearRange(
Shouldn't this be guarded with marking_mode?

https://codereview.chromium.org/2855143003/diff/220001/src/heap/mark-compact....
src/heap/mark-compact.cc:3832: if (!reduce_memory && !page->NeverEvacuate() &&
Let's extract this predicate into a separate function to share it with the full
collector.

[Michael Lippautz, 2017-05-09 13:00:14]

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

[commit-bot: I haz the power, 2017-05-09 13:00:25]

Dry run: CQ is trying da patch.

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

[Michael Lippautz, 2017-05-09 13:01:25]

https://codereview.chromium.org/2855143003/diff/220001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (right):

https://codereview.chromium.org/2855143003/diff/220001/src/heap/mark-compact....
src/heap/mark-compact.cc:2657:
full_collector->marking_state(p).bitmap()->ClearRange(
On 2017/05/09 11:31:54, ulan wrote:
> Shouldn't this be guarded with marking_mode?

Done.

https://codereview.chromium.org/2855143003/diff/220001/src/heap/mark-compact....
src/heap/mark-compact.cc:3832: if (!reduce_memory && !page->NeverEvacuate() &&
On 2017/05/09 11:31:53, ulan wrote:
> Let's extract this predicate into a separate function to share it with the
full
> collector.

Done.

[commit-bot: I haz the power, 2017-05-09 13:24:24]

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

[commit-bot: I haz the power, 2017-05-09 13:24:25]

Dry run: Try jobs failed on following builders:
  v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/27239)
  v8_win64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng_triggered/b...)

[Michael Lippautz, 2017-05-09 13:40:32]

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

[commit-bot: I haz the power, 2017-05-09 13:40: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, 2017-05-09 14:09:04]

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

[commit-bot: I haz the power, 2017-05-09 14:09:05]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2017-05-09 14:44:35]

Looking good. Two questions:

https://codereview.chromium.org/2855143003/diff/260001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (right):

https://codereview.chromium.org/2855143003/diff/260001/src/heap/mark-compact....
src/heap/mark-compact.cc:2636: void MinorMarkCompactCollector::MakeIterable(
I am wondering if we could re-use the sweeping method of MC here.

https://codereview.chromium.org/2855143003/diff/260001/src/heap/mark-compact....
src/heap/mark-compact.cc:3788: !p->Contains(age_mark) &&
heap()->CanExpandOldGeneration(live_bytes);
heap()->CanExpandOldGeneration(live_bytes)
If it is a new to new promotion we do not care if we can expand the old
generation?
How about going over the allocation limit with page promotions? It is very
likely that evacuating individual objects will also go over the limit.

[Michael Lippautz, 2017-05-09 17:01:12]

https://codereview.chromium.org/2855143003/diff/260001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (right):

https://codereview.chromium.org/2855143003/diff/260001/src/heap/mark-compact....
src/heap/mark-compact.cc:2636: void MinorMarkCompactCollector::MakeIterable(
On 2017/05/09 14:44:34, Hannes Payer wrote:
> I am wondering if we could re-use the sweeping method of MC here.

Both are similar but differ in critical parts, e.g., this one here needs to rest
the mark bits for the full collector for the freed areas. 

Since none of the RawSweep modes can be reused, I'd rather keep it.

https://codereview.chromium.org/2855143003/diff/260001/src/heap/mark-compact....
src/heap/mark-compact.cc:3788: !p->Contains(age_mark) &&
heap()->CanExpandOldGeneration(live_bytes);
On 2017/05/09 14:44:34, Hannes Payer wrote:
> heap()->CanExpandOldGeneration(live_bytes)
> If it is a new to new promotion we do not care if we can expand the old
> generation?
> How about going over the allocation limit with page promotions? It is very
> likely that evacuating individual objects will also go over the limit.

Right, this should not be needed. I remember that we  added it as a safety
mechanism, but both the full MC and the Scavenger/MinorMC use the AlwaysAllocate
scope and ignore the limit anyways.

Ulan, objections?

[ulan, 2017-05-09 17:05:57]

https://codereview.chromium.org/2855143003/diff/260001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (right):

https://codereview.chromium.org/2855143003/diff/260001/src/heap/mark-compact....
src/heap/mark-compact.cc:3788: !p->Contains(age_mark) &&
heap()->CanExpandOldGeneration(live_bytes);
On 2017/05/09 17:01:12, Michael Lippautz wrote:
> On 2017/05/09 14:44:34, Hannes Payer wrote:
> > heap()->CanExpandOldGeneration(live_bytes)
> > If it is a new to new promotion we do not care if we can expand the old
> > generation?
> > How about going over the allocation limit with page promotions? It is very
> > likely that evacuating individual objects will also go over the limit.
> 
> Right, this should not be needed. I remember that we  added it as a safety
> mechanism, but both the full MC and the Scavenger/MinorMC use the
AlwaysAllocate
> scope and ignore the limit anyways.
> 
> Ulan, objections?

Yes. This is the hard limit and all garbage collectors respect it even if always
allocate is on.

[Michael Lippautz, 2017-05-09 17:10:54]

Patchset #15 (id:280001) has been deleted

[Michael Lippautz, 2017-05-09 17:11:45]

https://codereview.chromium.org/2855143003/diff/260001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (right):

https://codereview.chromium.org/2855143003/diff/260001/src/heap/mark-compact....
src/heap/mark-compact.cc:3788: !p->Contains(age_mark) &&
heap()->CanExpandOldGeneration(live_bytes);
On 2017/05/09 17:05:57, ulan wrote:
> On 2017/05/09 17:01:12, Michael Lippautz wrote:
> > On 2017/05/09 14:44:34, Hannes Payer wrote:
> > > heap()->CanExpandOldGeneration(live_bytes)
> > > If it is a new to new promotion we do not care if we can expand the old
> > > generation?
> > > How about going over the allocation limit with page promotions? It is very
> > > likely that evacuating individual objects will also go over the limit.
> > 
> > Right, this should not be needed. I remember that we  added it as a safety
> > mechanism, but both the full MC and the Scavenger/MinorMC use the
> AlwaysAllocate
> > scope and ignore the limit anyways.
> > 
> > Ulan, objections?
> 
> Yes. This is the hard limit and all garbage collectors respect it even if
always
> allocate is on.

Acknowledged.  Thanks!

[Hannes Payer (out of office), 2017-05-09 20:59:33]

On 2017/05/09 17:11:45, Michael Lippautz wrote:
>
https://codereview.chromium.org/2855143003/diff/260001/src/heap/mark-compact.cc
> File src/heap/mark-compact.cc (right):
> 
>
https://codereview.chromium.org/2855143003/diff/260001/src/heap/mark-compact....
> src/heap/mark-compact.cc:3788: !p->Contains(age_mark) &&
> heap()->CanExpandOldGeneration(live_bytes);
> On 2017/05/09 17:05:57, ulan wrote:
> > On 2017/05/09 17:01:12, Michael Lippautz wrote:
> > > On 2017/05/09 14:44:34, Hannes Payer wrote:
> > > > heap()->CanExpandOldGeneration(live_bytes)
> > > > If it is a new to new promotion we do not care if we can expand the old
> > > > generation?
> > > > How about going over the allocation limit with page promotions? It is
very
> > > > likely that evacuating individual objects will also go over the limit.
> > > 
> > > Right, this should not be needed. I remember that we  added it as a safety
> > > mechanism, but both the full MC and the Scavenger/MinorMC use the
> > AlwaysAllocate
> > > scope and ignore the limit anyways.
> > > 
> > > Ulan, objections?
> > 
> > Yes. This is the hard limit and all garbage collectors respect it even if
> always
> > allocate is on.
> 
> Acknowledged.  Thanks!

Ah, that one checks against max. Right, we cannot get rid of this one.  For new
to new
it is not nice to do this check, however probably not an issue.

[Michael Lippautz, 2017-05-10 08:28:09]

Patchset #15 (id:300001) has been deleted

[Michael Lippautz, 2017-05-10 08:29:04]

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

[commit-bot: I haz the power, 2017-05-10 08:29:12]

Dry run: CQ is trying da patch.

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

[Hannes Payer (out of office), 2017-05-10 08:55:04]

lgtm

[commit-bot: I haz the power, 2017-05-10 09:01:27]

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

[commit-bot: I haz the power, 2017-05-10 09:01:28]

Dry run: This issue passed the CQ dry run.

[Michael Lippautz, 2017-05-10 09:04:14]

The CQ bit was checked by mlippautz@chromium.org

[Michael Lippautz, 2017-05-10 09:04:14]

The patchset sent to the CQ was uploaded after l-g-t-m from ulan@chromium.org
Link to the patchset: https://codereview.chromium.org/2855143003/#ps320001
(title: "Disable flag")

[commit-bot: I haz the power, 2017-05-10 09:04:19]

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, 2017-05-10 09:05:54]

CQ is committing da patch.

Bot data: {"patchset_id": 320001, "attempt_start_ts": 1494407054001680,
"parent_rev": "540419b660ae8da1d24767be5b95fc0421680580", "commit_rev":
"cf37556f0e78ff522cc8a14bad3922efab991844"}

[commit-bot: I haz the power, 2017-05-10 09:06:01]

Description was changed from

==========
[heap] Minor MC: Implement page moving

BUG=chromium:651354
==========

to

==========
[heap] Minor MC: Implement page moving

BUG=chromium:651354

Review-Url: https://codereview.chromium.org/2855143003
Cr-Commit-Position: refs/heads/master@{#45223}
Committed:
https://chromium.googlesource.com/v8/v8/+/cf37556f0e78ff522cc8a14bad3922efab9...
==========

[commit-bot: I haz the power, 2017-05-10 09:06:02]

Committed patchset #15 (id:320001) as
https://chromium.googlesource.com/v8/v8/+/cf37556f0e78ff522cc8a14bad3922efab9...