[dartdev] Use VmInteropHandler for invoking sub commands

Use VmInteropHandler for invoking sub commands instead of running them
in an isolate. Running sub commands in an isolate causes an increased footprint.
Changing this to use VmInteropHandler avoids the additional memory footprint.

Commands that need to use an AOT runtime for execution now exec the AOT
runtime and run the command.

TEST=ci

Change-Id: If7aed1cab2fec9d9940bd562ad5aa9c4e9a6ac7f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398604
Reviewed-by: Ben Konyi <[email protected]>
Reviewed-by: Brian Quinlan <[email protected]>
Commit-Queue: Siva Annamalai <[email protected]>

Author: a-siva

pkg/dartdev/lib/src/commands/compile.dart

@@ -17,6 +17,7 @@ import '../experiments.dart';
 import '../native_assets.dart';
 import '../sdk.dart';
 import '../utils.dart';
+import '../vm_interop_handler.dart';
 
 const int genericErrorExitCode = 255;
 const int compileErrorExitCode = 254;
@@ -93,17 +94,18 @@ class CompileJSCommand extends CompileSubcommandCommand {
     final args = argResults!;
     var snapshot = sdk.dart2jsAotSnapshot;
     var runtime = sdk.dartAotRuntime;
+    var isAOT = true;
     if (!Sdk.checkArtifactExists(snapshot, logError: false)) {
       // AOT snapshots cannot be generated on IA32, so we need this fallback
       // branch until support for IA32 is dropped (https://dartbug.com/49969).
       snapshot = sdk.dart2jsSnapshot;
-      runtime = sdk.dart;
       if (!Sdk.checkArtifactExists(snapshot)) {
         return genericErrorExitCode;
       }
+      runtime = sdk.dart;
+      isAOT = false;
     }
     final dart2jsCommand = [
-      runtime,
       snapshot,
       '--libraries-spec=${sdk.librariesJson}',
       '--cfe-invocation-modes=compile',
@@ -112,8 +114,13 @@ class CompileJSCommand extends CompileSubcommandCommand {
       if (args.rest.isNotEmpty) ...args.rest.sublist(0),
     ];
     try {
-      final exitCode = await runProcessInheritStdio(dart2jsCommand);
-      return exitCode;
+      VmInteropHandler.run(
+        runtime,
+        dart2jsCommand,
+        packageConfigOverride: null,
+        isAOT : isAOT,
+      );
+      return 0;
     } catch (e, st) {
       log.stderr('Error: JS compilation failed');
       log.stderr(e.toString());

pkg/dartdev/lib/src/vm_interop_handler.dart

@@ -18,6 +18,9 @@ abstract class VmInteropHandler {
   ///
   /// If [markMainIsolateAsSystemIsolate] is given and set to true, the spawned
   /// isolate will run with `--mark-main-isolate-as-system-isolate` enabled.
+  ///
+  /// If [isAOT] is given and set to true, the script is executed by
+  /// execing the dartaotruntime.
   static void run(
     String script,
     List<String> args, {
@@ -27,11 +30,12 @@ abstract class VmInteropHandler {
     //
     // See https://github.com/dart-lang/sdk/issues/53576
     bool markMainIsolateAsSystemIsolate = false,
+    bool isAOT = false,
   }) {
     final port = _port;
     if (port == null) return;
     final message = <dynamic>[
-      _kResultRun,
+      isAOT ? _kResultRunAOT : _kResultRunJIT,
       script,
       packageConfigOverride,
       markMainIsolateAsSystemIsolate,
@@ -51,8 +55,9 @@ abstract class VmInteropHandler {
   }
 
   // Note: keep in sync with runtime/bin/dartdev_isolate.h
-  static const int _kResultRun = 1;
-  static const int _kResultExit = 2;
+  static const int _kResultRunJIT = 1;
+  static const int _kResultRunAOT = 2;
+  static const int _kResultExit = 3;
 
   static SendPort? _port;
 }

runtime/bin/dartdev_isolate.cc

@@ -38,6 +38,7 @@ DartDevIsolate::DartDevRunner DartDevIsolate::runner_ =
     DartDevIsolate::DartDevRunner();
 bool DartDevIsolate::should_run_dart_dev_ = false;
 bool DartDevIsolate::print_usage_error_ = false;
+CommandLineOptions* DartDevIsolate::vm_options_ = nullptr;
 Monitor* DartDevIsolate::DartDevRunner::monitor_ = new Monitor();
 DartDevIsolate::DartDev_Result DartDevIsolate::DartDevRunner::result_ =
     DartDevIsolate::DartDev_Result_Unknown;
@@ -138,7 +139,7 @@ void DartDevIsolate::DartDevRunner::Run(
   Thread::Start("DartDev Runner", RunCallback, reinterpret_cast<uword>(this));
   monitor_->WaitMicros(Monitor::kNoTimeout);
 
-  if (result_ == DartDevIsolate::DartDev_Result_Run) {
+  if (result_ == DartDevIsolate::DartDev_Result_RunJIT) {
     // Clear the DartDev dart_options and replace them with the processed
     // options provided by DartDev.
     dart_options_->Reset();
@@ -157,8 +158,8 @@ void DartDevIsolate::DartDevRunner::DartDevResultCallback(
   ASSERT(message->type == Dart_CObject_kArray);
   int32_t type = GetArrayItem(message, 0)->value.as_int32;
   switch (type) {
-    case DartDevIsolate::DartDev_Result_Run: {
-      result_ = DartDevIsolate::DartDev_Result_Run;
+    case DartDevIsolate::DartDev_Result_RunJIT: {
+      result_ = DartDevIsolate::DartDev_Result_RunJIT;
       ASSERT(GetArrayItem(message, 1)->type == Dart_CObject_kString);
       auto item2 = GetArrayItem(message, 2);
 
@@ -204,6 +205,86 @@ void DartDevIsolate::DartDevRunner::DartDevResultCallback(
       }
       break;
     }
+    case DartDevIsolate::DartDev_Result_RunAOT: {
+      result_ = DartDevIsolate::DartDev_Result_RunAOT;
+      ASSERT(GetArrayItem(message, 1)->type == Dart_CObject_kString);
+      auto item2 = GetArrayItem(message, 2);
+
+      ASSERT(item2->type == Dart_CObject_kString ||
+             item2->type == Dart_CObject_kNull);
+
+      auto item3 = GetArrayItem(message, 3);
+
+      ASSERT(item3->type == Dart_CObject_kBool);
+      const bool mark_main_isolate_as_system_isolate = item3->value.as_bool;
+      if (mark_main_isolate_as_system_isolate) {
+        Options::set_mark_main_isolate_as_system_isolate(true);
+      }
+
+      if (*script_ != nullptr) {
+        free(*script_);
+      }
+      if (*package_config_override_ != nullptr) {
+        free(*package_config_override_);
+        *package_config_override_ = nullptr;
+      }
+      *script_ = Utils::StrDup(GetArrayItem(message, 1)->value.as_string);
+
+      if (item2->type == Dart_CObject_kString) {
+        *package_config_override_ = Utils::StrDup(item2->value.as_string);
+      }
+
+      intptr_t num_vm_options = 0;
+      const char** vm_options = nullptr;
+      ASSERT(GetArrayItem(message, 4)->type == Dart_CObject_kArray);
+      Dart_CObject* args = GetArrayItem(message, 4);
+      intptr_t argc = args->value.as_array.length;
+      Dart_CObject** dart_args = args->value.as_array.values;
+
+      if (vm_options_ != nullptr) {
+        num_vm_options = vm_options_->count();
+        vm_options = vm_options_->arguments();
+      }
+      auto deleter = [](char** args) {
+        for (intptr_t i = 0; i < argc_; ++i) {
+          free(args[i]);
+        }
+        delete[] args;
+      };
+      // Total count of arguments to be passed to the script being execed.
+      argc_ = argc + num_vm_options + 1;
+
+      // Array of arguments to be passed to the script being execed.
+      argv_ = std::unique_ptr<char*[], void (*)(char**)>(new char*[argc_ + 1],
+                                                         deleter);
+
+      intptr_t idx = 0;
+      // Copy in name of the script to run (dartaotruntime).
+      argv_[0] = Utils::StrDup(GetArrayItem(message, 1)->value.as_string);
+      idx += 1;
+      // Copy in any vm options that need to be passed to the execed process.
+      for (intptr_t i = 0; i < num_vm_options; ++i) {
+        argv_[i + idx] = Utils::StrDup(vm_options[i]);
+      }
+      idx += num_vm_options;
+      // Copy in the dart options that need to be passed to the command.
+      for (intptr_t i = 0; i < argc; ++i) {
+        argv_[i + idx] = Utils::StrDup(dart_args[i]->value.as_string);
+      }
+      // Null terminate the argv array.
+      argv_[argc + idx] = nullptr;
+
+      // Exec the script to be run and pass the arguments.
+      char err_msg[256];
+      err_msg[0] = '\0';
+      int ret = Process::Exec(nullptr, *script_,
+                              const_cast<const char**>(argv_.get()), argc_,
+                              nullptr, err_msg, sizeof(err_msg));
+      if (ret != 0) {
+        ProcessError(err_msg, kErrorExitCode);
+      }
+      break;
+    }
     case DartDevIsolate::DartDev_Result_Exit: {
       ASSERT(GetArrayItem(message, 1)->type == Dart_CObject_kInt32);
       int32_t dartdev_exit_code = GetArrayItem(message, 1)->value.as_int32;
@@ -314,7 +395,9 @@ DartDevIsolate::DartDev_Result DartDevIsolate::RunDartDev(
     Dart_IsolateGroupCreateCallback create_isolate,
     char** packages_file,
     char** script,
+    CommandLineOptions* vm_options,
     CommandLineOptions* dart_options) {
+  vm_options_ = vm_options;
   runner_.Run(create_isolate, packages_file, script, dart_options);
   return runner_.result();
 }

runtime/bin/dartdev_isolate.h

@@ -27,8 +27,9 @@ class DartDevIsolate {
   // Note: keep in sync with pkg/dartdev/lib/vm_interop_handler.dart
   typedef enum {
     DartDev_Result_Unknown = -1,
-    DartDev_Result_Run = 1,
-    DartDev_Result_Exit = 2,
+    DartDev_Result_RunJIT = 1,
+    DartDev_Result_RunAOT = 2,
+    DartDev_Result_Exit = 3,
   } DartDev_Result;
 
   // Returns true if there does not exist a file at |script_uri| or the URI is
@@ -58,6 +59,7 @@ class DartDevIsolate {
       Dart_IsolateGroupCreateCallback create_isolate,
       char** packages_file,
       char** script,
+      CommandLineOptions* vm_options,
       CommandLineOptions* dart_options);
 
  protected:
@@ -83,11 +85,11 @@ class DartDevIsolate {
     static char** package_config_override_;
     static std::unique_ptr<char*[], void (*)(char**)> argv_;
     static intptr_t argc_;
+    static Monitor* monitor_;
 
     Dart_IsolateGroupCreateCallback create_isolate_;
     CommandLineOptions* dart_options_;
     const char* packages_file_;
-    static Monitor* monitor_;
 
     DISALLOW_ALLOCATION();
   };
@@ -98,6 +100,7 @@ class DartDevIsolate {
   static DartDevRunner runner_;
   static bool should_run_dart_dev_;
   static bool print_usage_error_;
+  static CommandLineOptions* vm_options_;
 
   DISALLOW_ALLOCATION();
   DISALLOW_IMPLICIT_CONSTRUCTORS(DartDevIsolate);

runtime/bin/main_impl.cc

@@ -1417,11 +1417,11 @@ void main(int argc, char** argv) {
       Options::gen_snapshot_kind() == SnapshotKind::kNone) {
     DartDevIsolate::DartDev_Result dartdev_result = DartDevIsolate::RunDartDev(
         CreateIsolateGroupAndSetup, &package_config_override, &script_name,
-        &dart_options);
+        &vm_options, &dart_options);
     ASSERT(dartdev_result != DartDevIsolate::DartDev_Result_Unknown);
     ran_dart_dev = true;
     should_run_user_program =
-        (dartdev_result == DartDevIsolate::DartDev_Result_Run);
+        (dartdev_result == DartDevIsolate::DartDev_Result_RunJIT);
     if (should_run_user_program) {
       try_load_snapshots_lambda();
     }

runtime/bin/process.cc

@@ -114,9 +114,9 @@ void FUNCTION_NAME(Process_Start)(Dart_NativeArguments args) {
   const char* path = DartUtils::GetStringValue(path_handle);
   Dart_Handle arguments = Dart_GetNativeArgument(args, 3);
   intptr_t args_length = 0;
-  char** string_args =
+  const char** string_args = const_cast<const char**>(
       ExtractCStringList(arguments, status_handle,
-                         "Arguments must be builtin strings", &args_length);
+                         "Arguments must be builtin strings", &args_length));
   if (string_args == nullptr) {
     Dart_SetBooleanReturnValue(args, false);
     return;

runtime/bin/process.h

@@ -94,7 +94,7 @@ class Process {
   // process exit streams.
   static int Start(Namespace* namespc,
                    const char* path,
-                   char* arguments[],
+                   const char* arguments[],
                    intptr_t arguments_length,
                    const char* working_directory,
                    char* environment[],
@@ -114,6 +114,23 @@ class Process {
                    intptr_t exit_handler,
                    ProcessResult* result);
 
+  // Exec process.
+  // On systems that support 'exec' it will use it to replace
+  // the current process image with the image corresponding to 'path'
+  // On systems that do not support it (Windows) it will start in a
+  // child process in the same group as the parent so that when the parent
+  // is killed the child also dies.
+  // Returns 0 if the process could be execed successfully
+  // Returns -1 if the exec could not be done successfully and 'errmsg'
+  // points to the error message
+  static int Exec(Namespace* namespc,
+                  const char* path,
+                  const char* arguments[],
+                  intptr_t arguments_length,
+                  const char* working_directory,
+                  char* errmsg,
+                  intptr_t errmsg_len);
+
   // Kill a process with a given pid.
   static bool Kill(intptr_t id, int signal);
 

runtime/bin/process_fuchsia.cc

@@ -838,6 +838,19 @@ int Process::Start(Namespace* namespc,
   return starter.Start();
 }
 
+// The command line dart utility does not run on Fuchsia, this functionality
+// is not supported on that platform.
+int Process::Exec(Namespace* namespc,
+                  const char* path,
+                  const char** arguments,
+                  const char* working_directory,
+                  char* errmsg,
+                  intptr_t errmsg_len) {
+  snprintf(errmsg, errmsg_len,
+           "Process::Exec is not supported on this platform");
+  return -1;
+}
+
 intptr_t Process::SetSignalHandler(intptr_t signal) {
   errno = ENOSYS;
   return -1;