[Compiler] Introduce IsCompiledScope which prevents flushing of compiled code

Introduces a IsCompiledScope object which can be used to check whether a
function is compiled, and ensure it remains compiled for the lifetime
of the scope without being uncompiled by bytecode flushing. The Compile
functions are modified to take a scope so that calling code can ensure
the function remains compiled for the lifetime they require.

Also, don't allocate a feedback vector for asm-wasm code as this
is never used, and will be reallocated if the asm-wasm code fails to
instantiate the module and we fallback to regular JavaScript.

Also restructure Compiler::PostInstantiation() to allocate the feedback
vector once, and do the optimized code check before optimizing for
always opt.

BUG=v8:8395

Change-Id: I3f1a71143fcae3d1a0c01eefe91ebb4b8594221a
Reviewed-on: https://chromium-review.googlesource.com/c/1352295Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57971}

src/compiler.h

@@ -21,6 +21,7 @@ namespace internal {
 // Forward declarations.
 class AstRawString;
 class BackgroundCompileTask;
+class IsCompiledScope;
 class JavaScriptFrame;
 class OptimizedCompilationInfo;
 class OptimizedCompilationJob;
@@ -57,8 +58,10 @@ class V8_EXPORT_PRIVATE Compiler : public AllStatic {
   // given function holds (except for live-edit, which compiles the world).
 
   static bool Compile(Handle<SharedFunctionInfo> shared,
-                      ClearExceptionFlag flag);
-  static bool Compile(Handle<JSFunction> function, ClearExceptionFlag flag);
+                      ClearExceptionFlag flag,
+                      IsCompiledScope* is_compiled_scope);
+  static bool Compile(Handle<JSFunction> function, ClearExceptionFlag flag,
+                      IsCompiledScope* is_compiled_scope);
   static bool CompileOptimized(Handle<JSFunction> function, ConcurrencyMode);
 
   V8_WARN_UNUSED_RESULT static MaybeHandle<SharedFunctionInfo>

src/compiler/js-inlining.cc

@@ -452,8 +452,10 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
     return NoChange();
   }
 
