Add V8 extra utils for promise state

Add V8 extra utils for promise state

This will allow V8 extra consumers to track promise state without
using a side-table. This is used by streams as of
https://github.com/whatwg/streams/commit/173f9f67be56f6690f82ec26fe86c8e67256616b.

BUG=chromium:658144
Review-Url: https://codereview.chromium.org/2784213002
Cr-Commit-Position: refs/heads/master@{#44287}
Committed: https://chromium.googlesource.com/v8/v8/+/a6e635d6929c77e10c45ae069b15003b3f1cf73a

[domenic, 2017-03-30 05:55:30]

domenic@chromium.org changed reviewers:
+ gsathya@chromium.org, littledan@chromium.org

[Dan Ehrenberg, 2017-03-30 10:20:58]

Seems well-motivated to me. We expose the same information via the V8 C++ API.
Unfortunate that this is needed when JS libraries don't have access.

- Did you consider using the three strings as values rather than a numeric enum?
This would conceptually match the spec better and reduce the API surface.

- It seems like it might be a good idea to move the extras into their own JS
file rather than keeping them in prologue.js. Fine to do this in another patch.

[domenic, 2017-03-30 10:25:58]

On 2017/03/30 at 10:20:58, littledan wrote:
> Seems well-motivated to me. We expose the same information via the V8 C++ API.
Unfortunate that this is needed when JS libraries don't have access.
> 
> - Did you consider using the three strings as values rather than a numeric
enum? This would conceptually match the spec better and reduce the API surface.

I did, but it seemed like unnecessary translation work and memory usage, for an
API that is largely about saving small amounts of memory and performance. (If we
weren't worrying about those, we could just keep a side table.)

> - It seems like it might be a good idea to move the extras into their own JS
file rather than keeping them in prologue.js. Fine to do this in another patch.

Yeah, I was surprised they moved here since I last worked on them, but that was
Sathya's idea, so I just ran with it.

[Dan Ehrenberg, 2017-03-30 10:31:23]

Well, if this API may be used by Node, if it starts using V8 extras, and not
just by Chrome, I think I'd rather have the strings. We already do this
translation for DevTools, so in theory we can share the strings and the code
when we want to micro-optimize it. I think a switch statement would be enough
here.

[domenic, 2017-03-30 11:18:03]

On 2017/03/30 at 10:31:23, littledan wrote:
> Well, if this API may be used by Node, if it starts using V8 extras, and not
just by Chrome, I think I'd rather have the strings. We already do this
translation for DevTools, so in theory we can share the strings and the code
when we want to micro-optimize it. I think a switch statement would be enough
here.

I don't understand why Node would want strings instead of numbers. Node would
have the same concerns as we do.

The API is useless if it has strings; if that's required then we'll just not use
this and keep our own side table (using numbers) in the streams code. That will
be much more efficient for us than asking V8 to generate useless strings for us.

What is the actual problem you seem with numbers?

[Dan Ehrenberg, 2017-03-30 12:04:14]

On 2017/03/30 11:18:03, domenic wrote:
> On 2017/03/30 at 10:31:23, littledan wrote:
> > Well, if this API may be used by Node, if it starts using V8 extras, and not
> just by Chrome, I think I'd rather have the strings. We already do this
> translation for DevTools, so in theory we can share the strings and the code
> when we want to micro-optimize it. I think a switch statement would be enough
> here.
> 
> I don't understand why Node would want strings instead of numbers. Node would
> have the same concerns as we do.
> 
> The API is useless if it has strings; if that's required then we'll just not
use
> this and keep our own side table (using numbers) in the streams code. That
will
> be much more efficient for us than asking V8 to generate useless strings for
us.
> 
> What is the actual problem you seem with numbers?

We've been avoiding numbers all over the place in public APIs like this; I
thought it'd make sense to do that here too, if this is a public API. This
introduces another place where the constants are rewritten; if something changes
(e.g., if a cancellation state is added), we'd need to update both sides. I
don't see a big efficiency difference one way or another, for the same reason
you've made the argument that the web platform should export strings rather than
Numbers in cases like this.

Anyway, numbers don't seem like the end of the world here, so I'll trust your
judgement. lgtm

[Dan Ehrenberg, 2017-03-30 12:05:54]

https://codereview.chromium.org/2784213002/diff/1/src/js/prologue.js
File src/js/prologue.js (right):

https://codereview.chromium.org/2784213002/diff/1/src/js/prologue.js#newcode222
src/js/prologue.js:222: return %PromiseStatus(promise);
Is it OK for V8 extras to crash when not-a-promise is passed in here, or should
it throw a neat exception?

[domenic, 2017-03-31 01:04:13]

On 2017/03/30 at 12:05:54, littledan wrote:
> https://codereview.chromium.org/2784213002/diff/1/src/js/prologue.js
> File src/js/prologue.js (right):
> 
>
https://codereview.chromium.org/2784213002/diff/1/src/js/prologue.js#newcode222
> src/js/prologue.js:222: return %PromiseStatus(promise);
> Is it OK for V8 extras to crash when not-a-promise is passed in here, or
should it throw a neat exception?

Crashing is desired (similar to inside V8 code itself).

[gsathya, 2017-03-31 02:27:18]

lgtm

[domenic, 2017-03-31 05:11:22]

The CQ bit was checked by domenic@chromium.org

[commit-bot: I haz the power, 2017-03-31 05:11:27]

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, 2017-03-31 05:58:16]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1490937082738320, "parent_rev":
"5f41fbee57b27ee0853615a8d6258e2cdbf35300", "commit_rev":
"a6e635d6929c77e10c45ae069b15003b3f1cf73a"}

[commit-bot: I haz the power, 2017-03-31 05:58:22]

Description was changed from

==========
Add V8 extra utils for promise state

This will allow V8 extra consumers to track promise state without
using a side-table. This is used by streams as of
https://github.com/whatwg/streams/commit/173f9f67be56f6690f82ec26fe86c8e67256....

BUG=chromium:658144
==========

to

==========
Add V8 extra utils for promise state

This will allow V8 extra consumers to track promise state without
using a side-table. This is used by streams as of
https://github.com/whatwg/streams/commit/173f9f67be56f6690f82ec26fe86c8e67256....

BUG=chromium:658144

Review-Url: https://codereview.chromium.org/2784213002
Cr-Commit-Position: refs/heads/master@{#44287}
Committed:
https://chromium.googlesource.com/v8/v8/+/a6e635d6929c77e10c45ae069b15003b3f1...
==========

[commit-bot: I haz the power, 2017-03-31 05:58:23]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/v8/v8/+/a6e635d6929c77e10c45ae069b15003b3f1...