Avoid manual object's body traversal in GC.

Avoid manual object's body traversal in GC.

This CL introduces the following visitors:
1) RecordMigratedSlotVisitor which simplifies MarkCompactCollector::MigrateObject().
2) IteratePointersToFromSpaceVisitor which simplifies Heap::IteratePointersToFromSpace().
3) FindPointersToNewSpaceVisitor which simplifies StoreBuffer::IteratePointersToNewSpace().

These changes make the object's body descriptors the one and only place that knows how to traverse the object.

Committed: https://crrev.com/5ba9ea1802e5f88f8b54d30161869a3e905da182
Cr-Commit-Position: refs/heads/master@{#31992}

[Igor Sheludko, 2015-11-11 19:31:41]

Description was changed from

==========
Introducing RecordMigratedSlotVisitor which simplifies object handling in
MarkCompactCollector::MigrateObject().

FindPointersToNewSpaceVisitor and IteratePointersToFromSpaceVisitor also added.

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

to

==========
Introducing RecordMigratedSlotVisitor which simplifies object handling in
MarkCompactCollector::MigrateObject().

FindPointersToNewSpaceVisitor and IteratePointersToFromSpaceVisitor also added.

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

[Igor Sheludko, 2015-11-11 19:32:03]

Description was changed from

==========
Introducing RecordMigratedSlotVisitor which simplifies object handling in
MarkCompactCollector::MigrateObject().

FindPointersToNewSpaceVisitor and IteratePointersToFromSpaceVisitor also added.

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

to

==========
Introducing (virtual) RecordMigratedSlotVisitor which simplifies object handling
in MarkCompactCollector::MigrateObject().

FindPointersToNewSpaceVisitor and IteratePointersToFromSpaceVisitor also added.

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

[ulan, 2015-11-12 12:27:27]

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

[ulan, 2015-11-12 12:27:27]

I like it.

Please make sure that it doesn't regress GC times.

You can measure GC time as follows:

cd chrome_src

tools/perf/run_benchmark  v8.infinite_scroll --show-stdout
--extra-browser-args="--no-sandbox --js-flags='--trace-gc --trace-gc-nvp'" 
--browser-executable ./out/Release/chrome  > gc-nvp

v8/tools/eval_gc_nvp.py pause evacuate old_new nonlive_refs < gc-nvp

https://codereview.chromium.org/1441453002/diff/1/src/heap/objects-visiting.h
File src/heap/objects-visiting.h (right):

https://codereview.chromium.org/1441453002/diff/1/src/heap/objects-visiting.h...
src/heap/objects-visiting.h:200: V8_INLINE static ReturnType Visit(Map* map,
HeapObject* object,
Is this leftover from the previous patch?

[Igor Sheludko, 2015-11-13 09:13:26]

Description was changed from

==========
Introducing (virtual) RecordMigratedSlotVisitor which simplifies object handling
in MarkCompactCollector::MigrateObject().

FindPointersToNewSpaceVisitor and IteratePointersToFromSpaceVisitor also added.

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

to

==========
Avoid manual object's body traversal in GC.

This CL introduces the following visitors:
1) RecordMigratedSlotVisitor which simplifies
MarkCompactCollector::MigrateObject().
2) IteratePointersToFromSpaceVisitor which simplifies
Heap::IteratePointersToFromSpace().
3) FindPointersToNewSpaceVisitor which simplifies
StoreBuffer::IteratePointersToNewSpace().

These changes make the object's body descriptors the one and only entities that
knows how to traverse the object.
==========

[Igor Sheludko, 2015-11-13 09:14:07]

Description was changed from

==========
Avoid manual object's body traversal in GC.

This CL introduces the following visitors:
1) RecordMigratedSlotVisitor which simplifies
MarkCompactCollector::MigrateObject().
2) IteratePointersToFromSpaceVisitor which simplifies
Heap::IteratePointersToFromSpace().
3) FindPointersToNewSpaceVisitor which simplifies
StoreBuffer::IteratePointersToNewSpace().

These changes make the object's body descriptors the one and only entities that
knows how to traverse the object.
==========

to

==========
Avoid manual object's body traversal in GC.

