Don't schedule second pass callbacks if there are no callbacks

Don't schedule second pass callbacks if there are no callbacks

BUG=none
R=haraken@chromium.org,hpayer@chromium.org
LOG=n

Committed: https://crrev.com/ae1f32a1cccc9288a9eeb64e9b3576678fffacd8
Cr-Commit-Position: refs/heads/master@{#33658}

[haraken, 2016-02-01 14:09:41]

LGTM

However this can change Oilpan's performance, so I might want to wait for
landing this CL until Oilpan stops relying on the epilogue of the second-pass
callbacks. (I'll preparing the CL now.)

One more thing: It looks like that the IsFirstPass API in include/v8.h is 
wrong:

https://code.google.com/p/chromium/codesearch#chromium/src/v8/include/v8.h&q=...

IsFirstPass can return true when the second-pass callback is set (because
callback_ is not null in that case). But this wouldn't be a big deal right now
since IsFirstPass is unused.

https://codereview.chromium.org/1659433002/diff/1/src/global-handles.cc
File src/global-handles.cc (left):

https://codereview.chromium.org/1659433002/diff/1/src/global-handles.cc#oldco...
src/global-handles.cc:821: if (callback.callback() == nullptr) continue;

Shall we add DCHECK(!callback.callback())?

[haraken, 2016-02-01 14:09:59]

On 2016/02/01 14:09:41, haraken wrote:
> LGTM
> 
> However this can change Oilpan's performance, so I might want to wait for
> landing this CL until Oilpan stops relying on the epilogue of the second-pass
> callbacks. (I'll preparing the CL now.)
> 
> One more thing: It looks like that the IsFirstPass API in include/v8.h is 
> wrong:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/v8/include/v8.h&q=...
> 
> IsFirstPass can return true when the second-pass callback is set (because
> callback_ is not null in that case). But this wouldn't be a big deal right now
> since IsFirstPass is unused.
> 
> https://codereview.chromium.org/1659433002/diff/1/src/global-handles.cc
> File src/global-handles.cc (left):
> 
>
https://codereview.chromium.org/1659433002/diff/1/src/global-handles.cc#oldco...
> src/global-handles.cc:821: if (callback.callback() == nullptr) continue;
> 
> Shall we add DCHECK(!callback.callback())?

I meant DCHECK(callback.callback()).

[jochen (gone - plz use gerrit), 2016-02-01 14:29:11]

On 2016/02/01 at 14:09:59, haraken wrote:
> On 2016/02/01 14:09:41, haraken wrote:
> > LGTM
> > 
> > However this can change Oilpan's performance, so I might want to wait for
> > landing this CL until Oilpan stops relying on the epilogue of the
second-pass
> > callbacks. (I'll preparing the CL now.)
> > 
> > One more thing: It looks like that the IsFirstPass API in include/v8.h is 
> > wrong:
> > 
> >
https://code.google.com/p/chromium/codesearch#chromium/src/v8/include/v8.h&q=...
> > 
> > IsFirstPass can return true when the second-pass callback is set (because
> > callback_ is not null in that case). But this wouldn't be a big deal right
now
> > since IsFirstPass is unused.

deprecated the api

> > 
> > https://codereview.chromium.org/1659433002/diff/1/src/global-handles.cc
> > File src/global-handles.cc (left):
> > 
> >
https://codereview.chromium.org/1659433002/diff/1/src/global-handles.cc#oldco...
> > src/global-handles.cc:821: if (callback.callback() == nullptr) continue;
> > 
> > Shall we add DCHECK(!callback.callback())?
> 
> I meant DCHECK(callback.callback()).

we'll reliably crash when trying to invoke callback.callback(), so I'm not sure
this is worth slowing down the correct code in dcheck-on builds

[haraken, 2016-02-02 07:09:31]

Now that https://codereview.chromium.org/1659453002/ has been landed, feel free
to land this CL.

[Hannes Payer (out of office), 2016-02-02 08:46:19]

On 2016/02/02 07:09:31, haraken wrote:
> Now that https://codereview.chromium.org/1659453002/ has been landed, feel
free
> to land this CL.

Lgtm

[jochen (gone - plz use gerrit), 2016-02-02 08:46:34]

The CQ bit was checked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2016-02-02 08:46:35]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1659433002/#ps20001
(title: "updates")

[commit-bot: I haz the power, 2016-02-02 08:46:46]

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

[commit-bot: I haz the power, 2016-02-02 08:58:38]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-02 08:59:32]

Description was changed from

==========
Don't schedule second pass callbacks if there are no callbacks

BUG=none
R=haraken@chromium.org,hpayer@chromium.org
LOG=n
==========

to

==========
Don't schedule second pass callbacks if there are no callbacks

BUG=none
R=haraken@chromium.org,hpayer@chromium.org
LOG=n

Committed: https://crrev.com/ae1f32a1cccc9288a9eeb64e9b3576678fffacd8
Cr-Commit-Position: refs/heads/master@{#33658}
==========

[commit-bot: I haz the power, 2016-02-02 08:59:32]

Patchset 2 (id:??) landed as
https://crrev.com/ae1f32a1cccc9288a9eeb64e9b3576678fffacd8
Cr-Commit-Position: refs/heads/master@{#33658}