779601 - Minor cleanup for JS_ValueToECMAInt32 and friends

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: v1

Comment 2

[Jason Orendorff [:jorendorff]]

Comment on attachment 648058 [details] [diff] [review]
v1

Adding more changes after talking this over with bhackett.

Comment 3

[Jason Orendorff [:jorendorff]]

Attachment: v2

Comment 4

[Brian Hackett [Laid off!]]

Comment on attachment 648081 [details] [diff] [review]
v2

Review of attachment 648081 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsapi.h
@@ +2741,5 @@
>  JS_ValueToECMAInt32(JSContext *cx, jsval v, int32_t *ip);
>  
>  #ifdef __cplusplus
>  namespace js {
> +/* DO NOT CALL THIS.  Use JS::ToInt32. */

JS::ToUint16

Comment 5

[Jason Orendorff [:jorendorff]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a5f46bf6c00e

Comment 6

[Boris Zbarsky [:bzbarsky]]

How expensive is the root stuff in ToNumber and ToInt32?  Is there any way we could get rid of it?  Those are pretty hot paths in binding code...

Comment 7

[Terrence Cole [:terrence]]

(In reply to Boris Zbarsky (:bz) [In and out Aug 1 - 10, out Aug 11-20] from comment #6)
> How expensive is the root stuff in ToNumber and ToInt32?  Is there any way
> we could get rid of it?  Those are pretty hot paths in binding code...

Yes, absolutely.  We would like to take a HandleValue here and use the existing root rather than creating a new one.  Work should be starting soon under bug 773686 now that bug 777219 has landed.  We could certainly do this bit first, especially if you have time to help from the DOM side.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a5f46bf6c00e

Comment 9

[Boris Zbarsky [:bzbarsky]]

I can definitely help on the DOM side, modulo the whole "gone Aug 11-20 thing".

On the DOM side, the common case is that the value is coming out of a JSNative's vp or he equivalent for the new setup efaust added for ionmonkey.  A slightly more rare case is that I'm working with a property I just got off an object via JS_GetProperty or JS_GetElement.  So we could presumably push the bits from here out to the DOM code for now, pending those three things getting converted, but to get a real win perf-wise they'd need to be converted too, right?

Comment 10

[Jeff Walden [:Waldo]]

Yes, they'd need to be converted.  (HANDLE ALL THE THINGS.  Ahem.)  If we're changing JSNative, tho, we should probably just change it to JS::CallArgs, which is not far from including all the handle stuff.  (I see that JS::CallReceiver which it derives from is handle-ized, but CallArgs still appears to need a little work yet.)