Revert "[Compiler] Introduce IsCompiledScope which prevents flushing of compiled code"

This reverts commit 10ea3f8a.

Reason for revert: Causing failure on gc_stress bot: 
https://logs.chromium.org/logs/v8/buildbucket/cr-buildbucket.appspot.com/8928421099411850688/+/steps/Bisect_10ea3f8a/0/steps/Retry/0/logs/collections-construct../0

Original change's description:
> [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/1352295
> Reviewed-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}

TBR=rmcilroy@chromium.org,mstarzinger@chromium.org,jgruber@chromium.org

Change-Id: I1449a02a0aceb9757440757628e586df33972a40
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8395
Reviewed-on: https://chromium-review.googlesource.com/c/1357042Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57974}

src/compiler.h

@@ -21,7 +21,6 @@ namespace internal {
 // Forward declarations.
 class AstRawString;
 class BackgroundCompileTask;
-class IsCompiledScope;
 class JavaScriptFrame;
 class OptimizedCompilationInfo;
 class OptimizedCompilationJob;
@@ -58,10 +57,8 @@ 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,
-                      IsCompiledScope* is_compiled_scope);
-  static bool Compile(Handle<JSFunction> function, ClearExceptionFlag flag,
-                      IsCompiledScope* is_compiled_scope);
+                      ClearExceptionFlag flag);
+  static bool Compile(Handle<JSFunction> function, ClearExceptionFlag flag);
   static bool CompileOptimized(Handle<JSFunction> function, ConcurrencyMode);
 
   V8_WARN_UNUSED_RESULT static MaybeHandle<SharedFunctionInfo>

src/compiler/js-inlining.cc

@@ -452,10 +452,8 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
     return NoChange();
   }
 
-  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)) {
+  if (!shared_info->is_compiled() &&
+      !Compiler::Compile(shared_info, Compiler::CLEAR_EXCEPTION)) {
     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,9 +1272,7 @@ 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) {
-    IsCompiledScope is_compiled_scope;
-    Compiler::Compile(fun, Compiler::CLEAR_EXCEPTION, &is_compiled_scope);
-    DCHECK(is_compiled_scope.is_compiled());
+    Compiler::Compile(fun, Compiler::CLEAR_EXCEPTION);
     fun->set_code(*trampoline);
   }
 }
@@ -1322,7 +1320,6 @@ 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()) {
@@ -1339,17 +1336,13 @@ 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());
-      IsCompiledScope is_compiled_scope(candidate->is_compiled_scope());
-      if (!is_compiled_scope.is_compiled()) {
-        if (!Compiler::Compile(candidate, Compiler::CLEAR_EXCEPTION,
-                               &is_compiled_scope)) {
+      if (!candidate->is_compiled()) {
+        if (!Compiler::Compile(candidate, Compiler::CLEAR_EXCEPTION)) {
           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);
     }
@@ -1431,7 +1424,6 @@ 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);
@@ -1442,8 +1434,7 @@ Handle<Object> Debug::FindSharedFunctionInfoInScript(Handle<Script> script,
       shared = finder.Result();
       if (shared.is_null()) break;
       // We found it if it's already compiled.
-      is_compiled_scope = shared->is_compiled_scope();
-      if (is_compiled_scope.is_compiled()) {
+      if (shared->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
@@ -1460,11 +1451,9 @@ 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,
-                           &is_compiled_scope)) {
+    if (!Compiler::Compile(handle(shared, isolate_), Compiler::CLEAR_EXCEPTION))
       break;
   }
-  }
   return isolate_->factory()->undefined_value();
 }
 
