Skip to content

Commit

Permalink
Use import URIs when invalidating files
Browse files Browse the repository at this point in the history
Fixes flutter/flutter#16368
Note that this is mostly a copy of
https://dart-review.googlesource.com/c/sdk/+/48180

Change-Id: Ibdf7719d8b362a4c179fd7a4a6ab5ae7028a5429
Reviewed-on: https://dart-review.googlesource.com/50420
Commit-Queue: Jens Johansen <[email protected]>
Reviewed-by: Peter von der Ahé <[email protected]>
  • Loading branch information
jensjoha authored and [email protected] committed Apr 10, 2018
1 parent 23c59bd commit e696279
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 38 deletions.
62 changes: 36 additions & 26 deletions pkg/front_end/lib/src/fasta/incremental_compiler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
userBuilders = <Uri, LibraryBuilder>{};
platformBuilders = <LibraryBuilder>[];
dillLoadedData.loader.builders.forEach((uri, builder) {
if (builder.fileUri.scheme == "dart") {
if (builder.uri.scheme == "dart") {
platformBuilders.add(builder);
} else {
userBuilders[uri] = builder;
Expand All @@ -110,7 +110,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
if (userBuilders.isEmpty) userBuilders = null;
}

List<Uri> invalidatedUris = this.invalidatedUris.toList();
Set<Uri> invalidatedUris = this.invalidatedUris.toSet();
this.invalidatedUris.clear();
if (fullComponent) {
invalidatedUris.add(entryPoint);
Expand Down Expand Up @@ -156,7 +156,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
List<Library> librariesWithSdk = userCode.component.libraries;
List<Library> libraries = <Library>[];
for (Library lib in librariesWithSdk) {
if (lib.fileUri.scheme == "dart") continue;
if (lib.importUri.scheme == "dart") continue;
libraries.add(lib);
break;
}
Expand Down Expand Up @@ -199,20 +199,20 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
IncrementalCompilerData data) {
Map<Uri, Library> libraryMap = <Uri, Library>{};
for (Library library in libraries) {
libraryMap[library.fileUri] = library;
libraryMap[library.importUri] = library;
}
List<Uri> worklist = new List<Uri>.from(libraryMap.keys);
worklist.add(mainMethod?.enclosingLibrary?.fileUri);
worklist.add(mainMethod?.enclosingLibrary?.importUri);
if (entry != null) {
worklist.add(entry);
}

Map<Uri, Library> potentiallyReferencedLibraries = <Uri, Library>{};
for (LibraryBuilder library in reusedLibraries) {
if (library.fileUri.scheme == "dart") continue;
if (library.uri.scheme == "dart") continue;
Library lib = library.target;
potentiallyReferencedLibraries[library.fileUri] = lib;
libraryMap[library.fileUri] = lib;
potentiallyReferencedLibraries[library.uri] = lib;
libraryMap[library.uri] = lib;
}

LibraryGraph graph = new LibraryGraph(libraryMap);
Expand Down Expand Up @@ -325,45 +325,52 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
}

List<LibraryBuilder> computeReusedLibraries(
Iterable<Uri> invalidatedUris, UriTranslator uriTranslator) {
Set<Uri> invalidatedUris, UriTranslator uriTranslator) {
if (userCode == null && userBuilders == null) {
return <LibraryBuilder>[];
}

// [invalidatedUris] converted to a set.
Set<Uri> invalidatedFileUris = invalidatedUris.toSet();

// Maps all non-platform LibraryBuilders from their import URI.
Map<Uri, LibraryBuilder> builders = <Uri, LibraryBuilder>{};

// Invalidated URIs translated back to their import URI (package:, dart:,
// etc.).
List<Uri> invalidatedImportUris = <Uri>[];

bool isInvalidated(Uri importUri, Uri fileUri) {
if (invalidatedUris.contains(importUri) ||
(importUri != fileUri && invalidatedUris.contains(fileUri))) {
return true;
}
if (importUri.scheme == "package" &&
uriTranslator.translate(importUri, false) != fileUri) {
return true;
}
return false;
}

// Compute [builders] and [invalidatedImportUris].
addBuilderAndInvalidateUris(Uri uri, LibraryBuilder library,
[bool recursive = true]) {
builders[uri] = library;
if (invalidatedFileUris.contains(uri) ||
(uri != library.fileUri &&
invalidatedFileUris.contains(library.fileUri)) ||
(library is DillLibraryBuilder &&
uri != library.library.fileUri &&
invalidatedFileUris.contains(library.library.fileUri)) ||
(library.uri.scheme == "package" &&
uriTranslator.translate(library.uri, false) !=
library.target.fileUri)) {
if (isInvalidated(uri, library.target.fileUri)) {
invalidatedImportUris.add(uri);
}
if (!recursive) return;
if (library is SourceLibraryBuilder) {
for (LibraryBuilder part in library.parts) {
addBuilderAndInvalidateUris(part.uri, part, false);
if (isInvalidated(part.uri, part.fileUri)) {
invalidatedImportUris.add(part.uri);
builders[part.uri] = part;
}
}
} else if (library is DillLibraryBuilder) {
for (LibraryPart part in library.library.parts) {
for (LibraryPart part in library.target.parts) {
Uri partUri = library.uri.resolve(part.partUri);
addBuilderAndInvalidateUris(partUri, library, false);
Uri fileUri = library.library.fileUri.resolve(part.partUri);
if (isInvalidated(partUri, fileUri)) {
invalidatedImportUris.add(partUri);
builders[partUri] = library;
}
}
}
}
Expand Down Expand Up @@ -417,7 +424,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
List<LibraryBuilder> result = <LibraryBuilder>[];
for (LibraryBuilder builder in builders.values) {
if (builder.isPart) continue;
if (!seenUris.add(builder.fileUri)) continue;
// TODO(jensj/ahe): This line can probably go away once
// https://dart-review.googlesource.com/47442 lands.
if (builder.isPatch) continue;
if (!seenUris.add(builder.uri)) continue;
result.add(builder);
}
return result;
Expand Down
44 changes: 36 additions & 8 deletions pkg/front_end/test/incremental_load_from_dill_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -117,21 +117,30 @@ class RunCompilations extends Step<TestData, TestData, Context> {

void basicTest(Map<String, String> sourceFiles, String entryPoint, bool strong,
List<String> invalidate, Directory outDir) async {
Uri entryPointUri;
Uri entryPointUri = outDir.uri.resolve(entryPoint);
Set<String> invalidateFilenames = invalidate?.toSet() ?? new Set<String>();
List<Uri> invalidateUris = <Uri>[];
Uri packagesUri;
for (String filename in sourceFiles.keys) {
Uri uri = outDir.uri.resolve(filename);
if (filename == entryPoint) entryPointUri = uri;
if (invalidateFilenames.contains(filename)) invalidateUris.add(uri);
if (invalidateFilenames.contains(filename)) {
invalidateUris.add(uri);
invalidateFilenames.remove(filename);
}
String source = sourceFiles[filename];
if (filename == ".packages") {
source = substituteVariables(source, outDir.uri);
packagesUri = uri;
}
new File.fromUri(uri).writeAsStringSync(source);
}
for (String invalidateFilename in invalidateFilenames) {
if (invalidateFilename.startsWith('package:')) {
invalidateUris.add(Uri.parse(invalidateFilename));
} else {
throw "Error in test yaml: $invalidateFilename was not recognized.";
}
}

Uri output = outDir.uri.resolve("full.dill");
Uri initializedOutput = outDir.uri.resolve("full_from_initialized.dill");
Expand All @@ -145,9 +154,13 @@ void basicTest(Map<String, String> sourceFiles, String entryPoint, bool strong,
print("Normal compile took ${stopwatch.elapsedMilliseconds} ms");

stopwatch.reset();
options = getOptions(strong);
if (packagesUri != null) {
options.packagesFileUri = packagesUri;
}
bool initializedResult = await initializedCompile(
entryPointUri, initializedOutput, output, invalidateUris,
options: getOptions(strong));
options: options);
print("Initialized compile(s) from ${output.pathSegments.last} "
"took ${stopwatch.elapsedMilliseconds} ms");
Expect.isTrue(initializedResult);
Expand Down Expand Up @@ -249,7 +262,8 @@ void newWorldTest(bool strong, List worlds) async {
}

Stopwatch stopwatch = new Stopwatch()..start();
Component component = await compiler.computeDelta(fullComponent: true);
Component component = await compiler.computeDelta(
fullComponent: brandNewWorld ? false : true);
if (world["errors"] == true && !gotError) {
throw "Expected error, but didn't get any.";
} else if (world["errors"] != true && gotError) {
Expand Down Expand Up @@ -278,7 +292,7 @@ void newWorldTest(bool strong, List worlds) async {
if (world["checkInvalidatedFiles"] != false) {
if (world["invalidate"] != null) {
Expect.equals(world["invalidate"].length,
compiler.invalidatedImportUrisForTesting.length);
compiler.invalidatedImportUrisForTesting?.length ?? 0);
List expectedInvalidatedUri = world["expectedInvalidatedUri"];
if (expectedInvalidatedUri != null) {
Expect.setEquals(
Expand Down Expand Up @@ -357,6 +371,12 @@ Future<bool> initializedCompile(
throwOnEmptyMixinBodies(initializedComponent);
bool result = compiler.initializedFromDill;
await writeComponentToFile(initializedComponent, output);
int actuallyInvalidatedCount =
compiler.invalidatedImportUrisForTesting?.length ?? 0;
if (result && actuallyInvalidatedCount < invalidateUris.length) {
Expect.fail("Expected at least ${invalidateUris.length} invalidated uris, "
"got $actuallyInvalidatedCount");
}

var initializedComponent2 = await compiler.computeDelta(fullComponent: true);
throwOnEmptyMixinBodies(initializedComponent2);
Expand All @@ -371,6 +391,13 @@ Future<bool> initializedCompile(

var partialComponent = await compiler.computeDelta();
throwOnEmptyMixinBodies(partialComponent);
actuallyInvalidatedCount =
(compiler.invalidatedImportUrisForTesting?.length ?? 0);
if (actuallyInvalidatedCount < invalidateUris.length) {
Expect.fail("Expected at least ${invalidateUris.length} invalidated uris, "
"got $actuallyInvalidatedCount");
}

var emptyComponent = await compiler.computeDelta();
throwOnEmptyMixinBodies(emptyComponent);

Expand All @@ -384,6 +411,7 @@ Future<bool> initializedCompile(
Expect.isTrue(fullLibUris.length > partialLibUris.length ||
partialLibUris.length == invalidateUris.length);
Expect.isTrue(partialLibUris.isNotEmpty || invalidateUris.isEmpty);

Expect.isTrue(emptyLibUris.isEmpty);

return result;
Expand Down Expand Up @@ -417,7 +445,7 @@ String substituteVariables(String source, Uri base) {
}

class TestIncrementalCompiler extends IncrementalCompiler {
List<Uri> invalidatedImportUrisForTesting;
Set<Uri> invalidatedImportUrisForTesting;

TestIncrementalCompiler(CompilerOptions options, Uri entryPoint,
[Uri initializeFrom])
Expand All @@ -428,6 +456,6 @@ class TestIncrementalCompiler extends IncrementalCompiler {

@override
void recordInvalidatedImportUrisForTesting(List<Uri> uris) {
invalidatedImportUrisForTesting = uris.isEmpty ? null : uris.toList();
invalidatedImportUrisForTesting = uris.isEmpty ? null : uris.toSet();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ type: basic
entry: "package:dummy/main.dart"
strong: false
invalidate:
- b.dart
sources:
main.dart: |
part "b.dart";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
# for details. All rights reserved. Use of this source code is governed by a
# BSD-style license that can be found in the LICENSE.md file.

# Test that invalidating a part of a package works with file URI.
# Test that invalidating a part of a package works with package URI.

type: basic
entry: "package:dummy/main.dart"
strong: false
invalidate:
- "package:dummy/b.dart"
sources:
main.dart: |
part "b.dart";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,4 @@
# Status file for the test suite ../test/incremental_load_from_dill_yaml_test.dart.

calculated_bounds_no_strongmode: Crash
invalidate_package_part: Crash
invalidate_package_part_as_file: Crash
invalidate_package_part_as_package: Crash
strongmode_mixins_2: Crash

0 comments on commit e696279

Please sign in to comment.