Cleanup calls to ProcessMarkingDeque around ephemeral marking

We need to empty the deque after EmbedderHeapTracer::TracePrologue, and we don't have to empty the deque after ProcessEphemeralMarking, as the deque should be empty already.

BUG=468240
LOG=no

Committed: https://crrev.com/194a04d73513fc7193a8cd4415fd01192274e577
Cr-Commit-Position: refs/heads/master@{#35654}

[Marcel Hlopko, 2016-04-18 18:03:05]

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

[Marcel Hlopko, 2016-04-19 12:27:19]

Ptal :)

[Hannes Payer (out of office), 2016-04-20 08:26:00]

Instead of moving the DCHECK, why don't we empty the marking deque after
TracePrologue?

[Marcel Hlopko, 2016-04-20 08:39:49]

Like this? I'm fine with both :) Not directly related, is there a reason we call
ProcessMarkingDeque right after ProcessEphemeralMarking, which calls
ProcessMarkingDeque anyway? Is this line:

    work_to_do = !marking_deque_.IsEmpty();

sufficient when deque overflows? What I would expect:

    work_to_do = !marking_deque_.IsEmpty() || marking_deque_.overflowed();

Sorry for nitting, just want to understand stuff :) Thanks!

[Hannes Payer (out of office), 2016-04-20 08:43:44]

On 2016/04/20 at 08:39:49, hlopko wrote:
> Like this? I'm fine with both :) Not directly related, is there a reason we
call ProcessMarkingDeque right after ProcessEphemeralMarking, which calls
ProcessMarkingDeque anyway? Is this line:
> 
>     work_to_do = !marking_deque_.IsEmpty();
> 
> sufficient when deque overflows? What I would expect:
> 
>     work_to_do = !marking_deque_.IsEmpty() || marking_deque_.overflowed();
> 
> Sorry for nitting, just want to understand stuff :) Thanks!

Nope, that is useless. You can remove them :)

[Marcel Hlopko, 2016-04-20 08:49:53]

Description was changed from

==========
Move dcheck out of ephemeral marking

Currently the DCHECK cannot work for wrapper tracing, as right before the call
to processEphemeralMarking we call TracePrologue, which will mark blink roots =>
add stuff to the marking queue. I still want this check, so I moved it out of
ephemeral marking. Wdyt?

BUG=468240
LOG=no
==========

to

==========
We need to empty the deque after EmbedderHeapTracer::TracePrologue, and we don't
have to empty the deque after ProcessEphemeralMarking, as the deque should be
empty already.

BUG=468240
LOG=no
==========

[Marcel Hlopko, 2016-04-20 08:50:12]

Cleaned up the code *and* this cl :)

[Hannes Payer (out of office), 2016-04-20 10:36:52]

lgtm

[Marcel Hlopko, 2016-04-20 10:40:55]

The CQ bit was checked by hlopko@chromium.org

[commit-bot: I haz the power, 2016-04-20 10:40:59]

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

[commit-bot: I haz the power, 2016-04-20 10:42:29]

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

[commit-bot: I haz the power, 2016-04-20 10:42:30]

Try jobs failed on following builders:
  v8_android_arm_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux64_asan_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux64_avx2_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/4552)
  v8_linux_arm64_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_dbg_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/4575)
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_mipsel_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_nodcheck_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/13902)
  v8_win_nosnap_shared_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)

[Marcel Hlopko, 2016-04-20 10:47:02]

The CQ bit was checked by hlopko@chromium.org

[Marcel Hlopko, 2016-04-20 10:47:02]

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/1900783002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2016-04-20 10:47:11]

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

[commit-bot: I haz the power, 2016-04-20 11:11:10]

Committed patchset #4 (id:60001)

[nodir1, 2016-04-22 18:44:02]

Description was changed from

==========
We need to empty the deque after EmbedderHeapTracer::TracePrologue, and we don't
have to empty the deque after ProcessEphemeralMarking, as the deque should be
empty already.

BUG=468240
LOG=no
==========

to

==========
We need to empty the deque after EmbedderHeapTracer::TracePrologue, and we don't
have to empty the deque after ProcessEphemeralMarking, as the deque should be
empty already.

BUG=468240
LOG=no

Committed: https://crrev.com/194a04d73513fc7193a8cd4415fd01192274e577
Cr-Commit-Position: refs/heads/master@{#35654}
==========

[nodir1, 2016-04-22 18:44:03]

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