-  if (!shared_info->is_compiled() &&
-      !Compiler::Compile(shared_info, Compiler::CLEAR_EXCEPTION)) {
+  IsCompiledScope is_compiled_scope(shared_info->is_compiled_scope());
+  if (!is_compiled_scope.is_compiled() &&
+      !Compiler::Compile(shared_info, Compiler::CLEAR_EXCEPTION,
+                         &is_compiled_scope)) {
     TRACE("Not inlining %s into %s because bytecode generation failed\n",
           shared_info->DebugName()->ToCString().get(),
           info_->shared_info()->DebugName()->ToCString().get());

src/debug/debug.cc

@@ -1272,7 +1272,9 @@ void Debug::InstallDebugBreakTrampoline() {
   // By overwriting the function code with DebugBreakTrampoline, which tailcalls
   // to shared code, we bypass CompileLazy. Perform CompileLazy here instead.
   for (Handle<JSFunction> fun : needs_compile) {
-    Compiler::Compile(fun, Compiler::CLEAR_EXCEPTION);
+    IsCompiledScope is_compiled_scope;
+    Compiler::Compile(fun, Compiler::CLEAR_EXCEPTION, &is_compiled_scope);
+    DCHECK(is_compiled_scope.is_compiled());
     fun->set_code(*trampoline);
   }
 }
@@ -1320,6 +1322,7 @@ bool Debug::GetPossibleBreakpoints(Handle<Script> script, int start_position,
   while (true) {
     HandleScope scope(isolate_);
     std::vector<Handle<SharedFunctionInfo>> candidates;
+    std::vector<IsCompiledScope> compiled_scopes;
     SharedFunctionInfo::ScriptIterator iterator(isolate_, *script);
     for (SharedFunctionInfo info = iterator.Next(); !info.is_null();
          info = iterator.Next()) {
@@ -1336,13 +1339,17 @@ bool Debug::GetPossibleBreakpoints(Handle<Script> script, int start_position,
     for (const auto& candidate : candidates) {
       // Code that cannot be compiled lazily are internal and not debuggable.
       DCHECK(candidate->allows_lazy_compilation());
-      if (!candidate->is_compiled()) {
-        if (!Compiler::Compile(candidate, Compiler::CLEAR_EXCEPTION)) {
+      IsCompiledScope is_compiled_scope(candidate->is_compiled_scope());
+      if (!is_compiled_scope.is_compiled()) {
+        if (!Compiler::Compile(candidate, Compiler::CLEAR_EXCEPTION,
+                               &is_compiled_scope)) {
           return false;
         } else {
           was_compiled = true;
         }
       }
+      DCHECK(is_compiled_scope.is_compiled());
+      compiled_scopes.push_back(is_compiled_scope);
       if (!EnsureBreakInfo(candidate)) return false;
       PrepareFunctionForDebugExecution(candidate);
     }
@@ -1424,6 +1431,7 @@ Handle<Object> Debug::FindSharedFunctionInfoInScript(Handle<Script> script,
     // no point in looking for it by walking the heap.
 
     SharedFunctionInfo shared;
+    IsCompiledScope is_compiled_scope;
     {
       SharedFunctionInfoFinder finder(position);
       SharedFunctionInfo::ScriptIterator iterator(isolate_, *script);
@@ -1434,7 +1442,8 @@ Handle<Object> Debug::FindSharedFunctionInfoInScript(Handle<Script> script,
       shared = finder.Result();
       if (shared.is_null()) break;
       // We found it if it's already compiled.
-      if (shared->is_compiled()) {
+      is_compiled_scope = shared->is_compiled_scope();
+      if (is_compiled_scope.is_compiled()) {
         Handle<SharedFunctionInfo> shared_handle(shared, isolate_);
         // If the iteration count is larger than 1, we had to compile the outer
         // function in order to create this shared function info. So there can
@@ -1451,8 +1460,10 @@ Handle<Object> Debug::FindSharedFunctionInfoInScript(Handle<Script> script,
     HandleScope scope(isolate_);
     // Code that cannot be compiled lazily are internal and not debuggable.
     DCHECK(shared->allows_lazy_compilation());
-    if (!Compiler::Compile(handle(shared, isolate_), Compiler::CLEAR_EXCEPTION))
+    if (!Compiler::Compile(handle(shared, isolate_), Compiler::CLEAR_EXCEPTION,
+                           &is_compiled_scope)) {
       break;
+    }
   }
   return isolate_->factory()->undefined_value();
 }
@@ -1465,8 +1476,10 @@ bool Debug::EnsureBreakInfo(Handle<SharedFunctionInfo> shared) {
   if (!shared->IsSubjectToDebugging() && !CanBreakAtEntry(shared)) {
     return false;
   }
-  if (!shared->is_compiled() &&
-      !Compiler::Compile(shared, Compiler::CLEAR_EXCEPTION)) {
+  IsCompiledScope is_compiled_scope = shared->is_compiled_scope();
+  if (!is_compiled_scope.is_compiled() &&
+      !Compiler::Compile(shared, Compiler::CLEAR_EXCEPTION,
+                         &is_compiled_scope)) {
     return false;
   }
   CreateBreakInfo(shared);
@@ -2137,10 +2150,13 @@ bool Debug::PerformSideEffectCheck(Handle<JSFunction> function,
                                    Handle<Object> receiver) {
   DCHECK_EQ(isolate_->debug_execution_mode(), DebugInfo::kSideEffects);
   DisallowJavascriptExecution no_js(isolate_);
+  IsCompiledScope is_compiled_scope(function->shared()->is_compiled_scope());
   if (!function->is_compiled() &&
-      !Compiler::Compile(function, Compiler::KEEP_EXCEPTION)) {
+      !Compiler::Compile(function, Compiler::KEEP_EXCEPTION,
+                         &is_compiled_scope)) {
     return false;
   }
+  DCHECK(is_compiled_scope.is_compiled());
   Handle<SharedFunctionInfo> shared(function->shared(), isolate_);
   Handle<DebugInfo> debug_info = GetOrCreateDebugInfo(shared);
   DebugInfo::SideEffectState side_effect_state =

src/objects.cc

@@ -6237,12 +6237,14 @@ Handle<Object> JSFunction::GetName(Isolate* isolate,
 Maybe<int> JSFunction::GetLength(Isolate* isolate,
                                  Handle<JSFunction> function) {
   int length = 0;
-  if (function->shared()->is_compiled()) {
+  IsCompiledScope is_compiled_scope(function->shared()->is_compiled_scope());
+  if (is_compiled_scope.is_compiled()) {
     length = function->shared()->GetLength();
   } else {
     // If the function isn't compiled yet, the length is not computed
     // correctly yet. Compile it now and return the right length.
-    if (Compiler::Compile(function, Compiler::KEEP_EXCEPTION)) {
+    if (Compiler::Compile(function, Compiler::KEEP_EXCEPTION,
+                          &is_compiled_scope)) {
       length = function->shared()->GetLength();
     }
     if (isolate->has_pending_exception()) return Nothing<int>();
@@ -12683,9 +12685,11 @@ void JSFunction::MarkForOptimization(ConcurrencyMode mode) {
 // static
 void JSFunction::EnsureFeedbackVector(Handle<JSFunction> function) {
   Isolate* const isolate = function->GetIsolate();
+  DCHECK(function->shared()->is_compiled());
   if (function->feedback_cell()->value()->IsUndefined(isolate)) {
     Handle<SharedFunctionInfo> shared(function->shared(), isolate);
     if (!shared->HasAsmWasmData()) {
+      DCHECK(function->shared()->HasBytecodeArray());
       Handle<FeedbackVector> feedback_vector =
           FeedbackVector::New(isolate, shared);
       if (function->feedback_cell() == isolate->heap()->many_closures_cell()) {
@@ -13360,8 +13364,10 @@ void JSFunction::EnsureHasInitialMap(Handle<JSFunction> function) {
   // The constructor should be compiled for the optimization hints to be
   // available.
   int expected_nof_properties = 0;
-  if (function->shared()->is_compiled() ||
-      Compiler::Compile(function, Compiler::CLEAR_EXCEPTION)) {
+  IsCompiledScope is_compiled_scope(function->shared()->is_compiled_scope());
+  if (is_compiled_scope.is_compiled() ||
+      Compiler::Compile(function, Compiler::CLEAR_EXCEPTION,
+                        &is_compiled_scope)) {
     DCHECK(function->shared()->is_compiled());
     expected_nof_properties = function->shared()->expected_nof_properties();
   }
@@ -14214,8 +14220,10 @@ bool JSFunction::CalculateInstanceSizeForDerivedClass(
     // The super constructor should be compiled for the number of expected
     // properties to be available.
     Handle<SharedFunctionInfo> shared(func->shared(), isolate);
-    if (shared->is_compiled() ||
-        Compiler::Compile(func, Compiler::CLEAR_EXCEPTION)) {
+    IsCompiledScope is_compiled_scope(shared->is_compiled_scope());
+    if (is_compiled_scope.is_compiled() ||
+        Compiler::Compile(func, Compiler::CLEAR_EXCEPTION,
+                          &is_compiled_scope)) {
       DCHECK(shared->is_compiled());
       int count = shared->expected_nof_properties();
       // Check that the estimate is sane.
@@ -14224,7 +14232,7 @@ bool JSFunction::CalculateInstanceSizeForDerivedClass(
       } else {
         expected_nof_properties = JSObject::kMaxInObjectProperties;
       }
-    } else if (!shared->is_compiled()) {
+    } else {
       // In case there was a compilation error for the constructor we will
       // throw an error during instantiation. Hence we directly return 0;
       return false;

src/objects/compilation-cache-inl.h

@@ -71,6 +71,13 @@ uint32_t CompilationCacheShape::HashForObject(Isolate* isolate,
                     Smi::cast(val->get(JSRegExp::kFlagsIndex)));
 }
 
+InfoCellPair::InfoCellPair(SharedFunctionInfo shared,
+                           FeedbackCell* feedback_cell)
+    : is_compiled_scope_(!shared.is_null() ? shared->is_compiled_scope()
+                                           : IsCompiledScope()),
+      shared_(shared),
+      feedback_cell_(feedback_cell) {}
+
 }  // namespace internal
 }  // namespace v8
 

src/objects/compilation-cache.h

@@ -41,16 +41,29 @@ class CompilationCacheShape : public BaseShape<HashTableKey*> {
 class InfoCellPair {
  public:
   InfoCellPair() : feedback_cell_(nullptr) {}
-  InfoCellPair(SharedFunctionInfo shared, FeedbackCell* feedback_cell)
-      : shared_(shared), feedback_cell_(feedback_cell) {}
+  inline InfoCellPair(SharedFunctionInfo shared, FeedbackCell* feedback_cell);
 
-  FeedbackCell* feedback_cell() const { return feedback_cell_; }
-  SharedFunctionInfo shared() const { return shared_; }
+  FeedbackCell* feedback_cell() const {
+    DCHECK(is_compiled_scope_.is_compiled());
+    return feedback_cell_;
+  }
+  SharedFunctionInfo shared() const {
+    DCHECK(is_compiled_scope_.is_compiled());
+    return shared_;
+  }
 
-  bool has_feedback_cell() const { return feedback_cell_ != nullptr; }
-  bool has_shared() const { return !shared_.is_null(); }
+  bool has_feedback_cell() const {
+    return feedback_cell_ != nullptr && is_compiled_scope_.is_compiled();
+  }
+  bool has_shared() const {
+    // Only return true if SFI is compiled - the bytecode could have been
+    // flushed while it's in the compilation cache, and not yet have been
+    // removed form the compilation cache.
+    return !shared_.is_null() && is_compiled_scope_.is_compiled();
+  }
 
  private:
+  IsCompiledScope is_compiled_scope_;
   SharedFunctionInfo shared_;
   FeedbackCell* feedback_cell_;
 };

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

@@ -368,6 +368,19 @@ bool SharedFunctionInfo::is_compiled() const {
          !data->IsUncompiledData();
 }
 
+IsCompiledScope SharedFunctionInfo::is_compiled_scope() const {
+  return IsCompiledScope(*this, GetIsolate());
+}
+
+IsCompiledScope::IsCompiledScope(const SharedFunctionInfo shared,
+                                 Isolate* isolate)
+    : retain_bytecode_(shared->HasBytecodeArray()
+                           ? handle(shared->GetBytecodeArray(), isolate)
+                           : MaybeHandle<BytecodeArray>()),
+      is_compiled_(shared->is_compiled()) {
+  DCHECK_IMPLIES(!retain_bytecode_.is_null(), is_compiled());
+}
+
 uint16_t SharedFunctionInfo::GetLength() const {
   DCHECK(is_compiled());
   DCHECK(HasLength());

src/objects/shared-function-info.h

@@ -21,6 +21,7 @@ class AsmWasmData;
 class BytecodeArray;
 class CoverageInfo;
 class DebugInfo;
+class IsCompiledScope;
 class WasmExportedFunctionData;
 
 // Data collected by the pre-parser storing information about scopes and inner
@@ -246,9 +247,17 @@ class SharedFunctionInfo : public HeapObjectPtr {
   inline bool HasFeedbackMetadata() const;
   DECL_ACCESSORS(feedback_metadata, FeedbackMetadata)
 
-  // Returns if this function has been compiled to native code yet.
+  // Returns if this function has been compiled yet. Note: with bytecode
+  // flushing, any GC after this call is made could cause the function
+  // to become uncompiled. If you need to ensure the function remains compiled
+  // for some period of time, use IsCompiledScope instead.
   inline bool is_compiled() const;
 
+  // Returns an IsCompiledScope which reports whether the function is compiled,
+  // and if compiled, will avoid the function becoming uncompiled while it is
+  // held.
+  inline IsCompiledScope is_compiled_scope() const;
+
   // [length]: The function length - usually the number of declared parameters.
   // Use up to 2^16-2 parameters (16 bits of values, where one is reserved for
   // kDontAdaptArgumentsSentinel). The value is only reliable when the function
@@ -691,6 +700,21 @@ struct SourceCodeOf {
   int max_length;
 };
 
+// IsCompiledScope enables a caller to check if a function is compiled, and
+// ensure it remains compiled (i.e., doesn't have it's bytecode flushed) while
+// the scope is retained.
+class IsCompiledScope {
+ public:
+  inline IsCompiledScope(const SharedFunctionInfo shared, Isolate* isolate);
+  inline IsCompiledScope() : retain_bytecode_(), is_compiled_(false) {}
+
+  inline bool is_compiled() const { return is_compiled_; }
+
+ private:
+  MaybeHandle<BytecodeArray> retain_bytecode_;
+  bool is_compiled_;
+};
+
 std::ostream& operator<<(std::ostream& os, const SourceCodeOf& v);
 
 }  // namespace internal

src/runtime/runtime-compiler.cc

@@ -36,7 +36,9 @@ RUNTIME_FUNCTION(Runtime_CompileLazy) {
   if (check.JsHasOverflowed(kStackSpaceRequiredForCompilation * KB)) {
     return isolate->StackOverflow();
   }
-  if (!Compiler::Compile(function, Compiler::KEEP_EXCEPTION)) {
+  IsCompiledScope is_compiled_scope;
+  if (!Compiler::Compile(function, Compiler::KEEP_EXCEPTION,
+                         &is_compiled_scope)) {
     return ReadOnlyRoots(isolate).exception();
   }
   DCHECK(function->is_compiled());

src/runtime/runtime-test.cc

@@ -225,8 +225,10 @@ RUNTIME_FUNCTION(Runtime_OptimizeFunctionOnNextCall) {
   }
 
   // If function isn't compiled, compile it now.
-  if (!function->shared()->is_compiled() &&
-      !Compiler::Compile(function, Compiler::CLEAR_EXCEPTION)) {
+  IsCompiledScope is_compiled_scope(function->shared()->is_compiled_scope());
+  if (!is_compiled_scope.is_compiled() &&
+      !Compiler::Compile(function, Compiler::CLEAR_EXCEPTION,
+                         &is_compiled_scope)) {
     return ReadOnlyRoots(isolate).undefined_value();
   }
 
@@ -715,8 +717,9 @@ RUNTIME_FUNCTION(Runtime_DisassembleFunction) {
   DCHECK_EQ(1, args.length());
   // Get the function and make sure it is compiled.
   CONVERT_ARG_HANDLE_CHECKED(JSFunction, func, 0);
+  IsCompiledScope is_compiled_scope;
   if (!func->is_compiled() &&
-      !Compiler::Compile(func, Compiler::KEEP_EXCEPTION)) {
+      !Compiler::Compile(func, Compiler::KEEP_EXCEPTION, &is_compiled_scope)) {
     return ReadOnlyRoots(isolate).exception();
   }
   StdoutStream os;

test/cctest/compiler/function-tester.cc

@@ -142,8 +142,10 @@ Handle<JSFunction> FunctionTester::ForMachineGraph(Graph* graph,
 
 Handle<JSFunction> FunctionTester::Compile(Handle<JSFunction> function) {
   Handle<SharedFunctionInfo> shared(function->shared(), isolate);
-  CHECK(function->is_compiled() ||
-        Compiler::Compile(function, Compiler::CLEAR_EXCEPTION));
+  IsCompiledScope is_compiled_scope(shared->is_compiled_scope());
+  CHECK(is_compiled_scope.is_compiled() ||
+        Compiler::Compile(function, Compiler::CLEAR_EXCEPTION,
+                          &is_compiled_scope));
 
   Zone zone(isolate->allocator(), ZONE_NAME);
   OptimizedCompilationInfo info(&zone, isolate, shared, function);

test/unittests/compiler-dispatcher/optimizing-compile-dispatcher-unittest.cc

@@ -73,7 +73,9 @@ TEST_F(OptimizingCompileDispatcherTest, Construct) {
 TEST_F(OptimizingCompileDispatcherTest, NonBlockingFlush) {
   Handle<JSFunction> fun =
       RunJS<JSFunction>("function f() { function g() {}; return g;}; f();");
-  ASSERT_TRUE(Compiler::Compile(fun, Compiler::CLEAR_EXCEPTION));
+  IsCompiledScope is_compiled_scope;
+  ASSERT_TRUE(
+      Compiler::Compile(fun, Compiler::CLEAR_EXCEPTION, &is_compiled_scope));
   BlockingCompilationJob* job = new BlockingCompilationJob(i_isolate(), fun);
 
   OptimizingCompileDispatcher dispatcher(i_isolate());