Decouple root visitors from object visitors.

Decouple root visitors from object visitors.

This patch adds a new interface called RootVisitor and changes the root
iteration functions to accept a RootVisitor instead of an ObjectVisitor.

Future CLs will change ObjectVisitor to provide the host object to all
visiting functions, which will bring it in sync with static visitors.

Having separate visitors for roots and objects removes ambiguity in
VisitPointers and reduces chances of forgetting to record slots.

This is intended as pure refactoring. All places that require behavior
change are marked with TODO and will addressed in future CLs.

BUG=chromium:709075
Review-Url: https://codereview.chromium.org/2801073006
Cr-Commit-Position: refs/heads/master@{#44852}
Committed: https://chromium.googlesource.com/v8/v8/+/e671ed361026d01342d6032c3dab84abe48d19a6

[ulan, 2017-04-07 16:37:10]

Description was changed from

==========
Decouple root visitors from object visitors.

This patch adds a new interface called RootVisitor and changes the root
iteration functions to accept a RootVisitor instead of an ObjectVisitor.

Future CLs will change ObjectVisitor to provide the host object to all
visiting functions, which will bring it in sync with static visitors.

Having separate visitors for roots and objects removes ambiguity in
VisitPointers and reduces chances of forgetting to record slots.

BUG=chromium:709075
==========

to

==========
Decouple root visitors from object visitors.

This patch adds a new interface called RootVisitor and changes the root
iteration functions to accept a RootVisitor instead of an ObjectVisitor.

Future CLs will change ObjectVisitor to provide the host object to all
visiting functions, which will bring it in sync with static visitors.

Having separate visitors for roots and objects removes ambiguity in
VisitPointers and reduces chances of forgetting to record slots.

This is intended as pure refactoring. All places that require behavior
change are marked with TODO and will addressed in future CLs.

BUG=chromium:709075
==========

[ulan, 2017-04-07 16:39:23]

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

[ulan, 2017-04-07 16:39:24]

mlippautz@, hpayer@, ptal.
yangguo@, ptal at serializer.
mythria@, ptal at interpreter.

[Yang, 2017-04-10 11:51:23]

On 2017/04/07 16:39:24, ulan wrote:
> mlippautz@, hpayer@, ptal.
> yangguo@, ptal at serializer.
> mythria@, ptal at interpreter.

serializer lgtm.

[rmcilroy, 2017-04-10 13:54:56]

rmcilroy@chromium.org changed reviewers:
+ rmcilroy@chromium.org
- mythria@chromium.org

[rmcilroy, 2017-04-10 13:54:57]

Mythri's on vacation this week, so I'll take this one.

interpreter/ LGTM

[Michael Lippautz, 2017-04-20 09:01:29]

lgtm

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

https://codereview.chromium.org/2801073006/diff/80001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:53: class MarkingVerifier : public ObjectVisitor,
public RootVisitor {
Ugh, I am fine with this one but we should try hard to avoid multiple
inheritance.

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

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

https://codereview.chromium.org/2801073006/diff/80001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:53: class MarkingVerifier : public ObjectVisitor,
public RootVisitor {
On 2017/04/20 09:01:29, Michael Lippautz wrote:
> Ugh, I am fine with this one but we should try hard to avoid multiple
> inheritance.

Can't we compose ObjectVisitor and RootVisitor and have special methods for
visiting roots in ObjectVisitor?

[ulan, 2017-04-25 10:44:58]

On 2017/04/21 09:42:09, Hannes Payer wrote:
> https://codereview.chromium.org/2801073006/diff/80001/src/heap/mark-compact.cc
> File src/heap/mark-compact.cc (right):
> 
>
https://codereview.chromium.org/2801073006/diff/80001/src/heap/mark-compact.c...
> src/heap/mark-compact.cc:53: class MarkingVerifier : public ObjectVisitor,
> public RootVisitor {
> On 2017/04/20 09:01:29, Michael Lippautz wrote:
> > Ugh, I am fine with this one but we should try hard to avoid multiple
> > inheritance.
> 
> Can't we compose ObjectVisitor and RootVisitor and have special methods for
> visiting roots in ObjectVisitor?

Having two separate visitors gives us type checks and documentation. For
example, Frame::Iterate(RootVisitor* visitor) tells us that the visitor must
process roots.
On the other hand, it is not clear from Frame::Iterate(ObjectVisitor* visitor)
if the visitor has to process roots or object pointers.

We still can get a single visitor by inheriting from both RootVisitor and
ObjectVisitor.

[ulan, 2017-04-25 11:07:39]

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

[commit-bot: I haz the power, 2017-04-25 11:07:49]

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-25 11:39:53]

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

[commit-bot: I haz the power, 2017-04-25 11:39:54]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2017-04-25 13:26:27]

On 2017/04/25 10:44:58, ulan wrote:
> On 2017/04/21 09:42:09, Hannes Payer wrote:
> >
https://codereview.chromium.org/2801073006/diff/80001/src/heap/mark-compact.cc
> > File src/heap/mark-compact.cc (right):
> > 
> >
>
https://codereview.chromium.org/2801073006/diff/80001/src/heap/mark-compact.c...
> > src/heap/mark-compact.cc:53: class MarkingVerifier : public ObjectVisitor,
> > public RootVisitor {
> > On 2017/04/20 09:01:29, Michael Lippautz wrote:
> > > Ugh, I am fine with this one but we should try hard to avoid multiple
> > > inheritance.
> > 
> > Can't we compose ObjectVisitor and RootVisitor and have special methods for
> > visiting roots in ObjectVisitor?
> 
> Having two separate visitors gives us type checks and documentation. For
> example, Frame::Iterate(RootVisitor* visitor) tells us that the visitor must
> process roots.
> On the other hand, it is not clear from Frame::Iterate(ObjectVisitor* visitor)
> if the visitor has to process roots or object pointers.
> 
> We still can get a single visitor by inheriting from both RootVisitor and
> ObjectVisitor.

OK, I also think it is nice to call the visitor for roots only in the root
visiting phase and the
other visitor later.

LGTM

[ulan, 2017-04-25 13:30:22]

The CQ bit was checked by ulan@chromium.org

[ulan, 2017-04-25 13:30:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlippautz@chromium.org, rmcilroy@chromium.org, yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/2801073006/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-25 13:30:34]

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-25 13:32:19]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1493127022297300,
"parent_rev": "54818a635faba6ddc91122b8a73d02aabe243871", "commit_rev":
"e671ed361026d01342d6032c3dab84abe48d19a6"}

[commit-bot: I haz the power, 2017-04-25 13:32:25]

Description was changed from

==========
Decouple root visitors from object visitors.

This patch adds a new interface called RootVisitor and changes the root
iteration functions to accept a RootVisitor instead of an ObjectVisitor.

Future CLs will change ObjectVisitor to provide the host object to all
visiting functions, which will bring it in sync with static visitors.

Having separate visitors for roots and objects removes ambiguity in
VisitPointers and reduces chances of forgetting to record slots.

This is intended as pure refactoring. All places that require behavior
change are marked with TODO and will addressed in future CLs.

BUG=chromium:709075
==========

to

==========
Decouple root visitors from object visitors.

This patch adds a new interface called RootVisitor and changes the root
iteration functions to accept a RootVisitor instead of an ObjectVisitor.

Future CLs will change ObjectVisitor to provide the host object to all
visiting functions, which will bring it in sync with static visitors.

Having separate visitors for roots and objects removes ambiguity in
VisitPointers and reduces chances of forgetting to record slots.

This is intended as pure refactoring. All places that require behavior
change are marked with TODO and will addressed in future CLs.

BUG=chromium:709075

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

[commit-bot: I haz the power, 2017-04-25 13:32:26]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/v8/v8/+/e671ed361026d01342d6032c3dab84abe48...