Restore in-memory parser cache for V8 compile.

Source/bindings/core/v8/V8ScriptRunner.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 44 matching lines @@
         // If we are already handling a recursion level error, we should
         // not invoke v8::Function::Call.
         return v8::Undefined(isolate);
     }
     V8PerIsolateData::from(isolate)->setIsHandlingRecursionLevelError(true);
     v8::Local<v8::Value> result = v8::Function::New(isolate, throwStackOverflowException)->Call(v8::Undefined(isolate), 0, 0);
     V8PerIsolateData::from(isolate)->setIsHandlingRecursionLevelError(false);
     return result;
 }
 
-// Make a decision on whether we want to use V8 caching and how.
-// dataType, produceOption, consumeOption are out parameters.
-bool CacheDecider(
-    const v8::Handle<v8::String> code,
-    const ScriptResource* resource,
-    V8CacheOptions cacheOptions,
-    unsigned* dataType,
-    v8::ScriptCompiler::CompileOptions* compileOption,
-    bool* produce)
+v8::Local<v8::Script> CompileAndProduceCache(v8::Handle<v8::String> code, v8::ScriptOrigin origin, ScriptResource* resource, v8::Isolate* isolate, v8::ScriptCompiler::CompileOptions options, unsigned cacheTag, bool persistCache)

[haraken, 2014/09/03 04:11:43]

CompileAndProduceCache => compileAndProduceCache

[vogelheim, 2014/09/03 11:21:34]

Done.

 {
-    if (!resource || !resource->url().protocolIsInHTTPFamily() || code->Length() < 1024)
-        cacheOptions = V8CacheOptionsOff;
+    v8::ScriptCompiler::Source source(code, origin);
+    v8::Local<v8::Script> script = v8::ScriptCompiler::Compile(isolate, &source, options);
+    const v8::ScriptCompiler::CachedData* cachedData = source.GetCachedData();
+    if (resource && cachedData) {
+        resource->clearCachedMetadata();
+        resource->setCachedMetadata(
+            cacheTag,
+            reinterpret_cast<const char*>(cachedData->data),
+            cachedData->length,
+            persistCache);
+    }
+    return script;
+}
 
-    bool useCache = false;
-    switch (cacheOptions) {
-    case V8CacheOptionsOff:
-        *compileOption = v8::ScriptCompiler::kNoCompileOptions;
-        useCache = false;
-        break;
-    case V8CacheOptionsParse:
-        *dataType = StringHash::hash(v8::V8::GetVersion()) * 2;
-        *produce = !resource->cachedMetadata(*dataType);
-        *compileOption = *produce ? v8::ScriptCompiler::kProduceParserCache : v8::ScriptCompiler::kConsumeParserCache;
-        useCache = true;
-        break;
-    case V8CacheOptionsCode:
-        *dataType = StringHash::hash(v8::V8::GetVersion()) * 2 + 1;
-        *produce = !resource->cachedMetadata(*dataType);
-        *compileOption = *produce ? v8::ScriptCompiler::kProduceCodeCache : v8::ScriptCompiler::kConsumeCodeCache;
-        useCache = true;
-        break;
-    }
-    return useCache;
+v8::Local<v8::Script> CompileAndConsumeCache(v8::Handle<v8::String> code, v8::ScriptOrigin origin, ScriptResource* resource, v8::Isolate* isolate, v8::ScriptCompiler::CompileOptions options, unsigned cacheTag)
+{
+    // Consume existing cache data:
+    CachedMetadata* cachedMetadata = resource->cachedMetadata(cacheTag);
+    v8::ScriptCompiler::CachedData* cachedData = new v8::ScriptCompiler::CachedData(
+        reinterpret_cast<const uint8_t*>(cachedMetadata->data()),
+        cachedMetadata->size(),
+        v8::ScriptCompiler::CachedData::BufferNotOwned);
+    v8::ScriptCompiler::Source source(code, origin, cachedData);
+    return v8::ScriptCompiler::Compile(isolate, &source, options);
 }
 
