From e6962790a12806da87353730ffe604e5d2277445 Mon Sep 17 00:00:00 2001 From: Jens Johansen Date: Tue, 10 Apr 2018 13:52:42 +0000 Subject: [PATCH] Use import URIs when invalidating files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/flutter/flutter/issues/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 Reviewed-by: Peter von der Ahé --- .../lib/src/fasta/incremental_compiler.dart | 62 +++++++++++-------- .../test/incremental_load_from_dill_test.dart | 44 ++++++++++--- .../invalidate_package_part_as_file.yaml | 1 + .../invalidate_package_part_as_package.yaml | 3 +- .../status.status | 3 - 5 files changed, 75 insertions(+), 38 deletions(-) diff --git a/pkg/front_end/lib/src/fasta/incremental_compiler.dart b/pkg/front_end/lib/src/fasta/incremental_compiler.dart index 9af08b2bea4f2..f51d4a57edebf 100644 --- a/pkg/front_end/lib/src/fasta/incremental_compiler.dart +++ b/pkg/front_end/lib/src/fasta/incremental_compiler.dart @@ -101,7 +101,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator { userBuilders = {}; platformBuilders = []; dillLoadedData.loader.builders.forEach((uri, builder) { - if (builder.fileUri.scheme == "dart") { + if (builder.uri.scheme == "dart") { platformBuilders.add(builder); } else { userBuilders[uri] = builder; @@ -110,7 +110,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator { if (userBuilders.isEmpty) userBuilders = null; } - List invalidatedUris = this.invalidatedUris.toList(); + Set invalidatedUris = this.invalidatedUris.toSet(); this.invalidatedUris.clear(); if (fullComponent) { invalidatedUris.add(entryPoint); @@ -156,7 +156,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator { List librariesWithSdk = userCode.component.libraries; List libraries = []; for (Library lib in librariesWithSdk) { - if (lib.fileUri.scheme == "dart") continue; + if (lib.importUri.scheme == "dart") continue; libraries.add(lib); break; } @@ -199,20 +199,20 @@ class IncrementalCompiler implements IncrementalKernelGenerator { IncrementalCompilerData data) { Map libraryMap = {}; for (Library library in libraries) { - libraryMap[library.fileUri] = library; + libraryMap[library.importUri] = library; } List worklist = new List.from(libraryMap.keys); - worklist.add(mainMethod?.enclosingLibrary?.fileUri); + worklist.add(mainMethod?.enclosingLibrary?.importUri); if (entry != null) { worklist.add(entry); } Map potentiallyReferencedLibraries = {}; 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); @@ -325,14 +325,11 @@ class IncrementalCompiler implements IncrementalKernelGenerator { } List computeReusedLibraries( - Iterable invalidatedUris, UriTranslator uriTranslator) { + Set invalidatedUris, UriTranslator uriTranslator) { if (userCode == null && userBuilders == null) { return []; } - // [invalidatedUris] converted to a set. - Set invalidatedFileUris = invalidatedUris.toSet(); - // Maps all non-platform LibraryBuilders from their import URI. Map builders = {}; @@ -340,30 +337,40 @@ class IncrementalCompiler implements IncrementalKernelGenerator { // etc.). List invalidatedImportUris = []; + 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; + } } } } @@ -417,7 +424,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator { List result = []; 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; diff --git a/pkg/front_end/test/incremental_load_from_dill_test.dart b/pkg/front_end/test/incremental_load_from_dill_test.dart index 610c148903098..147487c8950b4 100644 --- a/pkg/front_end/test/incremental_load_from_dill_test.dart +++ b/pkg/front_end/test/incremental_load_from_dill_test.dart @@ -117,14 +117,16 @@ class RunCompilations extends Step { void basicTest(Map sourceFiles, String entryPoint, bool strong, List invalidate, Directory outDir) async { - Uri entryPointUri; + Uri entryPointUri = outDir.uri.resolve(entryPoint); Set invalidateFilenames = invalidate?.toSet() ?? new Set(); List invalidateUris = []; 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); @@ -132,6 +134,13 @@ void basicTest(Map sourceFiles, String entryPoint, bool strong, } 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"); @@ -145,9 +154,13 @@ void basicTest(Map 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); @@ -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) { @@ -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( @@ -357,6 +371,12 @@ Future 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); @@ -371,6 +391,13 @@ Future 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); @@ -384,6 +411,7 @@ Future initializedCompile( Expect.isTrue(fullLibUris.length > partialLibUris.length || partialLibUris.length == invalidateUris.length); Expect.isTrue(partialLibUris.isNotEmpty || invalidateUris.isEmpty); + Expect.isTrue(emptyLibUris.isEmpty); return result; @@ -417,7 +445,7 @@ String substituteVariables(String source, Uri base) { } class TestIncrementalCompiler extends IncrementalCompiler { - List invalidatedImportUrisForTesting; + Set invalidatedImportUrisForTesting; TestIncrementalCompiler(CompilerOptions options, Uri entryPoint, [Uri initializeFrom]) @@ -428,6 +456,6 @@ class TestIncrementalCompiler extends IncrementalCompiler { @override void recordInvalidatedImportUrisForTesting(List uris) { - invalidatedImportUrisForTesting = uris.isEmpty ? null : uris.toList(); + invalidatedImportUrisForTesting = uris.isEmpty ? null : uris.toSet(); } } diff --git a/pkg/front_end/testcases/incremental_initialize_from_dill/invalidate_package_part_as_file.yaml b/pkg/front_end/testcases/incremental_initialize_from_dill/invalidate_package_part_as_file.yaml index 43eb78b18a4f0..e2c631edeab6a 100644 --- a/pkg/front_end/testcases/incremental_initialize_from_dill/invalidate_package_part_as_file.yaml +++ b/pkg/front_end/testcases/incremental_initialize_from_dill/invalidate_package_part_as_file.yaml @@ -8,6 +8,7 @@ type: basic entry: "package:dummy/main.dart" strong: false invalidate: + - b.dart sources: main.dart: | part "b.dart"; diff --git a/pkg/front_end/testcases/incremental_initialize_from_dill/invalidate_package_part_as_package.yaml b/pkg/front_end/testcases/incremental_initialize_from_dill/invalidate_package_part_as_package.yaml index 43eb78b18a4f0..90d13acb13eec 100644 --- a/pkg/front_end/testcases/incremental_initialize_from_dill/invalidate_package_part_as_package.yaml +++ b/pkg/front_end/testcases/incremental_initialize_from_dill/invalidate_package_part_as_package.yaml @@ -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"; diff --git a/pkg/front_end/testcases/incremental_initialize_from_dill/status.status b/pkg/front_end/testcases/incremental_initialize_from_dill/status.status index 2dcebd246cd51..656e4372e6ff0 100644 --- a/pkg/front_end/testcases/incremental_initialize_from_dill/status.status +++ b/pkg/front_end/testcases/incremental_initialize_from_dill/status.status @@ -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