Run cli compilations in parallel dart isolates (#2078)

Co-authored-by: Natalie Weizenbaum <[email protected]>

Author: ntkme

bin/sass.dart

@@ -8,45 +8,21 @@ import 'package:path/path.dart' as p;
 import 'package:stack_trace/stack_trace.dart';
 import 'package:term_glyph/term_glyph.dart' as term_glyph;
 
-import 'package:sass/src/exception.dart';
-import 'package:sass/src/executable/compile_stylesheet.dart';
+import 'package:sass/src/executable/concurrent.dart';
 import 'package:sass/src/executable/options.dart';
 import 'package:sass/src/executable/repl.dart';
 import 'package:sass/src/executable/watch.dart';
 import 'package:sass/src/import_cache.dart';
 import 'package:sass/src/io.dart';
-import 'package:sass/src/io.dart' as io;
 import 'package:sass/src/logger/deprecation_handling.dart';
 import 'package:sass/src/stylesheet_graph.dart';
-import 'package:sass/src/util/map.dart';
 import 'package:sass/src/utils.dart';
 import 'package:sass/src/embedded/executable.dart'
     // Never load the embedded protocol when compiling to JS.
     if (dart.library.js) 'package:sass/src/embedded/unavailable.dart'
     as embedded;
 
 Future<void> main(List<String> args) async {
-  var printedError = false;
-
-  // Prints [error] to stderr, along with a preceding newline if anything else
-  // has been printed to stderr.
-  //
-  // If [trace] is passed, its terse representation is printed after the error.
-  void printError(String error, StackTrace? stackTrace) {
-    var buffer = StringBuffer();
-    if (printedError) buffer.writeln();
-    printedError = true;
-    buffer.write(error);
-
-    if (stackTrace != null) {
-      buffer.writeln();
-      buffer.writeln();
-      buffer.write(Trace.from(stackTrace).terse.toString().trimRight());
-    }
-
-    io.printError(buffer);
-  }
-
   if (args case ['--embedded', ...var rest]) {
     embedded.main(rest);
     return;
@@ -84,37 +60,8 @@ Future<void> main(List<String> args) async {
       return;
     }
 
-    for (var (source, destination) in options.sourcesToDestinations.pairs) {
-      try {
-        await compileStylesheet(options, graph, source, destination,
-            ifModified: options.update);
-      } on SassException catch (error, stackTrace) {
-        if (destination != null && !options.emitErrorCss) {
-          _tryDelete(destination);
-        }
-
-        printError(error.toString(color: options.color),
-            options.trace ? getTrace(error) ?? stackTrace : null);
-
-        // Exit code 65 indicates invalid data per
-        // https://www.freebsd.org/cgi/man.cgi?query=sysexits.
-        //
-        // We let exitCode 66 take precedence for deterministic behavior.
-        if (exitCode != 66) exitCode = 65;
-        if (options.stopOnError) return;
-      } on FileSystemException catch (error, stackTrace) {
-        var path = error.path;
-        printError(
-            path == null
-                ? error.message
-                : "Error reading ${p.relative(path)}: ${error.message}.",
-            options.trace ? getTrace(error) ?? stackTrace : null);
-
-        // Error 66 indicates no input.
-        exitCode = 66;
-        if (options.stopOnError) return;
-      }
-    }
+    await compileStylesheets(options, graph, options.sourcesToDestinations,
+        ifModified: options.update);
   } on UsageException catch (error) {
     print("${error.message}\n");
     print("Usage: sass <input.scss> [output.css]\n"
@@ -128,8 +75,11 @@ Future<void> main(List<String> args) async {
     if (options?.color ?? false) buffer.write('\u001b[0m');
     buffer.writeln();
     buffer.writeln(error);
-
-    printError(buffer.toString(), getTrace(error) ?? stackTrace);
+    buffer.writeln();
+    buffer.writeln();
+    buffer.write(
+        Trace.from(getTrace(error) ?? stackTrace).terse.toString().trimRight());
+    printError(buffer);
     exitCode = 255;
   }
 }
@@ -154,14 +104,3 @@ Future<String> _loadVersion() async {
       .split(" ")
       .last;
 }
-
-/// Delete [path] if it exists and do nothing otherwise.
-///
-/// This is a separate function to work around dart-lang/sdk#53082.
-void _tryDelete(String path) {
-  try {
-    deleteFile(path);
-  } on FileSystemException {
-    // If the file doesn't exist, that's fine.
-  }
-}

lib/src/executable/compile_stylesheet.dart

@@ -6,6 +6,7 @@ import 'dart:convert';
 
 import 'package:path/path.dart' as p;
 import 'package:source_maps/source_maps.dart';
+import 'package:stack_trace/stack_trace.dart';
 
 import '../async_import_cache.dart';
 import '../compile.dart';
@@ -30,8 +31,42 @@ import 'options.dart';
 /// If [ifModified] is `true`, only recompiles if [source]'s modification time
 /// or that of a file it imports is more recent than [destination]'s
 /// modification time. Note that these modification times are cached by [graph].
-Future<void> compileStylesheet(ExecutableOptions options, StylesheetGraph graph,
-    String? source, String? destination,
+///
+/// Returns `(exitCode, error, stackTrace)` when an error occurs.
+Future<(int, String, String?)?> compileStylesheet(ExecutableOptions options,
+    StylesheetGraph graph, String? source, String? destination,
+    {bool ifModified = false}) async {
+  try {
+    await _compileStylesheetWithoutErrorHandling(
+        options, graph, source, destination,
+        ifModified: ifModified);
+  } on SassException catch (error, stackTrace) {
+    if (destination != null && !options.emitErrorCss) {
+      _tryDelete(destination);
+    }
+    var message = error.toString(color: options.color);
+
+    // Exit code 65 indicates invalid data per
+    // https://www.freebsd.org/cgi/man.cgi?query=sysexits.
+    return _getErrorWithStackTrace(
+        65, message, options.trace ? getTrace(error) ?? stackTrace : null);
+  } on FileSystemException catch (error, stackTrace) {
+    var path = error.path;
+    var message = path == null
+        ? error.message
+        : "Error reading ${p.relative(path)}: ${error.message}.";
+
+    // Exit code 66 indicates no input.
+    return _getErrorWithStackTrace(
+        66, message, options.trace ? getTrace(error) ?? stackTrace : null);
+  }
+  return null;
+}
+
+/// Like [compileStylesheet], but throws errors instead of handling them
+/// internally.
+Future<void> _compileStylesheetWithoutErrorHandling(ExecutableOptions options,
+    StylesheetGraph graph, String? source, String? destination,
     {bool ifModified = false}) async {
   var importer = FilesystemImporter('.');
   if (ifModified) {
@@ -150,7 +185,7 @@ Future<void> compileStylesheet(ExecutableOptions options, StylesheetGraph graph,
   buffer.write('Compiled $sourceName to $destinationName.');
   if (options.color) buffer.write('\u001b[0m');
 
-  print(buffer);
+  safePrint(buffer);
 }
 
 /// Writes the source map given by [mapping] to disk (if necessary) according to
@@ -195,3 +230,26 @@ String _writeSourceMap(
   return (options.style == OutputStyle.compressed ? '' : '\n\n') +
       '/*# sourceMappingURL=$escapedUrl */';
 }
+
+/// Delete [path] if it exists and do nothing otherwise.
+///
+/// This is a separate function to work around dart-lang/sdk#53082.
+void _tryDelete(String path) {
+  try {
+    deleteFile(path);
+  } on FileSystemException {
+    // If the file doesn't exist, that's fine.
+  }
+}
+
+/// Return a Record of `(exitCode, error, stackTrace)` for the given error.
+(int, String, String?) _getErrorWithStackTrace(
+    int exitCode, String error, StackTrace? stackTrace) {
+  return (
+    exitCode,
+    error,
+    stackTrace != null
+        ? Trace.from(stackTrace).terse.toString().trimRight()
+        : null
+  );
+}

lib/src/executable/concurrent.dart

@@ -0,0 +1,66 @@
+// Copyright 2023 Google Inc. Use of this source code is governed by an
+// MIT-style license that can be found in the LICENSE file or at
+// https://opensource.org/licenses/MIT.
+
+import 'dart:math' as math;
+
+import '../io.dart';
+import '../stylesheet_graph.dart';
+import '../util/map.dart';
+import 'compile_stylesheet.dart';
+import 'concurrent/vm.dart'
+    // Never load the isolate library when compiling to JS.
+    if (dart.library.js) 'concurrent/js.dart';
+import 'options.dart';
+
+/// Compiles the stylesheets concurrently and returns whether all stylesheets are compiled
+/// successfully.
+Future<bool> compileStylesheets(ExecutableOptions options,
+    StylesheetGraph graph, Map<String?, String?> sourcesToDestinations,
+    {bool ifModified = false}) async {
+  var errorsWithStackTraces = switch ([...sourcesToDestinations.pairs]) {
+    // Concurrency does add some overhead, so avoid it in the common case of
+    // compiling a single stylesheet.
+    [(var source, var destination)] => [
+        await compileStylesheet(options, graph, source, destination,
+            ifModified: ifModified)
+      ],
+    var pairs => await Future.wait([
+        for (var (source, destination) in pairs)
+          compileStylesheetConcurrently(options, graph, source, destination,
+              ifModified: ifModified)
+      ], eagerError: options.stopOnError)
+  };
+
+  var printedError = false;
+
+  // Print all errors in deterministic order.
+  for (var errorWithStackTrace in errorsWithStackTraces) {
+    if (errorWithStackTrace == null) continue;
+    var (code, error, stackTrace) = errorWithStackTrace;
+
+    // We let the highest exitCode take precedence for deterministic behavior.
+    exitCode = math.max(exitCode, code);
+
+    _printError(error, stackTrace, printedError);
+    printedError = true;
+  }
+
+  return !printedError;
+}
+
+// Prints [error] to stderr, along with a preceding newline if anything else
+// has been printed to stderr.
+//
+// If [stackTrace] is passed, it is printed after the error.
+void _printError(String error, String? stackTrace, bool printedError) {
+  var buffer = StringBuffer();
+  if (printedError) buffer.writeln();
+  buffer.write(error);
+  if (stackTrace != null) {
+    buffer.writeln();
+    buffer.writeln();
+    buffer.write(stackTrace);
+  }
+  printError(buffer);
+}

lib/src/executable/concurrent/js.dart

@@ -0,0 +1,10 @@
+// Copyright 2023 Google Inc. Use of this source code is governed by an
+// MIT-style license that can be found in the LICENSE file or at
+// https://opensource.org/licenses/MIT.
+
+import '../compile_stylesheet.dart';
+
+/// We don't currently support concurrent compilation in JS.
+///
+/// In the future, we could add support using web workers.
+final compileStylesheetConcurrently = compileStylesheet;

lib/src/executable/concurrent/vm.dart

@@ -0,0 +1,33 @@
+// Copyright 2023 Google Inc. Use of this source code is governed by an
+// MIT-style license that can be found in the LICENSE file or at
+// https://opensource.org/licenses/MIT.
+
+import 'dart:isolate';
+
+import 'package:term_glyph/term_glyph.dart' as term_glyph;
+
+import '../options.dart';
+import '../../stylesheet_graph.dart';
+import '../compile_stylesheet.dart';
+
+/// Compiles the stylesheet at [source] to [destination].
+///
+/// Runs in a new Dart Isolate, unless [source] is `null`.
+Future<(int, String, String?)?> compileStylesheetConcurrently(
+    ExecutableOptions options,
+    StylesheetGraph graph,
+    String? source,
+    String? destination,
+    {bool ifModified = false}) {
+  // Reading from stdin does not work properly in dart isolate.
+  if (source == null) {
+    return compileStylesheet(options, graph, source, destination,
+        ifModified: ifModified);
+  }
+
+  return Isolate.run(() {
+    term_glyph.ascii = !options.unicode;
+    return compileStylesheet(options, graph, source, destination,
+        ifModified: ifModified);
+  });
+}