[nci] Consume cached code

When creating a new JSFunction (either through Factory::NewFunction or
the FastNewClosure builtin), install the cached Code object if one
exists. In the former, this happens explicitly; in the former implicitly
through %CompileLazy.

Drive-by: Clean up nci tracing methods.
Drive-by: Rename maybe_has_... to may_have_cached_code.

Bug: v8:8888
Change-Id: I98d87df10df496c45749b3fd76c072c36af45b68
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2294662Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Auto-Submit: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68863}

src/codegen/compilation-cache.cc

@@ -74,7 +74,10 @@ void CompilationSubCache::AgeCustom(CompilationSubCache* c) {
 void CompilationCacheScript::Age() { AgeCustom(this); }
 void CompilationCacheEval::Age() { AgeCustom(this); }
 void CompilationCacheRegExp::Age() { AgeByGeneration(this); }
-void CompilationCacheCode::Age() { AgeByGeneration(this); }
+void CompilationCacheCode::Age() {
+  if (FLAG_trace_turbo_nci) CompilationCacheCode::TraceAgeing();
+  AgeByGeneration(this);
+}
 
 void CompilationSubCache::Iterate(RootVisitor* v) {
   v->VisitRootPointers(Root::kCompilationCache, nullptr,
@@ -293,6 +296,27 @@ void CompilationCacheCode::Put(Handle<SharedFunctionInfo> key,
   SetFirstTable(CompilationCacheTable::PutCode(isolate(), table, key, value));
 }
 
+void CompilationCacheCode::TraceAgeing() {
+  DCHECK(FLAG_trace_turbo_nci);
+  StdoutStream os;
+  os << "NCI cache ageing: Removing oldest generation" << std::endl;
+}
+
+void CompilationCacheCode::TraceInsertion(Handle<SharedFunctionInfo> key,
+                                          Handle<Code> value) {
+  DCHECK(FLAG_trace_turbo_nci);
+  StdoutStream os;
+  os << "NCI cache insertion: " << Brief(*key) << ", " << Brief(*value)
+     << std::endl;
+}
+
+void CompilationCacheCode::TraceHit(Handle<SharedFunctionInfo> key,
+                                    Handle<Code> value) {
+  DCHECK(FLAG_trace_turbo_nci);
+  StdoutStream os;
+  os << "NCI cache hit: " << Brief(*key) << ", " << Brief(*value) << std::endl;
+}
+
 void CompilationCache::Remove(Handle<SharedFunctionInfo> function_info) {
   if (!IsEnabledScriptAndEval()) return;
 

src/codegen/compilation-cache.h

@@ -169,6 +169,11 @@ class CompilationCacheCode : public CompilationSubCache {
   // generations) in the future.
   static constexpr int kGenerations = 2;
 
+  static void TraceAgeing();
+  static void TraceInsertion(Handle<SharedFunctionInfo> key,
+                             Handle<Code> value);
+  static void TraceHit(Handle<SharedFunctionInfo> key, Handle<Code> value);
+
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(CompilationCacheCode);
 };

src/codegen/compiler.cc

@@ -790,14 +790,9 @@ void InsertCodeIntoCompilationCache(Isolate* isolate,
   cache->PutCode(sfi, code);
   DCHECK(!cache->LookupCode(sfi).is_null());
 
-  sfi->set_maybe_has_cached_code(true);
+  sfi->set_may_have_cached_code(true);
 
-  if (FLAG_trace_turbo_nci) {
-    StdoutStream os;
-    os << "NCI cache insertion: " << Brief(*sfi) << ", " << Brief(*code)
-       << std::endl
-       << std::flush;
-  }
+  if (FLAG_trace_turbo_nci) CompilationCacheCode::TraceInsertion(sfi, code);
 }
 
 bool GetOptimizedCodeNow(OptimizedCompilationJob* job, Isolate* isolate) {

src/heap/factory.cc

@@ -13,6 +13,7 @@
 #include "src/base/bits.h"
 #include "src/builtins/accessors.h"
 #include "src/builtins/constants-table-builder.h"
+#include "src/codegen/compilation-cache.h"
 #include "src/codegen/compiler.h"
 #include "src/common/globals.h"
 #include "src/diagnostics/basic-block-profiler.h"
@@ -1857,10 +1858,13 @@ Handle<JSFunction> Factory::NewFunction(Handle<Map> map,
   Handle<JSFunction> function(JSFunction::cast(New(map, allocation)),
                               isolate());
 
+  Handle<Code> code;
+  bool have_cached_code = info->TryGetCachedCode(isolate()).ToHandle(&code);
+
   function->initialize_properties(isolate());
   function->initialize_elements();
   function->set_shared(*info);
-  function->set_code(info->GetCode());
+  function->set_code(have_cached_code ? *code : info->GetCode());
   function->set_context(*context);
   function->set_raw_feedback_cell(*many_closures_cell());
   int header_size;
@@ -1871,6 +1875,13 @@ Handle<JSFunction> Factory::NewFunction(Handle<Map> map,
     header_size = JSFunction::kSizeWithoutPrototype;
   }
   InitializeJSObjectBody(function, map, header_size);
+
+  if (have_cached_code) {
+    IsCompiledScope is_compiled_scope(info->is_compiled_scope(isolate()));
+    JSFunction::EnsureFeedbackVector(function, &is_compiled_scope);
+    if (FLAG_trace_turbo_nci) CompilationCacheCode::TraceHit(info, code);
+  }
+
   return function;
 }
 

src/objects/js-objects.cc

@@ -5045,13 +5045,13 @@ void JSFunction::InitializeFeedbackCell(Handle<JSFunction> function,
     return;
   }
 
-  bool needs_feedback_vector = !FLAG_lazy_feedback_allocation;
-  // We need feedback vector for certain log events, collecting type profile
-  // and more precise code coverage.
-  if (FLAG_log_function_events) needs_feedback_vector = true;
-  if (!isolate->is_best_effort_code_coverage()) needs_feedback_vector = true;
-  if (isolate->is_collecting_type_profile()) needs_feedback_vector = true;
-  if (FLAG_always_opt) needs_feedback_vector = true;
+  const bool needs_feedback_vector =
+      !FLAG_lazy_feedback_allocation || FLAG_always_opt ||
+      function->shared().may_have_cached_code() ||
+      // We also need a feedback vector for certain log events, collecting type
+      // profile and more precise code coverage.
+      FLAG_log_function_events || !isolate->is_best_effort_code_coverage() ||
+      isolate->is_collecting_type_profile();
 
   if (needs_feedback_vector) {
     EnsureFeedbackVector(function, is_compiled_scope);

src/objects/shared-function-info-inl.h

@@ -186,8 +186,8 @@ BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags2,
                     has_static_private_methods_or_accessors,
                     SharedFunctionInfo::HasStaticPrivateMethodsOrAccessorsBit)
 
-BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags2, maybe_has_cached_code,
-                    SharedFunctionInfo::MaybeHasCachedCodeBit)
+BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags2, may_have_cached_code,
+                    SharedFunctionInfo::MayHaveCachedCodeBit)
 
 BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags, syntax_kind,
                     SharedFunctionInfo::FunctionSyntaxKindBits)

src/objects/shared-function-info.cc

@@ -6,6 +6,7 @@
 
 #include "src/ast/ast.h"
 #include "src/ast/scopes.h"
+#include "src/codegen/compilation-cache.h"
 #include "src/codegen/compiler.h"
 #include "src/objects/shared-function-info-inl.h"
 #include "src/strings/string-builder-inl.h"
@@ -412,6 +413,12 @@ std::ostream& operator<<(std::ostream& os, const SourceCodeOf& v) {
   }
 }
 
+MaybeHandle<Code> SharedFunctionInfo::TryGetCachedCode(Isolate* isolate) {
+  if (!may_have_cached_code()) return {};
+  Handle<SharedFunctionInfo> zis(*this, isolate);
+  return isolate->compilation_cache()->LookupCode(zis);
+}
+
 void SharedFunctionInfo::DisableOptimization(BailoutReason reason) {
   DCHECK_NE(reason, BailoutReason::kNoReason);
 

src/objects/shared-function-info.h

@@ -412,8 +412,12 @@ class SharedFunctionInfo : public HeapObject {
 
   // True if a Code object associated with this SFI has been inserted into the
   // compilation cache. Note that the cache entry may be removed by aging,
-  // hence the 'maybe'.
-  DECL_BOOLEAN_ACCESSORS(maybe_has_cached_code)
+  // hence the 'may'.
+  DECL_BOOLEAN_ACCESSORS(may_have_cached_code)
+
+  // Returns the cached Code object for this SFI if it exists, an empty handle
+  // otherwise.
+  MaybeHandle<Code> TryGetCachedCode(Isolate* isolate);
 
   // Is this function a top-level function (scripts, evals).
   DECL_BOOLEAN_ACCESSORS(is_toplevel)

src/objects/shared-function-info.tq

@@ -44,7 +44,7 @@ bitfield struct SharedFunctionInfoFlags extends uint32 {
 bitfield struct SharedFunctionInfoFlags2 extends uint8 {
   class_scope_has_private_brand: bool: 1 bit;
   has_static_private_methods_or_accessors: bool: 1 bit;
-  maybe_has_cached_code: bool: 1 bit;
+  may_have_cached_code: bool: 1 bit;
 }
 
 extern class SharedFunctionInfo extends HeapObject {

src/runtime/runtime-compiler.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "src/asmjs/asm-js.h"
+#include "src/codegen/compilation-cache.h"
 #include "src/codegen/compiler.h"
 #include "src/common/message-template.h"
 #include "src/compiler-dispatcher/optimizing-compile-dispatcher.h"
@@ -24,8 +25,10 @@ RUNTIME_FUNCTION(Runtime_CompileLazy) {
   DCHECK_EQ(1, args.length());
   CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
 
+  Handle<SharedFunctionInfo> sfi(function->shared(), isolate);
+
 #ifdef DEBUG
-  if (FLAG_trace_lazy && !function->shared().is_compiled()) {
+  if (FLAG_trace_lazy && !sfi->is_compiled()) {
     PrintF("[unoptimized: ");
     function->PrintName();
     PrintF("]\n");
@@ -41,6 +44,15 @@ RUNTIME_FUNCTION(Runtime_CompileLazy) {
                          &is_compiled_scope)) {
     return ReadOnlyRoots(isolate).exception();
   }
+  if (sfi->may_have_cached_code()) {
+    Handle<Code> code;
+    if (sfi->TryGetCachedCode(isolate).ToHandle(&code)) {
+      function->set_code(*code);
+      JSFunction::EnsureFeedbackVector(function, &is_compiled_scope);
+      if (FLAG_trace_turbo_nci) CompilationCacheCode::TraceHit(sfi, code);
+      return *code;
+    }
+  }
   DCHECK(function->is_compiled());
   return function->code();
 }
@@ -390,5 +402,6 @@ RUNTIME_FUNCTION(Runtime_ResolvePossiblyDirectEval) {
   return CompileGlobalEval(isolate, args.at<Object>(1), outer_info,
                            language_mode, args.smi_at(4), args.smi_at(5));
 }
+
 }  // namespace internal
 }  // namespace v8

test/cctest/cctest.status

@@ -641,6 +641,7 @@
   'test-api/FastApiCalls': [SKIP],
   'test-cpu-profiler/Deopt*': [SKIP],
   'test-cpu-profiler/DetailedSourcePositionAPI_Inlining': [SKIP],
+  'test-cpu-profiler/DetailedSourcePositionAPI': [SKIP],
   'test-heap/CellsInOptimizedCodeAreWeak': [SKIP],
   'test-heap/EnsureAllocationSiteDependentCodesProcessed': [SKIP],
   'test-heap/NewSpaceObjectsInOptimizedCode': [SKIP],

test/cctest/test-compiler.cc

@@ -968,13 +968,11 @@ static int AllocationSitesCount(Heap* heap) {
 TEST(DecideToPretenureDuringCompilation) {
   // The test makes use of optimization and relies on deterministic
   // compilation.
-  if (!i::FLAG_opt || i::FLAG_always_opt ||
-      i::FLAG_stress_incremental_marking || i::FLAG_optimize_for_size
-#ifdef ENABLE_MINOR_MC
-      || i::FLAG_minor_mc
-#endif
-  )
+  if (!i::FLAG_opt || i::FLAG_always_opt || i::FLAG_minor_mc ||
+      i::FLAG_stress_incremental_marking || i::FLAG_optimize_for_size ||
+      i::FLAG_turbo_nci || i::FLAG_turbo_nci_as_highest_tier) {
     return;
+  }
 
   FLAG_stress_gc_during_compilation = true;
   FLAG_allow_natives_syntax = true;