Reland "[heap] Ensure progress when incrementally marking wrappers"

Reland "[heap] Ensure progress when incrementally marking wrappers"

1) Alternate between processing v8 and wrappers
2) Once v8 is empty, try 3 rounds of finding the fixpoint between v8 and wrappers
3) After that, finalize once v8 marking deque is empty again

BUG=

Review-Url: https://codereview.chromium.org/2591383004
Cr-Commit-Position: refs/heads/master@{#41932}
Committed: https://chromium.googlesource.com/v8/v8/+/61a55548c50e01d84ed4aefa396324cbb4039b51

[Michael Lippautz, 2016-12-22 15:56:26]

Description was changed from

==========
Reland "[heap] Ensure progress when incrementally marking wrappers"

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

to

==========
Reland "[heap] Ensure progress when incrementally marking wrappers"

1) Alternate between processing v8 and wrappers, preferring v8
2) Once v8 is empty, try 3 rounds of finding the fixpoint between v8 and
wrappers
3) After that finalize once v8 marking deque is empty again

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

[Michael Lippautz, 2016-12-22 16:20:12]

Description was changed from

==========
Reland "[heap] Ensure progress when incrementally marking wrappers"

1) Alternate between processing v8 and wrappers, preferring v8
2) Once v8 is empty, try 3 rounds of finding the fixpoint between v8 and
wrappers
3) After that finalize once v8 marking deque is empty again

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

to

==========
Reland "[heap] Ensure progress when incrementally marking wrappers"

1) Alternate between processing v8 and wrappers
2) Once v8 is empty, try 3 rounds of finding the fixpoint between v8 and
wrappers
3) After that, finalize once v8 marking deque is empty again

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

[Michael Lippautz, 2016-12-22 16:22:16]

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

[Michael Lippautz, 2016-12-22 16:22:17]

Hannes, Ulan: This is a mix of what you both suggested. See the description.

[Michael Lippautz, 2016-12-22 16:25:23]

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

[commit-bot: I haz the power, 2016-12-22 16:25:31]

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-12-22 16:28:20]

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

[commit-bot: I haz the power, 2016-12-22 16:28:21]

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_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)

[ulan, 2016-12-22 16:32:47]

lgtm if bot redness is fixed :)

https://codereview.chromium.org/2591383004/diff/60001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/2591383004/diff/60001/src/heap/heap.cc#newcod...
src/heap/heap.cc:4179: bool Heap::MarkingDequesAreEmpty() {
can this be removed now?

[Michael Lippautz, 2016-12-22 16:33:47]

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

[commit-bot: I haz the power, 2016-12-22 16:33:52]

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-12-22 16:37:02]

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

[commit-bot: I haz the power, 2016-12-22 16:37:03]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  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_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_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/18366)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  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...)

[Michael Lippautz, 2016-12-22 17:39:17]

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

[commit-bot: I haz the power, 2016-12-22 17:39:23]

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-12-22 18:08:48]

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

[commit-bot: I haz the power, 2016-12-22 18:08:50]

Dry run: This issue passed the CQ dry run.

[Michael Lippautz, 2016-12-22 19:18:54]

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

[commit-bot: I haz the power, 2016-12-22 19:19:09]

Dry run: 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-12-22 19:24:16]

https://codereview.chromium.org/2591383004/diff/120001/src/heap/incremental-m...
File src/heap/incremental-marking.cc (right):

https://codereview.chromium.org/2591383004/diff/120001/src/heap/incremental-m...
src/heap/incremental-marking.cc:1164: ->ShouldFinalizeIncrementalMarking()) {
Do we really need that state? How about going there when v8 deque is empty? We
can do up to 3 finalization steps. Why don't we finish there?

[commit-bot: I haz the power, 2016-12-22 19:49:14]

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

[commit-bot: I haz the power, 2016-12-22 19:49:15]

Dry run: This issue passed the CQ dry run.

[Michael Lippautz, 2016-12-22 20:05:58]

On 2016/12/22 19:24:16, Hannes Payer wrote:
>
https://codereview.chromium.org/2591383004/diff/120001/src/heap/incremental-m...
> File src/heap/incremental-marking.cc (right):
> 
>
https://codereview.chromium.org/2591383004/diff/120001/src/heap/incremental-m...
> src/heap/incremental-marking.cc:1164: ->ShouldFinalizeIncrementalMarking()) {
> Do we really need that state? How about going there when v8 deque is empty? We
> can do up to 3 finalization steps. Why don't we finish there?

As discussed offline we should move that into FinalizeIncrementally. I will do
that in a followup since this also affects existing heuristics.

[Michael Lippautz, 2016-12-22 20:06:50]

https://codereview.chromium.org/2591383004/diff/60001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/2591383004/diff/60001/src/heap/heap.cc#newcod...
src/heap/heap.cc:4179: bool Heap::MarkingDequesAreEmpty() {
On 2016/12/22 16:32:47, ulan wrote:
> can this be removed now?

Yeah. It was only used in a DCHECK, which I inlined now.

[Michael Lippautz, 2016-12-22 20:08:52]

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

[commit-bot: I haz the power, 2016-12-22 20:09:00]

Dry run: 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-12-22 20:11:48]

LGTM, I am fine with landing this now and cleaning up later.

[commit-bot: I haz the power, 2016-12-22 20:37:30]

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

[commit-bot: I haz the power, 2016-12-22 20:37:31]

Dry run: This issue passed the CQ dry run.

[Michael Lippautz, 2016-12-22 20:50:06]

The CQ bit was checked by mlippautz@chromium.org

[Michael Lippautz, 2016-12-22 20:50:06]

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/2591383004/#ps140001
(title: "Remove unused method")

[commit-bot: I haz the power, 2016-12-22 20:50:10]

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-12-22 20:51:48]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1482439806427820,
"parent_rev": "b344f930c8b39296ff5a38b79b47c5d35cc5648c", "commit_rev":
"61a55548c50e01d84ed4aefa396324cbb4039b51"}

[commit-bot: I haz the power, 2016-12-22 20:51:53]

Description was changed from

==========
Reland "[heap] Ensure progress when incrementally marking wrappers"

1) Alternate between processing v8 and wrappers
2) Once v8 is empty, try 3 rounds of finding the fixpoint between v8 and
wrappers
3) After that, finalize once v8 marking deque is empty again

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

to

==========
Reland "[heap] Ensure progress when incrementally marking wrappers"

1) Alternate between processing v8 and wrappers
2) Once v8 is empty, try 3 rounds of finding the fixpoint between v8 and
wrappers
3) After that, finalize once v8 marking deque is empty again

BUG=

Review-Url: https://codereview.chromium.org/2591383004
Cr-Commit-Position: refs/heads/master@{#41932}
Committed:
https://chromium.googlesource.com/v8/v8/+/61a55548c50e01d84ed4aefa396324cbb40...
==========

[commit-bot: I haz the power, 2016-12-22 20:51:54]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/v8/v8/+/61a55548c50e01d84ed4aefa396324cbb40...

[Michael Lippautz, 2016-12-23 09:47:03]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/2592393003/ by mlippautz@chromium.org.

The reason for reverting is: Breaks webkit-unit-tests. Investigating...