From b2689ff9c2b31018ae72408a46e61fed7feca278 Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Wed, 8 Feb 2017 08:37:36 -0800 Subject: [PATCH] Use synchronous file writing in BlobStreamReference (#24) Doing so allows us to avoid dealing in futures all the way back to `encode()`. This becomes important when implementing replay since replay uses `noSuchMethod`, which can't await futures. This also extracts out a few constants in preparation for their use in replay. Part of #11 --- lib/src/backends/record_replay/common.dart | 52 ++++++++++++++++ lib/src/backends/record_replay/encoding.dart | 37 +++++------- lib/src/backends/record_replay/events.dart | 38 ++++++------ .../record_replay/mutable_recording.dart | 3 +- .../record_replay/recording_file.dart | 60 +++---------------- .../record_replay/result_reference.dart | 10 +--- 6 files changed, 97 insertions(+), 103 deletions(-) diff --git a/lib/src/backends/record_replay/common.dart b/lib/src/backends/record_replay/common.dart index 9ba9d1cedc469..afd95e0c72538 100644 --- a/lib/src/backends/record_replay/common.dart +++ b/lib/src/backends/record_replay/common.dart @@ -2,12 +2,64 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'events.dart'; + /// Encoded value of the file system in a recording. const String kFileSystemEncodedValue = '__fs__'; /// The name of the recording manifest file. const String kManifestName = 'MANIFEST.txt'; +/// The key in a serialized [InvocationEvent] map that is used to store the +/// type of invocation. +/// +/// See also: +/// - [kGetType] +/// - [kSetType] +/// - [kInvokeType] +const String kManifestTypeKey = 'type'; + +/// The key in a serialized [InvocationEvent] map that is used to store the +/// target of the invocation. +const String kManifestObjectKey = 'object'; + +/// The key in a serialized [InvocationEvent] map that is used to store the +/// result (return value) of the invocation. +const String kManifestResultKey = 'result'; + +/// The key in a serialized [InvocationEvent] map that is used to store the +/// timestamp of the invocation. +const String kManifestTimestampKey = 'timestamp'; + +/// The key in a serialized [PropertyGetEvent] or [PropertySetEvent] map that +/// is used to store the property that was accessed or mutated. +const String kManifestPropertyKey = 'property'; + +/// The key in a serialized [PropertySetEvent] map that is used to store the +/// value to which the property was set. +const String kManifestValueKey = 'value'; + +/// The key in a serialized [MethodEvent] map that is used to store the name of +/// the method that was invoked. +const String kManifestMethodKey = 'method'; + +/// The key in a serialized [MethodEvent] map that is used to store the +/// positional arguments that were passed to the method. +const String kManifestPositionalArgumentsKey = 'positionalArguments'; + +/// The key in a serialized [MethodEvent] map that is used to store the +/// named arguments that were passed to the method. +const String kManifestNamedArgumentsKey = 'namedArguments'; + +/// The serialized [kManifestTypeKey] for property retrievals. +const String kGetType = 'get'; + +/// The serialized [kManifestTypeKey] for property mutations. +const String kSetType = 'set'; + +/// The serialized [kManifestTypeKey] for method invocations. +const String kInvokeType = 'invoke'; + /// Gets an id guaranteed to be unique on this isolate for objects within this /// library. int newUid() => _nextUid++; diff --git a/lib/src/backends/record_replay/encoding.dart b/lib/src/backends/record_replay/encoding.dart index ac78f8aa1a611..086e95983dbc9 100644 --- a/lib/src/backends/record_replay/encoding.dart +++ b/lib/src/backends/record_replay/encoding.dart @@ -2,7 +2,6 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -import 'dart:async'; import 'dart:convert'; import 'package:file/file.dart'; @@ -20,7 +19,7 @@ import 'result_reference.dart'; /// Encodes an object into a JSON-ready representation. /// -/// It is legal for an encoder to return a future value. +/// Must return one of {number, boolean, string, null, list, or map}. typedef dynamic _Encoder(dynamic object); /// Known encoders. Types not covered here will be encoded using @@ -35,8 +34,8 @@ const Map, _Encoder> _kEncoders = const TypeMatcher(): _encodeRaw, const TypeMatcher(): _encodeRaw, const TypeMatcher(): _encodeRaw, - const TypeMatcher>(): encodeIterable, - const TypeMatcher>(): encodeMap, + const TypeMatcher>(): _encodeIterable, + const TypeMatcher>(): _encodeMap, const TypeMatcher(): getSymbolName, const TypeMatcher(): _encodeDateTime, const TypeMatcher(): _encodeUri, @@ -59,9 +58,9 @@ const Map, _Encoder> _kEncoders = /// Encodes an arbitrary [object] into a JSON-ready representation (a number, /// boolean, string, null, list, or map). /// -/// Returns a future that completes with a value suitable for conversion into -/// JSON using [JsonEncoder] without the need for a `toEncodable` argument. -Future encode(dynamic object) async { +/// Returns a value suitable for conversion into JSON using [JsonEncoder] +/// without the need for a `toEncodable` argument. +dynamic encode(dynamic object) { _Encoder encoder = _encodeDefault; for (TypeMatcher matcher in _kEncoders.keys) { if (matcher.matches(object)) { @@ -69,37 +68,31 @@ Future encode(dynamic object) async { break; } } - return await encoder(object); + return encoder(object); } /// Default encoder (used for types not covered in [_kEncoders]). String _encodeDefault(dynamic object) => object.runtimeType.toString(); -/// Pass-through encoder. +/// Pass-through encoder (used on `num`, `bool`, `String`, and `Null`). dynamic _encodeRaw(dynamic object) => object; /// Encodes the specified [iterable] into a JSON-ready list of encoded items. -/// -/// Returns a future that completes with a list suitable for conversion into -/// JSON using [JsonEncoder] without the need for a `toEncodable` argument. -Future> encodeIterable(Iterable iterable) async { +List _encodeIterable(Iterable iterable) { List encoded = []; for (dynamic element in iterable) { - encoded.add(await encode(element)); + encoded.add(encode(element)); } return encoded; } /// Encodes the specified [map] into a JSON-ready map of encoded key/value /// pairs. -/// -/// Returns a future that completes with a map suitable for conversion into -/// JSON using [JsonEncoder] without the need for a `toEncodable` argument. -Future> encodeMap(Map map) async { +Map _encodeMap(Map map) { Map encoded = {}; for (dynamic key in map.keys) { - String encodedKey = await encode(key); - encoded[encodedKey] = await encode(map[key]); + String encodedKey = encode(key); + encoded[encodedKey] = encode(map[key]); } return encoded; } @@ -115,10 +108,10 @@ Map _encodePathContext(p.Context context) { }; } -Future _encodeResultReference(ResultReference reference) => +dynamic _encodeResultReference(ResultReference reference) => reference.serializedValue; -Future> _encodeEvent(LiveInvocationEvent event) => +Map _encodeEvent(LiveInvocationEvent event) => event.serialize(); String _encodeFileSystem(FileSystem fs) => kFileSystemEncodedValue; diff --git a/lib/src/backends/record_replay/events.dart b/lib/src/backends/record_replay/events.dart index 821f4e4e639e8..75ffd15e76467 100644 --- a/lib/src/backends/record_replay/events.dart +++ b/lib/src/backends/record_replay/events.dart @@ -103,11 +103,11 @@ abstract class LiveInvocationEvent implements InvocationEvent { } /// Returns this event as a JSON-serializable object. - Future> serialize() async { + Map serialize() { return { - 'object': await encode(object), - 'result': await encode(_result), - 'timestamp': timestamp, + kManifestObjectKey: encode(object), + kManifestResultKey: encode(_result), + kManifestTimestampKey: timestamp, }; } @@ -126,11 +126,11 @@ class LivePropertyGetEvent extends LiveInvocationEvent final Symbol property; @override - Future> serialize() async { + Map serialize() { return { - 'type': 'get', - 'property': getSymbolName(property), - }..addAll(await super.serialize()); + kManifestTypeKey: kGetType, + kManifestPropertyKey: getSymbolName(property), + }..addAll(super.serialize()); } } @@ -148,12 +148,12 @@ class LivePropertySetEvent extends LiveInvocationEvent final T value; @override - Future> serialize() async { + Map serialize() { return { - 'type': 'set', - 'property': getSymbolName(property), - 'value': await encode(value), - }..addAll(await super.serialize()); + kManifestTypeKey: kSetType, + kManifestPropertyKey: getSymbolName(property), + kManifestValueKey: encode(value), + }..addAll(super.serialize()); } } @@ -185,12 +185,12 @@ class LiveMethodEvent extends LiveInvocationEvent final Map namedArguments; @override - Future> serialize() async { + Map serialize() { return { - 'type': 'invoke', - 'method': getSymbolName(method), - 'positionalArguments': await encodeIterable(positionalArguments), - 'namedArguments': await encodeMap(namedArguments), - }..addAll(await super.serialize()); + kManifestTypeKey: kInvokeType, + kManifestMethodKey: getSymbolName(method), + kManifestPositionalArgumentsKey: encode(positionalArguments), + kManifestNamedArgumentsKey: encode(namedArguments), + }..addAll(super.serialize()); } } diff --git a/lib/src/backends/record_replay/mutable_recording.dart b/lib/src/backends/record_replay/mutable_recording.dart index d6bac00527463..60ce88ee48bfb 100644 --- a/lib/src/backends/record_replay/mutable_recording.dart +++ b/lib/src/backends/record_replay/mutable_recording.dart @@ -46,8 +46,7 @@ class MutableRecording implements LiveRecording { .timeout(awaitPendingResults, onTimeout: () {}); } Directory dir = destination; - List encodedEvents = await encode(_events); - String json = new JsonEncoder.withIndent(' ').convert(encodedEvents); + String json = new JsonEncoder.withIndent(' ').convert(encode(_events)); String filename = dir.fileSystem.path.join(dir.path, kManifestName); await dir.fileSystem.file(filename).writeAsString(json, flush: true); } finally { diff --git a/lib/src/backends/record_replay/recording_file.dart b/lib/src/backends/record_replay/recording_file.dart index ddfe697f1b2a0..1453b7e24d57b 100644 --- a/lib/src/backends/record_replay/recording_file.dart +++ b/lib/src/backends/record_replay/recording_file.dart @@ -20,6 +20,7 @@ import 'result_reference.dart'; /// /// See also: /// - [_BlobReference] +/// - [_BlobStreamReference] typedef void _BlobDataSyncWriter(File file, T data); /// Callback responsible for asynchronously writing result [data] to the @@ -29,13 +30,6 @@ typedef void _BlobDataSyncWriter(File file, T data); /// - [_BlobFutureReference] typedef Future _BlobDataAsyncWriter(File file, T data); -/// Callback responsible writing streaming result [data] to the specified -/// [sink]. -/// -/// See also: -/// - [_BlobStreamReference] -typedef void _BlobDataStreamWriter(IOSink sink, T data); - /// [File] implementation that records all invocation activity to its file /// system's recording. class RecordingFile extends RecordingFileSystemEntity implements File { @@ -93,8 +87,8 @@ class RecordingFile extends RecordingFileSystemEntity implements File { return new _BlobStreamReference>( file: _newRecordingFile(), stream: delegate.openRead(start, end), - writer: (IOSink sink, List bytes) { - sink.add(bytes); + writer: (File file, List bytes) { + file.writeAsBytesSync(bytes, mode: FileMode.APPEND, flush: true); }, ); } @@ -209,7 +203,7 @@ class _BlobReference extends ResultReference { T get recordedValue => _value; @override - Future get serializedValue async => '!${_file.basename}'; + String get serializedValue => '!${_file.basename}'; } /// A [FutureReference] that serializes its value data to a separate file. @@ -235,64 +229,28 @@ class _BlobFutureReference extends FutureReference { } @override - Future get serializedValue async => '!${_file.basename}'; + String get serializedValue => '!${_file.basename}'; } /// A [StreamReference] that serializes its value data to a separate file. class _BlobStreamReference extends StreamReference { final File _file; - final _BlobDataStreamWriter _writer; - IOSink _sink; - Future _pendingFlush; + final _BlobDataSyncWriter _writer; _BlobStreamReference({ @required File file, @required Stream stream, - @required _BlobDataStreamWriter writer, + @required _BlobDataSyncWriter writer, }) : _file = file, _writer = writer, - _sink = file.openWrite(), super(stream); @override void onData(T event) { - if (_pendingFlush == null) { - _writer(_sink, event); - } else { - // It's illegal to write to an IOSink while a flush is pending. - // https://github.com/dart-lang/sdk/issues/28635 - _pendingFlush.whenComplete(() { - _writer(_sink, event); - }); - } - } - - @override - void onDone() { - if (_sink != null) { - _sink.close(); - } - } - - @override - Future get serializedValue async { - if (_pendingFlush != null) { - await _pendingFlush; - } else { - _pendingFlush = _sink.flush(); - try { - await _pendingFlush; - } finally { - _pendingFlush = null; - } - } - - return '!${_file.basename}'; + _writer(_file, event); } - // TODO(tvolkert): remove `.then()` once Dart 1.22 is in stable @override - Future get complete => - Future.wait(>[super.complete, _sink.done]).then((_) {}); + String get serializedValue => '!${_file.basename}'; } diff --git a/lib/src/backends/record_replay/result_reference.dart b/lib/src/backends/record_replay/result_reference.dart index 1fcdf3bcc321b..211d0067712ac 100644 --- a/lib/src/backends/record_replay/result_reference.dart +++ b/lib/src/backends/record_replay/result_reference.dart @@ -49,7 +49,7 @@ abstract class ResultReference { /// actually a byte array that was read from a file). In this case, the /// method can return a `ResultReference` to the list, and it will have a /// hook into the serialization process. - Future get serializedValue => encode(recordedValue); + dynamic get serializedValue => encode(recordedValue); /// A [Future] that completes when [value] has completed. /// @@ -139,7 +139,6 @@ class StreamReference extends ResultReference> { _controller.addError(error, stackTrace); }, onDone: () { - onDone(); _completer.complete(); _controller.close(); }, @@ -153,13 +152,6 @@ class StreamReference extends ResultReference> { @protected void onData(T event) {} - /// Called when the underlying delegate stream fires a "done" event. - /// - /// Subclasses may override this method to be notified when the underlying - /// stream is done. - @protected - void onDone() {} - @override Stream get value => _controller.stream;