Serialize barback assets more efficiently.

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

Index: sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart
diff --git a/sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart b/sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart
index 38938be500f17aec5d12bd33a354718345ff6028..9d35700e17b30dc2a9d0babb7b57734422eadf9f 100644
--- a/sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart
+++ b/sdk/lib/_internal/pub/lib/src/barback/load_transformers.dart
@@ -9,6 +9,7 @@ import 'dart:convert';
 import 'dart:isolate';
 
 import 'package:barback/barback.dart';
+import 'package:barback/src/internal_asset.dart';

[Bob Nystrom, 2013/11/06 21:29:08]

Importing from "src"!

How about making a separate "serialization.dart" library in lib in barback?

[nweiz, 2013/11/07 00:44:48]

I think it's a little gross but not unforgivable to have "friend" libraries that
access one another's src libraries, as long as each knows the other is going to
do so.
Same reason I didn't put the serialization stuff on Asset; I don't want to lock
us into this serialization API before we have time to thoroughly think through
how we want to handle custom assets.

[Bob Nystrom, 2013/11/07 18:04:56]

See my previous comment. I think we can put this in lib without being locked in
since we're <1.0.

I'm definitely not keen on pub importing stuff in src. The barback tests don't
cover the APIs of libraries directly in src, so it would be possible to break
pub like this while having all of barback's tests still pass.

[nweiz, 2013/11/07 22:36:08]

The fact that we're theoretically allowed to break the API doesn't mean we
should expose functionality that we plan to break, especially if we get no value
from exposing the functionality.
I'm not sure what you mean by this. I added a bunch of tests in asset_test that
test exactly the API that's being used by pub. Besides, we have bots that run
the pub tests on every check-in. There's no risk that we don't notice a change
in barback that breaks pub.

 import 'package:source_maps/source_maps.dart';
 
 import '../barback.dart';
@@ -30,6 +31,7 @@ import 'dart:mirrors';
 import 'http://<<HOST_AND_PORT>>/packages/source_maps/span.dart';
 import 'http://<<HOST_AND_PORT>>/packages/stack_trace/stack_trace.dart';
 import 'http://<<HOST_AND_PORT>>/packages/barback/barback.dart';
