Bug 94588 – [V8] Replace v8::String::NewSymbol() in CodeGeneratorV8.pm with v8String()

• Component: WebCore JavaScript
• Status: RESOLVED INVALID

[Kentaro Hara]

v8String() is much faster than v8::String::NewSymbol().

// 83 nano sec
static v8::Handle<v8::Value> attr1AttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
    static String str1 = String("foo");
    static String str2 = String("foo");
    v8::Handle<v8::Value> v1 = v8String(str1);
    v8::Handle<v8::Value> v2 = v8String(str2);
    if (!v1.IsEmpty() && !v2.IsEmpty())
        return v8Undefined();
    return v8::Null(); // Never reach.
}

// 86 nano sec
static v8::Handle<v8::Value> attr2AttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
    static String str1 = String("foo");
    static String str2 = String("bar");
    v8::Handle<v8::Value> v1 = v8String(str1);
    v8::Handle<v8::Value> v2 = v8String(str2);
    if (!v1.IsEmpty() && !v2.IsEmpty())
        return v8Undefined();
    return v8::Null(); // Never reach.
}

// 376 nano sec
static v8::Handle<v8::Value> attr3AttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
    v8::Handle<v8::Value> v1 = v8::String::NewSymbol("foo", 3);
    v8::Handle<v8::Value> v2 = v8::String::NewSymbol("foo", 3);
    if (!v1.IsEmpty() && !v2.IsEmpty())
        return v8Undefined();
    return v8::Null(); // Never reach.
}

// 377 nano sec
static v8::Handle<v8::Value> attr4AttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
    v8::Handle<v8::Value> v1 = v8::String::NewSymbol("foo", 3);
    v8::Handle<v8::Value> v2 = v8::String::NewSymbol("bar", 3);
    if (!v1.IsEmpty() && !v2.IsEmpty())
        return v8Undefined();
    return v8::Null(); // Never reach.
}

[Kentaro Hara]

Sorry, the above performance measurement was not fair. We need to compare the time for converting char* to v8::String.

Updated result:

// 272 nano sec
static v8::Handle<v8::Value> attr3AttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
    v8::Handle<v8::Value> v1 = v8String(String("foo"));
    v8::Handle<v8::Value> v2 = v8String(String("bar"));
    if (!v1.IsEmpty() && !v2.IsEmpty())
        return v8Undefined();
    return v8::Null(); // Never reach.
}

// 377 nano sec
static v8::Handle<v8::Value> attr4AttrGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
    v8::Handle<v8::Value> v1 = v8::String::NewSymbol("foo");
    v8::Handle<v8::Value> v2 = v8::String::NewSymbol("bar");
    if (!v1.IsEmpty() && !v2.IsEmpty())
        return v8Undefined();
    return v8::Null(); // Never reach.
}

Anyway, v8String() is faster than String::NewSymbol().

[Kentaro Hara]

Created attachment 159669 [details]
Patch

[Adam Barth]

Do you have any thoughts on https://bugs.webkit.org/show_bug.cgi?id=94574#c5 ?  I wonder if there's a place for NewSymbol in some cases.

[Eric Seidel (no email)]

Comment on attachment 159669 [details]
Patch

OK.

[WebKit Review Bot]

Comment on attachment 159669 [details]
Patch

Clearing flags on attachment: 159669

Committed r126348: <http://trac.webkit.org/changeset/126348>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Kentaro Hara]

Reverted r126348 for reason:

We should use v8::String::NewSymbol() for symbols. See https://bugs.webkit.org/show_bug.cgi?id=94574#c10 for more details

Committed r126397: <http://trac.webkit.org/changeset/126397>

[Kentaro Hara]

(In reply to comment #6)
> All reviewed patches have been landed.  Closing bug.

This patch is not quite right. As abarth pointed out, we should use String::NewSymbol() for symbols and use String::New() for other strings. See https://bugs.webkit.org/show_bug.cgi?id=94574#c10 for more details.