Load parallel transformers in the same isolate.

sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart

Index: sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart
diff --git a/sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart b/sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart
index 0f9016d8255f0983a88aa4371afe0f4a754a5bfa..4196a9e98f68d64745c55ca521c5af8ad9a7d11d 100644
--- a/sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart
+++ b/sdk/lib/_internal/pub/lib/src/barback/load_all_transformers.dart
@@ -98,8 +98,8 @@ Future loadAllTransformers(AssetEnvironment environment,
 
       var phases = environment.graph.packages[package].pubspec.transformers;
       return Future.forEach(phases, (phase) {
-        return Future.wait(phase.where((id) => id.package == package)
-            .map(loader.load)).then((_) {
+        return loader.load(phase.where((id) => id.package == package))
+            .then((_) {
           // If we've already loaded all the transformers in this package and no
           // other package imports it, there's no need to keep applying
           // transformers, so we can short-circuit.
@@ -118,7 +118,7 @@ Future loadAllTransformers(AssetEnvironment environment,
         // Now that we've applied all the transformers used by [package] via
         // [Barback.updateTransformers], we load any transformers defined in
         // [package] but used elsewhere.
-        return Future.wait(packageTransformers[package].map(loader.load));
+        return loader.load(packageTransformers[package]);
       });
     });
     loadingPackages[package] = future;
@@ -257,41 +257,45 @@ class _TransformerLoader {
     }
   }
 
-  /// Loads the transformer(s) defined in [id].
+  /// Loads the transformer(s) defined in [ids].
   ///
   /// Once the returned future completes, these transformers can be retrieved
-  /// using [transformersFor]. If [id] doesn't define any transformers, this
+  /// using [transformersFor]. If any id doesn't define any transformers, this
   /// will complete to an error.
-  Future load(TransformerId id) {
-    if (_transformers.containsKey(id)) return new Future.value();
+  Future load(Iterable<TransformerId> ids) {

[Bob Nystrom, 2014/06/11 20:33:35]

Doc: "Skip any transformers that are already loaded."

[nweiz, 2014/06/12 20:11:17]

Done.

+    ids = ids.where((id) => !_transformers.containsKey(id)).toList();
+    if (ids.isEmpty) return new Future.value();
 
     // TODO(nweiz): load multiple instances of the same transformer from the
     // same isolate rather than spinning up a separate isolate for each one.

[Bob Nystrom, 2014/06/11 20:33:35]

Is this TODO still meaningful?

[nweiz, 2014/06/12 20:11:17]

Yes. It's about loading multiple instances of the same transformer that appear
in various places in the package graph (potentially with different
configurations) from the same isolate, which this change doesn't do.

-    return log.progress("Loading $id transformers",
-        () => loadTransformers(_environment, _transformerServer, id))
-        .then((transformers) {
-      if (!transformers.isEmpty) {
-        _transformers[id] = transformers;
-        return;
+    return log.progress("Loading ${toSentence(ids)} transformers",
+        () => loadTransformers(_environment, _transformerServer, ids))
+        .then((allTransformers) {
+      for (var id in ids) {
+        var transformers = allTransformers[id];
+        if (transformers != null && transformers.isNotEmpty) {
+          _transformers[id] = transformers;
+          continue;
+        }
+
+        var message = "No transformers";
+        if (id.configuration.isNotEmpty) {
+          message += " that accept configuration";
+        }
+
+        var location;
+        if (id.path == null) {
+          location = 'package:${id.package}/transformer.dart or '
+            'package:${id.package}/${id.package}.dart';
+        } else {
+          location = 'package:$id.dart';
+        }
+        var pair = new Pair(id.package, id.path);
+
+        throw new ApplicationException(
+            "$message were defined in $location,\n"
+            "required by ${ordered(_transformerUsers[pair]).join(', ')}.");
       }
-
-      var message = "No transformers";
-      if (id.configuration.isNotEmpty) {
-        message += " that accept configuration";
-      }
-
-      var location;
-      if (id.path == null) {
-        location = 'package:${id.package}/transformer.dart or '
-          'package:${id.package}/${id.package}.dart';
-      } else {
-        location = 'package:$id.dart';
-      }
-      var pair = new Pair(id.package, id.path);
-
-      throw new ApplicationException(
-          "$message were defined in $location,\n"
-          "required by ${ordered(_transformerUsers[pair]).join(', ')}.");
     });
   }