New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 726040 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Feature

Blocking:
issue 651354



Sign in to add a comment

Parallel pointer updates should be one phase instead of three

Project Member Reported by mlippautz@chromium.org, May 24 2017

Issue description

Currently, we update pointers in parallel or sequential in three lockstep phases:
1) Update to space pointers (parallel)
2) Update roots (sequential)
3) Update old->new (parallel)

Using the parallel job based on items we can merge all 3 phases into a single parallel phase that is preceded by a short seeding phase, similar to parallel marking for minor MC. We can reuse the already existing pointer updating traits and just refactor them into the item-based job.

The benefits should be that we don't need to set up the parallel environment multiple times and load-balancing through items should be more effective than having lock steps between phases were the actual work varies. 

This immediately helps both, the full MC and the minor MC. Minor MC only: If we want to go all the way, we could even avoid iterating roots for parallel pointer updates and reuse the items that we generated for parallel marking.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/81d3427b104afbf28774d5a2fe332e2b04745214

commit 81d3427b104afbf28774d5a2fe332e2b04745214
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Mon May 29 16:16:32 2017

[heap] Move pointers updating to ItemParallelJob

Furthermore avoid lock-step between pointer updating phases as they
should execute in parallel without synchronization restrictions.

Bug:  chromium:726040 
Change-Id: I26ce0d1f2a4637ff5610cae556113e3d736788e2
Reviewed-on: https://chromium-review.googlesource.com/518103
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45574}
[modify] https://crrev.com/81d3427b104afbf28774d5a2fe332e2b04745214/src/heap/gc-tracer.cc
[modify] https://crrev.com/81d3427b104afbf28774d5a2fe332e2b04745214/src/heap/gc-tracer.h
[modify] https://crrev.com/81d3427b104afbf28774d5a2fe332e2b04745214/src/heap/mark-compact.cc
[modify] https://crrev.com/81d3427b104afbf28774d5a2fe332e2b04745214/src/heap/mark-compact.h

Project Member

Comment 2 by bugdroid1@chromium.org, May 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/68a723996bb39bacdf5e9fc7e11c329deab68cfa

commit 68a723996bb39bacdf5e9fc7e11c329deab68cfa
Author: Michael Achenbach <machenbach@chromium.org>
Date: Mon May 29 18:02:16 2017

Revert "[heap] Move pointers updating to ItemParallelJob"

This reverts commit 81d3427b104afbf28774d5a2fe332e2b04745214.

Reason for revert: Several gc related failures, e.g.:
https://build.chromium.org/p/client.v8/builders/V8%20Mac%20GC%20Stress/builds/13488

Original change's description:
> [heap] Move pointers updating to ItemParallelJob
> 
> Furthermore avoid lock-step between pointer updating phases as they
> should execute in parallel without synchronization restrictions.
> 
> Bug:  chromium:726040 
> Change-Id: I26ce0d1f2a4637ff5610cae556113e3d736788e2
> Reviewed-on: https://chromium-review.googlesource.com/518103
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Hannes Payer <hpayer@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#45574}

NOPRESUBMIT=true
NOTRY=true
NOTREECHECKS=true

TBR=ulan@chromium.org,hpayer@chromium.org,mlippautz@chromium.org
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:726040 

Change-Id: I60860aef390605d07bc520141cab9d5be9b712b3
Reviewed-on: https://chromium-review.googlesource.com/518106
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45575}
[modify] https://crrev.com/68a723996bb39bacdf5e9fc7e11c329deab68cfa/src/heap/gc-tracer.cc
[modify] https://crrev.com/68a723996bb39bacdf5e9fc7e11c329deab68cfa/src/heap/gc-tracer.h
[modify] https://crrev.com/68a723996bb39bacdf5e9fc7e11c329deab68cfa/src/heap/mark-compact.cc
[modify] https://crrev.com/68a723996bb39bacdf5e9fc7e11c329deab68cfa/src/heap/mark-compact.h

Project Member

Comment 3 by bugdroid1@chromium.org, May 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/33d5006e167e220fb5f7bfb04c59a564970bdd6b

commit 33d5006e167e220fb5f7bfb04c59a564970bdd6b
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Wed May 31 16:45:00 2017

Reland "[heap] Move pointers updating to ItemParallelJob""

Furthermore avoid lock-step between pointer updating phases as they
should execute in parallel without synchronization restrictions.

This reverts commit 68a723996bb39bacdf5e9fc7e11c329deab68cfa.

