Bug 104088 – [V8] Replace v8::Undefined() with v8Undefined() where possible (part 1)

• Component: WebCore JavaScript
• Status: RESOLVED WONTFIX

[Kentaro Hara]

v8Undefined() is faster than v8::Undefined(). See https://bugs.webkit.org/show_bug.cgi?id=93218#c17

[Kentaro Hara]

Created attachment 177669 [details]
Patch

[Kentaro Hara]

Comment on attachment 177669 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=177669&action=review

> Source/WebCore/bindings/v8/V8Binding.h:141
>      inline v8::Handle<v8::Value> v8Undefined()

Maybe should we rename this method to clarify the above situation?

[Adam Barth]

Comment on attachment 177669 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=177669&action=review

How did you verify that the difference was not important in these cases?

>> Source/WebCore/bindings/v8/V8Binding.h:141
>>      inline v8::Handle<v8::Value> v8Undefined()
> 
> Maybe should we rename this method to clarify the above situation?

v8EmptyHandle() ?

[Kentaro Hara]

> How did you verify that the difference was not important in these cases?

If the value is returned to V8 without being used, then the value can be safely replaced with Handle<Value>(). V8 converts Handle<Value>() to v8::Undefined(). (V8 bindings are already mixing Handle<Value>() and v8::Undefined() for an undefined value that is returned to V8.)

> >> Source/WebCore/bindings/v8/V8Binding.h:141
> >>      inline v8::Handle<v8::Value> v8Undefined()
> > 
> > Maybe should we rename this method to clarify the above situation?
> 
> v8EmptyHandle() ?

Approach 1: We name it v8EmptyHandle(). We use v8EmptyHandle() for an undefined value that is returned to V8. The disadvantage of this approach is that people won't understand why it is OK to return v8EmptyHandle() even though the spec requires to return an undefined value.

Approach 2: Given that Handle<Value>() and v8::Undefined(isolate) have the same speed, we can use v8::Undefined(isolate) for an undefined value that is returned to V8. This approach might be less confusing.

[Adam Barth]

Yeah, approach 2 sounds better.  Then we don't have to worry about the difference between undefined and empty handles.

I'd like to get to the point where we're passing around a non-0 isolate everywhere.  Approach 2 seems more consistent with that future.

[Kentaro Hara]

(In reply to comment #5)
> Yeah, approach 2 sounds better.  Then we don't have to worry about the difference between undefined and empty handles.

Sounds reasonable, I'll take the approach 2.