Return both toplevel SFI and Script from compilation cache

This is a partial reland of https://crrev.com/c/3597106 , except for the
changes in compiler.cc, which are just the minimal possible changes to
make the code compile.

With this change, it is possible that a call to
CompilationCache::LookupScript returns any of:
1. A Script and a toplevel SharedFunctionInfo (cache hit)
2. A Script but no toplevel SharedFunctionInfo (partial cache hit)
3. Nothing (cache miss)

Bug: v8:12808
Change-Id: Id33a4cd0cb28562d6b862fbb113ea9d03f255b2b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3687425Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#81193}

src/codegen/compilation-cache.cc

@@ -149,30 +149,35 @@ void CompilationCacheEvalOrScript::Remove(
   CompilationCacheTable::cast(table_).Remove(*function_info);
 }
 
-MaybeHandle<SharedFunctionInfo> CompilationCacheScript::Lookup(
+CompilationCacheScript::LookupResult CompilationCacheScript::Lookup(
     Handle<String> source, const ScriptDetails& script_details) {
-  MaybeHandle<SharedFunctionInfo> result;
+  LookupResult result;
+  LookupResult::RawObjects raw_result_for_escaping_handle_scope;
 
   // Probe the script table. Make sure not to leak handles
   // into the caller's handle scope.
   {
     HandleScope scope(isolate());
     Handle<CompilationCacheTable> table = GetTable();
-    MaybeHandle<SharedFunctionInfo> probe = CompilationCacheTable::LookupScript(
+    LookupResult probe = CompilationCacheTable::LookupScript(
         table, source, script_details, isolate());
-    Handle<SharedFunctionInfo> function_info;
-    if (probe.ToHandle(&function_info)) {
-      result = scope.CloseAndEscape(function_info);
-    }
+    raw_result_for_escaping_handle_scope = probe.GetRawObjects();
   }
+  result = LookupResult::FromRawObjects(raw_result_for_escaping_handle_scope,
+                                        isolate());
 
   // Once outside the manacles of the handle scope, we need to recheck
   // to see if we actually found a cached script. If so, we return a
   // handle created in the caller's handle scope.
-  Handle<SharedFunctionInfo> function_info;
-  if (result.ToHandle(&function_info)) {
-    isolate()->counters()->compilation_cache_hits()->Increment();
-    LOG(isolate(), CompilationCacheEvent("hit", "script", *function_info));
+  Handle<Script> script;
+  if (result.script().ToHandle(&script)) {
+    Handle<SharedFunctionInfo> sfi;
+    if (result.toplevel_sfi().ToHandle(&sfi)) {
+      isolate()->counters()->compilation_cache_hits()->Increment();
+      LOG(isolate(), CompilationCacheEvent("hit", "script", *sfi));
+    } else {
+      isolate()->counters()->compilation_cache_partial_hits()->Increment();
+    }
   } else {
     isolate()->counters()->compilation_cache_misses()->Increment();
   }
@@ -263,10 +268,10 @@ void CompilationCache::Remove(Handle<SharedFunctionInfo> function_info) {
   script_.Remove(function_info);
 }
 
-MaybeHandle<SharedFunctionInfo> CompilationCache::LookupScript(
+CompilationCacheScript::LookupResult CompilationCache::LookupScript(
     Handle<String> source, const ScriptDetails& script_details,
     LanguageMode language_mode) {
-  if (!IsEnabledScript(language_mode)) return MaybeHandle<SharedFunctionInfo>();
+  if (!IsEnabledScript(language_mode)) return {};
   return script_.Lookup(source, script_details);
 }
 

src/codegen/compilation-cache.h

@@ -54,8 +54,9 @@ class CompilationCacheScript : public CompilationCacheEvalOrScript {
   explicit CompilationCacheScript(Isolate* isolate)
       : CompilationCacheEvalOrScript(isolate) {}
 
-  MaybeHandle<SharedFunctionInfo> Lookup(Handle<String> source,
-                                         const ScriptDetails& script_details);
+  using LookupResult = CompilationCacheScriptLookupResult;
+  LookupResult Lookup(Handle<String> source,
+                      const ScriptDetails& script_details);
 
   void Put(Handle<String> source, Handle<SharedFunctionInfo> function_info);
 
@@ -143,10 +144,10 @@ class V8_EXPORT_PRIVATE CompilationCache {
   CompilationCache(const CompilationCache&) = delete;
   CompilationCache& operator=(const CompilationCache&) = delete;
 
-  // Finds the script shared function info for a source
-  // string. Returns an empty handle if the cache doesn't contain a
-  // script for the given source string with the right origin.
-  MaybeHandle<SharedFunctionInfo> LookupScript(
+  // Finds the Script and root SharedFunctionInfo for a script source string.
+  // Returns empty handles if the cache doesn't contain a script for the given
+  // source string with the right origin.
+  CompilationCacheScript::LookupResult LookupScript(
       Handle<String> source, const ScriptDetails& script_details,
       LanguageMode language_mode);
 

src/codegen/compiler.cc

@@ -3017,7 +3017,8 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
 
     // First check per-isolate compilation cache.
     maybe_result =
-        compilation_cache->LookupScript(source, script_details, language_mode);
+        compilation_cache->LookupScript(source, script_details, language_mode)
+            .toplevel_sfi();
     if (!maybe_result.is_null()) {
       compile_timer.set_hit_isolate_cache();
     } else if (can_consume_code_cache) {
@@ -3249,8 +3250,10 @@ Compiler::GetSharedFunctionInfoForStreamedScript(
   {
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
                  "V8.StreamingFinalization.CheckCache");
-    maybe_result = compilation_cache->LookupScript(
-        source, script_details, task->flags().outer_language_mode());
+    maybe_result = compilation_cache
+                       ->LookupScript(source, script_details,
+                                      task->flags().outer_language_mode())
+                       .toplevel_sfi();
     if (!maybe_result.is_null()) {
       compile_timer.set_hit_isolate_cache();
     }

src/logging/counters-definitions.h

@@ -278,13 +278,16 @@ namespace internal {
 // lines) rather than one macro (of length about 80 lines) to work around
 // this problem.  Please avoid using recursive macros of this length when
 // possible.
-#define STATS_COUNTER_LIST_1(SC)                          \
-  /* Global Handle Count*/                                \
-  SC(global_handles, V8.GlobalHandles)                    \
-  SC(alive_after_last_gc, V8.AliveAfterLastGC)            \
-  SC(compilation_cache_hits, V8.CompilationCacheHits)     \
-  SC(compilation_cache_misses, V8.CompilationCacheMisses) \
-  SC(objs_since_last_young, V8.ObjsSinceLastYoung)        \
+#define STATS_COUNTER_LIST_1(SC)                                     \
+  /* Global Handle Count*/                                           \
+  SC(global_handles, V8.GlobalHandles)                               \
+  SC(alive_after_last_gc, V8.AliveAfterLastGC)                       \
+  SC(compilation_cache_hits, V8.CompilationCacheHits)                \
+  SC(compilation_cache_misses, V8.CompilationCacheMisses)            \
+  /* Number of times the cache contained a reusable Script but not   \
+     the root SharedFunctionInfo */                                  \
+  SC(compilation_cache_partial_hits, V8.CompilationCachePartialHits) \
+  SC(objs_since_last_young, V8.ObjsSinceLastYoung)                   \
   SC(objs_since_last_full, V8.ObjsSinceLastFull)
 
 #define STATS_COUNTER_LIST_2(SC)                                               \

src/objects/compilation-cache-table.cc

@@ -351,19 +351,58 @@ Handle<Object> ScriptCacheKey::AsHandle(Isolate* isolate,
   return array;
 }
 
-MaybeHandle<SharedFunctionInfo> CompilationCacheTable::LookupScript(
+CompilationCacheScriptLookupResult::RawObjects
+CompilationCacheScriptLookupResult::GetRawObjects() const {
+  RawObjects result;
+  if (Handle<Script> script; script_.ToHandle(&script)) {
+    result.first = *script;
+  }
+  if (Handle<SharedFunctionInfo> toplevel_sfi;
+      toplevel_sfi_.ToHandle(&toplevel_sfi)) {
+    result.second = *toplevel_sfi;
+  }
+  return result;
+}
+
+CompilationCacheScriptLookupResult
+CompilationCacheScriptLookupResult::FromRawObjects(
+    CompilationCacheScriptLookupResult::RawObjects raw, Isolate* isolate) {
+  CompilationCacheScriptLookupResult result;
+  if (!raw.first.is_null()) {
+    result.script_ = handle(raw.first, isolate);
+  }
+  if (!raw.second.is_null()) {
+    result.is_compiled_scope_ = raw.second.is_compiled_scope(isolate);
+    if (result.is_compiled_scope_.is_compiled()) {
+      result.toplevel_sfi_ = handle(raw.second, isolate);
+    }
+  }
+  return result;
+}
+
+CompilationCacheScriptLookupResult CompilationCacheTable::LookupScript(
     Handle<CompilationCacheTable> table, Handle<String> src,
     const ScriptDetails& script_details, Isolate* isolate) {
   src = String::Flatten(isolate, src);
   ScriptCacheKey key(src, &script_details, isolate);
   InternalIndex entry = table->FindEntry(isolate, &key);
-  if (entry.is_not_found()) return MaybeHandle<SharedFunctionInfo>();
-  DCHECK(table->KeyAt(entry).IsWeakFixedArray());
+  if (entry.is_not_found()) return {};
+
+  DisallowGarbageCollection no_gc;
+  Object key_in_table = table->KeyAt(entry);
+  Script script = Script::cast(WeakFixedArray::cast(key_in_table)
+                                   .Get(ScriptCacheKey::kWeakScript)
+                                   .GetHeapObjectAssumeWeak());
+
   Object obj = table->PrimaryValueAt(entry);
-  if (obj.IsSharedFunctionInfo()) {
-    return handle(SharedFunctionInfo::cast(obj), isolate);
+  SharedFunctionInfo toplevel_sfi;
+  if (!obj.IsUndefined(isolate)) {
+    toplevel_sfi = SharedFunctionInfo::cast(obj);
+    DCHECK_EQ(toplevel_sfi.script(), script);
   }
-  return MaybeHandle<SharedFunctionInfo>();
+
+  return CompilationCacheScriptLookupResult::FromRawObjects(
+      std::make_pair(script, toplevel_sfi), isolate);
 }
 
 InfoCellPair CompilationCacheTable::LookupEval(

src/objects/compilation-cache-table.h

@@ -76,6 +76,34 @@ class InfoCellPair {
   FeedbackCell feedback_cell_;
 };
 
+// A lookup result from the compilation cache for scripts. There are three
+// possible states:
+//
+// 1. Cache miss: script and toplevel_sfi are both null.
+// 2. Cache hit: script and toplevel_sfi are both non-null. toplevel_sfi is
+//    guaranteed to be compiled, and to stay compiled while this lookup result
+//    instance is alive.
+// 3. Partial cache hit: script is non-null, but toplevel_sfi is null. The
+//    script may contain an uncompiled toplevel SharedFunctionInfo.
+class CompilationCacheScriptLookupResult {
+ public:
+  MaybeHandle<Script> script() const { return script_; }
+  MaybeHandle<SharedFunctionInfo> toplevel_sfi() const { return toplevel_sfi_; }
+  IsCompiledScope is_compiled_scope() const { return is_compiled_scope_; }
+
+  using RawObjects = std::pair<Script, SharedFunctionInfo>;
+
+  RawObjects GetRawObjects() const;
+
+  static CompilationCacheScriptLookupResult FromRawObjects(RawObjects raw,
+                                                           Isolate* isolate);
+
+ private:
+  MaybeHandle<Script> script_;
+  MaybeHandle<SharedFunctionInfo> toplevel_sfi_;
+  IsCompiledScope is_compiled_scope_;
+};
+
 EXTERN_DECLARE_HASH_TABLE(CompilationCacheTable, CompilationCacheShape)
 
 class CompilationCacheTable
@@ -83,8 +111,10 @@ class CompilationCacheTable
  public:
   NEVER_READ_ONLY_SPACE
 
-  // The 'script' cache contains SharedFunctionInfos.
-  static MaybeHandle<SharedFunctionInfo> LookupScript(
+  // The 'script' cache contains SharedFunctionInfos. Once a root
+  // SharedFunctionInfo has become old enough that its bytecode is flushed, the
+  // entry is still present and can be used to get the Script.
+  static CompilationCacheScriptLookupResult LookupScript(
       Handle<CompilationCacheTable> table, Handle<String> src,
       const ScriptDetails& script_details, Isolate* isolate);
   static Handle<CompilationCacheTable> PutScript(

test/cctest/heap/test-heap.cc

@@ -1474,7 +1474,7 @@ TEST(TestUseOfIncrementalBarrierOnCompileLazy) {
   CHECK(g_function->is_compiled());
 }
 
-TEST(CompilationCacheCachingBehavior) {
+void CompilationCacheCachingBehavior(bool retain_script) {
   // If we do not have the compilation cache turned off, this test is invalid.
   if (!FLAG_compilation_cache) {
     return;
@@ -1483,16 +1483,20 @@ TEST(CompilationCacheCachingBehavior) {
   Isolate* isolate = CcTest::i_isolate();
   Factory* factory = isolate->factory();
   CompilationCache* compilation_cache = isolate->compilation_cache();
-  LanguageMode language_mode = construct_language_mode(FLAG_use_strict);
+  LanguageMode language_mode = LanguageMode::kSloppy;
 
   v8::HandleScope outer_scope(CcTest::isolate());
-  const char* raw_source =
-      "function foo() {"
-      "  var x = 42;"
-      "  var y = 42;"
-      "  var z = x + y;"
-      "};"
-      "foo();";
+  const char* raw_source = retain_script ? "function foo() {"
+                                           "  var x = 42;"
+                                           "  var y = 42;"
+                                           "  var z = x + y;"
+                                           "};"
+                                           "foo();"
+                                         : "(function foo() {"
+                                           "  var x = 42;"
+                                           "  var y = 42;"
+                                           "  var z = x + y;"
+                                           "})();";
   Handle<String> source = factory->InternalizeUtf8String(raw_source);
 
   {
@@ -1505,9 +1509,9 @@ TEST(CompilationCacheCachingBehavior) {
     v8::HandleScope scope(CcTest::isolate());
     ScriptDetails script_details(Handle<Object>(),
                                  v8::ScriptOriginOptions(true, false));
-    MaybeHandle<SharedFunctionInfo> cached_script =
+    auto lookup_result =
         compilation_cache->LookupScript(source, script_details, language_mode);
-    CHECK(!cached_script.is_null());
+    CHECK(!lookup_result.toplevel_sfi().is_null());
   }
 
   // Check that the code cache entry survives at least one GC.
@@ -1516,12 +1520,13 @@ TEST(CompilationCacheCachingBehavior) {
     v8::HandleScope scope(CcTest::isolate());
     ScriptDetails script_details(Handle<Object>(),
                                  v8::ScriptOriginOptions(true, false));
-    MaybeHandle<SharedFunctionInfo> cached_script =
+    auto lookup_result =
         compilation_cache->LookupScript(source, script_details, language_mode);
-    CHECK(!cached_script.is_null());
+    CHECK(!lookup_result.toplevel_sfi().is_null());
 
     // Progress code age until it's old and ready for GC.
-    Handle<SharedFunctionInfo> shared = cached_script.ToHandleChecked();
+    Handle<SharedFunctionInfo> shared =
+        lookup_result.toplevel_sfi().ToHandleChecked();
     CHECK(shared->HasBytecodeArray());
     const int kAgingThreshold = 6;
     for (int i = 0; i < kAgingThreshold; i++) {
@@ -1536,12 +1541,21 @@ TEST(CompilationCacheCachingBehavior) {
     // Ensure code aging cleared the entry from the cache.
     ScriptDetails script_details(Handle<Object>(),
                                  v8::ScriptOriginOptions(true, false));
-    MaybeHandle<SharedFunctionInfo> cached_script =
+    auto lookup_result =
         compilation_cache->LookupScript(source, script_details, language_mode);
-    CHECK(cached_script.is_null());
+    CHECK(lookup_result.toplevel_sfi().is_null());
+    CHECK_EQ(retain_script, !lookup_result.script().is_null());
   }
 }
 
+TEST(CompilationCacheCachingBehaviorDiscardScript) {
+  CompilationCacheCachingBehavior(false);
+}
+
+TEST(CompilationCacheCachingBehaviorRetainScript) {
+  CompilationCacheCachingBehavior(true);
+}
+
 static void OptimizeEmptyFunction(const char* name) {
   HandleScope scope(CcTest::i_isolate());
   base::EmbeddedVector<char, 256> source;

test/cctest/test-serialize.cc

@@ -1822,10 +1822,9 @@ TEST(CodeSerializerPromotedToCompilationCache) {
     ScriptDetails script_details(src);
     script_details.host_defined_options =
         default_script_details.host_defined_options;
-    MaybeHandle<SharedFunctionInfo> shared =
-        isolate->compilation_cache()->LookupScript(src, script_details,
-                                                   LanguageMode::kSloppy);
-    CHECK_EQ(*shared.ToHandleChecked(), *copy);
+    auto lookup_result = isolate->compilation_cache()->LookupScript(
+        src, script_details, LanguageMode::kSloppy);
+    CHECK_EQ(*lookup_result.toplevel_sfi().ToHandleChecked(), *copy);
   }
 
   {
@@ -1839,10 +1838,9 @@ TEST(CodeSerializerPromotedToCompilationCache) {
             default_host_defined_option_1_string);
     host_defined_options->set(1, *host_defined_option_1);
     script_details.host_defined_options = host_defined_options;
-    MaybeHandle<SharedFunctionInfo> shared =
-        isolate->compilation_cache()->LookupScript(src, script_details,
-                                                   LanguageMode::kSloppy);
-    CHECK_EQ(*shared.ToHandleChecked(), *copy);
+    auto lookup_result = isolate->compilation_cache()->LookupScript(
+        src, script_details, LanguageMode::kSloppy);
+    CHECK_EQ(*lookup_result.toplevel_sfi().ToHandleChecked(), *copy);
   }
 
   {
@@ -1851,49 +1849,48 @@ TEST(CodeSerializerPromotedToCompilationCache) {
         isolate->factory()->NewStringFromAsciiChecked(source));
     script_details.host_defined_options =
         default_script_details.host_defined_options;
-    MaybeHandle<SharedFunctionInfo> shared =
-        isolate->compilation_cache()->LookupScript(src, script_details,
-                                                   LanguageMode::kSloppy);
-    CHECK_EQ(*shared.ToHandleChecked(), *copy);
+    auto lookup_result = isolate->compilation_cache()->LookupScript(
+        src, script_details, LanguageMode::kSloppy);
+    CHECK_EQ(*lookup_result.toplevel_sfi().ToHandleChecked(), *copy);
   }
 
   {
     // Lookup with different name string should fail:
     ScriptDetails script_details(
         isolate->factory()->NewStringFromAsciiChecked("other"));
-    MaybeHandle<SharedFunctionInfo> shared =
-        isolate->compilation_cache()->LookupScript(src, script_details,
-                                                   LanguageMode::kSloppy);
-    CHECK(shared.is_null());
+    auto lookup_result = isolate->compilation_cache()->LookupScript(
+        src, script_details, LanguageMode::kSloppy);
+    CHECK(lookup_result.script().is_null() &&
+          lookup_result.toplevel_sfi().is_null());
   }
 
   {
     // Lookup with different position should fail:
     ScriptDetails script_details(src);
     script_details.line_offset = 0xFF;
-    MaybeHandle<SharedFunctionInfo> shared =
-        isolate->compilation_cache()->LookupScript(src, script_details,
-                                                   LanguageMode::kSloppy);
-    CHECK(shared.is_null());
+    auto lookup_result = isolate->compilation_cache()->LookupScript(
+        src, script_details, LanguageMode::kSloppy);
+    CHECK(lookup_result.script().is_null() &&
+          lookup_result.toplevel_sfi().is_null());
   }
 
   {
     // Lookup with different position should fail:
     ScriptDetails script_details(src);
     script_details.column_offset = 0xFF;
-    MaybeHandle<SharedFunctionInfo> shared =
-        isolate->compilation_cache()->LookupScript(src, script_details,
-                                                   LanguageMode::kSloppy);
-    CHECK(shared.is_null());
+    auto lookup_result = isolate->compilation_cache()->LookupScript(
+        src, script_details, LanguageMode::kSloppy);
+    CHECK(lookup_result.script().is_null() &&
+          lookup_result.toplevel_sfi().is_null());
   }
 
   {
     // Lookup with different language mode should fail:
     ScriptDetails script_details(src);
-    MaybeHandle<SharedFunctionInfo> shared =
-        isolate->compilation_cache()->LookupScript(src, script_details,
-                                                   LanguageMode::kStrict);
-    CHECK(shared.is_null());
+    auto lookup_result = isolate->compilation_cache()->LookupScript(
+        src, script_details, LanguageMode::kStrict);
+    CHECK(lookup_result.script().is_null() &&
+          lookup_result.toplevel_sfi().is_null());
   }
 
   {
@@ -1901,20 +1898,20 @@ TEST(CodeSerializerPromotedToCompilationCache) {
     ScriptOriginOptions origin_options(false, true);
     CHECK_NE(ScriptOriginOptions().Flags(), origin_options.Flags());
     ScriptDetails script_details(src, origin_options);
-    MaybeHandle<SharedFunctionInfo> shared =
-        isolate->compilation_cache()->LookupScript(src, script_details,
-                                                   LanguageMode::kSloppy);
-    CHECK(shared.is_null());
+    auto lookup_result = isolate->compilation_cache()->LookupScript(
+        src, script_details, LanguageMode::kSloppy);
+    CHECK(lookup_result.script().is_null() &&
+          lookup_result.toplevel_sfi().is_null());
   }
 
   {
     // Lookup with different host_defined_options should fail:
     ScriptDetails script_details(src);
     script_details.host_defined_options = isolate->factory()->NewFixedArray(5);
-    MaybeHandle<SharedFunctionInfo> shared =
-        isolate->compilation_cache()->LookupScript(src, script_details,
-                                                   LanguageMode::kSloppy);
-    CHECK(shared.is_null());
+    auto lookup_result = isolate->compilation_cache()->LookupScript(
+        src, script_details, LanguageMode::kSloppy);
+    CHECK(lookup_result.script().is_null() &&
+          lookup_result.toplevel_sfi().is_null());
   }
 
   // Compile the script again with different options.
@@ -1932,19 +1929,18 @@ TEST(CodeSerializerPromotedToCompilationCache) {
     ScriptDetails script_details(src);
     script_details.host_defined_options =
         default_script_details.host_defined_options;
-    MaybeHandle<SharedFunctionInfo> shared =
-        isolate->compilation_cache()->LookupScript(src, script_details,
-                                                   LanguageMode::kSloppy);
-    CHECK_EQ(*shared.ToHandleChecked(), *copy);
+    auto lookup_result = isolate->compilation_cache()->LookupScript(
+        src, script_details, LanguageMode::kSloppy);
+    CHECK_EQ(*lookup_result.toplevel_sfi().ToHandleChecked(), *copy);
   }
 
   {
     // The new script can also be found.
     ScriptDetails script_details(src);
-    MaybeHandle<SharedFunctionInfo> shared =
-        isolate->compilation_cache()->LookupScript(src, script_details,
-                                                   LanguageMode::kSloppy);
-    CHECK_EQ(*shared.ToHandleChecked(), *alternative_toplevel_sfi);
+    auto lookup_result = isolate->compilation_cache()->LookupScript(
+        src, script_details, LanguageMode::kSloppy);
+    CHECK_EQ(*lookup_result.toplevel_sfi().ToHandleChecked(),
+             *alternative_toplevel_sfi);
   }
 
   delete cache;