+// Locally used enum to enumerate the actions for V8ScriptRunner::compileScript
+enum CompileScriptAction {
+    ConsumeParserCache,
+    ConsumeCodeCache,
+    ProduceParserCache,
+    ProduceCodeCache,
+    ProduceInMemoryParserCache,
+    Off
+};
+
 } // namespace
 
 v8::Local<v8::Script> V8ScriptRunner::compileScript(const ScriptSourceCode& source, v8::Isolate* isolate, AccessControlStatus corsStatus, V8CacheOptions cacheOptions)
 {
     return compileScript(v8String(isolate, source.source()), source.url(), source.startPosition(), source.resource(), isolate, corsStatus, cacheOptions);
 }
 
 v8::Local<v8::Script> V8ScriptRunner::compileScript(v8::Handle<v8::String> code, const String& fileName, const TextPosition& scriptStartPosition, ScriptResource* resource, v8::Isolate* isolate, AccessControlStatus corsStatus, V8CacheOptions cacheOptions)
 {
     TRACE_EVENT1("v8", "v8.compile", "fileName", fileName.utf8());
     TRACE_EVENT_SCOPED_SAMPLING_STATE("v8", "V8Compile");
 
     // NOTE: For compatibility with WebCore, ScriptSourceCode's line starts at
     // 1, whereas v8 starts at 0.
     v8::Handle<v8::String> name = v8String(isolate, fileName);
     v8::Handle<v8::Integer> line = v8::Integer::New(isolate, scriptStartPosition.m_line.zeroBasedInt());
     v8::Handle<v8::Integer> column = v8::Integer::New(isolate, scriptStartPosition.m_column.zeroBasedInt());
     v8::Handle<v8::Boolean> isSharedCrossOrigin = corsStatus == SharableCrossOrigin ? v8::True(isolate) : v8::False(isolate);
     v8::ScriptOrigin origin(name, line, column, isSharedCrossOrigin);
 
-    // V8 supports several forms of caching. Decide on the cache mode and call
-    // ScriptCompiler::Compile with suitable options.
-    unsigned dataTypeID = 0;
-    v8::ScriptCompiler::CompileOptions compileOption = v8::ScriptCompiler::kNoCompileOptions;
-    bool produce;
+    // Device on the compile options (caching).

[marja, 2014/09/03 07:17:47]

"Device on"? (Obvsly I'm not a native speaker but what does "device on" mean?)

[vogelheim, 2014/09/03 11:21:34]

I meant: "Decide on ...". Not sure what happened there. If had written this on
my phone, I'd blame autocorrect.

+    CompileScriptAction compileScriptAction;
+    unsigned cacheTag = 0;
+    if (!resource || !resource->url().protocolIsInHTTPFamily() || code->Length() < 1024) {
+        compileScriptAction = Off;
+    } else {
+        switch (cacheOptions) {
+        case V8CacheOptionsParse:
+            cacheTag = StringHash::hash(v8::V8::GetVersion()) * 2;
+            compileScriptAction = resource->cachedMetadata(cacheTag)
+                ? ConsumeParserCache : ProduceParserCache;
+            break;
+        case V8CacheOptionsCode:
+            cacheTag = StringHash::hash(v8::V8::GetVersion()) * 2 + 1;
+            compileScriptAction = resource->cachedMetadata(cacheTag)
+                ? ConsumeCodeCache : ProduceCodeCache;
+            break;
+        case V8CacheOptionsOff:
+        default:
+            // Previous behaviour was to always generate an in-memory parser
+            // cache. We emulate this here.
+            // TODO(vogelheim): Determine whether this should get its own
+            //                  setting, so we can also have a true 'off'.
+            cacheTag = StringHash::hash(v8::V8::GetVersion()) * 2;
+            compileScriptAction = resource->cachedMetadata(cacheTag)
+                ? ConsumeParserCache : ProduceInMemoryParserCache;
+            break;
+        };
+    }
+
     v8::Local<v8::Script> script;
-    if (CacheDecider(code, resource, cacheOptions, &dataTypeID, &compileOption, &produce)) {
-        if (produce) {
-            // Produce new cache data:
-            v8::ScriptCompiler::Source source(code, origin);
-            script = v8::ScriptCompiler::Compile(isolate, &source, compileOption);
-            const v8::ScriptCompiler::CachedData* cachedData = source.GetCachedData();
-            if (cachedData) {
-                resource->clearCachedMetadata();
-                resource->setCachedMetadata(
-                    dataTypeID,
-                    reinterpret_cast<const char*>(cachedData->data),
-                    cachedData->length);
-            }
-        } else {
-            // Consume existing cache data:
-            CachedMetadata* cachedMetadata = resource->cachedMetadata(dataTypeID);
-            v8::ScriptCompiler::CachedData* cachedData = new v8::ScriptCompiler::CachedData(
-                reinterpret_cast<const uint8_t*>(cachedMetadata->data()),
-                cachedMetadata->size(),
-                v8::ScriptCompiler::CachedData::BufferNotOwned);
-            v8::ScriptCompiler::Source source(code, origin, cachedData);
-            script = v8::ScriptCompiler::Compile(isolate, &source, compileOption);
-        }
-    } else {
-        // No caching:
+    switch (compileScriptAction) {

[marja, 2014/09/03 07:17:47]

Sounds a bit duplicate to first gather all the information in
compileScriptAction first, and right after that, split it to "cache option" &
"persist" & which function to call.

Can you do this in one go? Can't the switch above call the right function with
the right parameters? Does it get messy if you do that?

[vogelheim, 2014/09/03 11:21:34]

Hmm. Done. That was meant to separate the decision from the action, but I admit
it didn't really buy much. Please have a look at whether you like the new
version better.

+    case ProduceParserCache:
+        script = CompileAndProduceCache(code, origin, resource, isolate, v8::ScriptCompiler::kProduceParserCache, cacheTag, true);
+        break;
+    case ProduceCodeCache:
+        script = CompileAndProduceCache(code, origin, resource, isolate, v8::ScriptCompiler::kProduceCodeCache, cacheTag, true);
+        break;
+    case ConsumeParserCache:
+        script = CompileAndConsumeCache(code, origin, resource, isolate, v8::ScriptCompiler::kConsumeParserCache, cacheTag);
+        break;
+    case ConsumeCodeCache:
+        script = CompileAndConsumeCache(code, origin, resource, isolate, v8::ScriptCompiler::kConsumeCodeCache, cacheTag);
+        break;
+    case ProduceInMemoryParserCache:
+        script = CompileAndProduceCache(code, origin, resource, isolate, v8::ScriptCompiler::kProduceParserCache, cacheTag, false);
+        break;
+    case Off:
         v8::ScriptCompiler::Source source(code, origin);
-        script = v8::ScriptCompiler::Compile(
-            isolate, &source, v8::ScriptCompiler::kNoCompileOptions);
+        script = v8::ScriptCompiler::Compile(isolate, &source, v8::ScriptCompiler::kNoCompileOptions);
+        break;
     }
+
     return script;
 }
 
 v8::Local<v8::Value> V8ScriptRunner::runCompiledScript(v8::Handle<v8::Script> script, ExecutionContext* context, v8::Isolate* isolate)
 {
     if (script.IsEmpty())
         return v8::Local<v8::Value>();
     TRACE_EVENT_SCOPED_SAMPLING_STATE("v8", "V8Execution");
     TRACE_EVENT1("v8", "v8.run", "fileName", TRACE_STR_COPY(*v8::String::Utf8Value(script->GetUnboundScript()->GetScriptName())));
 
@@ skipping 103 matching lines @@
 {
     TRACE_EVENT0("v8", "v8.newInstance");
     TRACE_EVENT_SCOPED_SAMPLING_STATE("v8", "V8Execution");
     V8RecursionScope scope(isolate, context);
     v8::Local<v8::Object> result = function->NewInstance(argc, argv);
     crashIfV8IsDead();
     return result;
 }
 
 } // namespace blink