[Webkit-unassigned] [Bug 91850] add 7 bit strings capabilities to the v8 binding layer

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=91850





--- Comment #29 from Dan Carney <dcarney at google.com>  2012-10-18 07:07:59 PST ---
I've cleaned up the patch a bit.  It will fail until a v8 bug fix makes its way through the pipeline, but I'd still like another review so I can get this patch in the commit queue as soon as that happens. I have the feeling that this patch will get rolled back a few times, as it's using a bunch of v8 functionality never used before, and I'd like to work through that sooner rather than later.

> Since this should be a perf win, it seems appropriate to comment on how much of a win it is (in whatever example benchmarks you like) in the ChangeLog.

Okay, I forgot to add them in this patch, I'll add those in on the final submit. Some representative numbers are in https://bugs.webkit.org/attachment.cgi?id=163334, but like are perf numbers are probably somewhat wrong.

In particular:

Bindings/append-child
Bindings/create-element 
Bindings/id-getter
Dromaeo/jslib-style-jquery
Dromaeo/jslib-attr-jquery
Dromaeo/dromaeo-object-regexp
Dromaeo/sunspider-crypto-md5 

see some consistent improvement.  There are a few loses but they seem less drastic than the gains, and I have future patches to v8 which should mitigate them.

> > Source/WebCore/bindings/v8/V8ValueCache.cpp:44
> > +    if (string.is8Bit() && string.containsOnlyASCII()) {
> > +        WebCoreStringResource8* stringResource = new WebCoreStringResource8(string);
> > +        v8::Local<v8::String> newString = v8::String::NewExternal(stringResource);
> > +        if (newString.IsEmpty())
> > +            delete stringResource;
> > +        return newString;
> > +    }
> > +
> > +    WebCoreStringResource16* stringResource = new WebCoreStringResource16(string);
> 
> Can't we share this block by using a WebCoreStringResourceBase* ?  Or does it not have a virtual destructor?

It's not obvious here, but  v8::String::NewExternal() is the problem as it's overloaded and doesn't accept ExternalStringBase.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.