Bug:  chromium:726040 
Change-Id: I76bb31d146f8aa20f8b7c486eeae9f09efa0cd53
Reviewed-on: https://chromium-review.googlesource.com/518150
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45644}
[modify] https://crrev.com/33d5006e167e220fb5f7bfb04c59a564970bdd6b/src/heap/gc-tracer.cc
[modify] https://crrev.com/33d5006e167e220fb5f7bfb04c59a564970bdd6b/src/heap/gc-tracer.h
[modify] https://crrev.com/33d5006e167e220fb5f7bfb04c59a564970bdd6b/src/heap/mark-compact.cc
[modify] https://crrev.com/33d5006e167e220fb5f7bfb04c59a564970bdd6b/src/heap/mark-compact.h

Project Member

Comment 4 by bugdroid1@chromium.org, May 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/33d5006e167e220fb5f7bfb04c59a564970bdd6b

commit 33d5006e167e220fb5f7bfb04c59a564970bdd6b
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Wed May 31 16:45:00 2017

Reland "[heap] Move pointers updating to ItemParallelJob""

Furthermore avoid lock-step between pointer updating phases as they
should execute in parallel without synchronization restrictions.

This reverts commit 68a723996bb39bacdf5e9fc7e11c329deab68cfa.

Bug:  chromium:726040 
Change-Id: I76bb31d146f8aa20f8b7c486eeae9f09efa0cd53
Reviewed-on: https://chromium-review.googlesource.com/518150
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45644}
[modify] https://crrev.com/33d5006e167e220fb5f7bfb04c59a564970bdd6b/src/heap/gc-tracer.cc
[modify] https://crrev.com/33d5006e167e220fb5f7bfb04c59a564970bdd6b/src/heap/gc-tracer.h
[modify] https://crrev.com/33d5006e167e220fb5f7bfb04c59a564970bdd6b/src/heap/mark-compact.cc
[modify] https://crrev.com/33d5006e167e220fb5f7bfb04c59a564970bdd6b/src/heap/mark-compact.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/1dd21fb5811bffb0fbe953ce4601e5ad6bde7105

commit 1dd21fb5811bffb0fbe953ce4601e5ad6bde7105
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Thu Jun 01 07:39:27 2017

[runtime] Allow Map::HasFastPointerLayout during GC

Use relaxed reads to allow changing the pointer concurrently in the 
GC. The layout descriptor will either be fast and there's nothing
to be done, or it will be slow, and we will use the forwarding
pointer to determine the proper version to use.

Bug:  chromium:726040 
Change-Id: I0a376752c3a99abf0874070387fcaeb3cee0dcb2
Reviewed-on: https://chromium-review.googlesource.com/519346
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45652}
[modify] https://crrev.com/1dd21fb5811bffb0fbe953ce4601e5ad6bde7105/src/objects-inl.h
[modify] https://crrev.com/1dd21fb5811bffb0fbe953ce4601e5ad6bde7105/src/objects/map.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/4b7ce1446d8ee881132c16622fbf6aea7672bcc0

commit 4b7ce1446d8ee881132c16622fbf6aea7672bcc0
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Thu Jun 01 11:19:39 2017

Reland "[heap] Move pointers updating to ItemParallelJob"

Furthermore avoid lock-step between pointer updating phases as they
should execute in parallel without synchronization restrictions.

This reverts commit bc6adb868d93a37c6b695d05fc492e12fada89c3.

Bug:  chromium:726040 
Change-Id: I1713d4333f0ce1604ff51c02461f3ef91e4bdaed
Reviewed-on: https://chromium-review.googlesource.com/521062
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45657}
[modify] https://crrev.com/4b7ce1446d8ee881132c16622fbf6aea7672bcc0/src/heap/gc-tracer.cc
[modify] https://crrev.com/4b7ce1446d8ee881132c16622fbf6aea7672bcc0/src/heap/gc-tracer.h
[modify] https://crrev.com/4b7ce1446d8ee881132c16622fbf6aea7672bcc0/src/heap/mark-compact.cc
[modify] https://crrev.com/4b7ce1446d8ee881132c16622fbf6aea7672bcc0/src/heap/mark-compact.h

Labels: -OS-Fuchsia
Labels: -Pri-2 Pri-3
Status: Fixed (was: Assigned)
There are various dependencies that prohibiting a complete merge of all phases (e.g. ArrayBufferTracker). 

Sign in to add a comment