Simplify TONATIVE_*_EXCEPTIONSTATE* macros

Simplify TONATIVE_*_EXCEPTIONSTATE* macros

By assuming that the |value| expression throws exceptions via the
ExceptionState object, and only via it, we can skip the v8::TryCatch
declaration and check. This is good since it is less code, and since
v8::TryCatch objects cost a bit of CPU time to initialize.

The correctness of this patch depends on
  https://codereview.chromium.org/559553003/
and
  https://codereview.chromium.org/556273003/
that make sure all type conversion helpers with an ExceptionState&
argument actually throw all exceptions via it.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181796

[Jens Widell, 2014-09-10 11:01:31]

jl@opera.com changed reviewers:
+ haraken@chromium.org

[Jens Widell, 2014-09-10 11:01:31]

PTAL

Note that this is quite not ready to land; once
https://codereview.chromium.org/561633003/ lands, dictionary conversions won't
follow the rules, and this change to the macros would cause incorrect behavior.

The purpose here is to optimize by eliminating the need for TryCatch locals. The
conversion functions (e.g. toInt32()) have their own, and often don't actually
initialize them in the "fast case" where the V8 value is of the required type
already.

[haraken, 2014-09-10 11:14:22]

LGTM

This might slightly speed up the binding code, since the allocation of
v8::TryCatch is heavy.

[Jens Widell, 2014-09-10 11:17:41]

On 2014/09/10 11:14:22, haraken wrote:
> LGTM
> 
> This might slightly speed up the binding code, since the allocation of
> v8::TryCatch is heavy.

Yes, that was my hope as well. I think I've seen a 10 % or so speed-up on a
micro-benchmark assigning the same value over and over to an element's tabIndex
attribute. Micro-benchmarks are always tricky to draw conclusions from.

But executing less code doing less work is almost always faster, in my
experience. :-)

[Jens Widell, 2014-09-10 16:09:34]

On 2014/09/10 11:14:22, haraken wrote:
> This might slightly speed up the binding code, since the allocation of
> v8::TryCatch is heavy.

I think we can probably get rid of the argument conversion v8::TryCatch entirely
without too much difficulty.

We'd need to use toInt32(), toFloat() and toDouble() (the latter doesn't exist
yet) instead of calling v8::Value accessors directly for some types, and convert
the to*NativeArray() helpers to use an ExceptionState, and then we're pretty
much done, I think.

This ought to be a win performance-wise (and we could also play around with
inlining the fast cases of toInt32() and friends) and would also allow us to
drop all the complexity surrounding that v8::TryCatch local that I added with
https://codereview.chromium.org/265293004/.

WDYT?

[haraken, 2014-09-11 01:04:07]

On 2014/09/10 16:09:34, Jens Widell wrote:
> On 2014/09/10 11:14:22, haraken wrote:
> > This might slightly speed up the binding code, since the allocation of
> > v8::TryCatch is heavy.
> 
> I think we can probably get rid of the argument conversion v8::TryCatch
entirely
> without too much difficulty.
> 
> We'd need to use toInt32(), toFloat() and toDouble() (the latter doesn't exist
> yet) instead of calling v8::Value accessors directly for some types, and
convert
> the to*NativeArray() helpers to use an ExceptionState, and then we're pretty
> much done, I think.
> 
> This ought to be a win performance-wise (and we could also play around with
> inlining the fast cases of toInt32() and friends) and would also allow us to
> drop all the complexity surrounding that v8::TryCatch local that I added with
> https://codereview.chromium.org/265293004/.
> 
> WDYT?

I think that is a super great suggestion!

[Jens Widell, 2014-09-11 05:59:18]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2014-09-11 05:59:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/552143004/60001

[commit-bot: I haz the power, 2014-09-11 06:45:31]

Committed patchset #4 (id:60001) as 181796