This CL introduces the following visitors:
1) RecordMigratedSlotVisitor which simplifies
MarkCompactCollector::MigrateObject().
2) IteratePointersToFromSpaceVisitor which simplifies
Heap::IteratePointersToFromSpace().
3) FindPointersToNewSpaceVisitor which simplifies
StoreBuffer::IteratePointersToNewSpace().

These changes make the object's body descriptors the one and only place that
knows how to traverse the object.
==========

[Igor Sheludko, 2015-11-13 09:46:31]

Patchset #2 (id:20001) has been deleted

[Igor Sheludko, 2015-11-13 09:46:39]

Patchset #2 (id:40001) has been deleted

[Igor Sheludko, 2015-11-13 10:04:23]

I addressed comments and rebased on top of a body descriptors refactoring CL.

The results looks good:

================== base ==================
pause
  len: 217
  min: 0.2
  max: 118.7
  avg: 14.9456221198
  [0,5[: 97
  [5,10[: 49
  [10,15[: 18
  [15,20[: 5
  [20,25[: 3
  [25,30[: 7
  [30,35[: 2
  [35,40[: 11
  [40,45[: 3
  [45,50[: 3
  [50,55[: 3
  [55,60[: 1
  [60,65[: 2
  [65,70[: 4
  [70,75[: 3
  [75,80[: 1
  [80,85[: 2
  [85,90[: 1
  [90,95[: 1
  [95,100[: 0
  [100,105[: 0
  [105,110[: 0
  [110,115[: 0
  [115,120[: 1
evacuate
  len: 80
  min: 0.0
  max: 25.3
  avg: 2.2025
  [0,5[: 70
  [5,10[: 6
  [10,15[: 1
  [15,20[: 2
  [20,25[: 0
  [25,30[: 1
old_new
  len: 217
  min: 0.0
  max: 8.84
  avg: 0.932949308756
  [0,5[: 210
  [5,10[: 7
nonlive_refs
  len: 78
  min: 0.0
  max: 5.8
  avg: 1.20769230769
  [0,5[: 77
  [5,10[: 1

================== Patch Set 2 ==================
pause
  len: 218
  min: 0.4
  max: 104.7
  avg: 14.6252293578
  [0,5[: 100
  [5,10[: 45
  [10,15[: 20
  [15,20[: 3
  [20,25[: 5
  [25,30[: 5
  [30,35[: 9
  [35,40[: 3
  [40,45[: 6
  [45,50[: 3
  [50,55[: 3
  [55,60[: 2
  [60,65[: 3
  [65,70[: 2
  [70,75[: 5
  [75,80[: 1
  [80,85[: 2
  [85,90[: 0
  [90,95[: 0
  [95,100[: 0
  [100,105[: 1
evacuate
  len: 78
  min: 0.0
  max: 25.1
  avg: 1.97948717949
  [0,5[: 68
  [5,10[: 6
  [10,15[: 2
  [15,20[: 1
  [20,25[: 0
  [25,30[: 1
old_new
  len: 218
  min: 0.0
  max: 11.83
  avg: 0.972844036697
  [0,5[: 211
  [5,10[: 6
  [10,15[: 1
nonlive_refs
  len: 77
  min: 0.0
  max: 4.5
  avg: 1.2025974026
  [0,5[: 77

[Igor Sheludko, 2015-11-13 10:07:10]

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

[commit-bot: I haz the power, 2015-11-13 10:07:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1441453002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1441453002/60001

[commit-bot: I haz the power, 2015-11-13 10:26:41]

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

[commit-bot: I haz the power, 2015-11-13 10:26:42]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2015-11-13 17:00:36]

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

[Hannes Payer (out of office), 2015-11-13 17:00:37]

awesome, lgtm

https://codereview.chromium.org/1441453002/diff/60001/src/heap/heap.h
File src/heap/heap.h (right):

https://codereview.chromium.org/1441453002/diff/60001/src/heap/heap.h#newcode...
src/heap/heap.h:2366: friend class IteratePointersToFromSpaceVisitor;
alphabetic oder

https://codereview.chromium.org/1441453002/diff/60001/src/heap/mark-compact.h
File src/heap/mark-compact.h (right):

https://codereview.chromium.org/1441453002/diff/60001/src/heap/mark-compact.h...
src/heap/mark-compact.h:571: friend class IncrementalMarkingMarkingVisitor;
alphabetic order everywhere

https://codereview.chromium.org/1441453002/diff/60001/src/heap/store-buffer.h
File src/heap/store-buffer.h (right):

https://codereview.chromium.org/1441453002/diff/60001/src/heap/store-buffer.h...
src/heap/store-buffer.h:166: friend class DontMoveStoreBufferEntriesScope;
all friends like to be in alphabetic order

[Igor Sheludko, 2015-11-13 17:35:19]

https://codereview.chromium.org/1441453002/diff/60001/src/heap/heap.h
File src/heap/heap.h (right):

https://codereview.chromium.org/1441453002/diff/60001/src/heap/heap.h#newcode...
src/heap/heap.h:2366: friend class IteratePointersToFromSpaceVisitor;
On 2015/11/13 17:00:36, Hannes Payer wrote:
> alphabetic oder

Done.

https://codereview.chromium.org/1441453002/diff/60001/src/heap/mark-compact.h
File src/heap/mark-compact.h (right):

https://codereview.chromium.org/1441453002/diff/60001/src/heap/mark-compact.h...
src/heap/mark-compact.h:571: friend class IncrementalMarkingMarkingVisitor;
On 2015/11/13 17:00:36, Hannes Payer wrote:
> alphabetic order everywhere

Done.

https://codereview.chromium.org/1441453002/diff/60001/src/heap/store-buffer.h
File src/heap/store-buffer.h (right):

https://codereview.chromium.org/1441453002/diff/60001/src/heap/store-buffer.h...
src/heap/store-buffer.h:166: friend class DontMoveStoreBufferEntriesScope;
On 2015/11/13 17:00:36, Hannes Payer wrote:
> all friends like to be in alphabetic order

Done.

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

https://codereview.chromium.org/1441453002/diff/80001/src/heap/spaces.cc#oldc...
src/heap/spaces.cc:3211: CHECK(object->ContentType() !=
HeapObjectContents::kMixedValues);
As discussed offline I'm removing this check because JSObjects must never appear
in large space at all (see CalculateInstanceSizeHelper() in objects.cc, which
ensures that the object is not bigger than JSObject::kMaxInstanceSize) and this
is checked by the check two lines below.
The other mixed objects are typed arrays and bytecode arrays have tagged fields
only in the header.

[Igor Sheludko, 2015-11-13 17:35:25]

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

[commit-bot: I haz the power, 2015-11-13 17:35:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1441453002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1441453002/80001

[commit-bot: I haz the power, 2015-11-13 17:59:13]

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

[commit-bot: I haz the power, 2015-11-13 17:59:13]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2015-11-13 18:54:38]

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

https://codereview.chromium.org/1441453002/diff/80001/src/heap/spaces.cc#oldc...
src/heap/spaces.cc:3211: CHECK(object->ContentType() !=
HeapObjectContents::kMixedValues);
On 2015/11/13 17:35:18, Igor Sheludko wrote:
> As discussed offline I'm removing this check because JSObjects must never
appear
> in large space at all (see CalculateInstanceSizeHelper() in objects.cc, which
> ensures that the object is not bigger than JSObject::kMaxInstanceSize) and
this
> is checked by the check two lines below.
> The other mixed objects are typed arrays and bytecode arrays have tagged
fields
> only in the header.

Acknowledged.

[Igor Sheludko, 2015-11-13 19:10:13]

The CQ bit was checked by ishell@chromium.org

[Igor Sheludko, 2015-11-13 19:10:13]

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/1441453002/#ps80001
(title: "Addressed comments + removed HeapObject::ContentType()")

[commit-bot: I haz the power, 2015-11-13 19:10:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1441453002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1441453002/80001

[commit-bot: I haz the power, 2015-11-13 19:11:31]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2015-11-14 23:21:54]

Patchset 3 (id:??) landed as
https://crrev.com/5ba9ea1802e5f88f8b54d30161869a3e905da182
Cr-Commit-Position: refs/heads/master@{#31992}