When compiled delta is rejected, reset incremental compiler back to last known good state(last accepted state).
This is needed so that compileExpression requests are serviced from that accepted state.
Bug: https://github.com/dart-lang/sdk/issues/34025
Change-Id: I592d2af50e59a721e1feb1699c6d56bcd568465f
Reviewed-on: https://dart-review.googlesource.com/72540
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
diff --git a/pkg/front_end/lib/src/api_prototype/incremental_kernel_generator.dart b/pkg/front_end/lib/src/api_prototype/incremental_kernel_generator.dart
index 8d4e803..eef20ee 100644
--- a/pkg/front_end/lib/src/api_prototype/incremental_kernel_generator.dart
+++ b/pkg/front_end/lib/src/api_prototype/incremental_kernel_generator.dart
@@ -25,6 +25,13 @@
initializeFromDillUri);
}
+ factory IncrementalKernelGenerator.fromComponent(
+ CompilerOptions options, Uri entryPoint, Component component) {
+ return new IncrementalCompiler.fromComponent(
+ new CompilerContext(new ProcessedOptions(options, [entryPoint])),
+ component);
+ }
+
/// Returns a component whose libraries are the recompiled libraries,
/// or - in the case of [fullComponent] - a full Component.
Future<Component> computeDelta({Uri entryPoint, bool fullComponent});
diff --git a/pkg/front_end/lib/src/fasta/incremental_compiler.dart b/pkg/front_end/lib/src/fasta/incremental_compiler.dart
index 0697cc1..f9a4ca8 100644
--- a/pkg/front_end/lib/src/fasta/incremental_compiler.dart
+++ b/pkg/front_end/lib/src/fasta/incremental_compiler.dart
@@ -86,13 +86,20 @@
List<LibraryBuilder> platformBuilders;
Map<Uri, LibraryBuilder> userBuilders;
final Uri initializeFromDillUri;
+ Component componentToInitializeFrom;
bool initializedFromDill = false;
bool hasToCheckPackageUris = false;
KernelIncrementalTarget userCode;
+ IncrementalCompiler.fromComponent(
+ this.context, Component this.componentToInitializeFrom)
+ : ticker = context.options.ticker,
+ initializeFromDillUri = null;
+
IncrementalCompiler(this.context, [this.initializeFromDillUri])
- : ticker = context.options.ticker;
+ : ticker = context.options.ticker,
+ componentToInitializeFrom = null;
@override
Future<Component> computeDelta(
@@ -142,6 +149,8 @@
null);
}
}
+ } else if (componentToInitializeFrom != null) {
+ initializeFromComponent(summaryBytes, uriTranslator, c, data);
}
appendLibraries(data, bytesLength);
@@ -171,9 +180,19 @@
}
Set<Uri> invalidatedUris = this.invalidatedUris.toSet();
- if (data.includeUserLoadedLibraries || fullComponent) {
+ if ((data.includeUserLoadedLibraries &&
+ componentToInitializeFrom ==
+ null) // when loading state from component no need to invalidate anything
+ ||
+ fullComponent) {
invalidatedUris.add(entryPoint);
}
+ if (componentToInitializeFrom != null) {
+ // Once compiler was initialized from component, no need to skip logic
+ // above that adds entryPoint to invalidatedUris for successive
+ // [computeDelta] calls.
+ componentToInitializeFrom = null;
+ }
ClassHierarchy hierarchy = userCode?.loader?.hierarchy;
Set<LibraryBuilder> notReusedLibraries;
@@ -414,6 +433,43 @@
return bytesLength;
}
+ // This procedure will set up compiler from [componentToInitializeFrom].
+ void initializeFromComponent(
+ List<int> summaryBytes,
+ UriTranslator uriTranslator,
+ CompilerContext c,
+ IncrementalCompilerData data) {
+ ticker.logMs("Read initializeFromComponent");
+
+ // [libraries] and [uriToSource] from [componentToInitializeFrom] take
+ // precedence over what was already read into [data.component]. Assumption
+ // is that [data.component] is initialized with standard prebuilt various
+ // platform libraries.
+ List<Library> combinedLibs = <Library>[];
+ Set<Uri> readLibs =
+ componentToInitializeFrom.libraries.map((lib) => lib.fileUri).toSet();
+ combinedLibs.addAll(componentToInitializeFrom.libraries);
+ for (Library lib in data.component.libraries) {
+ if (!readLibs.contains(lib.fileUri)) {
+ combinedLibs.add(lib);
+ }
+ }
+ Map<Uri, Source> combinedMaps = new Map<Uri, Source>();
+ combinedMaps.addAll(componentToInitializeFrom.uriToSource);
+ Set<Uri> uris = combinedMaps.keys.toSet();
+ for (MapEntry<Uri, Source> entry in data.component.uriToSource.entries) {
+ if (!uris.contains(entry.key)) {
+ combinedMaps[entry.key] = entry.value;
+ }
+ }
+
+ data.component =
+ new Component(libraries: combinedLibs, uriToSource: combinedMaps)
+ ..mainMethod = componentToInitializeFrom.mainMethod;
+ data.userLoadedUriMain = data.component.mainMethod;
+ data.includeUserLoadedLibraries = true;
+ }
+
void appendLibraries(IncrementalCompilerData data, int bytesLength) {
if (data.component != null) {
dillLoadedData.loader
diff --git a/pkg/vm/lib/frontend_server.dart b/pkg/vm/lib/frontend_server.dart
index b63071f..6397f17 100644
--- a/pkg/vm/lib/frontend_server.dart
+++ b/pkg/vm/lib/frontend_server.dart
@@ -159,6 +159,10 @@
/// won't recompile sources that were previously reported as changed.
void acceptLastDelta();
+ /// Rejects results of previous compilation and sets compiler back to last
+ /// accepted state.
+ Future<void> rejectLastDelta();
+
/// This let's compiler know that source file identifed by `uri` was changed.
void invalidate(Uri uri);
@@ -494,6 +498,14 @@
}
@override
+ Future<void> rejectLastDelta() async {
+ await _generator.reject();
+ final String boundaryKey = new Uuid().generateV4();
+ _outputStream.writeln('result $boundaryKey');
+ _outputStream.writeln(boundaryKey);
+ }
+
+ @override
void invalidate(Uri uri) {
_generator.invalidate(uri);
}
@@ -626,6 +638,8 @@
state = _State.COMPILE_EXPRESSION_EXPRESSION;
} else if (string == 'accept') {
compiler.acceptLastDelta();
+ } else if (string == 'reject') {
+ await compiler.rejectLastDelta();
} else if (string == 'reset') {
compiler.resetIncrementalCompiler();
} else if (string == 'quit') {
diff --git a/pkg/vm/lib/incremental_compiler.dart b/pkg/vm/lib/incremental_compiler.dart
index 8238343..4cfedcd 100644
--- a/pkg/vm/lib/incremental_compiler.dart
+++ b/pkg/vm/lib/incremental_compiler.dart
@@ -17,16 +17,26 @@
/// accepted.
class IncrementalCompiler {
IncrementalKernelGenerator _generator;
+
+ // Component that reflect current state of the compiler, which has not
+ // been yet accepted by the client. Is [null] if no compilation was done
+ // since last accept/reject acknowledgement by the client.
+ Component _candidate;
+ // Component that reflect the state that was most recently accepted by the
+ // client. Is [null], if no compilation results were accepted by the client.
+ Component _lastKnownGood;
+
List<Component> _pendingDeltas;
CompilerOptions _compilerOptions;
bool initialized = false;
bool fullComponent = false;
Uri initializeFromDillUri;
+ Uri _entryPoint;
- IncrementalCompiler(this._compilerOptions, Uri entryPoint,
+ IncrementalCompiler(this._compilerOptions, this._entryPoint,
{this.initializeFromDillUri}) {
_generator = new IncrementalKernelGenerator(
- _compilerOptions, entryPoint, initializeFromDillUri);
+ _compilerOptions, _entryPoint, initializeFromDillUri);
_pendingDeltas = <Component>[];
}
@@ -41,25 +51,24 @@
fullComponent = false;
final bool firstDelta = _pendingDeltas.isEmpty;
_pendingDeltas.add(component);
- if (firstDelta) {
- return component;
- }
-
- // If more than one delta is pending, we need to combine them.
- Procedure mainMethod;
- Map<Uri, Library> combined = <Uri, Library>{};
- for (Component delta in _pendingDeltas) {
- if (delta.mainMethod != null) {
- mainMethod = delta.mainMethod;
+ if (!firstDelta) {
+ // If more than one delta is pending, we need to combine them.
+ Procedure mainMethod;
+ Map<Uri, Library> combined = <Uri, Library>{};
+ for (Component delta in _pendingDeltas) {
+ if (delta.mainMethod != null) {
+ mainMethod = delta.mainMethod;
+ }
+ for (Library library in delta.libraries) {
+ combined[library.importUri] = library;
+ }
}
- for (Library library in delta.libraries) {
- combined[library.importUri] = library;
- }
+ // TODO(vegorov) this needs to merge metadata repositories from deltas.
+ component = new Component(libraries: combined.values.toList())
+ ..mainMethod = mainMethod;
}
-
- // TODO(vegorov) this needs to merge metadata repositories from deltas.
- return new Component(libraries: combined.values.toList())
- ..mainMethod = mainMethod;
+ _candidate = component;
+ return component;
}
/// This lets incremental compiler know that results of last [compile] call
@@ -67,6 +76,40 @@
/// results.
accept() {
_pendingDeltas.clear();
+
+ Map<Uri, Library> combined = <Uri, Library>{};
+ if (_lastKnownGood != null) {
+ // TODO(aam): Figure out how to skip no-longer-used libraries from
+ // [_lastKnownGood] libraries.
+ for (Library library in _lastKnownGood.libraries) {
+ combined[library.importUri] = library;
+ }
+ }
+ for (Library library in _candidate.libraries) {
+ combined[library.importUri] = library;
+ }
+ _lastKnownGood = new Component(
+ libraries: combined.values.toList(),
+ uriToSource: _candidate.uriToSource,
+ )..mainMethod = _candidate.mainMethod;
+ for (final repo in _candidate.metadata.values) {
+ _lastKnownGood.addMetadataRepository(repo);
+ }
+
+ _candidate = null;
+ }
+
+ /// This lets incremental compiler know that results of last [compile] call
+ /// were rejected. Subsequent [compile] or [compileExpression] calls need to
+ /// be processed without changes picked up by rejected [compile] call.
+ reject() async {
+ _pendingDeltas.clear();
+ _candidate = null;
+ // Need to reset and warm up compiler so that expression evaluation requests
+ // are processed in that known good state.
+ _generator = new IncrementalKernelGenerator.fromComponent(
+ _compilerOptions, _entryPoint, _lastKnownGood);
+ await _generator.computeDelta(entryPoint: _entryPoint);
}
/// This tells incremental compiler that it needs rescan [uri] file during
diff --git a/pkg/vm/test/frontend_server_test.dart b/pkg/vm/test/frontend_server_test.dart
index 89dfafa..4ca4370 100644
--- a/pkg/vm/test/frontend_server_test.dart
+++ b/pkg/vm/test/frontend_server_test.dart
@@ -589,6 +589,137 @@
expect(await result, 0);
});
+ test('compile expression when delta is rejected', () async {
+ var fileLib = new File('${tempDir.path}/lib.dart')..createSync();
+ fileLib.writeAsStringSync("foo() => 42;\n");
+ var file = new File('${tempDir.path}/foo.dart')..createSync();
+ file.writeAsStringSync("import 'lib.dart'; main1() => print(foo);\n");
+ var dillFile = new File('${tempDir.path}/app.dill');
+ expect(dillFile.existsSync(), equals(false));
+ final List<String> args = <String>[
+ '--sdk-root=${sdkRoot.toFilePath()}',
+ '--strong',
+ '--incremental',
+ '--platform=${platformKernel.path}',
+ '--output-dill=${dillFile.path}'
+ ];
+
+ final StreamController<List<int>> inputStreamController =
+ new StreamController<List<int>>();
+ final StreamController<List<int>> stdoutStreamController =
+ new StreamController<List<int>>();
+ final IOSink ioSink = new IOSink(stdoutStreamController.sink);
+ StreamController<String> receivedResults = new StreamController<String>();
+
+ String boundaryKey;
+ stdoutStreamController.stream
+ .transform(utf8.decoder)
+ .transform(const LineSplitter())
+ .listen((String s) {
+ print(s);
+ const String RESULT_OUTPUT_SPACE = 'result ';
+ if (boundaryKey == null) {
+ if (s.startsWith(RESULT_OUTPUT_SPACE)) {
+ boundaryKey = s.substring(RESULT_OUTPUT_SPACE.length);
+ }
+ } else {
+ if (s.startsWith(boundaryKey)) {
+ receivedResults.add(s.length > boundaryKey.length
+ ? s.substring(boundaryKey.length + 1)
+ : null);
+ boundaryKey = null;
+ }
+ }
+ });
+
+ final Future<int> result =
+ starter(args, input: inputStreamController.stream, output: ioSink);
+ inputStreamController.add('compile ${file.path}\n'.codeUnits);
+ int count = 0;
+ receivedResults.stream.listen((String outputFilenameAndErrorCount) {
+ if (count == 0) {
+ // First request was to 'compile', which resulted in full kernel file.
+ CompilationResult result =
+ new CompilationResult.parse(outputFilenameAndErrorCount);
+
+ expect(dillFile.existsSync(), equals(true));
+ expect(result.filename, dillFile.path);
+ expect(result.errorsCount, equals(0));
+ inputStreamController.add('accept\n'.codeUnits);
+
+ // 'compile-expression <boundarykey>
+ // expression
+ // definitions (one per line)
+ // ...
+ // <boundarykey>
+ // type-defintions (one per line)
+ // ...
+ // <boundarykey>
+ // <libraryUri: String>
+ // <klass: String>
+ // <isStatic: true|false>
+ inputStreamController.add('''
+compile-expression abc
+main1
+abc
+abc
+${file.uri}
+
+true
+'''
+ .codeUnits);
+ count += 1;
+ } else if (count == 1) {
+ // Second request was to 'compile-expression', which resulted in
+ // kernel file with a function that wraps compiled expression.
+ expect(outputFilenameAndErrorCount, isNotNull);
+ CompilationResult result =
+ new CompilationResult.parse(outputFilenameAndErrorCount);
+ print(outputFilenameAndErrorCount);
+
+ expect(result.errorsCount, equals(0));
+ File outputFile = new File(result.filename);
+ expect(outputFile.existsSync(), equals(true));
+ expect(outputFile.lengthSync(), isPositive);
+
+ file.writeAsStringSync("import 'lib.dart'; main() => foo();\n");
+ inputStreamController.add('recompile ${file.path} abc\n'
+ '${file.path}\n'
+ 'abc\n'
+ .codeUnits);
+
+ count += 1;
+ } else if (count == 2) {
+ // Third request was to recompile the script after renaming a function.
+ expect(outputFilenameAndErrorCount, isNotNull);
+ CompilationResult result =
+ new CompilationResult.parse(outputFilenameAndErrorCount);
+ expect(result.errorsCount, equals(0));
+
+ inputStreamController.add('reject\n'.codeUnits);
+ count += 1;
+ } else if (count == 3) {
+ // Fourth request was to reject the compilation results.
+ inputStreamController.add(
+ 'compile-expression abc\nmain1\nabc\nabc\n${file.uri}\n\ntrue\n'
+ .codeUnits);
+ count += 1;
+ } else {
+ expect(count, 4);
+ // Fifth request was to 'compile-expression' that references original
+ // function, which should still be successful.
+ expect(outputFilenameAndErrorCount, isNotNull);
+ CompilationResult result =
+ new CompilationResult.parse(outputFilenameAndErrorCount);
+ expect(result.errorsCount, equals(0));
+ inputStreamController.add('quit\n'.codeUnits);
+ }
+ });
+
+ expect(await result, 0);
+ inputStreamController.close();
+ }, timeout: Timeout.factor(100));
+
test('recompile request keeps incremental output dill filename', () async {
var file = new File('${tempDir.path}/foo.dart')..createSync();
file.writeAsStringSync("main() {}\n");
@@ -630,7 +761,6 @@
starter(args, input: inputStreamController.stream, output: ioSink);
inputStreamController.add('compile ${file.path}\n'.codeUnits);
int count = 0;
- Completer<bool> allDone = new Completer<bool>();
receivedResults.stream.listen((String outputFilenameAndErrorCount) {
CompilationResult result =
new CompilationResult.parse(outputFilenameAndErrorCount);
@@ -655,11 +785,9 @@
expect(result.filename, dillIncFile.path);
expect(result.errorsCount, 0);
expect(dillIncFile.existsSync(), equals(true));
- allDone.complete(true);
+ inputStreamController.add('quit\n'.codeUnits);
}
});
- expect(await allDone.future, true);
- inputStreamController.add('quit\n'.codeUnits);
expect(await result, 0);
inputStreamController.close();
});
@@ -815,7 +943,7 @@
'--sdk-root=${sdkRoot.toFilePath()}',
'--strong',
'--incremental',
- '--platform=$platformKernel',
+ '--platform=${platformKernel.path}',
'--output-dill=${dillFile.path}',
'--output-incremental-dill=${incrementalDillFile.path}'
];
@@ -901,8 +1029,7 @@
}
// Include platform and verify.
- component =
- loadComponentFromBinary(platformKernel.toFilePath(), component);
+ component = loadComponentFromBinary(platformKernel.path, component);
expect(component.mainMethod, isNotNull);
verifyComponent(component);
@@ -927,8 +1054,7 @@
reason: "Expect the same number of sources after a reset.");
// Include platform and verify.
- component =
- loadComponentFromBinary(platformKernel.toFilePath(), component);
+ component = loadComponentFromBinary(platformKernel.path, component);
expect(component.mainMethod, isNotNull);
verifyComponent(component);
@@ -953,11 +1079,10 @@
// Reload with 1 change
inputStreamController.add('accept\n'.codeUnits);
- inputStreamController
- .add('recompile ${dart2jsOtherFile.path} x$count\n'
- '${dart2jsOtherFile.uri}\n'
- 'x$count\n'
- .codeUnits);
+ inputStreamController.add('recompile ${dart2js.path} x$count\n'
+ '${dart2jsOtherFile.path}\n'
+ 'x$count\n'
+ .codeUnits);
} else if (count == 3) {
// Partial file. Expect to not be empty.
expect(incrementalDillFile.existsSync(), equals(true));
