[webkit-reviews] review granted: [Bug 38661] Persist V8's ScriptData to cache : [Attachment 57269] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Tony Gentilcore
<tonyg at chromium.org>'s request for review:
Bug 38661: Persist V8's ScriptData to cache
https://bugs.webkit.org/show_bug.cgi?id=38661

Attachment 57269: Patch
https://bugs.webkit.org/attachment.cgi?id=57269&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
This looks good.  I'm sad about the magic number, as we've discussed before. 
Also, you're comments aren't they way we usually do things in WebKit-land. 
Maybe worth tweaking slightly before landing?

WebCore/ChangeLog:12
 +	    Chromium's morejs benchmark shows a 3-4% improvement on fast
hardware.
Awesome.

WebCore/bindings/v8/V8Proxy.cpp:345
 +	static const unsigned dataTypeID = 0xECC13BD7;
:(

WebCore/bindings/v8/V8Proxy.cpp:353
 +	// If there is cached data, use it.
We usually don't comment about *what* the code is doing, but more *why* it's
doing that.

WebCore/bindings/v8/V8Proxy.cpp:359
 +	v8::ScriptData* scriptData =
v8::ScriptData::PreCompile(sourceCode.source().utf8().data(),
sourceCode.source().utf8().length());
Is the conversion to utf8 expensive?  Maybe we should do that once instead of
twice?