[heap] Make LiveObjectIterator concurrency safe

[heap] Make LiveObjectIterator concurrency safe

LiveObjectIterator sometimes runs concurrently to the main thread. In this
scenarios we are not allowed to access memory of live objects in non-atomic
ways. Use synchronized reads where needed.

Correctness (already ok in current state):
- Reading a larger size is fine per definition.
- Reading a smaller size is fine since are guaranteed that one word fillers will
  follow.

BUG=v8:5583
R=ulan@chromium.org,hpayer@chromium.org

Committed: https://crrev.com/142d4f97c009b1d9898448308da07ff7a7f0b3cf
Cr-Commit-Position: refs/heads/master@{#40798}

[Michael Lippautz, 2016-11-04 13:00:22]

ptal

I think we should try doing it that way and watch for potential regressions
because of the synchronized read.

[ulan, 2016-11-04 13:09:13]

lgtm

[Hannes Payer (out of office), 2016-11-07 09:26:55]

https://codereview.chromium.org/2477823003/diff/1/src/heap/mark-compact-inl.h
File src/heap/mark-compact-inl.h (right):

https://codereview.chromium.org/2477823003/diff/1/src/heap/mark-compact-inl.h...
src/heap/mark-compact-inl.h:172: map = black_object->synchronized_map();
Size() uses no barrier accessor. No need to synchronize here, however we can if
it is not a performance issue.

https://codereview.chromium.org/2477823003/diff/1/src/heap/mark-compact-inl.h...
src/heap/mark-compact-inl.h:203: if (map == nullptr || map ==
heap()->one_pointer_filler_map()) {
How can map become nullptr?

[Michael Lippautz, 2016-11-07 09:36:50]

https://codereview.chromium.org/2477823003/diff/1/src/heap/mark-compact-inl.h
File src/heap/mark-compact-inl.h (right):

https://codereview.chromium.org/2477823003/diff/1/src/heap/mark-compact-inl.h...
src/heap/mark-compact-inl.h:172: map = black_object->synchronized_map();
On 2016/11/07 09:26:55, Hannes Payer wrote:
> Size() uses no barrier accessor. No need to synchronize here, however we can
if
> it is not a performance issue.

Changed to NoBarrier read.

https://codereview.chromium.org/2477823003/diff/1/src/heap/mark-compact-inl.h...
src/heap/mark-compact-inl.h:203: if (map == nullptr || map ==
heap()->one_pointer_filler_map()) {
On 2016/11/07 09:26:55, Hannes Payer wrote:
> How can map become nullptr?

As discussed offline: This happens when we take the branch for gray objects.

[Michael Lippautz, 2016-11-07 09:37:06]

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

[commit-bot: I haz the power, 2016-11-07 09:37:12]

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, 2016-11-07 09:40:22]

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

[commit-bot: I haz the power, 2016-11-07 09:40:23]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_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_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/15689)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/12014)

[Michael Lippautz, 2016-11-07 09:44:57]

The CQ bit was checked by mlippautz@chromium.org

[Michael Lippautz, 2016-11-07 09:44:58]

The patchset sent to the CQ was uploaded after l-g-t-m from ulan@chromium.org
Link to the patchset: https://codereview.chromium.org/2477823003/#ps40001
(title: "Use right address")

[commit-bot: I haz the power, 2016-11-07 09:45:02]

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, 2016-11-07 10:00:11]

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

[commit-bot: I haz the power, 2016-11-07 10:00:12]

Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)

[Michael Lippautz, 2016-11-07 10:44:30]

The CQ bit was checked by mlippautz@chromium.org

[Michael Lippautz, 2016-11-07 10:44:30]

The patchset sent to the CQ was uploaded after l-g-t-m from ulan@chromium.org
Link to the patchset: https://codereview.chromium.org/2477823003/#ps60001
(title: "Fix condition")

[commit-bot: I haz the power, 2016-11-07 10:44:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Hannes Payer (out of office), 2016-11-07 10:45:05]

lgtm

[commit-bot: I haz the power, 2016-11-07 11:11:14]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-17 22:24:26]

Description was changed from

==========
[heap] Make LiveObjectIterator concurrency safe

LiveObjectIterator sometimes runs concurrently to the main thread. In this
scenarios we are not allowed to access memory of live objects in non-atomic
ways. Use synchronized reads where needed.

Correctness (already ok in current state):
- Reading a larger size is fine per definition.
- Reading a smaller size is fine since are guaranteed that one word fillers will
  follow.

BUG=v8:5583
R=ulan@chromium.org,hpayer@chromium.org
==========

to

==========
[heap] Make LiveObjectIterator concurrency safe

LiveObjectIterator sometimes runs concurrently to the main thread. In this
scenarios we are not allowed to access memory of live objects in non-atomic
ways. Use synchronized reads where needed.

Correctness (already ok in current state):
- Reading a larger size is fine per definition.
- Reading a smaller size is fine since are guaranteed that one word fillers will
  follow.

BUG=v8:5583
R=ulan@chromium.org,hpayer@chromium.org

Committed: https://crrev.com/142d4f97c009b1d9898448308da07ff7a7f0b3cf
Cr-Commit-Position: refs/heads/master@{#40798}
==========

[commit-bot: I haz the power, 2016-11-17 22:24:27]

Patchset 4 (id:??) landed as
https://crrev.com/142d4f97c009b1d9898448308da07ff7a7f0b3cf
Cr-Commit-Position: refs/heads/master@{#40798}