Use the V8 one-pass compilation API in Blink.

Use the V8 one-pass compilation API in Blink.

With this API, we can get rid of the precompilation phase. When the script is
compiled the first time, the compilation will produce the lazy function
data (the same data which the precompilation used to produce). This data is then
cached and used when the script is compiled again. Note that the caching only
happens inside one renderer, and the data is not stored on disk.

Getting rid of the separate precompilation phase enables various improvements on
the V8 side, for example, using multiple threads for parsing: one will run a
fast parser to produce the lazy function data, another will run the real
(slower) parser and use the lazy function data to skip over functions.

Perf note: This doesn't regress morejs warm times. That means that the data is
generated and used correctly.

BUG=
R=haraken@chromium.org

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

[marja, 2014-03-27 12:45:30]

jochen, ptal

[haraken, 2014-03-27 12:55:59]

https://codereview.chromium.org/212983011/diff/10001/Source/bindings/v8/V8Scr...
File Source/bindings/v8/V8ScriptRunner.cpp (right):

https://codereview.chromium.org/212983011/diff/10001/Source/bindings/v8/V8Scr...
Source/bindings/v8/V8ScriptRunner.cpp:77: v8::ScriptCompiler::Source
source(code, origin, cachedData.release().leakPtr());

Just help me understand: Does the |source| delete the cachedData when the
|source| is garbage-collected? Actually I don't understand why you need to use
OwnPtr here.

Nit: You can use cachedData.leakPtr().

https://codereview.chromium.org/212983011/diff/10001/Source/bindings/v8/V8Scr...
Source/bindings/v8/V8ScriptRunner.cpp:119: v8::Handle<v8::Script> script =
V8ScriptRunner::compileScript(source, fileName, scriptStartPosition, 0,
isolate);

Is it intentional that we don't cache the compiled result of internal scripts to
ScriptResource?

https://codereview.chromium.org/212983011/diff/10001/Source/bindings/v8/V8Scr...
File Source/bindings/v8/V8ScriptRunner.h (right):

https://codereview.chromium.org/212983011/diff/10001/Source/bindings/v8/V8Scr...
Source/bindings/v8/V8ScriptRunner.h:45: static v8::Local<v8::Script>
compileScript(const ScriptSourceCode&, v8::Isolate*, AccessControlStatus =
SharableCrossOrigin);

Just help me understand: Is it OK to share the compiled result in cross origin?

[marja, 2014-03-27 13:07:28]

haraken: Thx for comments! Since you're already at it, could you review this
instead of jochen@? (He's afk.)

https://codereview.chromium.org/212983011/diff/10001/Source/bindings/v8/V8Scr...
File Source/bindings/v8/V8ScriptRunner.cpp (right):

https://codereview.chromium.org/212983011/diff/10001/Source/bindings/v8/V8Scr...
Source/bindings/v8/V8ScriptRunner.cpp:77: v8::ScriptCompiler::Source
source(code, origin, cachedData.release().leakPtr());
On 2014/03/27 12:55:59, haraken wrote:
> 
> Just help me understand: Does the |source| delete the cachedData when the
> |source| is garbage-collected? Actually I don't understand why you need to use
> OwnPtr here.
> 
> Nit: You can use cachedData.leakPtr().

Yes, Source takes the ownership of the CachedData* passed in the ctor (see v8.h
for comments about how ownership works throughout the new compilation api).

Using cachedData.leakPtr() -> Done.

A raw pointer would also do fine because we only own it for a short moment and
we never need to delete it, and it's easy to see that the code is correct. Otoh,
why not use an owned pointer instead of a raw pointer. So, no profound reasons
here to use one or the other, it's mostly just style.

https://codereview.chromium.org/212983011/diff/10001/Source/bindings/v8/V8Scr...
Source/bindings/v8/V8ScriptRunner.cpp:119: v8::Handle<v8::Script> script =
V8ScriptRunner::compileScript(source, fileName, scriptStartPosition, 0,
isolate);
On 2014/03/27 12:55:59, haraken wrote:
> 
> Is it intentional that we don't cache the compiled result of internal scripts
to
> ScriptResource?

This function was never called with a ScriptData* other than 0, so it's
unnecessary.

https://codereview.chromium.org/212983011/diff/10001/Source/bindings/v8/V8Scr...
File Source/bindings/v8/V8ScriptRunner.h (right):

https://codereview.chromium.org/212983011/diff/10001/Source/bindings/v8/V8Scr...
Source/bindings/v8/V8ScriptRunner.h:45: static v8::Local<v8::Script>
compileScript(const ScriptSourceCode&, v8::Isolate*, AccessControlStatus =
SharableCrossOrigin);
On 2014/03/27 12:55:59, haraken wrote:
> 
> Just help me understand: Is it OK to share the compiled result in cross
origin?

I don't know, but this CL doesn't change the behavior related to that. Before
this CL, precompiling was always done (independent of access control status) and
associated to the Resource.

[haraken, 2014-03-27 13:23:08]

Thanks for the clarification. LGTM.

[marja, 2014-03-27 13:28:40]

Thanks for review!

[marja, 2014-03-27 13:28:43]

The CQ bit was checked by marja@chromium.org

[commit-bot: I haz the power, 2014-03-27 13:28:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/212983011/30001

[commit-bot: I haz the power, 2014-03-27 14:36:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-27 14:36:51]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[marja, 2014-03-27 14:59:10]

On 2014/03/27 14:36:51, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   tryserver.blink on linux_blink_dbg

Try failures are unrelated to this cl, see here
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1717
for another try job where the same tests fail.

[marja, 2014-03-27 15:01:21]

Committed patchset #3 manually as r170183 (presubmit successful).