Change key format for script cache

This is a partial reland of https://crrev.com/c/3597106 including fixes
from https://crrev.com/c/3654413

Before this change, a script cache key is the same format as an eval
cache key, which is a FixedArray containing:
- The SharedFunctionInfo of the containing function
- The source text
- The language mode in which the code was parsed
- The position in the source where eval was called

After this change, a script cache key is a WeakFixedArray containing:
- A weak pointer to the Script
- The hash value of the source text

This sets up for a subsequent change which can cause these keys to
outlive their corresponding values (top-level SharedFunctionInfos)
without leaking any memory beyond the key itself.

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

src/codegen/compilation-cache.cc

@@ -71,7 +71,7 @@ void CompilationCacheScript::Age() {
   for (InternalIndex entry : table.IterateEntries()) {
     Object key;
     if (!table.ToKey(isolate(), entry, &key)) continue;
-    DCHECK(key.IsFixedArray());
+    DCHECK(key.IsWeakFixedArray());
 
     Object value = table.PrimaryValueAt(entry);
     if (!value.IsUndefined(isolate())) {
@@ -208,8 +208,7 @@ bool HasOrigin(Isolate* isolate, Handle<SharedFunctionInfo> function_info,
 // will be cached, but subsequent code from different source / line
 // won't.
 MaybeHandle<SharedFunctionInfo> CompilationCacheScript::Lookup(
-    Handle<String> source, const ScriptDetails& script_details,
-    LanguageMode language_mode) {
+    Handle<String> source, const ScriptDetails& script_details) {
   MaybeHandle<SharedFunctionInfo> result;
 
   // Probe the script generation tables. Make sure not to leak handles
@@ -217,8 +216,8 @@ MaybeHandle<SharedFunctionInfo> CompilationCacheScript::Lookup(
   {
     HandleScope scope(isolate());
     Handle<CompilationCacheTable> table = GetTable();
-    MaybeHandle<SharedFunctionInfo> probe = CompilationCacheTable::LookupScript(
-        table, source, language_mode, isolate());
+    MaybeHandle<SharedFunctionInfo> probe =
+        CompilationCacheTable::LookupScript(table, source, isolate());
     Handle<SharedFunctionInfo> function_info;
     if (probe.ToHandle(&function_info)) {
       // Break when we've found a suitable shared function info that
@@ -246,12 +245,11 @@ MaybeHandle<SharedFunctionInfo> CompilationCacheScript::Lookup(
 }
 
 void CompilationCacheScript::Put(Handle<String> source,
-                                 LanguageMode language_mode,
                                  Handle<SharedFunctionInfo> function_info) {
   HandleScope scope(isolate());
   Handle<CompilationCacheTable> table = GetTable();
-  table_ = *CompilationCacheTable::PutScript(table, source, language_mode,
-                                             function_info, isolate());
+  table_ = *CompilationCacheTable::PutScript(table, source, function_info,
+                                             isolate());
 }
 
 InfoCellPair CompilationCacheEval::Lookup(Handle<String> source,
@@ -333,8 +331,8 @@ void CompilationCache::Remove(Handle<SharedFunctionInfo> function_info) {
 MaybeHandle<SharedFunctionInfo> CompilationCache::LookupScript(
     Handle<String> source, const ScriptDetails& script_details,
     LanguageMode language_mode) {
-  if (!IsEnabledScriptAndEval()) return MaybeHandle<SharedFunctionInfo>();
-  return script_.Lookup(source, script_details, language_mode);
+  if (!IsEnabledScript(language_mode)) return MaybeHandle<SharedFunctionInfo>();
+  return script_.Lookup(source, script_details);
 }
 
 InfoCellPair CompilationCache::LookupEval(Handle<String> source,
@@ -375,10 +373,10 @@ MaybeHandle<FixedArray> CompilationCache::LookupRegExp(Handle<String> source,
 void CompilationCache::PutScript(Handle<String> source,
                                  LanguageMode language_mode,
                                  Handle<SharedFunctionInfo> function_info) {
-  if (!IsEnabledScriptAndEval()) return;
+  if (!IsEnabledScript(language_mode)) return;
   LOG(isolate(), CompilationCacheEvent("put", "script", *function_info));
 
-  script_.Put(source, language_mode, function_info);
+  script_.Put(source, function_info);
 }
 
 void CompilationCache::PutEval(Handle<String> source,

src/codegen/compilation-cache.h

@@ -55,11 +55,9 @@ class CompilationCacheScript : public CompilationCacheEvalOrScript {
       : CompilationCacheEvalOrScript(isolate) {}
 
   MaybeHandle<SharedFunctionInfo> Lookup(Handle<String> source,
-                                         const ScriptDetails& script_details,
-                                         LanguageMode language_mode);
+                                         const ScriptDetails& script_details);
 
-  void Put(Handle<String> source, LanguageMode language_mode,
-           Handle<SharedFunctionInfo> function_info);
+  void Put(Handle<String> source, Handle<SharedFunctionInfo> function_info);
 
   void Age();
 
@@ -215,6 +213,11 @@ class V8_EXPORT_PRIVATE CompilationCache {
   bool IsEnabledScriptAndEval() const {
     return FLAG_compilation_cache && enabled_script_and_eval_;
   }
+  bool IsEnabledScript(LanguageMode language_mode) {
+    // Tests can change FLAG_use_strict at runtime. The compilation cache only
+    // contains scripts which were compiled with the default language mode.
+    return IsEnabledScriptAndEval() && language_mode == LanguageMode::kSloppy;
+  }
 
   Isolate* isolate() const { return isolate_; }
 

src/objects/compilation-cache-table-inl.h

@@ -46,14 +46,59 @@ void CompilationCacheTable::SetEvalFeedbackValueAt(InternalIndex entry,
   set(EntryToIndex(entry) + 2, value, mode);
 }
 
+// The key in a script cache is a WeakFixedArray containing a weak pointer to
+// the Script. The corresponding value can be either the root SharedFunctionInfo
+// or undefined. The purpose of storing the root SharedFunctionInfo as the value
+// is to keep it alive, not to save a lookup on the Script. A newly added entry
+// always contains the root SharedFunctionInfo.
+//
+// TODO(v8:12808): After the root SharedFunctionInfo has aged sufficiently, it
+// should be replaced with undefined. In this way, all strong references to
+// large objects are dropped, but there is still a way to get the Script if it
+// happens to still be alive.
+class ScriptCacheKey : public HashTableKey {
+ public:
+  enum Index {
+    kHash,
+    kWeakScript,
+    kEnd,
+  };
+
+  explicit ScriptCacheKey(Handle<String> source);
+
+  bool IsMatch(Object other) override;
+
+  Handle<Object> AsHandle(Isolate* isolate, Handle<SharedFunctionInfo> shared);
+
+  static base::Optional<String> SourceFromObject(Object obj) {
+    DisallowGarbageCollection no_gc;
+    DCHECK(obj.IsWeakFixedArray());
+    WeakFixedArray array = WeakFixedArray::cast(obj);
+    DCHECK_EQ(array.length(), kEnd);
+
+    MaybeObject maybe_script = array.Get(kWeakScript);
+    if (HeapObject script; maybe_script.GetHeapObjectIfWeak(&script)) {
+      PrimitiveHeapObject source_or_undefined = Script::cast(script).source();
+      // Scripts stored in the script cache should always have a source string.
+      return String::cast(source_or_undefined);
+    }
+
+    DCHECK(maybe_script.IsCleared());
+    return {};
+  }
+
+ private:
+  Handle<String> source_;
+};
+
 uint32_t CompilationCacheShape::RegExpHash(String string, Smi flags) {
   return string.EnsureHash() + flags.value();
 }
 
-uint32_t CompilationCacheShape::StringSharedHash(String source,
-                                                 SharedFunctionInfo shared,
-                                                 LanguageMode language_mode,
-                                                 int position) {
+uint32_t CompilationCacheShape::EvalHash(String source,
+                                         SharedFunctionInfo shared,
+                                         LanguageMode language_mode,
+                                         int position) {
   uint32_t hash = source.EnsureHash();
   if (shared.HasSourceCode()) {
     // Instead of using the SharedFunctionInfo pointer in the hash
@@ -70,14 +115,6 @@ uint32_t CompilationCacheShape::StringSharedHash(String source,
   return hash;
 }
 
-uint32_t CompilationCacheShape::StringSharedHash(String source,
-                                                 LanguageMode language_mode) {
-  uint32_t hash = source.EnsureHash();
-  static_assert(LanguageModeSize == 2);
-  if (is_strict(language_mode)) hash ^= 0x8000;
-  return hash;
-}
-
 uint32_t CompilationCacheShape::HashForObject(ReadOnlyRoots roots,
                                               Object object) {
   // Eval: The key field contains the hash as a Number.
@@ -88,7 +125,24 @@ uint32_t CompilationCacheShape::HashForObject(ReadOnlyRoots roots,
     return SharedFunctionInfo::cast(object).Hash();
   }
 
-  // Script: See StringSharedKey::ToHandle for the encoding.
+  // Script.
+  if (object.IsWeakFixedArray()) {
+    uint32_t result = static_cast<uint32_t>(Smi::ToInt(
+        WeakFixedArray::cast(object).Get(ScriptCacheKey::kHash).ToSmi()));
+#ifdef DEBUG
+    base::Optional<String> script_key =
+        ScriptCacheKey::SourceFromObject(object);
+    if (script_key) {
+      uint32_t source_hash;
+      if (script_key->TryGetHash(&source_hash)) {
+        DCHECK_EQ(result, source_hash);
+      }
+    }
+#endif
+    return result;
+  }
+
+  // Eval: See EvalCacheKey::ToHandle for the encoding.
   FixedArray val = FixedArray::cast(object);
   if (val.map() == roots.fixed_cow_array_map()) {
     DCHECK_EQ(4, val.length());
@@ -97,14 +151,9 @@ uint32_t CompilationCacheShape::HashForObject(ReadOnlyRoots roots,
     DCHECK(is_valid_language_mode(language_unchecked));
     LanguageMode language_mode = static_cast<LanguageMode>(language_unchecked);
     int position = Smi::ToInt(val.get(3));
-    Object shared_or_smi = val.get(0);
-    if (shared_or_smi.IsSmi()) {
-      DCHECK_EQ(position, kNoSourcePosition);
-      return StringSharedHash(source, language_mode);
-    } else {
-      return StringSharedHash(source, SharedFunctionInfo::cast(shared_or_smi),
-                              language_mode, position);
-    }
+    Object shared = val.get(0);
+    return EvalHash(source, SharedFunctionInfo::cast(shared), language_mode,
+                    position);
   }
 
   // RegExp: The key field (and the value field) contains the

src/objects/compilation-cache-table.cc

@@ -129,8 +129,8 @@ FeedbackCell SearchLiteralsMap(CompilationCacheTable cache,
   return result;
 }
 
-// StringSharedKeys are used as keys in the eval cache.
-class StringSharedKey : public HashTableKey {
+// EvalCacheKeys are used as keys in the eval cache.
+class EvalCacheKey : public HashTableKey {
  public:
   // This tuple unambiguously identifies calls to eval() or
   // CreateDynamicFunction() (such as through the Function() constructor).
@@ -142,23 +142,15 @@ class StringSharedKey : public HashTableKey {
   // * When positive, position is the position in the source where eval is
   //   called. When negative, position is the negation of the position in the
   //   dynamic function's effective source where the ')' ends the parameters.
-  StringSharedKey(Handle<String> source, Handle<SharedFunctionInfo> shared,
-                  LanguageMode language_mode, int position)
-      : HashTableKey(CompilationCacheShape::StringSharedHash(
-            *source, *shared, language_mode, position)),
+  EvalCacheKey(Handle<String> source, Handle<SharedFunctionInfo> shared,
+               LanguageMode language_mode, int position)
+      : HashTableKey(CompilationCacheShape::EvalHash(*source, *shared,
+                                                     language_mode, position)),
         source_(source),
         shared_(shared),
         language_mode_(language_mode),
         position_(position) {}
 
-  // This tuple unambiguously identifies script compilation.
-  StringSharedKey(Handle<String> source, LanguageMode language_mode)
-      : HashTableKey(
-            CompilationCacheShape::StringSharedHash(*source, language_mode)),
-        source_(source),
-        language_mode_(language_mode),
-        position_(kNoSourcePosition) {}
-
   bool IsMatch(Object other) override {
     DisallowGarbageCollection no_gc;
     if (!other.IsFixedArray()) {
@@ -167,14 +159,8 @@ class StringSharedKey : public HashTableKey {
       return Hash() == other_hash;
     }
     FixedArray other_array = FixedArray::cast(other);
-    DCHECK(other_array.get(0).IsSharedFunctionInfo() ||
-           other_array.get(0) == Smi::zero());
-    Handle<SharedFunctionInfo> shared;
-    if (shared_.ToHandle(&shared)) {
-      if (*shared != other_array.get(0)) return false;
-    } else {
-      if (Smi::zero() != other_array.get(0)) return false;
-    }
+    DCHECK(other_array.get(0).IsSharedFunctionInfo());
+    if (*shared_ != other_array.get(0)) return false;
     int language_unchecked = Smi::ToInt(other_array.get(2));
     DCHECK(is_valid_language_mode(language_unchecked));
     LanguageMode language_mode = static_cast<LanguageMode>(language_unchecked);
@@ -187,12 +173,7 @@ class StringSharedKey : public HashTableKey {
 
   Handle<Object> AsHandle(Isolate* isolate) {
     Handle<FixedArray> array = isolate->factory()->NewFixedArray(4);
-    Handle<SharedFunctionInfo> shared;
-    if (shared_.ToHandle(&shared)) {
-      array->set(0, *shared);
-    } else {
-      array->set(0, Smi::zero());
-    }
+    array->set(0, *shared_);
     array->set(1, *source_);
     array->set(2, Smi::FromEnum(language_mode_));
     array->set(3, Smi::FromInt(position_));
@@ -202,7 +183,7 @@ class StringSharedKey : public HashTableKey {
 
  private:
   Handle<String> source_;
-  MaybeHandle<SharedFunctionInfo> shared_;
+  Handle<SharedFunctionInfo> shared_;
   LanguageMode language_mode_;
   int position_;
 };
@@ -244,14 +225,41 @@ class CodeKey : public HashTableKey {
 
 }  // namespace
 
+ScriptCacheKey::ScriptCacheKey(Handle<String> source)
+    : HashTableKey(source->EnsureHash()), source_(source) {
+  // Hash values must fit within a Smi.
+  static_assert(Name::HashBits::kSize <= kSmiValueSize);
+  DCHECK_EQ(
+      static_cast<uint32_t>(Smi::ToInt(Smi::FromInt(static_cast<int>(Hash())))),
+      Hash());
+}
+
+bool ScriptCacheKey::IsMatch(Object other) {
+  DisallowGarbageCollection no_gc;
+  base::Optional<String> other_source = SourceFromObject(other);
+  return other_source && other_source->Equals(*source_);
+}
+
+Handle<Object> ScriptCacheKey::AsHandle(Isolate* isolate,
+                                        Handle<SharedFunctionInfo> shared) {
+  Handle<WeakFixedArray> array = isolate->factory()->NewWeakFixedArray(kEnd);
+  // Any SharedFunctionInfo being stored in the script cache should have a
+  // Script.
+  DCHECK(shared->script().IsScript());
+  array->Set(kHash,
+             MaybeObject::FromObject(Smi::FromInt(static_cast<int>(Hash()))));
+  array->Set(kWeakScript,
+             MaybeObject::MakeWeak(MaybeObject::FromObject(shared->script())));
+  return array;
+}
+
 MaybeHandle<SharedFunctionInfo> CompilationCacheTable::LookupScript(
-    Handle<CompilationCacheTable> table, Handle<String> src,
-    LanguageMode language_mode, Isolate* isolate) {
+    Handle<CompilationCacheTable> table, Handle<String> src, Isolate* isolate) {
   src = String::Flatten(isolate, src);
-  StringSharedKey key(src, language_mode);
+  ScriptCacheKey key(src);
   InternalIndex entry = table->FindEntry(isolate, &key);
   if (entry.is_not_found()) return MaybeHandle<SharedFunctionInfo>();
-  DCHECK(table->KeyAt(entry).IsFixedArray());
+  DCHECK(table->KeyAt(entry).IsWeakFixedArray());
   Object obj = table->PrimaryValueAt(entry);
   if (obj.IsSharedFunctionInfo()) {
     return handle(SharedFunctionInfo::cast(obj), isolate);
@@ -267,7 +275,7 @@ InfoCellPair CompilationCacheTable::LookupEval(
   Isolate* isolate = native_context->GetIsolate();
   src = String::Flatten(isolate, src);
 
-  StringSharedKey key(src, outer_info, language_mode, position);
+  EvalCacheKey key(src, outer_info, language_mode, position);
   InternalIndex entry = table->FindEntry(isolate, &key);
   if (entry.is_not_found()) return empty_result;
 
@@ -293,11 +301,10 @@ Handle<Object> CompilationCacheTable::LookupRegExp(Handle<String> src,
 
 Handle<CompilationCacheTable> CompilationCacheTable::PutScript(
     Handle<CompilationCacheTable> cache, Handle<String> src,
-    LanguageMode language_mode, Handle<SharedFunctionInfo> value,
-    Isolate* isolate) {
+    Handle<SharedFunctionInfo> value, Isolate* isolate) {
   src = String::Flatten(isolate, src);
-  StringSharedKey key(src, language_mode);
-  Handle<Object> k = key.AsHandle(isolate);
+  ScriptCacheKey key(src);
+  Handle<Object> k = key.AsHandle(isolate, value);
   cache = EnsureCapacity(isolate, cache);
   InternalIndex entry = cache->FindInsertionEntry(isolate, key.Hash());
   cache->SetKeyAt(entry, *k);
@@ -313,7 +320,7 @@ Handle<CompilationCacheTable> CompilationCacheTable::PutEval(
     int position) {
   Isolate* isolate = native_context->GetIsolate();
   src = String::Flatten(isolate, src);
-  StringSharedKey key(src, outer_info, value->language_mode(), position);
+  EvalCacheKey key(src, outer_info, value->language_mode(), position);
 
   // This block handles 'real' insertions, i.e. the initial dummy insert
   // (below) has already happened earlier.

src/objects/compilation-cache-table.h

@@ -29,13 +29,8 @@ class CompilationCacheShape : public BaseShape<HashTableKey*> {
 
   static inline uint32_t RegExpHash(String string, Smi flags);
 
-  static inline uint32_t StringSharedHash(String source,
-                                          SharedFunctionInfo shared,
-                                          LanguageMode language_mode,
-                                          int position);
-
-  static inline uint32_t StringSharedHash(String source,
-                                          LanguageMode language_mode);
+  static inline uint32_t EvalHash(String source, SharedFunctionInfo shared,
+                                  LanguageMode language_mode, int position);
 
   static inline uint32_t HashForObject(ReadOnlyRoots roots, Object object);
 
@@ -89,11 +84,10 @@ class CompilationCacheTable
   // The 'script' cache contains SharedFunctionInfos.
   static MaybeHandle<SharedFunctionInfo> LookupScript(
       Handle<CompilationCacheTable> table, Handle<String> src,
-      LanguageMode language_mode, Isolate* isolate);
+      Isolate* isolate);
   static Handle<CompilationCacheTable> PutScript(
       Handle<CompilationCacheTable> cache, Handle<String> src,
-      LanguageMode language_mode, Handle<SharedFunctionInfo> value,
-      Isolate* isolate);
+      Handle<SharedFunctionInfo> value, Isolate* isolate);
 
   // Eval code only gets cached after a second probe for the
   // code object. To do so, on first "put" only a hash identifying the