@@ -1476,10 +1465,8 @@ bool Debug::EnsureBreakInfo(Handle<SharedFunctionInfo> shared) {
   if (!shared->IsSubjectToDebugging() && !CanBreakAtEntry(shared)) {
     return false;
   }
-  IsCompiledScope is_compiled_scope = shared->is_compiled_scope();
-  if (!is_compiled_scope.is_compiled() &&
-      !Compiler::Compile(shared, Compiler::CLEAR_EXCEPTION,
-                         &is_compiled_scope)) {
+  if (!shared->is_compiled() &&
+      !Compiler::Compile(shared, Compiler::CLEAR_EXCEPTION)) {
     return false;
   }
   CreateBreakInfo(shared);
@@ -2150,13 +2137,10 @@ 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,
-                         &is_compiled_scope)) {
+      !Compiler::Compile(function, Compiler::KEEP_EXCEPTION)) {
     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,14 +6237,12 @@ Handle<Object> JSFunction::GetName(Isolate* isolate,
 Maybe<int> JSFunction::GetLength(Isolate* isolate,
                                  Handle<JSFunction> function) {
   int length = 0;
-  IsCompiledScope is_compiled_scope(function->shared()->is_compiled_scope());
-  if (is_compiled_scope.is_compiled()) {
+  if (function->shared()->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,
-                          &is_compiled_scope)) {
+    if (Compiler::Compile(function, Compiler::KEEP_EXCEPTION)) {
       length = function->shared()->GetLength();
     }
     if (isolate->has_pending_exception()) return Nothing<int>();
@@ -12685,11 +12683,9 @@ 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()) {
@@ -13364,10 +13360,8 @@ void JSFunction::EnsureHasInitialMap(Handle<JSFunction> function) {
   // The constructor should be compiled for the optimization hints to be
   // available.
   int expected_nof_properties = 0;
-  IsCompiledScope is_compiled_scope(function->shared()->is_compiled_scope());
-  if (is_compiled_scope.is_compiled() ||
-      Compiler::Compile(function, Compiler::CLEAR_EXCEPTION,
-                        &is_compiled_scope)) {
+  if (function->shared()->is_compiled() ||
+      Compiler::Compile(function, Compiler::CLEAR_EXCEPTION)) {
     DCHECK(function->shared()->is_compiled());
     expected_nof_properties = function->shared()->expected_nof_properties();
   }
@@ -14220,10 +14214,8 @@ bool JSFunction::CalculateInstanceSizeForDerivedClass(
     // The super constructor should be compiled for the number of expected
     // properties to be available.
     Handle<SharedFunctionInfo> shared(func->shared(), isolate);
-    IsCompiledScope is_compiled_scope(shared->is_compiled_scope());
-    if (is_compiled_scope.is_compiled() ||
-        Compiler::Compile(func, Compiler::CLEAR_EXCEPTION,
-                          &is_compiled_scope)) {
+    if (shared->is_compiled() ||
+        Compiler::Compile(func, Compiler::CLEAR_EXCEPTION)) {
       DCHECK(shared->is_compiled());
       int count = shared->expected_nof_properties();
       // Check that the estimate is sane.
@@ -14232,7 +14224,7 @@ bool JSFunction::CalculateInstanceSizeForDerivedClass(
       } else {
         expected_nof_properties = JSObject::kMaxInObjectProperties;
       }
-    } else {
+    } else if (!shared->is_compiled()) {
       // 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,13 +71,6 @@ 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,29 +41,16 @@ class CompilationCacheShape : public BaseShape<HashTableKey*> {
 class InfoCellPair {
  public:
   InfoCellPair() : feedback_cell_(nullptr) {}
-  inline InfoCellPair(SharedFunctionInfo shared, FeedbackCell* feedback_cell);
+  InfoCellPair(SharedFunctionInfo shared, FeedbackCell* feedback_cell)
+      : shared_(shared), feedback_cell_(feedback_cell) {}
 
-  FeedbackCell* feedback_cell() const {
-    DCHECK(is_compiled_scope_.is_compiled());
-    return feedback_cell_;
-  }
-  SharedFunctionInfo shared() const {
-    DCHECK(is_compiled_scope_.is_compiled());
-    return shared_;
-  }
+  FeedbackCell* feedback_cell() const { return feedback_cell_; }
+  SharedFunctionInfo shared() const { return shared_; }
 
-  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();
-  }
+  bool has_feedback_cell() const { return feedback_cell_ != nullptr; }
+  bool has_shared() const { return !shared_.is_null(); }
 
  private:
-  IsCompiledScope is_compiled_scope_;
   SharedFunctionInfo shared_;
   FeedbackCell* feedback_cell_;
 };

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

@@ -368,19 +368,6 @@ 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,7 +21,6 @@ class AsmWasmData;
 class BytecodeArray;
 class CoverageInfo;
 class DebugInfo;
-class IsCompiledScope;
 class WasmExportedFunctionData;
 
 // Data collected by the pre-parser storing information about scopes and inner
@@ -247,17 +246,9 @@ class SharedFunctionInfo : public HeapObjectPtr {
   inline bool HasFeedbackMetadata() const;
   DECL_ACCESSORS(feedback_metadata, FeedbackMetadata)
 
-  // 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.
+  // Returns if this function has been compiled to native code yet.
   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
@@ -700,21 +691,6 @@ 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,9 +36,7 @@ RUNTIME_FUNCTION(Runtime_CompileLazy) {
   if (check.JsHasOverflowed(kStackSpaceRequiredForCompilation * KB)) {
     return isolate->StackOverflow();
   }
-  IsCompiledScope is_compiled_scope;
-  if (!Compiler::Compile(function, Compiler::KEEP_EXCEPTION,
-                         &is_compiled_scope)) {
+  if (!Compiler::Compile(function, Compiler::KEEP_EXCEPTION)) {
     return ReadOnlyRoots(isolate).exception();
   }
   DCHECK(function->is_compiled());

src/runtime/runtime-test.cc

@@ -225,10 +225,8 @@ RUNTIME_FUNCTION(Runtime_OptimizeFunctionOnNextCall) {
   }
 
   // If function isn't compiled, compile it now.
-  IsCompiledScope is_compiled_scope(function->shared()->is_compiled_scope());
-  if (!is_compiled_scope.is_compiled() &&
-      !Compiler::Compile(function, Compiler::CLEAR_EXCEPTION,
-                         &is_compiled_scope)) {
+  if (!function->shared()->is_compiled() &&
+      !Compiler::Compile(function, Compiler::CLEAR_EXCEPTION)) {
     return ReadOnlyRoots(isolate).undefined_value();
   }
 
@@ -717,9 +715,8 @@ 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, &is_compiled_scope)) {
+      !Compiler::Compile(func, Compiler::KEEP_EXCEPTION)) {
     return ReadOnlyRoots(isolate).exception();
   }
   StdoutStream os;

test/cctest/compiler/function-tester.cc

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

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

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