Introduce debug events for Microtask queue.

Introduce debug events for Microtask queue.

R=yangguo@chromium.org, adamk@chromium.org, rafaelw@chromium.org, rossberg@chromium.org
BUG=chromium:272416
LOG=Y

Committed: https://code.google.com/p/v8/source/detail?r=22204

[aandrey, 2014-07-01 14:15:00]

PTAL

[rossberg, 2014-07-01 14:32:30]

Added rafaelw and adamk, since they are the microtask queue experts.

[adamk, 2014-07-02 23:04:32]

I'll have to defer to yangguo for the debugger stuff. Just want to note that the
approach taken here only logs a debug event for the ObserveMicrotaskRunner,
which itself will run a variety of callbacks. It's not clear to me whether
that's the right approach given I don't know how these debug events are meant to
be used. If it's important that each observer callback is individually tagged,
you'll need a different approach in object-observe.js

[aandrey, 2014-07-03 07:03:50]

On 2014/07/02 23:04:32, adamk wrote:
> I'll have to defer to yangguo for the debugger stuff. Just want to note that
the
> approach taken here only logs a debug event for the ObserveMicrotaskRunner,
> which itself will run a variety of callbacks. It's not clear to me whether
> that's the right approach given I don't know how these debug events are meant
to
> be used. If it's important that each observer callback is individually tagged,
> you'll need a different approach in object-observe.js

There is no need to tag callbacks individually.
BTW, same debug events were implemented for MutationObserver in Blink.

Yang, could you please take a look?

[Yang, 2014-07-03 14:19:45]

Looking good so far. But I'm concerned about the overlap of promise events and
async events. They share a lot of the same code in C++, and in promise.js, will
overlap as we add more promise events. Would it make sense to have promise
events be a subset of async events? I'm aware that the "new promise" event is
not actually async, but I don't think that's a big deal.

https://codereview.chromium.org/362783002/diff/20001/src/promise.js
File src/promise.js (right):

https://codereview.chromium.org/362783002/diff/20001/src/promise.js#newcode132
src/promise.js:132: }
I was actually thinking about adding add promise events here. This way we have
an overlap.

https://codereview.chromium.org/362783002/diff/20001/test/mjsunit/es7/object-...
File test/mjsunit/es7/object-observe-debug-event.js (right):

https://codereview.chromium.org/362783002/diff/20001/test/mjsunit/es7/object-...
test/mjsunit/es7/object-observe-debug-event.js:18: {
move this bracket to the previous line please.

[aandrey, 2014-07-03 14:38:09]

On 2014/07/03 14:19:45, Yang wrote:
> Looking good so far. But I'm concerned about the overlap of promise events and
> async events. They share a lot of the same code in C++, and in promise.js,
will
> overlap as we add more promise events. Would it make sense to have promise
> events be a subset of async events? I'm aware that the "new promise" event is
> not actually async, but I don't think that's a big deal.

Actually, these events are not related.

"Your" promise instrumentation events should be:
- a new promise was created
- promise has changed it's state and/or value
- promise has changed it's parent (iff not handled by one of the above)

I.e. your events should be dispatched right after the promise has actually
changed, but not queued into a microtask. Should we drop the async stacks
support for promises, we would not need to instrument the microtask at all, so
these are not related.

BTW, at this point, if it makes sense, we can do the rest of promise events
ourself, thank to your first pathfinder patch :)

Cross ref for Blink Promise instrumentation:
https://codereview.chromium.org/177773002/

[Yang, 2014-07-03 14:41:06]

On 2014/07/03 14:38:09, aandrey wrote:
> On 2014/07/03 14:19:45, Yang wrote:
> > Looking good so far. But I'm concerned about the overlap of promise events
and
> > async events. They share a lot of the same code in C++, and in promise.js,
> will
> > overlap as we add more promise events. Would it make sense to have promise
> > events be a subset of async events? I'm aware that the "new promise" event
is
> > not actually async, but I don't think that's a big deal.
> 
> Actually, these events are not related.
> 
> "Your" promise instrumentation events should be:
> - a new promise was created
> - promise has changed it's state and/or value
> - promise has changed it's parent (iff not handled by one of the above)
> 
> I.e. your events should be dispatched right after the promise has actually
> changed, but not queued into a microtask. Should we drop the async stacks
> support for promises, we would not need to instrument the microtask at all, so
> these are not related.
> 
> BTW, at this point, if it makes sense, we can do the rest of promise events
> ourself, thank to your first pathfinder patch :)
> 
> Cross ref for Blink Promise instrumentation:
> https://codereview.chromium.org/177773002/

LGTM.

I'd be happy to offload that to you guys. Especially since you know the
requirements better than me. Please add either me or rossberg@ as reviewer
still. Thanks.

[aandrey, 2014-07-03 14:48:26]

The CQ bit was checked by aandrey@chromium.org

[commit-bot: I haz the power, 2014-07-03 14:50:44]

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

[commit-bot: I haz the power, 2014-07-03 14:50:45]

Commit queue rejected this change because it did not recognize the base URL.
Please commit your change manually.

[aandrey, 2014-07-03 15:56:34]

Committed patchset #3 manually as r22204 (presubmit successful).

[yurys, 2014-07-10 12:16:03]

Sometimes we don't collect information about microtasks but still want to be
able to debug the script. In that case we may want to avoid additional overhead
add by these hooks so it might make sense to have additional option that would
enable this instrumentation.