[Compiler] Don't save FeedbackVector in CompilationCache for Scripts.

The compilation logic never used the saved FeedbackVector for Script
compiles when looking up the CompilationCache, so remove it and
simplify the return value of LookupScript to be a
MaybeHandle<SharedFunctionInfo>

Change-Id: Ib1d833f997b299e2e79621bd8509bdfd911d4e10
Reviewed-on: https://chromium-review.googlesource.com/924002
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51443}

src/compilation-cache.cc

@@ -123,11 +123,11 @@ bool CompilationCacheScript::HasOrigin(Handle<SharedFunctionInfo> function_info,
 // be cached in the same script generation. Currently the first use
 // will be cached, but subsequent code from different source / line
 // won't.
-InfoVectorPair CompilationCacheScript::Lookup(
+MaybeHandle<SharedFunctionInfo> CompilationCacheScript::Lookup(
     Handle<String> source, MaybeHandle<Object> name, int line_offset,
     int column_offset, ScriptOriginOptions resource_options,
     Handle<Context> context, LanguageMode language_mode) {
-  InfoVectorPair result;
+  MaybeHandle<SharedFunctionInfo> result;
 
   // Probe the script generation tables. Make sure not to leak handles
   // into the caller's handle scope.
@@ -135,19 +135,15 @@ InfoVectorPair CompilationCacheScript::Lookup(
     const int generation = 0;
     DCHECK_EQ(generations(), 1);
     Handle<CompilationCacheTable> table = GetTable(generation);
-    InfoVectorPair probe = table->LookupScript(source, context, language_mode);
-    if (probe.has_shared()) {
-      Handle<SharedFunctionInfo> function_info(probe.shared(), isolate());
-      Handle<Cell> vector_handle;
-      if (probe.has_vector()) {
-        vector_handle = Handle<Cell>(probe.vector(), isolate());
-      }
+    MaybeHandle<SharedFunctionInfo> probe =
+        table->LookupScript(source, context, language_mode);
+    Handle<SharedFunctionInfo> function_info;
+    if (probe.ToHandle(&function_info)) {
       // Break when we've found a suitable shared function info that
       // matches the origin.
       if (HasOrigin(function_info, name, line_offset, column_offset,
                     resource_options)) {
-        result = InfoVectorPair(*function_info,
-                                probe.has_vector() ? *vector_handle : nullptr);
+        result = scope.CloseAndEscape(function_info);
       }
     }
   }
@@ -155,19 +151,13 @@ InfoVectorPair CompilationCacheScript::Lookup(
   // 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.
-  if (result.has_shared()) {
+  Handle<SharedFunctionInfo> function_info;
+  if (result.ToHandle(&function_info)) {
 #ifdef DEBUG
     // Since HasOrigin can allocate, we need to protect the SharedFunctionInfo
-    // and the FeedbackVector with handles during the call.
-    Handle<SharedFunctionInfo> shared(result.shared(), isolate());
-    Handle<Cell> vector_handle;
-    if (result.has_vector()) {
-      vector_handle = Handle<Cell>(result.vector(), isolate());
-    }
-    DCHECK(
-        HasOrigin(shared, name, line_offset, column_offset, resource_options));
-    result =
-        InfoVectorPair(*shared, result.has_vector() ? *vector_handle : nullptr);
+    // with handles during the call.
+    DCHECK(HasOrigin(function_info, name, line_offset, column_offset,
+                     resource_options));
 #endif
     isolate()->counters()->compilation_cache_hits()->Increment();
   } else {
@@ -178,12 +168,11 @@ InfoVectorPair CompilationCacheScript::Lookup(
 
 void CompilationCacheScript::Put(Handle<String> source, Handle<Context> context,
                                  LanguageMode language_mode,
-                                 Handle<SharedFunctionInfo> function_info,
-                                 Handle<Cell> literals) {
+                                 Handle<SharedFunctionInfo> function_info) {
   HandleScope scope(isolate());
   Handle<CompilationCacheTable> table = GetFirstTable();
-  SetFirstTable(CompilationCacheTable::PutScript(
-      table, source, context, language_mode, function_info, literals));
+  SetFirstTable(CompilationCacheTable::PutScript(table, source, context,
+                                                 language_mode, function_info));
 }
 
 InfoVectorPair CompilationCacheEval::Lookup(
@@ -263,12 +252,11 @@ void CompilationCache::Remove(Handle<SharedFunctionInfo> function_info) {
   script_.Remove(function_info);
 }
 
-InfoVectorPair CompilationCache::LookupScript(
+MaybeHandle<SharedFunctionInfo> CompilationCache::LookupScript(
     Handle<String> source, MaybeHandle<Object> name, int line_offset,
     int column_offset, ScriptOriginOptions resource_options,
     Handle<Context> context, LanguageMode language_mode) {
-  InfoVectorPair empty_result;
-  if (!IsEnabled()) return empty_result;
+  if (!IsEnabled()) return MaybeHandle<SharedFunctionInfo>();
 
   return script_.Lookup(source, name, line_offset, column_offset,
                         resource_options, context, language_mode);
@@ -302,11 +290,10 @@ MaybeHandle<FixedArray> CompilationCache::LookupRegExp(Handle<String> source,
 
 void CompilationCache::PutScript(Handle<String> source, Handle<Context> context,
                                  LanguageMode language_mode,
-                                 Handle<SharedFunctionInfo> function_info,
-                                 Handle<Cell> literals) {
+                                 Handle<SharedFunctionInfo> function_info) {
   if (!IsEnabled()) return;
 
-  script_.Put(source, context, language_mode, function_info, literals);
+  script_.Put(source, context, language_mode, function_info);
 }
 
 void CompilationCache::PutEval(Handle<String> source,

src/compilation-cache.h

@@ -79,14 +79,16 @@ class CompilationCacheScript : public CompilationSubCache {
  public:
   explicit CompilationCacheScript(Isolate* isolate);
 
-  InfoVectorPair Lookup(Handle<String> source, MaybeHandle<Object> name,
-                        int line_offset, int column_offset,
-                        ScriptOriginOptions resource_options,
-                        Handle<Context> context, LanguageMode language_mode);
+  MaybeHandle<SharedFunctionInfo> Lookup(Handle<String> source,
+                                         MaybeHandle<Object> name,
+                                         int line_offset, int column_offset,
+                                         ScriptOriginOptions resource_options,
+                                         Handle<Context> context,
+                                         LanguageMode language_mode);
 
   void Put(Handle<String> source, Handle<Context> context,
-           LanguageMode language_mode, Handle<SharedFunctionInfo> function_info,
-           Handle<Cell> literals);
+           LanguageMode language_mode,
+           Handle<SharedFunctionInfo> function_info);
 
  private:
   bool HasOrigin(Handle<SharedFunctionInfo> function_info,
@@ -152,11 +154,10 @@ class CompilationCache {
   // 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.
-  InfoVectorPair LookupScript(Handle<String> source, MaybeHandle<Object> name,
-                              int line_offset, int column_offset,
-                              ScriptOriginOptions resource_options,
-                              Handle<Context> context,
-                              LanguageMode language_mode);
+  MaybeHandle<SharedFunctionInfo> LookupScript(
+      Handle<String> source, MaybeHandle<Object> name, int line_offset,
+      int column_offset, ScriptOriginOptions resource_options,
+      Handle<Context> context, LanguageMode language_mode);
 
   // Finds the shared function info for a source string for eval in a
   // given context.  Returns an empty handle if the cache doesn't
@@ -175,8 +176,7 @@ class CompilationCache {
   // info. This may overwrite an existing mapping.
   void PutScript(Handle<String> source, Handle<Context> context,
                  LanguageMode language_mode,
-                 Handle<SharedFunctionInfo> function_info,
-                 Handle<Cell> literals);
+                 Handle<SharedFunctionInfo> function_info);
 
   // Associate the (source, context->closure()->shared(), kind) triple
   // with the shared function info. This may overwrite an existing mapping.

src/compiler.cc

@@ -1658,7 +1658,6 @@ MaybeHandle<SharedFunctionInfo> Compiler::GetSharedFunctionInfoForScript(
 
   // Do a lookup in the compilation cache but not for extensions.
   MaybeHandle<SharedFunctionInfo> maybe_result;
-  Handle<Cell> vector;
   if (extension == nullptr) {
     bool can_consume_code_cache =
         compile_options == ScriptCompiler::kConsumeCodeCache &&
@@ -1668,11 +1667,13 @@ MaybeHandle<SharedFunctionInfo> Compiler::GetSharedFunctionInfoForScript(
     }
 
     // First check per-isolate compilation cache.
-    InfoVectorPair pair = compilation_cache->LookupScript(
+    maybe_result = compilation_cache->LookupScript(
         source, script_details.name_obj, script_details.line_offset,
         script_details.column_offset, origin_options, isolate->native_context(),
         language_mode);
-    if (can_consume_code_cache && !pair.has_shared()) {
+    if (!maybe_result.is_null()) {
+      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.
       HistogramTimerScope timer(isolate->counters()->compile_deserialize());
@@ -1685,11 +1686,8 @@ MaybeHandle<SharedFunctionInfo> Compiler::GetSharedFunctionInfoForScript(
               .ToHandle(&inner_result)) {
         // Promote to per-isolate compilation cache.
         DCHECK(inner_result->is_compiled());
-        Handle<FeedbackVector> feedback_vector =
-            FeedbackVector::New(isolate, inner_result);
-        vector = isolate->factory()->NewCell(feedback_vector);
         compilation_cache->PutScript(source, isolate->native_context(),
-                                     language_mode, inner_result, vector);
+                                     language_mode, inner_result);
         Handle<Script> script(Script::cast(inner_result->script()), isolate);
         isolate->debug()->OnAfterCompile(script);
         if (isolate->NeedsSourcePositionsForProfiling()) {
@@ -1699,14 +1697,6 @@ MaybeHandle<SharedFunctionInfo> Compiler::GetSharedFunctionInfoForScript(
       }
       // Deserializer failed. Fall through to compile.
       compile_timer.set_consuming_code_cache_failed();
-    } else {
-      if (pair.has_shared()) {
-        maybe_result = MaybeHandle<SharedFunctionInfo>(pair.shared(), isolate);
-        compile_timer.set_hit_isolate_cache();
-      }
-      if (pair.has_vector()) {
-        vector = Handle<Cell>(pair.vector(), isolate);
-      }
     }
   }
 
@@ -1727,13 +1717,9 @@ MaybeHandle<SharedFunctionInfo> Compiler::GetSharedFunctionInfoForScript(
     maybe_result = CompileToplevel(&parse_info, isolate);
     Handle<SharedFunctionInfo> result;
     if (extension == nullptr && maybe_result.ToHandle(&result)) {
-      // We need a feedback vector.
       DCHECK(result->is_compiled());
-      Handle<FeedbackVector> feedback_vector =
-          FeedbackVector::New(isolate, result);
-      vector = isolate->factory()->NewCell(feedback_vector);
       compilation_cache->PutScript(source, isolate->native_context(),
-                                   language_mode, result, vector);
+                                   language_mode, result);
     }
 
     if (maybe_result.is_null()) {

src/objects.cc

@@ -17520,23 +17520,19 @@ Cell* SearchLiteralsMap(CompilationCacheTable* cache, int cache_entry,
 
 }  // namespace
 
-InfoVectorPair CompilationCacheTable::LookupScript(Handle<String> src,
-                                                   Handle<Context> context,
-                                                   LanguageMode language_mode) {
-  InfoVectorPair empty_result;
+MaybeHandle<SharedFunctionInfo> CompilationCacheTable::LookupScript(
+    Handle<String> src, Handle<Context> context, LanguageMode language_mode) {
   Handle<SharedFunctionInfo> shared(context->closure()->shared());
   StringSharedKey key(src, shared, language_mode, kNoSourcePosition);
   int entry = FindEntry(&key);
-  if (entry == kNotFound) return empty_result;
+  if (entry == kNotFound) return MaybeHandle<SharedFunctionInfo>();
   int index = EntryToIndex(entry);
-  if (!get(index)->IsFixedArray()) return empty_result;
+  if (!get(index)->IsFixedArray()) return MaybeHandle<SharedFunctionInfo>();
   Object* obj = get(index + 1);
   if (obj->IsSharedFunctionInfo()) {
-    Cell* literals =
-        SearchLiteralsMap(this, index + 2, context->native_context());
-    return InfoVectorPair(SharedFunctionInfo::cast(obj), literals);
+    return handle(SharedFunctionInfo::cast(obj));
   }
-  return empty_result;
+  return MaybeHandle<SharedFunctionInfo>();
 }
 
 InfoVectorPair CompilationCacheTable::LookupEval(
@@ -17586,7 +17582,7 @@ Handle<CompilationCacheTable> CompilationCacheTable::Put(
 Handle<CompilationCacheTable> CompilationCacheTable::PutScript(
     Handle<CompilationCacheTable> cache, Handle<String> src,
     Handle<Context> context, LanguageMode language_mode,
-    Handle<SharedFunctionInfo> value, Handle<Cell> literals) {
+    Handle<SharedFunctionInfo> value) {
   Isolate* isolate = cache->GetIsolate();
   Handle<SharedFunctionInfo> shared(context->closure()->shared());
   Handle<Context> native_context(context->native_context());
@@ -17596,7 +17592,6 @@ Handle<CompilationCacheTable> CompilationCacheTable::PutScript(
   int entry = cache->FindInsertionEntry(key.Hash());
   cache->set(EntryToIndex(entry), *k);
   cache->set(EntryToIndex(entry) + 1, *value);
-  AddToLiteralsMap(cache, EntryToIndex(entry) + 2, native_context, literals);
   cache->ElementAdded();
   return cache;
 }

src/objects/compilation-cache.h

@@ -71,8 +71,9 @@ class CompilationCacheTable
   // Find cached value for a string key, otherwise return null.
   Handle<Object> Lookup(Handle<String> src, Handle<Context> context,
                         LanguageMode language_mode);
-  InfoVectorPair LookupScript(Handle<String> src, Handle<Context> context,
-                              LanguageMode language_mode);
+  MaybeHandle<SharedFunctionInfo> LookupScript(Handle<String> src,
+                                               Handle<Context> context,
+                                               LanguageMode language_mode);
   InfoVectorPair LookupEval(Handle<String> src,
                             Handle<SharedFunctionInfo> shared,
                             Handle<Context> native_context,
@@ -86,7 +87,7 @@ class CompilationCacheTable
   static Handle<CompilationCacheTable> PutScript(
       Handle<CompilationCacheTable> cache, Handle<String> src,
       Handle<Context> context, LanguageMode language_mode,
-      Handle<SharedFunctionInfo> value, Handle<Cell> literals);
+      Handle<SharedFunctionInfo> value);
   static Handle<CompilationCacheTable> PutEval(
       Handle<CompilationCacheTable> cache, Handle<String> src,
       Handle<SharedFunctionInfo> outer_info, Handle<SharedFunctionInfo> value,

test/cctest/heap/test-heap.cc

@@ -1331,35 +1331,45 @@ TEST(CompilationCacheCachingBehavior) {
   }
 
   // The script should be in the cache now.
-  InfoVectorPair pair = compilation_cache->LookupScript(
-      source, Handle<Object>(), 0, 0, v8::ScriptOriginOptions(true, false),
-      native_context, language_mode);
-  CHECK(pair.has_shared());
-
-  // Check that the code cache entry survives at least on GC.
-  // (Unless --optimize-for-size, in which case it might get collected
-  // immediately.)
-  if (!FLAG_optimize_for_size) {
-    CcTest::CollectAllGarbage();
-    pair = compilation_cache->LookupScript(source, Handle<Object>(), 0, 0,
-                                           v8::ScriptOriginOptions(true, false),
-                                           native_context, language_mode);
-    CHECK(pair.has_shared());
+  {
+    v8::HandleScope scope(CcTest::isolate());
+    MaybeHandle<SharedFunctionInfo> cached_script =
+        compilation_cache->LookupScript(source, Handle<Object>(), 0, 0,
+                                        v8::ScriptOriginOptions(true, false),
+                                        native_context, language_mode);
+    CHECK(!cached_script.is_null());
   }
 
-  // Progress code age until it's old and ready for GC.
-  const int kAgingThreshold = 6;
-  for (int i = 0; i < kAgingThreshold; i++) {
-    CHECK(pair.shared()->HasBytecodeArray());
-    pair.shared()->bytecode_array()->MakeOlder();
+  // Check that the code cache entry survives at least one GC.
+  {
+    CcTest::CollectAllGarbage();
+    v8::HandleScope scope(CcTest::isolate());
+    MaybeHandle<SharedFunctionInfo> cached_script =
+        compilation_cache->LookupScript(source, Handle<Object>(), 0, 0,
+                                        v8::ScriptOriginOptions(true, false),
+                                        native_context, language_mode);
+    CHECK(!cached_script.is_null());
+
+    // Progress code age until it's old and ready for GC.
+    Handle<SharedFunctionInfo> shared = cached_script.ToHandleChecked();
+    CHECK(shared->HasBytecodeArray());
+    const int kAgingThreshold = 6;
+    for (int i = 0; i < kAgingThreshold; i++) {
+      shared->bytecode_array()->MakeOlder();
+    }
   }
 
   CcTest::CollectAllGarbage();
-  // Ensure code aging cleared the entry from the cache.
-  pair = compilation_cache->LookupScript(source, Handle<Object>(), 0, 0,
-                                         v8::ScriptOriginOptions(true, false),
-                                         native_context, language_mode);
-  CHECK(!pair.has_shared());
+
+  {
+    v8::HandleScope scope(CcTest::isolate());
+    // Ensure code aging cleared the entry from the cache.
+    MaybeHandle<SharedFunctionInfo> cached_script =
+        compilation_cache->LookupScript(source, Handle<Object>(), 0, 0,
+                                        v8::ScriptOriginOptions(true, false),
+                                        native_context, language_mode);
+    CHECK(cached_script.is_null());
+  }
 }
 
 

test/cctest/test-serialize.cc

@@ -1322,11 +1322,12 @@ TEST(CodeSerializerPromotedToCompilationCache) {
   Handle<SharedFunctionInfo> copy = CompileScript(
       isolate, src, src, &cache, v8::ScriptCompiler::kConsumeCodeCache);
 
-  InfoVectorPair pair = isolate->compilation_cache()->LookupScript(
-      src, src, 0, 0, v8::ScriptOriginOptions(), isolate->native_context(),
-      LanguageMode::kSloppy);
+  MaybeHandle<SharedFunctionInfo> shared =
+      isolate->compilation_cache()->LookupScript(
+          src, src, 0, 0, v8::ScriptOriginOptions(), isolate->native_context(),
+          LanguageMode::kSloppy);
 
-  CHECK(pair.shared() == *copy);
+  CHECK(*shared.ToHandleChecked() == *copy);
 
   delete cache;
 }