[VM] More cleanup in the loader code

- Removed ResolveDependenciesAsFilePaths functionality as the loader
  does not track any dependencies (this is now provided by the kernel
  isolate)

- Removed ResolveAsFilePath functionality from the loader class

- Removed some left over code related to deferred loading

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

Author: a-siva

runtime/bin/isolate_data.cc

@@ -17,7 +17,6 @@ IsolateGroupData::IsolateGroupData(const char* url,
     : script_url((url != NULL) ? strdup(url) : NULL),
       package_root(NULL),
       app_snapshot_(app_snapshot),
-      dependencies_(NULL),
       resolved_packages_config_(NULL),
       kernel_buffer_(NULL),
       kernel_buffer_size_(0),
@@ -41,7 +40,6 @@ IsolateGroupData::~IsolateGroupData() {
   resolved_packages_config_ = NULL;
   kernel_buffer_ = NULL;
   kernel_buffer_size_ = 0;
-  delete dependencies_;
 }
 
 IsolateData::IsolateData(IsolateGroupData* isolate_group_data)

runtime/bin/isolate_data.h

@@ -88,11 +88,6 @@ class IsolateGroupData {
     resolved_packages_config_ = strdup(packages_config);
   }
 
-  MallocGrowableArray<char*>* dependencies() const { return dependencies_; }
-  void set_dependencies(MallocGrowableArray<char*>* deps) {
-    dependencies_ = deps;
-  }
-
   bool RunFromAppSnapshot() const {
     // If the main isolate is using an app snapshot the [app_snapshot_] pointer
     // will be still nullptr (see main.cc:CreateIsolateGroupAndSetupHelper)
@@ -106,7 +101,6 @@ class IsolateGroupData {
   friend class IsolateData;  // For packages_file_
 
   std::unique_ptr<AppSnapshot> app_snapshot_;
-  MallocGrowableArray<char*>* dependencies_;
   char* resolved_packages_config_;
   std::shared_ptr<uint8_t> kernel_buffer_;
   intptr_t kernel_buffer_size_;

runtime/bin/loader.cc

@@ -28,7 +28,6 @@ extern DFE dfe;
 // 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;
 
 Loader::Loader(IsolateData* isolate_data)
     : port_(ILLEGAL_PORT),
@@ -259,46 +258,6 @@ static bool PathContainsSeparator(const char* path) {
           (strstr(path, File::PathSeparator()) != NULL));
 }
 
-void Loader::AddDependencyLocked(Loader* loader, const char* resolved_uri) {
-  MallocGrowableArray<char*>* dependencies =
-      loader->isolate_group_data()->dependencies();
-  if (dependencies == NULL) {
-    return;
-  }
-  dependencies->Add(strdup(resolved_uri));
-}
-
-void Loader::ResolveDependenciesAsFilePaths() {
-  IsolateGroupData* isolate_group_data =
-      reinterpret_cast<IsolateGroupData*>(Dart_CurrentIsolateGroupData());
-  ASSERT(isolate_group_data != NULL);
-  MallocGrowableArray<char*>* dependencies = isolate_group_data->dependencies();
-  if (dependencies == NULL) {
-    return;
-  }
-
-  for (intptr_t i = 0; i < dependencies->length(); i++) {
-    char* resolved_uri = (*dependencies)[i];
-
-    uint8_t* file_path = NULL;
-    intptr_t file_path_length = -1;
-    Dart_Handle uri = Dart_NewStringFromCString(resolved_uri);
-    ASSERT(!Dart_IsError(uri));
-    Dart_Handle result =
-        Loader::ResolveAsFilePath(uri, &file_path, &file_path_length);
-    if (Dart_IsError(result)) {
-      Syslog::Print("Error resolving dependency: %s\n", Dart_GetError(result));
-      return;
-    }
-
-    // Convert buffer buffer to NUL-terminated string.
-    (*dependencies)[i] = Utils::StrNDup(
-        reinterpret_cast<const char*>(file_path), file_path_length);
-    free(file_path);
-    free(resolved_uri);
-  }
-}
-
 class ScopedDecompress : public ValueObject {
  public:
   ScopedDecompress(const uint8_t** payload, intptr_t* payload_length)
@@ -343,17 +302,14 @@ bool Loader::ProcessResultLocked(Loader* loader, Loader::IOResult* result) {
         Dart_NewStringFromCString(reinterpret_cast<char*>(result->library_uri));
   }
 
-  AddDependencyLocked(loader, result->resolved_uri);
-
   // A negative result tag indicates a loading error occurred in the service
   // isolate. The payload is a C string of the error message.
   if (result->tag < 0) {
     Dart_Handle library = Dart_LookupLibrary(uri);
     Dart_Handle error =
         Dart_NewStringFromUTF8(result->payload, result->payload_length);
     // If a library with the given uri exists, give it a chance to handle
-    // the error. If the load requests stems from a deferred library load,
-    // an IO error is not fatal.
+    // the error.
     if (LibraryHandleError(library, error)) {
       return true;
     }
@@ -421,23 +377,6 @@ bool Loader::ProcessResultLocked(Loader* loader, Loader::IOResult* result) {
   return false;
 }
 
-bool Loader::ProcessPayloadResultLocked(Loader* loader,
-                                        Loader::IOResult* result) {
-  // A negative result tag indicates a loading error occurred in the service
-  // isolate. The payload is a C string of the error message.
-  if (result->tag < 0) {
-    Dart_Handle error =
-        Dart_NewStringFromUTF8(result->payload, result->payload_length);
-    loader->error_ = Dart_NewUnhandledExceptionError(error);
-    return false;
-  }
-  loader->payload_length_ = result->payload_length;
-  loader->payload_ =
-      reinterpret_cast<uint8_t*>(::malloc(loader->payload_length_));
-  memmove(loader->payload_, result->payload, loader->payload_length_);
-  return true;
-}
-
 bool Loader::ProcessQueueLocked(ProcessResult process_result) {
   bool hit_error = false;
   for (intptr_t i = 0; i < results_length(); i++) {
@@ -510,53 +449,6 @@ Dart_Handle Loader::ReloadNativeExtensions() {
   return Dart_True();
 }
 
-Dart_Handle Loader::SendAndProcessReply(intptr_t tag,
-                                        Dart_Handle url,
-                                        uint8_t** payload,
-                                        intptr_t* payload_length) {
-  auto isolate_data = reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
-  ASSERT(isolate_data != NULL);
-  ASSERT(!isolate_data->HasLoader());
-  Loader* loader = NULL;
-
-  // Setup the loader. The constructor does a bunch of leg work.
-  loader = new Loader(isolate_data);
-  loader->Init(isolate_data->isolate_group_data()->package_root,
-               isolate_data->packages_file(),
-               DartUtils::original_working_directory, NULL);
-  ASSERT(loader != NULL);
-  ASSERT(isolate_data->HasLoader());
-
-  // Now send a load request to the service isolate.
-  loader->SendRequest(tag, url, Dart_Null());
-
-  // Wait for a reply to the load request.
-  loader->BlockUntilComplete(ProcessPayloadResultLocked);
-
-  // Copy fields from the loader before deleting it.
-  // The payload array itself which was malloced above is freed by
-  // the caller of LoadUrlContents.
-  Dart_Handle error = loader->error();
-  *payload = loader->payload_;
-  *payload_length = loader->payload_length_;
-
-  // Destroy the loader. The destructor does a bunch of leg work.
-  delete loader;
-
-  // An error occurred during loading.
-  if (!Dart_IsNull(error)) {
-    return error;
-  }
-  return Dart_Null();
-}
-
-Dart_Handle Loader::ResolveAsFilePath(Dart_Handle url,
-                                      uint8_t** payload,
-                                      intptr_t* payload_length) {
-  return SendAndProcessReply(_Dart_kResolveAsFilePath, url, payload,
-                             payload_length);
-}
-
 IsolateGroupData* Loader::isolate_group_data() {
   return isolate_data_->isolate_group_data();
 }
@@ -662,13 +554,6 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
   // Grab this isolate's loader.
   Loader* loader = NULL;
 
-  // The outer invocation of the tag handler for this isolate. We make the outer
-  // invocation block and allow any nested invocations to operate in parallel.
-  const bool blocking_call = !isolate_data->HasLoader();
-
-  // If we are the outer invocation of the tag handler and the tag is an import
-  // this means that we are starting a deferred library load.
-  const bool is_deferred_import = blocking_call && (tag == Dart_kImportTag);
   if (!isolate_data->HasLoader()) {
     // The isolate doesn't have a loader -- this is the outer invocation which
     // will block.
@@ -717,42 +602,25 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
     }
   }
 
-  if (blocking_call) {
-    // The outer invocation of the tag handler will block here until all nested
-    // invocations complete.
-    loader->BlockUntilComplete(ProcessResultLocked);
-
-    // Remember the error (if any).
-    Dart_Handle error = loader->error();
-    // Destroy the loader. The destructor does a bunch of leg work.
-    delete loader;
-
-    // An error occurred during loading.
-    if (!Dart_IsNull(error)) {
-      if (false && is_deferred_import) {
-        // This blocks handles transitive load errors caused by a deferred
-        // import. Non-transitive load errors are handled above (see callers of
-        // |LibraryHandleError|). To handle the transitive case, we give the
-        // originating deferred library an opportunity to handle it.
-        Dart_Handle deferred_library = Dart_LookupLibrary(url);
-        if (!LibraryHandleError(deferred_library, error)) {
-          // Library did not handle it, return to caller as an unhandled
-          // exception.
-          return Dart_NewUnhandledExceptionError(error);
-        }
-      } else {
-        // We got an error during loading but we aren't loading a deferred
-        // library, return the error to the caller.
-        return error;
-      }
-    }
+  // The outer invocation of the tag handler will block here until all nested
+  // invocations complete.
+  loader->BlockUntilComplete(ProcessResultLocked);
 
-    // Finalize loading. This will complete any futures for completed deferred
-    // loads.
-    error = Dart_FinalizeLoading(true);
-    if (Dart_IsError(error)) {
-      return error;
-    }
+  // Remember the error (if any).
+  Dart_Handle error = loader->error();
+  // Destroy the loader. The destructor does a bunch of leg work.
+  delete loader;
+
+  // An error occurred during loading.
+  if (!Dart_IsNull(error)) {
+    // We got an error during loading, return the error to the caller.
+    return error;
+  }
+
+  // Finalize loading.
+  error = Dart_FinalizeLoading(true);
+  if (Dart_IsError(error)) {
+    return error;
   }
   return Dart_Null();
 }

runtime/bin/loader.h

@@ -25,8 +25,6 @@ class Loader {
 
   static Dart_Handle ReloadNativeExtensions();
 
-  static void ResolveDependenciesAsFilePaths();
-
   // A static tag handler that hides all usage of a loader for an isolate.
   static Dart_Handle LibraryTagHandler(Dart_LibraryTag tag,
                                        Dart_Handle library,
@@ -90,30 +88,15 @@ class Loader {
   // Send a request from the tag handler to the service isolate.
   void SendRequest(intptr_t tag, Dart_Handle url, Dart_Handle library_url);
 
-  static Dart_Handle SendAndProcessReply(intptr_t tag,
-                                         Dart_Handle url,
-                                         uint8_t** payload,
-                                         intptr_t* payload_length);
-
-  static Dart_Handle ResolveAsFilePath(Dart_Handle url,
-                                       uint8_t** payload,
-                                       intptr_t* payload_length);
-
   /// Queue |message| and notify the loader that a message is available.
   void QueueMessage(Dart_CObject* message);
 
   /// Blocks the caller until the loader is finished.
   void BlockUntilComplete(ProcessResult process_result);
 
-  /// Saves a script dependency when applicable.
-  static void AddDependencyLocked(Loader* loader, const char* resolved_uri);
-
   /// Returns false if |result| is an error and the loader should quit.
   static bool ProcessResultLocked(Loader* loader, IOResult* result);
 
-  /// Returns false if |result| is an error and the loader should quit.
-  static bool ProcessPayloadResultLocked(Loader* loader, IOResult* result);
-
   /// Returns false if an error occurred and the loader should quit.
   bool ProcessQueueLocked(ProcessResult process_result);
 

runtime/bin/main.cc

@@ -131,12 +131,6 @@ static void WriteDepsFile(Dart_Isolate isolate) {
   if (Options::depfile() == NULL) {
     return;
   }
-  Loader::ResolveDependenciesAsFilePaths();
-  auto isolate_group_data =
-      reinterpret_cast<IsolateGroupData*>(Dart_IsolateGroupData(isolate));
-  ASSERT(isolate_group_data != NULL);
-  MallocGrowableArray<char*>* dependencies = isolate_group_data->dependencies();
-  ASSERT(dependencies != NULL);
   File* file =
       File::Open(NULL, Options::depfile(), File::kWriteTruncate);
   if (file == NULL) {
@@ -149,11 +143,6 @@ static void WriteDepsFile(Dart_Isolate isolate) {
   } else {
     success &= file->Print("%s: ", Options::depfile_output_filename());
   }
-  for (intptr_t i = 0; i < dependencies->length(); i++) {
-    char* dep = dependencies->At(i);
-    success &= file->Print("%s ", dep);
-    free(dep);
-  }
   if (kernel_isolate_is_running) {
     Dart_KernelCompilationResult result = Dart_KernelListDependencies();
     if (result.status != Dart_KernelCompilationStatus_Ok) {
@@ -171,7 +160,6 @@ static void WriteDepsFile(Dart_Isolate isolate) {
               Options::depfile());
   }
   file->Release();
-  dependencies->Clear();
 }
 
 static void OnExitHook(int64_t exit_code) {
@@ -682,9 +670,6 @@ static Dart_Isolate CreateIsolateGroupAndSetupHelper(
                                                     kernel_buffer_size);
     }
   }
-  if (is_main_isolate && (Options::depfile() != NULL)) {
-    isolate_group_data->set_dependencies(new MallocGrowableArray<char*>());
-  }
 
   Dart_Isolate isolate = NULL;
 

runtime/bin/vmservice/loader.dart

@@ -962,7 +962,6 @@ const _Dart_kResolvePackageUri = 8; // Resolve a package: uri.
 
 // Extra requests. Keep these in sync between loader.dart and loader.cc.
 const _Dart_kImportExtension = 9; // Import a dart-ext: file.
-const _Dart_kResolveAsFilePath = 10; // Resolve uri to file path.
 
 // External entry point for loader requests.
 _processLoadRequest(request) {
@@ -1091,38 +1090,6 @@ _processLoadRequest(request) {
           break;
       }
       break;
-    case _Dart_kResolveAsFilePath:
-      loaderState._triggerPackageResolution(() {
-        String uri = request[4];
-        Uri resolvedUri = Uri.parse(_sanitizeWindowsPath(uri));
-        try {
-          if (resolvedUri.scheme == 'package') {
-            resolvedUri = loaderState._resolvePackageUri(resolvedUri);
-          }
-          if (resolvedUri.scheme == '' || resolvedUri.scheme == 'file') {
-            resolvedUri = loaderState._workingDirectory.resolveUri(resolvedUri);
-            var msg = new List(5);
-            msg[0] = tag;
-            msg[1] = uri;
-            msg[2] = resolvedUri.toString();
-            msg[3] = null;
-            msg[4] = resolvedUri.toFilePath();
-            sp.send(msg);
-          } else {
-            throw "Cannot resolve scheme (${resolvedUri.scheme}) to file path"
-                " for $resolvedUri";
-          }
-        } catch (e) {
-          var msg = new List(5);
-          msg[0] = -tag;
-          msg[1] = uri;
-          msg[2] = resolvedUri.toString();
-          msg[3] = null;
-          msg[4] = e.toString();
-          sp.send(msg);
-        }
-      });
-      break;
     default:
       _log('Unknown loader request tag=$tag from $isolateId');
   }