- reorganize DartUtils::PrepareForScriptLoading so that it does not have the wait for service load …

runtime/bin/dartutils.cc

Index: runtime/bin/dartutils.cc
diff --git a/runtime/bin/dartutils.cc b/runtime/bin/dartutils.cc
index 89150111d022a6e160a37ed824a636d2629916c8..1cf9937c5c4c7e55137025a98a90cd25f8d596ac 100644
--- a/runtime/bin/dartutils.cc
+++ b/runtime/bin/dartutils.cc
@@ -305,14 +305,19 @@ Dart_Handle DartUtils::MakeUint8Array(const uint8_t* buffer, intptr_t len) {
 }
 
 
-Dart_Handle DartUtils::SetWorkingDirectory(Dart_Handle builtin_lib) {
+Dart_Handle DartUtils::SetWorkingDirectory() {
+  IsolateData* isolate_data =

[zra, 2016/02/04 04:12:41]

This pattern is repeated a lot. Maybe something like this would useful:

Dart_Handle DartUtils::BuiltinLib() {
  IsolateData* isolate_data =
    reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
  return isolate_data->builtin_lib();
}

[siva, 2016/02/04 16:59:00]

Done.

+      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+  Dart_Handle builtin_lib = isolate_data->builtin_lib();
   Dart_Handle directory = NewString(original_working_directory);

[zra, 2016/02/04 04:12:41]

RETURN_IF_ERROR(directory)

Sometimes we check the result of NewString to see if it's an error and sometimes
we don't. I suspect that checking all of them is the safer bet so that we don't
end up going back into Dart code after we could have detected e.g. an OOM
exception.

Maybe not in this CL, though.

[siva, 2016/02/04 16:59:00]

NewString is used in a number of places sometimes inline as a parameter, it is
going to require a lot of changes. Maybe another CL or a cleanup when time
permits.

   return SingleArgDart_Invoke(builtin_lib, "_setWorkingDirectory", directory);
 }
 
 
-Dart_Handle DartUtils::ResolveUriInWorkingDirectory(Dart_Handle script_uri,
-                                                    Dart_Handle builtin_lib) {
+Dart_Handle DartUtils::ResolveUriInWorkingDirectory(Dart_Handle script_uri) {
+  IsolateData* isolate_data =
+      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+  Dart_Handle builtin_lib = isolate_data->builtin_lib();
   const int kNumArgs = 1;
   Dart_Handle dart_args[kNumArgs];
   dart_args[0] = script_uri;
@@ -323,8 +328,10 @@ Dart_Handle DartUtils::ResolveUriInWorkingDirectory(Dart_Handle script_uri,
 }
 
 
-Dart_Handle DartUtils::FilePathFromUri(Dart_Handle script_uri,
-                                       Dart_Handle builtin_lib) {
+Dart_Handle DartUtils::FilePathFromUri(Dart_Handle script_uri) {
+  IsolateData* isolate_data =
+      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+  Dart_Handle builtin_lib = isolate_data->builtin_lib();
   const int kNumArgs = 1;
   Dart_Handle dart_args[kNumArgs];
   dart_args[0] = script_uri;
@@ -335,8 +342,10 @@ Dart_Handle DartUtils::FilePathFromUri(Dart_Handle script_uri,
 }
 
 
-Dart_Handle DartUtils::ExtensionPathFromUri(Dart_Handle extension_uri,
-                                            Dart_Handle builtin_lib) {
+Dart_Handle DartUtils::ExtensionPathFromUri(Dart_Handle extension_uri) {
+  IsolateData* isolate_data =
+      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+  Dart_Handle builtin_lib = isolate_data->builtin_lib();
   const int kNumArgs = 1;
   Dart_Handle dart_args[kNumArgs];
   dart_args[0] = extension_uri;
@@ -347,9 +356,10 @@ Dart_Handle DartUtils::ExtensionPathFromUri(Dart_Handle extension_uri,
 }
 
 
-Dart_Handle DartUtils::ResolveUri(Dart_Handle library_url,
-                                  Dart_Handle url,
-                                  Dart_Handle builtin_lib) {
+Dart_Handle DartUtils::ResolveUri(Dart_Handle library_url, Dart_Handle url) {
+  IsolateData* isolate_data =
+      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+  Dart_Handle builtin_lib = isolate_data->builtin_lib();
   const int kNumArgs = 2;
   Dart_Handle dart_args[kNumArgs];
   dart_args[0] = library_url;
@@ -361,8 +371,10 @@ Dart_Handle DartUtils::ResolveUri(Dart_Handle library_url,
 
 static Dart_Handle LoadDataAsync_Invoke(Dart_Handle tag,
                                         Dart_Handle url,
-                                        Dart_Handle library_url,
-                                        Dart_Handle builtin_lib) {
+                                        Dart_Handle library_url) {
+  IsolateData* isolate_data =
+      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+  Dart_Handle builtin_lib = isolate_data->builtin_lib();
   const int kNumArgs = 3;
   Dart_Handle dart_args[kNumArgs];
   dart_args[0] = tag;
@@ -407,10 +419,7 @@ Dart_Handle DartUtils::LibraryTagHandler(Dart_LibraryTag tag,
       return url;
     }
     // Resolve the url within the context of the library's URL.
-    Dart_Handle builtin_lib =
-        Builtin::LoadAndCheckLibrary(Builtin::kBuiltinLibrary);
-    RETURN_IF_ERROR(builtin_lib);
-    return ResolveUri(library_url, url, builtin_lib);
+    return ResolveUri(library_url, url);
   }
 
   // Handle 'import' of dart scheme URIs (i.e they start with 'dart:').
@@ -447,15 +456,12 @@ Dart_Handle DartUtils::LibraryTagHandler(Dart_LibraryTag tag,
     }
   }
 
-  Dart_Handle builtin_lib =
-      Builtin::LoadAndCheckLibrary(Builtin::kBuiltinLibrary);
-  RETURN_IF_ERROR(builtin_lib);
   if (DartUtils::IsDartExtensionSchemeURL(url_string)) {
     // Load a native code shared library to use in a native extension
     if (tag != Dart_kImportTag) {
       return NewError("Dart extensions must use import: '%s'", url_string);
     }
-    Dart_Handle path_parts = DartUtils::ExtensionPathFromUri(url, builtin_lib);
+    Dart_Handle path_parts = DartUtils::ExtensionPathFromUri(url);
     if (Dart_IsError(path_parts)) {
       return path_parts;
     }
@@ -474,10 +480,7 @@ Dart_Handle DartUtils::LibraryTagHandler(Dart_LibraryTag tag,
 
   // Handle 'import' or 'part' requests for all other URIs. Call dart code to
   // read the source code asynchronously.
-  return LoadDataAsync_Invoke(Dart_NewInteger(tag),
-                              url,
-                              library_url,
-                              builtin_lib);
+  return LoadDataAsync_Invoke(Dart_NewInteger(tag), url, library_url);
 }
 
 
@@ -509,13 +512,12 @@ void DartUtils::WriteMagicNumber(File* file) {
 }
 
 
-Dart_Handle DartUtils::LoadScript(const char* script_uri,
-                                  Dart_Handle builtin_lib) {
+Dart_Handle DartUtils::LoadScript(const char* script_uri) {
   Dart_Handle uri = Dart_NewStringFromCString(script_uri);
   IsolateData* isolate_data =
       reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
   Dart_TimelineAsyncBegin("LoadScript", &(isolate_data->load_async_id));
-  return LoadDataAsync_Invoke(Dart_Null(), uri, Dart_Null(), builtin_lib);
+  return LoadDataAsync_Invoke(Dart_Null(), uri, Dart_Null());
 }
 
 
@@ -656,9 +658,7 @@ void FUNCTION_NAME(Builtin_GetCurrentDirectory)(Dart_NativeArguments args) {
 Dart_Handle DartUtils::PrepareBuiltinLibrary(Dart_Handle builtin_lib,
                                              Dart_Handle internal_lib,
                                              bool is_service_isolate,
-                                             bool trace_loading,
-                                             const char* package_root,
-                                             const char* packages_config) {
+                                             bool trace_loading) {
   // Setup the internal library's 'internalPrint' function.
   Dart_Handle print = Dart_Invoke(
       builtin_lib, NewString("_getPrintClosure"), 0, NULL);
@@ -678,41 +678,7 @@ Dart_Handle DartUtils::PrepareBuiltinLibrary(Dart_Handle builtin_lib,
       RETURN_IF_ERROR(result);
     }
     // Set current working directory.
-    result = SetWorkingDirectory(builtin_lib);
-    RETURN_IF_ERROR(result);
-    // Wait for the service isolate to initialize the load port.
-    Dart_Port load_port = Dart_ServiceWaitForLoadPort();
-    if (load_port == ILLEGAL_PORT) {
-      return Dart_NewUnhandledExceptionError(
-          NewDartUnsupportedError("Service did not return load port."));
-    }
-    result = Builtin::SetLoadPort(load_port);
-    RETURN_IF_ERROR(result);
-  }
-
-  // Set up package root if specified.
-  if (package_root != NULL) {
-    ASSERT(packages_config == NULL);
-    result = NewString(package_root);
-    RETURN_IF_ERROR(result);
-    const int kNumArgs = 1;
-    Dart_Handle dart_args[kNumArgs];
-    dart_args[0] = result;
-    result = Dart_Invoke(builtin_lib,
-                         NewString("_setPackageRoot"),
-                         kNumArgs,
-                         dart_args);
-    RETURN_IF_ERROR(result);
-  } else if (packages_config != NULL) {
-    result = NewString(packages_config);
-    RETURN_IF_ERROR(result);
-    const int kNumArgs = 1;
-    Dart_Handle dart_args[kNumArgs];
-    dart_args[0] = result;
-    result = Dart_Invoke(builtin_lib,
-                         NewString("_loadPackagesMap"),
-                         kNumArgs,
-                         dart_args);
+    result = SetWorkingDirectory();
     RETURN_IF_ERROR(result);
   }
   return Dart_True();
@@ -759,11 +725,56 @@ Dart_Handle DartUtils::PrepareIsolateLibrary(Dart_Handle isolate_lib) {
 }
 
 
-Dart_Handle DartUtils::PrepareForScriptLoading(const char* package_root,
-                                               const char* packages_config,
-                                               bool is_service_isolate,
-                                               bool trace_loading,
-                                               Dart_Handle builtin_lib) {
+Dart_Handle DartUtils::SetupServiceLoadPort() {
+  // Wait for the service isolate to initialize the load port.
+  Dart_Port load_port = Dart_ServiceWaitForLoadPort();
+  if (load_port == ILLEGAL_PORT) {
+    return Dart_NewUnhandledExceptionError(
+        NewDartUnsupportedError("Service did not return load port."));
+  }
+  return Builtin::SetLoadPort(load_port);
+}
+
+
+Dart_Handle DartUtils::SetupPackageRoot(const char* package_root,
+                                        const char* packages_config) {
+  // Set up package root if specified.
+  if (package_root != NULL) {
+    ASSERT(packages_config == NULL);
+    Dart_Handle result = NewString(package_root);
+    RETURN_IF_ERROR(result);
+    IsolateData* isolate_data =
+        reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+    Dart_Handle builtin_lib = isolate_data->builtin_lib();
+    const int kNumArgs = 1;
+    Dart_Handle dart_args[kNumArgs];
+    dart_args[0] = result;
+    result = Dart_Invoke(builtin_lib,
+                         NewString("_setPackageRoot"),
+                         kNumArgs,
+                         dart_args);
+    RETURN_IF_ERROR(result);
+  } else if (packages_config != NULL) {
+    Dart_Handle result = NewString(packages_config);
+    RETURN_IF_ERROR(result);
+    IsolateData* isolate_data =
+        reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+    Dart_Handle builtin_lib = isolate_data->builtin_lib();
+    const int kNumArgs = 1;
+    Dart_Handle dart_args[kNumArgs];
+    dart_args[0] = result;
+    result = Dart_Invoke(builtin_lib,
+                         NewString("_loadPackagesMap"),
+                         kNumArgs,
+                         dart_args);
+    RETURN_IF_ERROR(result);
+  }
+  return Dart_True();
+}
+
+
+Dart_Handle DartUtils::PrepareForScriptLoading(bool is_service_isolate,
+                                               bool trace_loading) {
   // First ensure all required libraries are available.
   Dart_Handle url = NewString(kCoreLibURL);
   RETURN_IF_ERROR(url);
@@ -781,9 +792,19 @@ Dart_Handle DartUtils::PrepareForScriptLoading(const char* package_root,
   RETURN_IF_ERROR(url);
   Dart_Handle internal_lib = Dart_LookupLibrary(url);
   RETURN_IF_ERROR(internal_lib);
+  Dart_Handle builtin_lib =
+      Builtin::LoadAndCheckLibrary(Builtin::kBuiltinLibrary);
+  RETURN_IF_ERROR(builtin_lib);
   Dart_Handle io_lib = Builtin::LoadAndCheckLibrary(Builtin::kIOLibrary);
   RETURN_IF_ERROR(io_lib);
 
+  // Setup the builtin library in a persistent handle attached the isolate
+  // specific data as we seem to lookup and use builtin lib a lot.
+  IsolateData* isolate_data =
+      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+  ASSERT(isolate_data != NULL);
+  isolate_data->set_builtin_lib(builtin_lib);
+
   // We need to ensure that all the scripts loaded so far are finalized
   // as we are about to invoke some Dart code below to setup closures.
   Dart_Handle result = Dart_FinalizeLoading(false);
@@ -792,9 +813,7 @@ Dart_Handle DartUtils::PrepareForScriptLoading(const char* package_root,
   result = PrepareBuiltinLibrary(builtin_lib,
                                  internal_lib,
                                  is_service_isolate,
-                                 trace_loading,
-                                 package_root,
-                                 packages_config);
+                                 trace_loading);
   RETURN_IF_ERROR(result);
 
   RETURN_IF_ERROR(PrepareAsyncLibrary(async_lib, isolate_lib));