[heap] Refactor MC and introduce MarkCompactCollectorBase

[heap] Refactor MC and introduce MarkCompactCollectorBase

- Dispatch evacuation of a page to its concrete evacuator.
- Create MC base class to accommodate shared state.

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

[Michael Lippautz, 2017-04-21 11:42:56]

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

[Michael Lippautz, 2017-04-21 11:42:57]

pure refactoring, ptal

[Michael Lippautz, 2017-04-21 12:14:54]

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

[commit-bot: I haz the power, 2017-04-21 12:14:59]

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-04-21 12:17:49]

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

[commit-bot: I haz the power, 2017-04-21 12:17:49]

Dry run: Try jobs failed on following builders:
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/24758)
  v8_linux64_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/24666)
  v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/21259)

[Michael Lippautz, 2017-04-21 12:32:18]

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

[commit-bot: I haz the power, 2017-04-21 12:32:29]

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-04-21 13:03:32]

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

[commit-bot: I haz the power, 2017-04-21 13:03:33]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2017-04-21 13:09:15]

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

https://codereview.chromium.org/2828323004/diff/1/src/heap/mark-compact.cc#ne...
src/heap/mark-compact.cc:3235: bool EvacuatePageImpl(Page* page, const
MarkingState& state) override;
I do not like the name. I do not think this is how we call the dispatch to a
concrete implementation. I may be wrong.

https://codereview.chromium.org/2828323004/diff/1/src/heap/mark-compact.cc#ne...
src/heap/mark-compact.cc:3339: namespace {
Why are you moving this into a separate namespace?

[Michael Lippautz, 2017-04-21 15:02:25]

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

https://codereview.chromium.org/2828323004/diff/1/src/heap/mark-compact.cc#ne...
src/heap/mark-compact.cc:3235: bool EvacuatePageImpl(Page* page, const
MarkingState& state) override;
On 2017/04/21 13:09:15, Hannes Payer wrote:
> I do not like the name. I do not think this is how we call the dispatch to a
> concrete implementation. I may be wrong.

In other parts of V8 we use *Impl suffixes. Changed to RawEvacuatePage for
consistency with sweeping though.

https://codereview.chromium.org/2828323004/diff/1/src/heap/mark-compact.cc#ne...
src/heap/mark-compact.cc:3339: namespace {
On 2017/04/21 13:09:15, Hannes Payer wrote:
> Why are you moving this into a separate namespace?

Will be used by the minor MC. Let's see if we add a base class that can hold the
few bits of shared state.

[Michael Lippautz, 2017-04-21 15:56:32]

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

[commit-bot: I haz the power, 2017-04-21 15:56:35]

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-04-21 16:23:06]

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

[commit-bot: I haz the power, 2017-04-21 16:23:06]

Dry run: This issue passed the CQ dry run.

[Michael Lippautz, 2017-04-24 12:18:24]

Description was changed from

==========
[heap] Refactor MC evacuation

- Dispatch evacuation of a page to its concrete evacuator.
- Factor out computation of tasks as it is independent from the
  collector.

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

to

==========
[heap] Refactor MC and introduce MarkCompactCollectorBase

- Dispatch evacuation of a page to its concrete evacuator.
- Create MC base class to accommodate shared state.

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

[Michael Lippautz, 2017-04-24 13:10:55]

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

[commit-bot: I haz the power, 2017-04-24 13:11: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, 2017-04-24 13:40:17]

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

[commit-bot: I haz the power, 2017-04-24 13:40:18]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2017-04-24 14:54:28]

lgtm

[Michael Lippautz, 2017-04-24 14:58:00]

The CQ bit was checked by mlippautz@chromium.org

[commit-bot: I haz the power, 2017-04-24 14:58:10]

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-04-24 15:04:19]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493045880686790,
"parent_rev": "852a20b08c031690db262e257f87d0bb664a86ea", "commit_rev":
"2c9fc184743dac8627b78a5990522525f35a0007"}

[commit-bot: I haz the power, 2017-04-24 15:04:28]

Description was changed from

==========
[heap] Refactor MC and introduce MarkCompactCollectorBase

- Dispatch evacuation of a page to its concrete evacuator.
- Create MC base class to accommodate shared state.

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

to

==========
[heap] Refactor MC and introduce MarkCompactCollectorBase

- Dispatch evacuation of a page to its concrete evacuator.
- Create MC base class to accommodate shared state.

BUG=chromium:651354

Review-Url: https://codereview.chromium.org/2828323004
Cr-Commit-Position: refs/heads/master@{#44807}
Committed:
https://chromium.googlesource.com/v8/v8/+/2c9fc184743dac8627b78a5990522525f35...
==========

[commit-bot: I haz the power, 2017-04-24 15:04:29]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/v8/v8/+/2c9fc184743dac8627b78a5990522525f35...