Implement parallel pointer updates after evacuation.

Implement parallel pointer updates after evacuation.

BUG=chromium:568495
LOG=NO

Committed: https://crrev.com/561e1629023726e2d7d71e86938088edcdf7331f
Cr-Commit-Position: refs/heads/master@{#34665}

[ulan, 2016-03-08 20:09:35]

Description was changed from

==========
Parallel pointer updates.

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

to

==========
Implement parallel pointer updates after evacuation.

BUG=chromium:568495
LOG=NO
==========

[ulan, 2016-03-08 20:09:35]

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

[ulan, 2016-03-08 20:09:47]

PTAL

[Michael Lippautz, 2016-03-09 09:48:53]

lgtm, only nits.

As far as I see it should already be possible to refactor compaction to use this
pattern (which was obviously also a goal ;))

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

https://codereview.chromium.org/1775003003/diff/60001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:2792: static void UpdateOldToNewSlot(HeapObject**
address, HeapObject* object) {
If its only use is within PointerUpdateJobTraits, just move it there?

https://codereview.chromium.org/1775003003/diff/60001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3162: 1, static_cast<int>(
Thanks for the drive-by-fix ;)

https://codereview.chromium.org/1775003003/diff/60001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3593: typedef int PerPageData;
Maybe add a comment that PerPageData is unused for this trait.

https://codereview.chromium.org/1775003003/diff/60001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3597: PointersUpdatingVisitor* visitor,
Not sure if the equivalent version of the decl
  static bool ProcessPageInParallel(Heap*, __ PerTaskData __, MemoryChunk*,
PerPageData) 
is more readable

https://codereview.chromium.org/1775003003/diff/60001/src/heap/page-parallel-...
File src/heap/page-parallel-job.h (right):

https://codereview.chromium.org/1775003003/diff/60001/src/heap/page-parallel-...
src/heap/page-parallel-job.h:65: if (num_items_ == 0) return;
Maybe add
  DCHECK_GE(num_tasks, 1)
after the bailout.

Currently we compute the number of tasks on the outside and they will be 0 iff
num_items == 0. It seems slightly fragile though.

https://codereview.chromium.org/1775003003/diff/60001/src/heap/page-parallel-...
src/heap/page-parallel-job.h:119: Item(MemoryChunk* chunk, typename
JobTraits::PerPageData data, Item* next)
nit: move constructor to top

[ulan, 2016-03-09 17:34:18]

Thanks for comments!

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

https://codereview.chromium.org/1775003003/diff/60001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3162: 1, static_cast<int>(
On 2016/03/09 09:48:53, Michael Lippautz wrote:
> Thanks for the drive-by-fix ;)

Done.

https://codereview.chromium.org/1775003003/diff/60001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3593: typedef int PerPageData;
On 2016/03/09 09:48:53, Michael Lippautz wrote:
> Maybe add a comment that PerPageData is unused for this trait.

Done.

https://codereview.chromium.org/1775003003/diff/60001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3597: PointersUpdatingVisitor* visitor,
On 2016/03/09 09:48:53, Michael Lippautz wrote:
> Not sure if the equivalent version of the decl
>   static bool ProcessPageInParallel(Heap*, __ PerTaskData __, MemoryChunk*,
> PerPageData) 
> is more readable

Done. It is shorter :)

https://codereview.chromium.org/1775003003/diff/60001/src/heap/page-parallel-...
File src/heap/page-parallel-job.h (right):

https://codereview.chromium.org/1775003003/diff/60001/src/heap/page-parallel-...
src/heap/page-parallel-job.h:65: if (num_items_ == 0) return;
On 2016/03/09 09:48:53, Michael Lippautz wrote:
> Maybe add
>   DCHECK_GE(num_tasks, 1)
> after the bailout.
> 
> Currently we compute the number of tasks on the outside and they will be 0 iff
> num_items == 0. It seems slightly fragile though.

Done.

https://codereview.chromium.org/1775003003/diff/60001/src/heap/page-parallel-...
src/heap/page-parallel-job.h:119: Item(MemoryChunk* chunk, typename
JobTraits::PerPageData data, Item* next)
On 2016/03/09 09:48:53, Michael Lippautz wrote:
> nit: move constructor to top

Done.

[ulan, 2016-03-09 17:34:28]

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

[commit-bot: I haz the power, 2016-03-09 17:34:38]

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

[commit-bot: I haz the power, 2016-03-09 18:01:51]

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

[commit-bot: I haz the power, 2016-03-09 18:01:52]

Dry run: This issue passed the CQ dry run.

[ulan, 2016-03-10 11:45:40]

The CQ bit was checked by ulan@chromium.org

[ulan, 2016-03-10 11:45:40]

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

[commit-bot: I haz the power, 2016-03-10 11:45:52]

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

[commit-bot: I haz the power, 2016-03-10 12:18:29]

Description was changed from

==========
Implement parallel pointer updates after evacuation.

BUG=chromium:568495
LOG=NO
==========

to

==========
Implement parallel pointer updates after evacuation.

BUG=chromium:568495
LOG=NO
==========

[commit-bot: I haz the power, 2016-03-10 12:18:30]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-03-10 12:19:23]

Description was changed from

==========
Implement parallel pointer updates after evacuation.

BUG=chromium:568495
LOG=NO
==========

to

==========
Implement parallel pointer updates after evacuation.

BUG=chromium:568495
LOG=NO

Committed: https://crrev.com/561e1629023726e2d7d71e86938088edcdf7331f
Cr-Commit-Position: refs/heads/master@{#34665}
==========

[commit-bot: I haz the power, 2016-03-10 12:19:24]

Patchset 6 (id:??) landed as
https://crrev.com/561e1629023726e2d7d71e86938088edcdf7331f
Cr-Commit-Position: refs/heads/master@{#34665}