Reapply "[kernel/cfe/etc] Split outline transformation into performOutlineTransformations and performOutlineComponentOperations"
This reverts commit 264098c85f.
Currently outline transformations aren't run via the incremental
compiler which causes problems in
https://dart-review.googlesource.com/c/sdk/+/491702 which fixes it by
calling the current transformation in the incremental compiler. This
calls it twice though (because it's run again in
`frontend_server/lib/compute_kernel.dart`, but removing it there doesn't
work because a filtering is done which doesn't apply through the
incremental compiler.
This CL splits up the outline transformation stage into a call that can
actually transform the libraries and one that can do the filtering,
which should fix the issue.
Original CL was reverted because it caused errors in google3. The
original CL is in patchset 1. The error has been reproduced and
recreated in a test added in patchset 2. The fix is in patchset 3.
The dwds failure @
https://github.com/dart-lang/webdev/actions/runs/24516059718/job/71660245069
has been verified as fixed as well.
The problem was this:
Previously outline transformations were not run by the incremental
compiler, but only outside. When it was moved to the incremental
compiler it had on old - outdated - `target` which for the ddc/dart2js
summary target would hold a list of source files that it was initially
created with, not the ones currently being compiled. This meant that the
transformation step that removed "unrelated" libraries actually removed
the newly compiled libraries instead. It could cause one of two issues:
1) Empty output: With no overlap between the combined output of the
compile and the sources of the first compile (i.e. the ones in the
outdated `target`) all libraries were filtered out. If a later
compile was given this as a summary input the compile could fail with
a file not found error because the given summary - which should
contain the missing library didn't.
2) Non-empty output: With an overlap between the combined output of the
compile and the sources of the first compile only the overlap would
be included. In practise this would mean that the output would be a
(potentially partial) copy of the first compile. If then a later
compile was given both the summary from the first compile and the
output with the copy it would throw when loading because it got the
same library from two different summaries.
Change-Id: If712663acdbd7d25ccb3beab54a7efac0c0b0568
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/496181
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
This commit is contained in:
committed by
dart-scoped@luci-project-accounts.iam.gserviceaccount.com
parent
ae0fa469c4
commit
41aa8d42bc
@@ -361,6 +361,20 @@ class Dart2jsSummaryTarget extends Dart2jsTarget with SummaryMixin {
|
||||
this.excludeNonSources,
|
||||
TargetFlags targetFlags,
|
||||
) : super(name, targetFlags);
|
||||
|
||||
@override
|
||||
bool isModularlyCompatibleWith(Target other) {
|
||||
if (other is! Dart2jsSummaryTarget) return false;
|
||||
if (excludeNonSources != other.excludeNonSources) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
@override
|
||||
void updateModularCompatibilityAs(Target other) {
|
||||
assert(other is Dart2jsSummaryTarget);
|
||||
sources.clear();
|
||||
sources.addAll((other as Dart2jsSummaryTarget).sources);
|
||||
}
|
||||
}
|
||||
|
||||
class Dart2jsConstantsBackend extends ConstantsBackend {
|
||||
|
||||
@@ -83,7 +83,9 @@ Future<InitializedCompilerState> initializeIncrementalCompiler(
|
||||
!equalSets(oldState.tags, tags) ||
|
||||
(sdkSummary != null &&
|
||||
(cachedSdkInput == null ||
|
||||
!digestsEqual(cachedSdkInput.digest, sdkDigest)))) {
|
||||
!digestsEqual(cachedSdkInput.digest, sdkDigest))) ||
|
||||
(oldState.options.target == null ||
|
||||
!oldState.options.target!.isModularlyCompatibleWith(target))) {
|
||||
// No - or immediately not correct - previous state.
|
||||
// We'll load a new sdk, anything loaded already will have a wrong root.
|
||||
workerInputCache.clear();
|
||||
@@ -148,6 +150,7 @@ Future<InitializedCompilerState> initializeIncrementalCompiler(
|
||||
options.packagesFileUri = packagesFile;
|
||||
options.fileSystem = fileSystem;
|
||||
processedOpts.clearFileSystemCache();
|
||||
options.target!.updateModularCompatibilityAs(target);
|
||||
}
|
||||
|
||||
// Then read all the input summary components.
|
||||
|
||||
@@ -441,6 +441,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
|
||||
);
|
||||
componentWithDill = buildResult.component;
|
||||
}
|
||||
// Coverage-ignore(suite): Not run.
|
||||
else if (componentWithDill != null) {
|
||||
context.options.target.performOutlineTransformations(componentWithDill);
|
||||
}
|
||||
|
||||
_benchmarker
|
||||
// Coverage-ignore(suite): Not run.
|
||||
@@ -578,6 +582,11 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
|
||||
// about other libraries.
|
||||
result.metadata.addAll(componentWithDill.metadata);
|
||||
|
||||
if (outlineOnly) {
|
||||
// Coverage-ignore-block(suite): Not run.
|
||||
context.options.target.performOutlineComponentOperations(result);
|
||||
}
|
||||
|
||||
// We're now done. Allow any waiting compile to start.
|
||||
Completer<dynamic> currentlyCompilingLocal = _currentlyCompiling!;
|
||||
_currentlyCompiling = null;
|
||||
|
||||
@@ -224,6 +224,7 @@ Future<InternalCompilerResult> _buildInternal(
|
||||
// the only need we have for these transformations).
|
||||
if (!buildComponent) {
|
||||
options.target.performOutlineTransformations(trimmedSummaryComponent);
|
||||
options.target.performOutlineComponentOperations(trimmedSummaryComponent);
|
||||
options.ticker.logMs("Transformed outline");
|
||||
}
|
||||
if (serializeIfBuildingSummary) {
|
||||
|
||||
@@ -392,7 +392,11 @@ Future<ComputeKernelResult> computeKernel(
|
||||
previousState,
|
||||
{
|
||||
"target=$targetName",
|
||||
// trackWidgetCreation is in TargetFlags.
|
||||
"trackWidgetCreation=$trackWidgetCreation",
|
||||
// includeUnsupportedPlatformLibraryStubs is in TargetFlags.
|
||||
"includeUnsupportedPlatformLibraryStubs="
|
||||
"$includeUnsupportedPlatformLibraryStubs",
|
||||
"multiRootScheme=${mrfs.markerScheme}",
|
||||
"multiRootRoots=${mrfs.roots}",
|
||||
},
|
||||
@@ -483,7 +487,6 @@ Future<ComputeKernelResult> computeKernel(
|
||||
incrementalComponent.uriToSource.clear();
|
||||
incrementalComponent.problemsAsJson = null;
|
||||
incrementalComponent.setMainMethodAndMode(null, true);
|
||||
target.performOutlineTransformations(incrementalComponent);
|
||||
makeStable(incrementalComponent);
|
||||
return new Future.value(
|
||||
fe.serializeComponent(
|
||||
@@ -615,6 +618,20 @@ class DevCompilerSummaryTarget extends DevCompilerTarget with SummaryMixin {
|
||||
this.excludeNonSources,
|
||||
TargetFlags targetFlags,
|
||||
) : super(targetFlags);
|
||||
|
||||
@override
|
||||
bool isModularlyCompatibleWith(Target other) {
|
||||
if (other is! DevCompilerSummaryTarget) return false;
|
||||
if (excludeNonSources != other.excludeNonSources) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
@override
|
||||
void updateModularCompatibilityAs(Target other) {
|
||||
assert(other is DevCompilerSummaryTarget);
|
||||
sources.clear();
|
||||
sources.addAll((other as DevCompilerSummaryTarget).sources);
|
||||
}
|
||||
}
|
||||
|
||||
Uri? toUriNullable(String? uriString) {
|
||||
|
||||
@@ -0,0 +1,297 @@
|
||||
// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
|
||||
// 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:io';
|
||||
|
||||
import 'package:front_end/src/api_unstable/compiler_state.dart';
|
||||
import 'package:front_end/src/compute_platform_binaries_location.dart'
|
||||
show computePlatformBinariesLocation;
|
||||
import 'package:frontend_server/compute_kernel.dart';
|
||||
import 'package:kernel/ast.dart';
|
||||
import 'package:kernel/binary/ast_from_binary.dart';
|
||||
|
||||
Future<void> main() async {
|
||||
await runHelper(
|
||||
ddc_summary_sources_change_standalone_standalone_require_both,
|
||||
);
|
||||
await runHelper(
|
||||
ddc_summary_sources_change_standalone_require_one_require_both,
|
||||
);
|
||||
|
||||
if (_countFailures != 0) {
|
||||
// Set the exit code so the bots go red.
|
||||
throw "Got $_countFailures failures.";
|
||||
}
|
||||
}
|
||||
|
||||
int _countFailures = 0;
|
||||
|
||||
/// Compile one standalone package, then compile another standalone package,
|
||||
/// then compile a third requiring both.
|
||||
/// At one point we had a bug where the target wasn't updated properly and the
|
||||
/// output of the second compile was empty. In that case the third compile would
|
||||
/// be missing sources.
|
||||
Future<void> ddc_summary_sources_change_standalone_standalone_require_both(
|
||||
Directory dir,
|
||||
) async {
|
||||
Uri outDirUri = dir.uri.resolve("out");
|
||||
new Directory.fromUri(outDirUri)..createSync();
|
||||
Uri packagesFileUri = dir.uri.resolve("packages.json");
|
||||
_writePackageFilePkg1To3(packagesFileUri);
|
||||
|
||||
Uri ddcOutlineUri = computePlatformBinariesLocation(
|
||||
forceBuildDir: true,
|
||||
).resolve("ddc_outline.dill");
|
||||
|
||||
final Map<Uri, List<int>> inputDigests = {
|
||||
ddcOutlineUri: [0],
|
||||
};
|
||||
|
||||
// Compile package:pkg1/file.dart - a standalone package.
|
||||
Uri pkg1File = dir.uri.resolve("live/pkg1/lib/file.dart");
|
||||
new File.fromUri(pkg1File)
|
||||
..createSync(recursive: true)
|
||||
..writeAsStringSync("int pkg1() { return 42; }");
|
||||
|
||||
InitializedCompilerState? previousState;
|
||||
Uri pkg1Output = outDirUri.resolve("1.dill");
|
||||
ComputeKernelResult result = await _compileSummaryAndCheck(
|
||||
pkg1Output,
|
||||
packagesFileUri,
|
||||
ddcOutlineUri,
|
||||
inputDigests,
|
||||
"package:pkg1/file.dart",
|
||||
previousState,
|
||||
);
|
||||
previousState = result.previousState;
|
||||
new File.fromUri(pkg1File).deleteSync();
|
||||
|
||||
// Compile package:pkg2/file.dart - a standalone package.
|
||||
Uri pkg2File = dir.uri.resolve("live/pkg2/lib/file.dart");
|
||||
new File.fromUri(pkg2File)
|
||||
..createSync(recursive: true)
|
||||
..writeAsStringSync("int pkg2() { return 43; }");
|
||||
|
||||
Uri pkg2Output = outDirUri.resolve("2.dill");
|
||||
result = await _compileSummaryAndCheck(
|
||||
pkg2Output,
|
||||
packagesFileUri,
|
||||
ddcOutlineUri,
|
||||
inputDigests,
|
||||
"package:pkg2/file.dart",
|
||||
previousState,
|
||||
);
|
||||
previousState = result.previousState;
|
||||
new File.fromUri(pkg2File).deleteSync();
|
||||
|
||||
// Compile package:pkg3/file.dart - require both previous.
|
||||
Uri pkg3File = dir.uri.resolve("live/pkg3/lib/file.dart");
|
||||
new File.fromUri(pkg3File)
|
||||
..createSync(recursive: true)
|
||||
..writeAsStringSync("""
|
||||
import "package:pkg1/file.dart";
|
||||
import "package:pkg2/file.dart";
|
||||
int pkg3() { pkg1() + pkg2() + 3; }
|
||||
""");
|
||||
|
||||
Uri pkg3Output = outDirUri.resolve("3.dill");
|
||||
inputDigests[pkg1Output] = [1];
|
||||
inputDigests[pkg2Output] = [2];
|
||||
result = await _compileSummaryAndCheck(
|
||||
pkg3Output,
|
||||
packagesFileUri,
|
||||
ddcOutlineUri,
|
||||
inputDigests,
|
||||
"package:pkg3/file.dart",
|
||||
previousState,
|
||||
inputSummaries: [pkg1Output.toFilePath(), pkg2Output.toFilePath()],
|
||||
);
|
||||
previousState = result.previousState;
|
||||
}
|
||||
|
||||
/// Compile one standalone package, then compile another that requires the
|
||||
/// first. Then compile a third that requires both the two previous.
|
||||
/// At one point we had a bug where the target wasn't updated properly and the
|
||||
/// second output had the same output as the first output, and trying to compile
|
||||
/// the third threw because it had two summaries as input with the same library
|
||||
/// inside.
|
||||
Future<void> ddc_summary_sources_change_standalone_require_one_require_both(
|
||||
Directory dir,
|
||||
) async {
|
||||
Uri outDirUri = dir.uri.resolve("out");
|
||||
new Directory.fromUri(outDirUri)..createSync();
|
||||
Uri packagesFileUri = dir.uri.resolve("packages.json");
|
||||
_writePackageFilePkg1To3(packagesFileUri);
|
||||
|
||||
Uri ddcOutlineUri = computePlatformBinariesLocation(
|
||||
forceBuildDir: true,
|
||||
).resolve("ddc_outline.dill");
|
||||
|
||||
final Map<Uri, List<int>> inputDigests = {
|
||||
ddcOutlineUri: [0],
|
||||
};
|
||||
|
||||
// Compile package:pkg1/file.dart - a standalone package.
|
||||
Uri pkg1File = dir.uri.resolve("live/pkg1/lib/file.dart");
|
||||
new File.fromUri(pkg1File)
|
||||
..createSync(recursive: true)
|
||||
..writeAsStringSync("int pkg1() { return 42; }");
|
||||
|
||||
InitializedCompilerState? previousState;
|
||||
Uri pkg1Output = outDirUri.resolve("1.dill");
|
||||
ComputeKernelResult result = await _compileSummaryAndCheck(
|
||||
pkg1Output,
|
||||
packagesFileUri,
|
||||
ddcOutlineUri,
|
||||
inputDigests,
|
||||
"package:pkg1/file.dart",
|
||||
previousState,
|
||||
);
|
||||
previousState = result.previousState;
|
||||
new File.fromUri(pkg1File).deleteSync();
|
||||
|
||||
// Compile package:pkg2/file.dart - needs pkg1.
|
||||
Uri pkg2File = dir.uri.resolve("live/pkg2/lib/file.dart");
|
||||
new File.fromUri(pkg2File)
|
||||
..createSync(recursive: true)
|
||||
..writeAsStringSync("""
|
||||
import "package:pkg1/file.dart";
|
||||
int pkg2() { return pkg1() + 1; }
|
||||
""");
|
||||
inputDigests[pkg1Output] = [1];
|
||||
|
||||
Uri pkg2Output = outDirUri.resolve("2.dill");
|
||||
result = await _compileSummaryAndCheck(
|
||||
pkg2Output,
|
||||
packagesFileUri,
|
||||
ddcOutlineUri,
|
||||
inputDigests,
|
||||
"package:pkg2/file.dart",
|
||||
previousState,
|
||||
inputSummaries: [pkg1Output.toFilePath()],
|
||||
);
|
||||
previousState = result.previousState;
|
||||
new File.fromUri(pkg2File).deleteSync();
|
||||
|
||||
// Compile package:pkg3/file.dart - needs pkg1 and pkg2.
|
||||
Uri pkg3File = dir.uri.resolve("live/pkg3/lib/file.dart");
|
||||
new File.fromUri(pkg3File)
|
||||
..createSync(recursive: true)
|
||||
..writeAsStringSync("""
|
||||
import "package:pkg1/file.dart";
|
||||
import "package:pkg2/file.dart";
|
||||
int pkg3() { return pkg1() + pkg2() + 2; }
|
||||
""");
|
||||
inputDigests[pkg2Output] = [2];
|
||||
|
||||
Uri pkg3Output = outDirUri.resolve("3.dill");
|
||||
result = await _compileSummaryAndCheck(
|
||||
pkg3Output,
|
||||
packagesFileUri,
|
||||
ddcOutlineUri,
|
||||
inputDigests,
|
||||
"package:pkg3/file.dart",
|
||||
previousState,
|
||||
inputSummaries: [pkg1Output.toFilePath(), pkg2Output.toFilePath()],
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> runHelper(Future<void> Function(Directory) runThis) async {
|
||||
Directory tmpDir = Directory.systemTemp.createTempSync('compute_kernel_test');
|
||||
try {
|
||||
await runThis(tmpDir);
|
||||
} catch (e, st) {
|
||||
stderr.writeln("Failure running $runThis:\n\n$e\n\n");
|
||||
stderr.writeln(st);
|
||||
stderr.writeln("\n-----\n");
|
||||
_countFailures++;
|
||||
exitCode = 1;
|
||||
} finally {
|
||||
try {
|
||||
tmpDir.deleteSync(recursive: true);
|
||||
} catch (e) {
|
||||
// Wait a little and retry.
|
||||
sleep(const Duration(milliseconds: 42));
|
||||
try {
|
||||
tmpDir.deleteSync(recursive: true);
|
||||
} catch (e) {
|
||||
print('Warning: Got exception when deleting temp dir: $e');
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Future<ComputeKernelResult> _compileSummaryAndCheck(
|
||||
Uri outDill,
|
||||
Uri packagesFileUri,
|
||||
Uri ddcOutlineUri,
|
||||
Map<Uri, List<int>> inputDigests,
|
||||
String source,
|
||||
InitializedCompilerState? previousState, {
|
||||
List<String>? inputSummaries,
|
||||
}) async {
|
||||
ComputeKernelResult result = await computeKernel(
|
||||
[
|
||||
"--output=${outDill.toFilePath()}",
|
||||
"--packages-file=${packagesFileUri.toFilePath()}",
|
||||
"--dart-sdk-summary=${ddcOutlineUri.toFilePath()}",
|
||||
"--exclude-non-sources",
|
||||
"--summary-only",
|
||||
"--reuse-compiler-result",
|
||||
"--use-incremental-compiler",
|
||||
"--sound-null-safety",
|
||||
"--source=$source",
|
||||
if (inputSummaries != null)
|
||||
for (String summary in inputSummaries) "--input-summary=$summary",
|
||||
],
|
||||
isWorker: true,
|
||||
outputBuffer: null,
|
||||
inputDigests: inputDigests,
|
||||
previousState: previousState,
|
||||
);
|
||||
if (!result.succeeded) throw "Failed to compile.";
|
||||
File outFile = new File.fromUri(outDill);
|
||||
|
||||
Component component = new Component();
|
||||
new BinaryBuilder(outFile.readAsBytesSync()).readComponent(component);
|
||||
List<String> outputLibs = component.libraries
|
||||
.map((lib) => lib.importUri.toString())
|
||||
.toList();
|
||||
|
||||
if (outputLibs.length != 1 || outputLibs.single != source) {
|
||||
throw "Failure: Output contained ${outputLibs.length} libraries, "
|
||||
"expected exactly 1 with uri $source: "
|
||||
"${outputLibs}";
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
void _writePackageFilePkg1To3(Uri packagesFileUri) {
|
||||
new File.fromUri(packagesFileUri)..writeAsStringSync("""
|
||||
{
|
||||
"configVersion": 2,
|
||||
"packages": [
|
||||
{
|
||||
"name": "pkg1",
|
||||
"rootUri": "live/pkg1",
|
||||
"packageUri": "lib/",
|
||||
"languageVersion": "3.10"
|
||||
},
|
||||
{
|
||||
"name": "pkg2",
|
||||
"rootUri": "live/pkg2",
|
||||
"packageUri": "lib/",
|
||||
"languageVersion": "3.10"
|
||||
},
|
||||
{
|
||||
"name": "pkg3",
|
||||
"rootUri": "live/pkg3",
|
||||
"packageUri": "lib/",
|
||||
"languageVersion": "3.10"
|
||||
}
|
||||
]
|
||||
}
|
||||
""");
|
||||
}
|
||||
@@ -334,13 +334,24 @@ abstract class Target {
|
||||
/// Perform target-specific transformations on the outlines stored in
|
||||
/// [Component] when generating summaries.
|
||||
///
|
||||
/// This transformation is used to add metadata on outlines and to filter
|
||||
/// unnecessary information before generating program summaries. This
|
||||
/// transformation is not applied when compiling full kernel programs to
|
||||
/// This is used to transform the libraries, but not for instance
|
||||
/// filtering the output libraries or adding metadata. Do this in
|
||||
/// [performOutlineComponentOperations] instead.
|
||||
/// This transformation is not applied when compiling full kernel programs to
|
||||
/// prevent affecting the internal invariants of the compiler and accidentally
|
||||
/// slowing down compilation.
|
||||
void performOutlineTransformations(Component component) {}
|
||||
|
||||
/// Perform target-specific operations on the [Component] storing the outlines
|
||||
/// when generating summaries.
|
||||
///
|
||||
/// This is not for transforming the libraries, but can be used to add
|
||||
/// metadata and filter libraries.
|
||||
/// This is not applied when compiling full kernel programs to prevent
|
||||
/// affecting the internal invariants of the compiler and accidentally
|
||||
/// slowing down compilation.
|
||||
void performOutlineComponentOperations(Component component) {}
|
||||
|
||||
/// Perform target-specific transformations on the given libraries that must
|
||||
/// run before constant evaluation.
|
||||
void performPreConstantEvaluationTransformations(
|
||||
@@ -605,6 +616,17 @@ abstract class Target {
|
||||
/// invalidation was only within the body of the mixin member.
|
||||
bool get incrementalCompilerIncludeMixinApplicationInvalidatedLibraries =>
|
||||
false;
|
||||
|
||||
/// If this target is - or can be made - to be compatible with [other].
|
||||
///
|
||||
/// Used for the modular incremental compilation pipeline.
|
||||
bool isModularlyCompatibleWith(Target other) => true;
|
||||
|
||||
/// Update this target to be compatible with [other]. Assumes
|
||||
/// [isModularlyCompatibleWith] returns true.
|
||||
///
|
||||
/// Used for the modular incremental compilation pipeline.
|
||||
void updateModularCompatibilityAs(Target other) {}
|
||||
}
|
||||
|
||||
class NoneConstantsBackend extends ConstantsBackend {
|
||||
@@ -1022,6 +1044,11 @@ class TargetWrapper extends Target {
|
||||
_target.performOutlineTransformations(component);
|
||||
}
|
||||
|
||||
@override
|
||||
void performOutlineComponentOperations(Component component) {
|
||||
_target.performOutlineComponentOperations(component);
|
||||
}
|
||||
|
||||
@override
|
||||
void performPreConstantEvaluationTransformations(
|
||||
Component component,
|
||||
@@ -1100,8 +1127,8 @@ mixin SummaryMixin on Target {
|
||||
bool get excludeNonSources;
|
||||
|
||||
@override
|
||||
void performOutlineTransformations(Component component) {
|
||||
super.performOutlineTransformations(component);
|
||||
void performOutlineComponentOperations(Component component) {
|
||||
super.performOutlineComponentOperations(component);
|
||||
if (!excludeNonSources) return;
|
||||
|
||||
List<Library> libraries = new List.of(component.libraries);
|
||||
|
||||
Reference in New Issue
Block a user