+import 'http://<<HOST_AND_PORT>>/packages/barback/src/internal_asset.dart';
 
 /// Sets up the initial communication with the host isolate.
 void main(_, SendPort replyTo) {
@@ -99,7 +101,8 @@ class ForeignTransform implements Transform {
   /// Creates a transform from a serializable map sent from the host isolate.
   ForeignTransform(Map transform)
       : _port = transform['port'],
-        primaryInput = _deserializeAsset(transform['primaryInput']) {
+        primaryInput =
+            new InternalAsset.deserialize(transform['primaryInput']) {
     _logger = new TransformLogger((assetId, level, message, span) {
       _call(_port, {
         'type': 'log',
@@ -115,7 +118,7 @@ class ForeignTransform implements Transform {
     return _call(_port, {
       'type': 'getInput',
       'id': _serializeId(id)
-    }).then(_deserializeAsset);
+    }).then((message) => new InternalAsset.deserialize(message));
   }
 
   Future<String> readInputAsString(AssetId id, {Encoding encoding}) {
@@ -129,7 +132,7 @@ class ForeignTransform implements Transform {
   void addOutput(Asset output) {
     _call(_port, {
       'type': 'addOutput',
-      'output': _serializeAsset(output)
+      'output': (output as InternalAsset).serialize()
     });
   }
 }
@@ -184,7 +187,8 @@ Map _serializeTransformer(Transformer transformer) {
   port.listen((wrappedMessage) {
     _respond(wrappedMessage, (message) {
       if (message['type'] == 'isPrimary') {
-        return transformer.isPrimary(_deserializeAsset(message['asset']));
+        return transformer.isPrimary(
+            new InternalAsset.deserialize(message['asset']));
       } else {
         assert(message['type'] == 'apply');
         return transformer.apply(
@@ -211,13 +215,6 @@ Map _serializeTransformerGroup(TransformerGroup group) {
   };
 }
 
-/// Converts a serializable map into an [Asset].
-Asset _deserializeAsset(Map asset) {
-  return new Asset.fromStream(
-      _deserializeId(asset['id']),
-      _deserializeStream(asset['stream']));
-}
-
 /// The body of a [StreamTransformer] that deserializes the values in a stream
 /// sent by [_serializeStream].
 StreamSubscription _deserializeTransformer(Stream input, bool cancelOnError) {
@@ -238,49 +235,9 @@ StreamSubscription _deserializeTransformer(Stream input, bool cancelOnError) {
   return subscription;
 }
 
-/// Convert a [SendPort] whose opposite is waiting to send us a stream into a
-/// [Stream].
-///
-/// No stream data will actually be sent across the isolate boundary until
-/// someone subscribes to the returned stream.
-Stream _deserializeStream(SendPort sendPort) {
-  return callbackStream(() {
-    var receivePort = new ReceivePort();
-    sendPort.send(receivePort.sendPort);
-    return receivePort.transform(
-        const StreamTransformer(_deserializeTransformer));
-  });
-}
-
 /// Converts a serializable map into an [AssetId].
 AssetId _deserializeId(Map id) => new AssetId(id['package'], id['path']);
 
-/// Converts [asset] into a serializable map.
-Map _serializeAsset(Asset asset) {
-  return {
-    'id': _serializeId(asset.id),
-    'stream': _serializeStream(asset.read())
-  };
-}
-
-/// Converts [stream] into a [SendPort] with which another isolate can request
-/// the data from [stream].
-SendPort _serializeStream(Stream stream) {
-  var receivePort = new ReceivePort();
-  receivePort.first.then((sendPort) {
-    stream.listen((data) => sendPort.send({'type': 'data', 'data': data}),
-        onDone: () => sendPort.send({'type': 'done'}),
-        onError: (error, stackTrace) {
-      sendPort.send({
-        'type': 'error',
-        'error': CrossIsolateException.serialize(error, stackTrace)
-      });
-    });
-  });
-
-  return receivePort.sendPort;
-}
-
 /// Converts [id] into a serializable map.
 Map _serializeId(AssetId id) => {'package': id.package, 'path': id.path};
 
@@ -503,7 +460,7 @@ class _ForeignTransformer extends Transformer {
   Future<bool> isPrimary(Asset asset) {
     return _call(_port, {
       'type': 'isPrimary',
-      'asset': _serializeAsset(asset)
+      'asset': (asset as InternalAsset).serialize()
     });
   }
 
@@ -547,11 +504,11 @@ Map _serializeTransform(Transform transform) {
     _respond(wrappedMessage, (message) {
       if (message['type'] == 'getInput') {
         return transform.getInput(_deserializeId(message['id']))
-            .then(_serializeAsset);
+            .then((asset) => (asset as InternalAsset).serialize());
       }
 
       if (message['type'] == 'addOutput') {
-        transform.addOutput(_deserializeAsset(message['output']));
+        transform.addOutput(new InternalAsset.deserialize(message['output']));
         return null;
       }
 
@@ -576,17 +533,10 @@ Map _serializeTransform(Transform transform) {
 
   return {
     'port': receivePort.sendPort,
-    'primaryInput': _serializeAsset(transform.primaryInput)
+    'primaryInput': (transform.primaryInput as InternalAsset).serialize()
   };
 }
 
-/// Converts a serializable map into an [Asset].
-Asset _deserializeAsset(Map asset) {
-  return new Asset.fromStream(
-      _deserializeId(asset['id']),
-      _deserializeStream(asset['stream']));
-}
-
 /// A transformer that deserializes the values in a stream sent by
 /// [_serializeStream].
 StreamSubscription _deserializeTransformer(Stream input, bool cancelOnError) {
@@ -607,20 +557,6 @@ StreamSubscription _deserializeTransformer(Stream input, bool cancelOnError) {
   return subscription;
 }
 
-/// Convert a [SendPort] whose opposite is waiting to send us a stream into a
-/// [Stream].
-///
-/// No stream data will actually be sent across the isolate boundary until
-/// someone subscribes to the returned stream.
-Stream _deserializeStream(SendPort sendPort) {
-  return callbackStream(() {
-    var receivePort = new ReceivePort();
-    sendPort.send(receivePort.sendPort);
-    return receivePort.transform(
-        const StreamTransformer(_deserializeTransformer));
-  });
-}
-
 /// Converts a serializable map into an [AssetId].
 AssetId _deserializeId(Map id) => new AssetId(id['package'], id['path']);
 
@@ -639,34 +575,6 @@ Location _deserializeLocation(Map location) {
       location['line'], location['column']);
 }
 
-// TODO(nweiz): add custom serialization code for assets that can be more
-// efficiently serialized.
-/// Converts [asset] into a serializable map.
-Map _serializeAsset(Asset asset) {
-  return {
-    'id': _serializeId(asset.id),
-    'stream': _serializeStream(asset.read())
-  };
-}
-
-/// Converts [stream] into a [SendPort] with which another isolate can request
-/// the data from [stream].
-SendPort _serializeStream(Stream stream) {
-  var receivePort = new ReceivePort();
-  receivePort.first.then((sendPort) {
-    stream.listen((data) => sendPort.send({'type': 'data', 'data': data}),
-        onDone: () => sendPort.send({'type': 'done'}),
-        onError: (error, stackTrace) {
-      sendPort.send({
-        'type': 'error',
-        'error': CrossIsolateException.serialize(error, stackTrace)
-      });
-    });
-  });
-
-  return receivePort.sendPort;
-}
-
 /// Converts [id] into a serializable map.
 Map _serializeId(AssetId id) => {'package': id.package, 'path': id.path};