[vm/runtime] Create separate background compilation queues for optimized and unoptimized compilations.

- create separate compilation queues for optimized and compilation
queues
- remove option FLAG_background_compilation_stop_alot doesn't seem to be used in any format testing
- remove some class finalization checks before starting the background compiler

Change-Id: I98f1773987e33a43e7b43ace4c383d17bd0a1710
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/98453
Reviewed-by: Zach Anderson <zra@google.com>

runtime/tests/vm/vm.status

@@ -267,7 +267,6 @@ dart/data_uri*test: Skip # Data uri's not supported by dart2js or the analyzer.
 # Tests that use functionality not supported in Dart 2.
 [ ($compiler == dartk || $compiler == dartkb) || $compiler == dartkp ]
 cc/DartAPI_IsolateSetCheckedMode: SkipByDesign # Checked mode is not relevant for dart 2?
-cc/CompileFunctionOnHelperThread: SkipByDesign
 cc/CompileFunction: SkipByDesign
 cc/InvokeDynamic_CompileError: SkipByDesign
 cc/InvokeStatic_CompileError: SkipByDesign

runtime/vm/compiler/jit/compiler.cc

@@ -361,7 +361,7 @@ class CompileParsedFunctionHelper : public ValueObject {
   RawCode* FinalizeCompilation(Assembler* assembler,
                                FlowGraphCompiler* graph_compiler,
                                FlowGraph* flow_graph);
-  void CheckIfBackgroundCompilerIsBeingStopped();
+  void CheckIfBackgroundCompilerIsBeingStopped(bool optimizing_compiler);
 
   ParsedFunction* parsed_function_;
   const bool optimized_;
@@ -556,12 +556,22 @@ RawCode* CompileParsedFunctionHelper::FinalizeCompilation(
   return code.raw();
 }
 
-void CompileParsedFunctionHelper::CheckIfBackgroundCompilerIsBeingStopped() {
+void CompileParsedFunctionHelper::CheckIfBackgroundCompilerIsBeingStopped(
+    bool optimizing_compiler) {
   ASSERT(Compiler::IsBackgroundCompilation());
-  if (!isolate()->background_compiler()->is_running()) {
-    // The background compiler is being stopped.
-    Compiler::AbortBackgroundCompilation(
-        DeoptId::kNone, "Background compilation is being stopped");
+  if (optimizing_compiler) {
+    if (!isolate()->optimizing_background_compiler()->is_running()) {
+      // The background compiler is being stopped.
+      Compiler::AbortBackgroundCompilation(
+          DeoptId::kNone, "Optimizing Background compilation is being stopped");
+    }
+  } else {
+    if (FLAG_enable_interpreter &&
+        !isolate()->background_compiler()->is_running()) {
+      // The background compiler is being stopped.
+      Compiler::AbortBackgroundCompilation(
+          DeoptId::kNone, "Background compilation is being stopped");
+    }
   }
 }
 
@@ -708,12 +718,12 @@ RawCode* CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
           // changes code page access permissions (makes them temporary not
           // executable).
           {
-            CheckIfBackgroundCompilerIsBeingStopped();
+            CheckIfBackgroundCompilerIsBeingStopped(optimized());
             SafepointOperationScope safepoint_scope(thread());
             // Do not Garbage collect during this stage and instead allow the
             // heap to grow.
             NoHeapGrowthControlScope no_growth_control;
-            CheckIfBackgroundCompilerIsBeingStopped();
+            CheckIfBackgroundCompilerIsBeingStopped(optimized());
             *result =
                 FinalizeCompilation(&assembler, &graph_compiler, flow_graph);
           }
@@ -1474,7 +1484,7 @@ void BackgroundCompiler::Run() {
   }
 }
 
-void BackgroundCompiler::CompileOptimized(const Function& function) {
+void BackgroundCompiler::Compile(const Function& function) {
   ASSERT(Thread::Current()->IsMutatorThread());
   // TODO(srdjan): Checking different strategy for collecting garbage
   // accumulated by background compiler.
@@ -1516,18 +1526,6 @@ void BackgroundCompiler::Start() {
   ASSERT(thread->IsMutatorThread());
   ASSERT(!thread->IsAtSafepoint());
 
-  // Finalize NoSuchMethodError, _Mint; occasionally needed in optimized
-  // compilation.
-  Class& cls = Class::Handle(
-      thread->zone(), Library::LookupCoreClass(Symbols::NoSuchMethodError()));
-  ASSERT(!cls.IsNull());
-  Error& error = Error::Handle(thread->zone(), cls.EnsureIsFinalized(thread));
-  ASSERT(error.IsNull());
-  cls = Library::LookupCoreClass(Symbols::_Mint());
-  ASSERT(!cls.IsNull());
-  error = cls.EnsureIsFinalized(thread);
-  ASSERT(error.IsNull());
-
   MonitorLocker ml(done_monitor_);
   if (running_ || !done_) return;
   running_ = true;
@@ -1653,7 +1651,7 @@ void Compiler::AbortBackgroundCompilation(intptr_t deopt_id, const char* msg) {
   UNREACHABLE();
 }
 
-void BackgroundCompiler::CompileOptimized(const Function& function) {
+void BackgroundCompiler::Compile(const Function& function) {
   UNREACHABLE();
 }
 

runtime/vm/compiler/jit/compiler.h

@@ -157,46 +157,57 @@ class BackgroundCompiler {
 
   static void Start(Isolate* isolate) {
     ASSERT(Thread::Current()->IsMutatorThread());
-    if (isolate->background_compiler() != NULL) {
+    if (FLAG_enable_interpreter && isolate->background_compiler() != NULL) {
       isolate->background_compiler()->Start();
     }
+    if (isolate->optimizing_background_compiler() != NULL) {
+      isolate->optimizing_background_compiler()->Start();
+    }
   }
   static void Stop(Isolate* isolate) {
     ASSERT(Thread::Current()->IsMutatorThread());
-    if (isolate->background_compiler() != NULL) {
+    if (FLAG_enable_interpreter && isolate->background_compiler() != NULL) {
       isolate->background_compiler()->Stop();
     }
+    if (isolate->optimizing_background_compiler() != NULL) {
+      isolate->optimizing_background_compiler()->Stop();
+    }
   }
   static void Enable(Isolate* isolate) {
     ASSERT(Thread::Current()->IsMutatorThread());
-    if (isolate->background_compiler() != NULL) {
+    if (FLAG_enable_interpreter && isolate->background_compiler() != NULL) {
       isolate->background_compiler()->Enable();
     }
+    if (isolate->optimizing_background_compiler() != NULL) {
+      isolate->optimizing_background_compiler()->Enable();
+    }
   }
   static void Disable(Isolate* isolate) {
     ASSERT(Thread::Current()->IsMutatorThread());
-    if (isolate->background_compiler() != NULL) {
+    if (FLAG_enable_interpreter && isolate->background_compiler() != NULL) {
       isolate->background_compiler()->Disable();
     }
-  }
-  static bool IsDisabled(Isolate* isolate) {
-    ASSERT(Thread::Current()->IsMutatorThread());
-    if (isolate->background_compiler() != NULL) {
-      return isolate->background_compiler()->IsDisabled();
+    if (isolate->optimizing_background_compiler() != NULL) {
+      isolate->optimizing_background_compiler()->Disable();
     }
-    return false;
   }
-  static bool IsRunning(Isolate* isolate) {
+  static bool IsDisabled(Isolate* isolate, bool optimizing_compiler) {
     ASSERT(Thread::Current()->IsMutatorThread());
-    if (isolate->background_compiler() != NULL) {
-      return isolate->background_compiler()->IsRunning();
+    if (optimizing_compiler) {
+      if (isolate->optimizing_background_compiler() != NULL) {
+        return isolate->optimizing_background_compiler()->IsDisabled();
+      }
+    } else {
+      if (FLAG_enable_interpreter && isolate->background_compiler() != NULL) {
+        return isolate->background_compiler()->IsDisabled();
+      }
     }
     return false;
   }
 
-  // Call to optimize a function in the background, enters the function in the
-  // compilation queue.
-  void CompileOptimized(const Function& function);
+  // Call to compile (unoptimized or optimized) a function in the background,
+  // enters the function in the compilation queue.
+  void Compile(const Function& function);
 
   void VisitPointers(ObjectPointerVisitor* visitor);
 

runtime/vm/compiler_test.cc

@@ -37,12 +37,13 @@ ISOLATE_UNIT_TEST_CASE(CompileFunction) {
       "    // A.foo();\n"
       "  }\n"
       "}\n";
-  String& url = String::Handle(String::New("dart-test:CompileFunction"));
-  String& source = String::Handle(String::New(kScriptChars));
-  Script& script =
-      Script::Handle(Script::New(url, source, RawScript::kScriptTag));
-  Library& lib = Library::Handle(Library::CoreLibrary());
-  EXPECT(CompilerTest::TestCompileScript(lib, script));
+  Dart_Handle library;
+  {
+    TransitionVMToNative transition(thread);
+    library = TestCase::LoadTestScript(kScriptChars, NULL);
+  }
+  const Library& lib =
+      Library::Handle(Library::RawCast(Api::UnwrapHandle(library)));
   EXPECT(ClassFinalizer::ProcessPendingClasses());
   Class& cls =
       Class::Handle(lib.LookupClass(String::Handle(Symbols::New(thread, "A"))));
@@ -68,19 +69,19 @@ ISOLATE_UNIT_TEST_CASE(CompileFunction) {
                function_source.ToCString());
 }
 
-ISOLATE_UNIT_TEST_CASE(CompileFunctionOnHelperThread) {
+ISOLATE_UNIT_TEST_CASE(OptimizeCompileFunctionOnHelperThread) {
   // Create a simple function and compile it without optimization.
   const char* kScriptChars =
       "class A {\n"
       "  static foo() { return 42; }\n"
       "}\n";
-  String& url =
-      String::Handle(String::New("dart-test:CompileFunctionOnHelperThread"));
-  String& source = String::Handle(String::New(kScriptChars));
-  Script& script =
-      Script::Handle(Script::New(url, source, RawScript::kScriptTag));
-  Library& lib = Library::Handle(Library::CoreLibrary());
-  EXPECT(CompilerTest::TestCompileScript(lib, script));
+  Dart_Handle library;
+  {
+    TransitionVMToNative transition(thread);
+    library = TestCase::LoadTestScript(kScriptChars, NULL);
+  }
+  const Library& lib =
+      Library::Handle(Library::RawCast(Api::UnwrapHandle(library)));
   EXPECT(ClassFinalizer::ProcessPendingClasses());
   Class& cls =
       Class::Handle(lib.LookupClass(String::Handle(Symbols::New(thread, "A"))));
@@ -98,7 +99,7 @@ ISOLATE_UNIT_TEST_CASE(CompileFunctionOnHelperThread) {
 #endif
   Isolate* isolate = thread->isolate();
   BackgroundCompiler::Start(isolate);
-  isolate->background_compiler()->CompileOptimized(func);
+  isolate->optimizing_background_compiler()->Compile(func);
   Monitor* m = new Monitor();
   {
     MonitorLocker ml(m);
@@ -110,6 +111,50 @@ ISOLATE_UNIT_TEST_CASE(CompileFunctionOnHelperThread) {
   BackgroundCompiler::Stop(isolate);
 }
 
+ISOLATE_UNIT_TEST_CASE(CompileFunctionOnHelperThread) {
+  // Create a simple function and compile it without optimization.
+  const char* kScriptChars =
+      "class A {\n"
+      "  static foo() { return 42; }\n"
+      "}\n";
+  Dart_Handle library;
+  {
+    TransitionVMToNative transition(thread);
+    library = TestCase::LoadTestScript(kScriptChars, NULL);
+  }
+  const Library& lib =
+      Library::Handle(Library::RawCast(Api::UnwrapHandle(library)));
+  EXPECT(ClassFinalizer::ProcessPendingClasses());
+  Class& cls =
+      Class::Handle(lib.LookupClass(String::Handle(Symbols::New(thread, "A"))));
+  EXPECT(!cls.IsNull());
+  String& function_foo_name = String::Handle(String::New("foo"));
+  Function& func =
+      Function::Handle(cls.LookupStaticFunction(function_foo_name));
+  EXPECT(!func.HasCode());
+  if (!FLAG_enable_interpreter) {
+    CompilerTest::TestCompileFunction(func);
+    EXPECT(func.HasCode());
+    return;
+  }
+#if !defined(PRODUCT)
+  // Constant in product mode.
+  FLAG_background_compilation = true;
+#endif
+  Isolate* isolate = thread->isolate();
+  BackgroundCompiler::Start(isolate);
+  isolate->background_compiler()->Compile(func);
+  Monitor* m = new Monitor();
+  {
+    MonitorLocker ml(m);
+    while (!func.HasCode()) {
+      ml.WaitWithSafepointCheck(thread, 1);
+    }
+  }
+  delete m;
+  BackgroundCompiler::Stop(isolate);
+}
+
 ISOLATE_UNIT_TEST_CASE(RegenerateAllocStubs) {
   const char* kScriptChars =
       "class A {\n"

runtime/vm/flag_list.h

@@ -57,8 +57,6 @@ constexpr bool kDartPrecompiledRuntime = false;
     "Debugger support async functions.")                                       \
   P(background_compilation, bool, USING_MULTICORE,                             \
     "Run optimizing compilation in background")                                \
-  R(background_compilation_stop_alot, false, bool, false,                      \
-    "Stress test system: stop background compiler often.")                     \
   P(causal_async_stacks, bool, !USING_PRODUCT, "Improved async stacks")        \
   P(collect_code, bool, true, "Attempt to GC infrequently used code.")         \
   P(collect_dynamic_function_names, bool, true,                                \

runtime/vm/isolate.cc

@@ -866,6 +866,7 @@ Isolate::Isolate(const Dart_IsolateFlags& api_flags)
       heap_(NULL),
       isolate_flags_(0),
       background_compiler_(NULL),
+      optimizing_background_compiler_(NULL),
 #if !defined(PRODUCT)
       debugger_(NULL),
       last_resume_timestamp_(OS::GetCurrentTimeMillis()),
@@ -951,7 +952,11 @@ Isolate::Isolate(const Dart_IsolateFlags& api_flags)
         "         See dartbug.com/30524 for more information.\n");
   }
 
-  NOT_IN_PRECOMPILED(background_compiler_ = new BackgroundCompiler(this));
+  if (FLAG_enable_interpreter) {
+    NOT_IN_PRECOMPILED(background_compiler_ = new BackgroundCompiler(this));
+  }
+  NOT_IN_PRECOMPILED(optimizing_background_compiler_ =
+                         new BackgroundCompiler(this));
 }
 
 #undef REUSABLE_HANDLE_SCOPE_INIT
@@ -966,8 +971,13 @@ Isolate::~Isolate() {
   delete reverse_pc_lookup_cache_;
   reverse_pc_lookup_cache_ = nullptr;
 
-  delete background_compiler_;
-  background_compiler_ = NULL;
+  if (FLAG_enable_interpreter) {
+    delete background_compiler_;
+    background_compiler_ = NULL;
+  }
+
+  delete optimizing_background_compiler_;
+  optimizing_background_compiler_ = NULL;
 
 #if !defined(PRODUCT)
   delete debugger_;
@@ -1862,8 +1872,12 @@ void Isolate::MaybeIncreaseReloadEveryNStackOverflowChecks() {
 void Isolate::Shutdown() {
   ASSERT(this == Isolate::Current());
   BackgroundCompiler::Stop(this);
-  delete background_compiler_;
-  background_compiler_ = NULL;
+  if (FLAG_enable_interpreter) {
+    delete background_compiler_;
+    background_compiler_ = NULL;
+  }
+  delete optimizing_background_compiler_;
+  optimizing_background_compiler_ = NULL;
 
 #if defined(DEBUG)
   if (heap_ != NULL && FLAG_verify_on_transition) {
@@ -1986,6 +2000,9 @@ void Isolate::VisitObjectPointers(ObjectPointerVisitor* visitor,
   if (background_compiler() != NULL) {
     background_compiler()->VisitPointers(visitor);
   }
+  if (optimizing_background_compiler() != NULL) {
+    optimizing_background_compiler()->VisitPointers(visitor);
+  }
 
 #if !defined(PRODUCT)
   // Visit objects in the debugger.

runtime/vm/isolate.h

@@ -479,6 +479,10 @@ class Isolate : public BaseIsolate {
     return background_compiler_;
   }
 
+  BackgroundCompiler* optimizing_background_compiler() const {
+    return optimizing_background_compiler_;
+  }
+
 #if !defined(PRODUCT)
   void UpdateLastAllocationProfileAccumulatorResetTimestamp() {
     last_allocationprofile_accumulator_reset_timestamp_ =
@@ -909,9 +913,12 @@ class Isolate : public BaseIsolate {
 
   uint32_t isolate_flags_;
 
-  // Background compilation.
+  // Unoptimized background compilation.
   BackgroundCompiler* background_compiler_;
 
+  // Optimized background compilation.
+  BackgroundCompiler* optimizing_background_compiler_;
+
 // Fields that aren't needed in a product build go here with boolean flags at
 // the top.
 #if !defined(PRODUCT)

runtime/vm/runtime_entry.cc

@@ -2144,6 +2144,9 @@ DEFINE_RUNTIME_ENTRY(OptimizeInvokedFunction, 1) {
   if ((!optimizing_compilation) ||
       Compiler::CanOptimizeFunction(thread, function)) {
     if (FLAG_background_compilation) {
+      if (FLAG_enable_inlining_annotations) {
+        FATAL("Cannot enable inlining annotations and background compilation");
+      }
       Field& field = Field::Handle(zone, isolate->GetDeoptimizingBoxedField());
       while (!field.IsNull()) {
         if (FLAG_trace_optimization || FLAG_trace_field_guards) {
@@ -2154,25 +2157,21 @@ DEFINE_RUNTIME_ENTRY(OptimizeInvokedFunction, 1) {
         // Get next field.
         field = isolate->GetDeoptimizingBoxedField();
       }
-    }
-    // TODO(srdjan): Fix background compilation of regular expressions.
-    if (FLAG_background_compilation) {
-      if (FLAG_enable_inlining_annotations) {
-        FATAL("Cannot enable inlining annotations and background compilation");
-      }
-      if (!BackgroundCompiler::IsDisabled(isolate) &&
+      if (!BackgroundCompiler::IsDisabled(isolate, optimizing_compilation) &&
           function.is_background_optimizable()) {
-        if (FLAG_background_compilation_stop_alot) {
-          BackgroundCompiler::Stop(isolate);
-        }
-        // Reduce the chance of triggering optimization while the function is
-        // being optimized in the background. INT_MIN should ensure that it
-        // takes long time to trigger optimization.
+        // Ensure background compiler is running, if not start it.
+        BackgroundCompiler::Start(isolate);
+        // Reduce the chance of triggering a compilation while the function is
+        // being compiled in the background. INT_MIN should ensure that it
+        // takes long time to trigger a compilation.
         // Note that the background compilation queue rejects duplicate entries.
         function.SetUsageCounter(INT_MIN);
-        BackgroundCompiler::Start(isolate);
-        isolate->background_compiler()->CompileOptimized(function);
-
+        if (optimizing_compilation) {
+          isolate->optimizing_background_compiler()->Compile(function);
+        } else {
+          ASSERT(FLAG_enable_interpreter);
+          isolate->background_compiler()->Compile(function);
+        }
         // Continue in the same code.
         arguments.SetReturn(function);
         return;