[VM] Start the kernel isolate only when compilations are needed

- Start the kernel isolate only when a compilation is needed
- Remove some dead code and state in the loader path

Change-Id: I39e14cd79cda18b155af6334a11635c44e2fc11f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/121923
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Siva Annamalai <asiva@google.com>

Author: a-siva

runtime/bin/loader.cc

@@ -26,6 +26,7 @@ extern DFE dfe;
 #endif
 
 // Keep in sync with loader.dart.
+static const intptr_t _Dart_kInitLoader = 4;
 static const intptr_t _Dart_kImportExtension = 9;
 static const intptr_t _Dart_kResolveAsFilePath = 10;
 
@@ -136,9 +137,6 @@ void Loader::Init(const char* package_root,
   Dart_Port loader_port = Builtin::LoadPort();
   ASSERT(loader_port != ILLEGAL_PORT);
 
-  // Keep in sync with loader.dart.
-  const intptr_t _Dart_kInitLoader = 4;
-
   Dart_Handle request = Dart_NewList(9);
   Dart_ListSetAt(request, 0, trace_loader ? Dart_True() : Dart_False());
   Dart_ListSetAt(request, 1, Dart_NewInteger(Dart_GetMainPortId()));
@@ -552,12 +550,6 @@ Dart_Handle Loader::SendAndProcessReply(intptr_t tag,
   return Dart_Null();
 }
 
-Dart_Handle Loader::LoadUrlContents(Dart_Handle url,
-                                    uint8_t** payload,
-                                    intptr_t* payload_length) {
-  return SendAndProcessReply(Dart_kScriptTag, url, payload, payload_length);
-}
-
 Dart_Handle Loader::ResolveAsFilePath(Dart_Handle url,
                                       uint8_t** payload,
                                       intptr_t* payload_length) {
@@ -575,14 +567,6 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
                                       Dart_Handle url) {
   return Dart_Null();
 }
-
-Dart_Handle Loader::DartColonLibraryTagHandler(Dart_LibraryTag tag,
-                                               Dart_Handle library,
-                                               Dart_Handle url,
-                                               const char* library_url_string,
-                                               const char* url_string) {
-  return Dart_Null();
-}
 #else
 static void MallocFinalizer(void* isolate_callback_data,
                             Dart_WeakPersistentHandle handle,
@@ -593,18 +577,28 @@ static void MallocFinalizer(void* isolate_callback_data,
 Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
                                       Dart_Handle library,
                                       Dart_Handle url) {
+  const char* url_string = NULL;
+  Dart_Handle result = Dart_StringToCString(url, &url_string);
+  if (Dart_IsError(result)) {
+    return result;
+  }
   if (tag == Dart_kCanonicalizeUrl) {
     Dart_Handle library_url = Dart_LibraryUrl(library);
     if (Dart_IsError(library_url)) {
       return library_url;
     }
+    const char* library_url_string = NULL;
+    result = Dart_StringToCString(library_url, &library_url_string);
+    if (Dart_IsError(result)) {
+      return result;
+    }
+    bool is_dart_scheme_url = DartUtils::IsDartSchemeURL(url_string);
+    bool is_dart_library = DartUtils::IsDartSchemeURL(library_url_string);
+    if (is_dart_scheme_url || is_dart_library) {
+      return url;
+    }
     return Dart_DefaultCanonicalizeUrl(library_url, url);
   }
-  const char* url_string = NULL;
-  Dart_Handle result = Dart_StringToCString(url, &url_string);
-  if (Dart_IsError(result)) {
-    return result;
-  }
   if (tag == Dart_kKernelTag) {
     uint8_t* kernel_buffer = NULL;
     intptr_t kernel_buffer_size = 0;
@@ -650,27 +644,6 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
     free(lib_path);
     return result;
   }
-  if (tag != Dart_kScriptTag) {
-    // Special case for handling dart: imports and parts.
-    // Grab the library's url.
-    Dart_Handle library_url = Dart_LibraryUrl(library);
-    if (Dart_IsError(library_url)) {
-      return library_url;
-    }
-    const char* library_url_string = NULL;
-    result = Dart_StringToCString(library_url, &library_url_string);
-    if (Dart_IsError(result)) {
-      return result;
-    }
-
-    bool is_dart_scheme_url = DartUtils::IsDartSchemeURL(url_string);
-    bool is_dart_library = DartUtils::IsDartSchemeURL(library_url_string);
-
-    if (is_dart_scheme_url || is_dart_library) {
-      return DartColonLibraryTagHandler(tag, library, url, library_url_string,
-                                        url_string);
-    }
-  }
   if (DartUtils::IsDartExtensionSchemeURL(url_string)) {
     // Handle early error cases for dart-ext: imports.
     if (tag != Dart_kImportTag) {
@@ -685,15 +658,7 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
 
   auto isolate_data = reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
   ASSERT(isolate_data != NULL);
-  if ((tag == Dart_kScriptTag) && Dart_IsString(library)) {
-    // Update packages file for isolate.
-    const char* packages_file = NULL;
-    Dart_Handle result = Dart_StringToCString(library, &packages_file);
-    if (Dart_IsError(result)) {
-      return result;
-    }
-    isolate_data->UpdatePackagesFile(packages_file);
-  }
+
   // Grab this isolate's loader.
   Loader* loader = NULL;
 
@@ -712,10 +677,8 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
     loader = new Loader(isolate_data);
     loader->Init(isolate_data->isolate_group_data()->package_root,
                  isolate_data->packages_file(),
-                 DartUtils::original_working_directory,
-                 (tag == Dart_kScriptTag) ? url_string : NULL);
+                 DartUtils::original_working_directory, NULL);
   } else {
-    ASSERT(tag != Dart_kScriptTag);
     // The isolate has a loader -- this is an inner invocation that will queue
     // work with the service isolate.
     // Use the existing loader.
@@ -793,21 +756,6 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
   }
   return Dart_Null();
 }
-
-Dart_Handle Loader::DartColonLibraryTagHandler(Dart_LibraryTag tag,
-                                               Dart_Handle library,
-                                               Dart_Handle url,
-                                               const char* library_url_string,
-                                               const char* url_string) {
-  // Handle canonicalization, 'import' and 'part' of 'dart:' libraries.
-  if (tag == Dart_kCanonicalizeUrl) {
-    // These will be handled internally.
-    return url;
-  }
-  // All cases should have been handled above.
-  UNREACHABLE();
-  return Dart_Null();
-}
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
 
 void Loader::InitOnce() {

runtime/bin/loader.h

@@ -25,11 +25,6 @@ class Loader {
 
   static Dart_Handle ReloadNativeExtensions();
 
-  // Loads contents of the specified url.
-  static Dart_Handle LoadUrlContents(Dart_Handle url,
-                                     uint8_t** payload,
-                                     intptr_t* payload_length);
-
   static void ResolveDependenciesAsFilePaths();
 
   // A static tag handler that hides all usage of a loader for an isolate.
@@ -122,13 +117,6 @@ class Loader {
   /// Returns false if an error occurred and the loader should quit.
   bool ProcessQueueLocked(ProcessResult process_result);
 
-  // Special inner tag handler for dart: uris.
-  static Dart_Handle DartColonLibraryTagHandler(Dart_LibraryTag tag,
-                                                Dart_Handle library,
-                                                Dart_Handle url,
-                                                const char* library_url_string,
-                                                const char* url_string);
-
   // We use one native message handler callback for N loaders. The native
   // message handler callback provides us with the Dart_Port which we use as a
   // key into our map of active loaders from |port| to |isolate_data|.

runtime/bin/main.cc

@@ -408,11 +408,7 @@ static Dart_Isolate IsolateSetupHelper(Dart_Isolate isolate,
     Dart_Handle uri =
         DartUtils::ResolveScript(Dart_NewStringFromCString(script_uri));
     CHECK_RESULT(uri);
-    if (kernel_buffer == NULL) {
-      result = Loader::LibraryTagHandler(Dart_kScriptTag, Dart_Null(), uri);
-      CHECK_RESULT(result);
-    } else {
-      // Various core-library parts will send requests to the Loader to resolve
+    if (kernel_buffer != NULL) {
       // relative URIs and perform other related tasks. We need Loader to be
       // initialized for this to work because loading from Kernel binary
       // bypasses normal source code loading paths that initialize it.
@@ -421,7 +417,6 @@ static Dart_Isolate IsolateSetupHelper(Dart_Isolate isolate,
       CHECK_RESULT(result);
       Loader::InitForSnapshot(resolved_script_uri, isolate_data);
     }
-
     Dart_TimelineEvent("LoadScript", Dart_TimelineGetMicros(),
                        Dart_GetMainPortId(), Dart_Timeline_Event_Async_End, 0,
                        NULL, NULL);

runtime/bin/vmservice/loader.dart

@@ -952,9 +952,6 @@ void shutdownLoaders() {
 
 // See Dart_LibraryTag in dart_api.h
 const _Dart_kCanonicalizeUrl = 0; // Canonicalize the URL.
-const _Dart_kScriptTag = 1; // Load the root script.
-const _Dart_kSourceTag = 2; // Load a part source.
-const _Dart_kImportTag = 3; // Import a library.
 
 // Extra requests. Keep these in sync between loader.dart and builtin.dart.
 const _Dart_kInitLoader = 4; // Initialize the loader.

runtime/include/dart_api.h

@@ -2908,8 +2908,6 @@ DART_EXPORT Dart_Handle Dart_GetNativeSymbol(Dart_Handle library,
 
 typedef enum {
   Dart_kCanonicalizeUrl = 0,
-  Dart_kScriptTag,
-  Dart_kSourceTag,
   Dart_kImportTag,
   Dart_kKernelTag,
   Dart_kImportExtensionTag,
@@ -2930,14 +2928,6 @@ typedef enum {
  * of this tag.  The return value should be a string holding the
  * canonicalized url.
  *
- * Dart_kScriptTag
- *
- * No longer used.
- *
- * Dart_kSourceTag
- *
- * No longer used.
- *
  * Dart_kImportTag
  *
  * This tag is used to load a library from IsolateMirror.loadUri. The embedder

runtime/vm/dart.cc

@@ -401,7 +401,7 @@ char* Dart::Init(const uint8_t* vm_isolate_snapshot,
 
 #ifndef DART_PRECOMPILED_RUNTIME
   if (start_kernel_isolate) {
-    KernelIsolate::Run();
+    KernelIsolate::InitializeState();
   }
 #endif  // DART_PRECOMPILED_RUNTIME
 

runtime/vm/kernel_isolate.cc

@@ -61,7 +61,7 @@ const int KernelIsolate::kNotifyIsolateShutdown = 6;
 const char* KernelIsolate::kName = DART_KERNEL_ISOLATE_NAME;
 Dart_IsolateGroupCreateCallback KernelIsolate::create_group_callback_ = NULL;
 Monitor* KernelIsolate::monitor_ = new Monitor();
-KernelIsolate::State KernelIsolate::state_ = KernelIsolate::kStopped;
+KernelIsolate::State KernelIsolate::state_ = KernelIsolate::kNotStarted;
 Isolate* KernelIsolate::isolate_ = NULL;
 Dart_Port KernelIsolate::kernel_port_ = ILLEGAL_PORT;
 
@@ -217,30 +217,45 @@ class RunKernelTask : public ThreadPool::Task {
   }
 };
 
-void KernelIsolate::Run() {
-  {
-    MonitorLocker ml(monitor_);
-    ASSERT(state_ == kStopped);
-    state_ = kStarting;
-    ml.NotifyAll();
-  }
+void KernelIsolate::InitializeState() {
   // Grab the isolate create callback here to avoid race conditions with tests
   // that change this after Dart_Initialize returns.
+  if (FLAG_trace_kernel) {
+    OS::PrintErr(DART_KERNEL_ISOLATE_NAME ": InitializeState\n");
+  }
   create_group_callback_ = Isolate::CreateGroupCallback();
   if (create_group_callback_ == NULL) {
     KernelIsolate::InitializingFailed();
     return;
   }
-  bool task_started = Dart::thread_pool()->Run<RunKernelTask>();
-  ASSERT(task_started);
+}
+
+bool KernelIsolate::Start() {
+  bool start_task = false;
+  {
+    MonitorLocker ml(monitor_);
+    if (state_ == kNotStarted) {
+      if (FLAG_trace_kernel) {
+        OS::PrintErr(DART_KERNEL_ISOLATE_NAME ": Start\n");
+      }
+      start_task = true;
+      state_ = kStarting;
+      ml.NotifyAll();
+    }
+  }
+  bool task_started = true;
+  if (start_task) {
+    task_started = Dart::thread_pool()->Run<RunKernelTask>();
+  }
+  return task_started;
 }
 
 void KernelIsolate::Shutdown() {
   MonitorLocker ml(monitor_);
   while (state_ == kStarting) {
     ml.Wait();
   }
-  if (state_ == kStopped) {
+  if (state_ == kStopped || state_ == kNotStarted) {
     return;
   }
   ASSERT(state_ == kStarted);
@@ -824,6 +839,14 @@ Dart_KernelCompilationResult KernelIsolate::CompileToKernel(
     const char* package_config,
     const char* multiroot_filepaths,
     const char* multiroot_scheme) {
+  // Start the kernel Isolate if it is not already running.
+  if (!Start()) {
+    Dart_KernelCompilationResult result = {};
+    result.status = Dart_KernelCompilationStatus_Unknown;
+    result.error = strdup("Error while starting Kernel isolate task");
+    return result;
+  }
+
   // This must be the main script to be loaded. Wait for Kernel isolate
   // to finish initialization.
   Dart_Port kernel_port = WaitForKernelPort();

runtime/vm/kernel_isolate.h

@@ -29,7 +29,8 @@ class KernelIsolate : public AllStatic {
   static const int kListDependenciesTag;
   static const int kNotifyIsolateShutdown;
 
-  static void Run();
+  static void InitializeState();
+  static bool Start();
   static void Shutdown();
 
   static bool NameEquals(const char* name);
@@ -83,6 +84,7 @@ class KernelIsolate : public AllStatic {
   static Dart_IsolateGroupCreateCallback create_group_callback_;
   static Monitor* monitor_;
   enum State {
+    kNotStarted,
     kStopped,
     kStarting,
     kStarted,

runtime/vm/unit_test.cc

@@ -359,11 +359,6 @@ static Dart_Handle LibraryTagHandler(Dart_LibraryTag tag,
     }
     return Dart_DefaultCanonicalizeUrl(library_url, url);
   }
-  if (tag == Dart_kScriptTag) {
-    // Reload request.
-    UNREACHABLE();
-    return Dart_Null();
-  }
   if (!Dart_IsLibrary(library)) {
     return Dart_NewApiError("not a library");
   }
@@ -428,14 +423,8 @@ static Dart_Handle LibraryTagHandler(Dart_LibraryTag tag,
   // Do sync loading since unit_test doesn't support async.
   Dart_Handle source = DartUtils::ReadStringFromFile(resolved_url_chars);
   EXPECT_VALID(source);
-  if (tag == Dart_kImportTag) {
-    UNREACHABLE();
-    return Dart_Null();
-  } else {
-    ASSERT(tag == Dart_kSourceTag);
-    UNREACHABLE();
-    return Dart_Null();
-  }
+  UNREACHABLE();
+  return Dart_Null();
 }
 
 static intptr_t BuildSourceFilesArray(

sdk/lib/_internal/vm/bin/builtin.dart

@@ -45,7 +45,6 @@ int _isolateId;
 // Requests made to the service isolate over the load port.
 
 // Extra requests. Keep these in sync between loader.dart and builtin.dart.
-const _Dart_kInitLoader = 4; // Initialize the loader.
 const _Dart_kResourceLoad = 5; // Resource class support.
 const _Dart_kGetPackageRootUri = 6; // Uri of the packages/ directory.
 const _Dart_kGetPackageConfigUri = 7; // Uri of the .packages file.