[compiler] support isolate compilation cache in CompileFunction()

joyeecheung · V8 LUCI CQ · commit f4b3f6eb4595 · 2024-01-04T15:58:02.000Z
Previously there was no isolate compilation cache support for scripts compiled Script::CompileFunction() with wrapped arguments. This patch adds support for that. Refs: nodejs/node#35375 Change-Id: Id1849961ecd1282eb2dac95829157d167a3aa9a1 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4962094 Reviewed-by: Camillo Bruni <cbruni@chromium.org> Commit-Queue: Joyee Cheung <joyee@igalia.com> Cr-Commit-Position: refs/heads/main@{#91681}
diff --git a/src/api/api.cc b/src/api/api.cc
@@ -2766,6 +2766,7 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInternal(
         i_isolate, source->resource_name, source->resource_line_offset,
         source->resource_column_offset, source->source_map_url,
         source->host_defined_options, source->resource_options);
+    script_details.wrapped_arguments = arguments_list;
 
     std::unique_ptr<i::AlignedCachedData> cached_data;
     if (options == kConsumeCodeCache) {
@@ -2778,8 +2779,8 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInternal(
     i::Handle<i::JSFunction> scoped_result;
     has_exception =
         !i::Compiler::GetWrappedFunction(
-             Utils::OpenHandle(*source->source_string), arguments_list, context,
-             script_details, cached_data.get(), options, no_cache_reason)
+             Utils::OpenHandle(*source->source_string), context, script_details,
+             cached_data.get(), options, no_cache_reason)
              .ToHandle(&scoped_result);
     if (options == kConsumeCodeCache) {
       source->cached_data->rejected = cached_data->rejected();
diff --git a/src/codegen/compilation-cache.cc b/src/codegen/compilation-cache.cc
@@ -189,8 +189,8 @@ void CompilationCacheScript::Put(Handle<String> source,
                                  Handle<SharedFunctionInfo> function_info) {
   HandleScope scope(isolate());
   Handle<CompilationCacheTable> table = GetTable();
-  table_ = *CompilationCacheTable::PutScript(table, source, function_info,
-                                             isolate());
+  table_ = *CompilationCacheTable::PutScript(table, source, kNullMaybeHandle,
+                                             function_info, isolate());
 }
 
 InfoCellPair CompilationCacheEval::Lookup(Handle<String> source,
diff --git a/src/codegen/compiler.cc b/src/codegen/compiler.cc
@@ -3373,14 +3373,13 @@ struct ScriptCompileTimerScope {
   }
 };
 
-Handle<Script> NewScript(
-    Isolate* isolate, ParseInfo* parse_info, Handle<String> source,
-    ScriptDetails script_details, NativesFlag natives,
-    MaybeHandle<FixedArray> maybe_wrapped_arguments = kNullMaybeHandle) {
+Handle<Script> NewScript(Isolate* isolate, ParseInfo* parse_info,
+                         Handle<String> source, ScriptDetails script_details,
+                         NativesFlag natives) {
   // Create a script object describing the script to be compiled.
-  Handle<Script> script =
-      parse_info->CreateScript(isolate, source, maybe_wrapped_arguments,
-                               script_details.origin_options, natives);
+  Handle<Script> script = parse_info->CreateScript(
+      isolate, source, script_details.wrapped_arguments,
+      script_details.origin_options, natives);
   DisallowGarbageCollection no_gc;
   SetScriptFieldsFromDetails(isolate, *script, script_details, &no_gc);
   LOG(isolate, ScriptDetails(*script));
@@ -3790,9 +3789,8 @@ Compiler::GetSharedFunctionInfoForScriptWithCompileHints(
 
 // static
 MaybeHandle<JSFunction> Compiler::GetWrappedFunction(
-    Handle<String> source, Handle<FixedArray> arguments,
-    Handle<Context> context, const ScriptDetails& script_details,
-    AlignedCachedData* cached_data,
+    Handle<String> source, Handle<Context> context,
+    const ScriptDetails& script_details, AlignedCachedData* cached_data,
     v8::ScriptCompiler::CompileOptions compile_options,
     v8::ScriptCompiler::NoCacheReason no_cache_reason) {
   Isolate* isolate = context->GetIsolate();
@@ -3802,16 +3800,28 @@ MaybeHandle<JSFunction> Compiler::GetWrappedFunction(
 
   if (compile_options == ScriptCompiler::kConsumeCodeCache) {
     DCHECK(cached_data);
+    DCHECK_EQ(script_details.repl_mode, REPLMode::kNo);
   } else {
     DCHECK_NULL(cached_data);
   }
 
   LanguageMode language_mode = construct_language_mode(v8_flags.use_strict);
-
+  DCHECK(!script_details.wrapped_arguments.is_null());
   MaybeHandle<SharedFunctionInfo> maybe_result;
+  Handle<SharedFunctionInfo> result;
+  Handle<Script> script;
+  IsCompiledScope is_compiled_scope;
   bool can_consume_code_cache =
       compile_options == ScriptCompiler::kConsumeCodeCache;
-  if (can_consume_code_cache) {
+  CompilationCache* compilation_cache = isolate->compilation_cache();
+  // First check per-isolate compilation cache.
+  CompilationCacheScript::LookupResult lookup_result =
+      compilation_cache->LookupScript(source, script_details, language_mode);
+  maybe_result = lookup_result.toplevel_sfi();
+  if (maybe_result.ToHandle(&result)) {
+    is_compiled_scope = result->is_compiled_scope(isolate);
+    compile_timer.set_hit_isolate_cache();
+  } else if (can_consume_code_cache) {
     compile_timer.set_consuming_code_cache();
     // Then check cached code provided by embedder.
     NestedTimedHistogramScope timer(isolate->counters()->compile_deserialize());
@@ -3820,16 +3830,22 @@ MaybeHandle<JSFunction> Compiler::GetWrappedFunction(
                  "V8.CompileDeserialize");
     maybe_result = CodeSerializer::Deserialize(isolate, cached_data, source,
                                                script_details.origin_options);
-    if (maybe_result.is_null()) {
+    bool consuming_code_cache_succeeded = false;
+    if (maybe_result.ToHandle(&result)) {
+      is_compiled_scope = result->is_compiled_scope(isolate);
+      if (is_compiled_scope.is_compiled()) {
+        consuming_code_cache_succeeded = true;
+        // Promote to per-isolate compilation cache.
+        compilation_cache->PutScript(source, language_mode, result);
+      }
+    }
+    if (!consuming_code_cache_succeeded) {
       // Deserializer failed. Fall through to compile.
       compile_timer.set_consuming_code_cache_failed();
     }
   }
 
-  Handle<SharedFunctionInfo> wrapped;
-  Handle<Script> script;
-  IsCompiledScope is_compiled_scope;
-  if (!maybe_result.ToHandle(&wrapped)) {
+  if (maybe_result.is_null()) {
     UnoptimizedCompileFlags flags = UnoptimizedCompileFlags::ForToplevelCompile(
         isolate, true, language_mode, script_details.repl_mode,
         ScriptType::kClassic, v8_flags.lazy);
@@ -3849,9 +3865,8 @@ MaybeHandle<JSFunction> Compiler::GetWrappedFunction(
     if (!IsNativeContext(*context)) {
       maybe_outer_scope_info = handle(context->scope_info(), isolate);
     }
-
     script = NewScript(isolate, &parse_info, source, script_details,
-                       NOT_NATIVES_CODE, arguments);
+                       NOT_NATIVES_CODE);
 
     Handle<SharedFunctionInfo> top_level;
     maybe_result = v8::internal::CompileToplevel(&parse_info, script,
@@ -3864,18 +3879,23 @@ MaybeHandle<JSFunction> Compiler::GetWrappedFunction(
     for (Tagged<SharedFunctionInfo> info = infos.Next(); !info.is_null();
          info = infos.Next()) {
       if (info->is_wrapped()) {
-        wrapped = Handle<SharedFunctionInfo>(info, isolate);
+        result = Handle<SharedFunctionInfo>(info, isolate);
         break;
       }
     }
-    DCHECK(!wrapped.is_null());
-  } else {
-    is_compiled_scope = wrapped->is_compiled_scope(isolate);
-    script = Handle<Script>(Script::cast(wrapped->script()), isolate);
+    DCHECK(!result.is_null());
+
+    is_compiled_scope = result->is_compiled_scope(isolate);
+    script = Handle<Script>(Script::cast(result->script()), isolate);
+    // Add the result to the isolate cache if there's no context extension.
+    if (maybe_outer_scope_info.is_null()) {
+      compilation_cache->PutScript(source, language_mode, result);
+    }
   }
+
   DCHECK(is_compiled_scope.is_compiled());
 
-  return Factory::JSFunctionBuilder{isolate, wrapped, context}
+  return Factory::JSFunctionBuilder{isolate, result, context}
       .set_allocation_type(AllocationType::kYoung)
       .Build();
 }
diff --git a/src/codegen/compiler.h b/src/codegen/compiler.h
@@ -161,9 +161,8 @@ class V8_EXPORT_PRIVATE Compiler : public AllStatic {
   // Create a function that results from wrapping |source| in a function,
   // with |arguments| being a list of parameters for that function.
   V8_WARN_UNUSED_RESULT static MaybeHandle<JSFunction> GetWrappedFunction(
-      Handle<String> source, Handle<FixedArray> arguments,
-      Handle<Context> context, const ScriptDetails& script_details,
-      AlignedCachedData* cached_data,
+      Handle<String> source, Handle<Context> context,
+      const ScriptDetails& script_details, AlignedCachedData* cached_data,
       v8::ScriptCompiler::CompileOptions compile_options,
       v8::ScriptCompiler::NoCacheReason no_cache_reason);
 
diff --git a/src/codegen/script-details.h b/src/codegen/script-details.h
@@ -30,6 +30,7 @@ struct ScriptDetails {
   MaybeHandle<Object> name_obj;
   MaybeHandle<Object> source_map_url;
   MaybeHandle<Object> host_defined_options;
+  MaybeHandle<FixedArray> wrapped_arguments;
   REPLMode repl_mode;
   const ScriptOriginOptions origin_options;
 };
diff --git a/src/objects/compilation-cache-table-inl.h b/src/objects/compilation-cache-table-inl.h
@@ -68,10 +68,12 @@ class ScriptCacheKey : public HashTableKey {
   ScriptCacheKey(Handle<String> source, MaybeHandle<Object> name,
                  int line_offset, int column_offset,
                  v8::ScriptOriginOptions origin_options,
-                 MaybeHandle<Object> host_defined_options, Isolate* isolate);
+                 MaybeHandle<Object> host_defined_options,
+                 MaybeHandle<FixedArray> maybe_wrapped_arguments,
+                 Isolate* isolate);
 
   bool IsMatch(Tagged<Object> other) override;
-  bool MatchesOrigin(Tagged<Script> script);
+  bool MatchesScript(Tagged<Script> script);
 
   Handle<Object> AsHandle(Isolate* isolate, Handle<SharedFunctionInfo> shared);
 
@@ -100,6 +102,7 @@ class ScriptCacheKey : public HashTableKey {
   int column_offset_;
   v8::ScriptOriginOptions origin_options_;
   MaybeHandle<Object> host_defined_options_;
+  MaybeHandle<FixedArray> wrapped_arguments_;
   Isolate* isolate_;
 };
 
diff --git a/src/objects/compilation-cache-table.cc b/src/objects/compilation-cache-table.cc
@@ -246,7 +246,7 @@ Tagged<Smi> ScriptHash(Tagged<String> source, MaybeHandle<Object> maybe_name,
 // We only re-use a cached function for some script source code if the
 // script originates from the same place. This is to avoid issues
 // when reporting errors, etc.
-bool ScriptCacheKey::MatchesOrigin(Tagged<Script> script) {
+bool ScriptCacheKey::MatchesScript(Tagged<Script> script) {
   DisallowGarbageCollection no_gc;
 
   // If the script name isn't set, the boilerplate script should have
@@ -270,6 +270,30 @@ bool ScriptCacheKey::MatchesOrigin(Tagged<Script> script) {
     return false;
   }
 
+  Handle<FixedArray> wrapped_arguments_handle;
+  if (wrapped_arguments_.ToHandle(&wrapped_arguments_handle)) {
+    if (!script->is_wrapped()) {
+      return false;
+    }
+    Tagged<FixedArray> wrapped_arguments = *wrapped_arguments_handle;
+    Tagged<FixedArray> other_wrapped_arguments = script->wrapped_arguments();
+    int length = wrapped_arguments->length();
+    if (length != other_wrapped_arguments->length()) {
+      return false;
+    }
+    for (int i = 0; i < length; i++) {
+      Tagged<Object> arg = wrapped_arguments->get(i);
+      Tagged<Object> other_arg = other_wrapped_arguments->get(i);
+      DCHECK(IsString(arg));
+      DCHECK(IsString(other_arg));
+      if (!String::cast(arg)->Equals(String::cast(other_arg))) {
+        return false;
+      }
+    }
+  } else if (script->is_wrapped()) {
+    return false;
+  }
+
   // Don't compare host options if the script was deserialized because we didn't
   // serialize host options (see CodeSerializer::SerializeObjectImpl())
   if (script->deserialized() &&
@@ -307,12 +331,14 @@ ScriptCacheKey::ScriptCacheKey(Handle<String> source,
     : ScriptCacheKey(source, script_details->name_obj,
                      script_details->line_offset, script_details->column_offset,
                      script_details->origin_options,
-                     script_details->host_defined_options, isolate) {}
+                     script_details->host_defined_options,
+                     script_details->wrapped_arguments, isolate) {}
 
 ScriptCacheKey::ScriptCacheKey(Handle<String> source, MaybeHandle<Object> name,
                                int line_offset, int column_offset,
                                v8::ScriptOriginOptions origin_options,
                                MaybeHandle<Object> host_defined_options,
+                               MaybeHandle<FixedArray> maybe_wrapped_arguments,
                                Isolate* isolate)
     : HashTableKey(static_cast<uint32_t>(ScriptHash(*source, name, line_offset,
                                                     column_offset,
@@ -324,8 +350,19 @@ ScriptCacheKey::ScriptCacheKey(Handle<String> source, MaybeHandle<Object> name,
       column_offset_(column_offset),
       origin_options_(origin_options),
       host_defined_options_(host_defined_options),
+      wrapped_arguments_(maybe_wrapped_arguments),
       isolate_(isolate) {
   DCHECK(Smi::IsValid(static_cast<int>(Hash())));
+#ifdef DEBUG
+  Handle<FixedArray> wrapped_arguments;
+  if (maybe_wrapped_arguments.ToHandle(&wrapped_arguments)) {
+    int length = wrapped_arguments->length();
+    for (int i = 0; i < length; i++) {
+      Tagged<Object> arg = wrapped_arguments->get(i);
+      DCHECK(IsString(arg));
+    }
+  }
+#endif
 }
 
 bool ScriptCacheKey::IsMatch(Tagged<Object> other) {
@@ -347,7 +384,8 @@ bool ScriptCacheKey::IsMatch(Tagged<Object> other) {
   }
   Tagged<Script> other_script = Script::cast(other_script_object);
   Tagged<String> other_source = String::cast(other_script->source());
-  return other_source->Equals(*source_) && MatchesOrigin(other_script);
+
+  return other_source->Equals(*source_) && MatchesScript(other_script);
 }
 
 Handle<Object> ScriptCacheKey::AsHandle(Isolate* isolate,
@@ -474,6 +512,7 @@ Handle<CompilationCacheTable> CompilationCacheTable::EnsureScriptTableCapacity(
 
 Handle<CompilationCacheTable> CompilationCacheTable::PutScript(
     Handle<CompilationCacheTable> cache, Handle<String> src,
+    MaybeHandle<FixedArray> maybe_wrapped_arguments,
     Handle<SharedFunctionInfo> value, Isolate* isolate) {
   src = String::Flatten(isolate, src);
   Handle<Script> script = handle(Script::cast(value->script()), isolate);
@@ -485,7 +524,7 @@ Handle<CompilationCacheTable> CompilationCacheTable::PutScript(
                                           isolate);
   ScriptCacheKey key(src, script_name, script->line_offset(),
                      script->column_offset(), script->origin_options(),
-                     host_defined_options, isolate);
+                     host_defined_options, maybe_wrapped_arguments, isolate);
   Handle<Object> k = key.AsHandle(isolate, value);
 
   // Check whether there is already a matching entry. If so, we must overwrite
diff --git a/src/objects/compilation-cache-table.h b/src/objects/compilation-cache-table.h
@@ -121,6 +121,7 @@ class CompilationCacheTable
       const ScriptDetails& script_details, Isolate* isolate);
   static Handle<CompilationCacheTable> PutScript(
       Handle<CompilationCacheTable> cache, Handle<String> src,
+      MaybeHandle<FixedArray> maybe_wrapped_arguments,
       Handle<SharedFunctionInfo> value, Isolate* isolate);
 
   // Eval code only gets cached after a second probe for the
diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc
