From 41aa8d42bced4a358e98d68b5694e5bb9f9efe79 Mon Sep 17 00:00:00 2001 From: Jens Johansen Date: Tue, 21 Apr 2026 23:26:41 -0700 Subject: [PATCH] Reapply "[kernel/cfe/etc] Split outline transformation into performOutlineTransformations and performOutlineComponentOperations" This reverts commit 264098c85fa326bcc5c88cd4ccd7628980a58c14. 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 Reviewed-by: Johnni Winther Commit-Queue: Jens Johansen --- .../lib/src/kernel/dart2js_target.dart | 14 + .../modular_incremental_compilation.dart | 5 +- .../lib/src/base/incremental_compiler.dart | 9 + .../lib/src/kernel_generator_impl.dart | 1 + pkg/frontend_server/lib/compute_kernel.dart | 19 +- .../test/compute_kernel_test.dart | 297 ++++++++++++++++++ pkg/kernel/lib/target/targets.dart | 37 ++- 7 files changed, 375 insertions(+), 7 deletions(-) create mode 100644 pkg/frontend_server/test/compute_kernel_test.dart diff --git a/pkg/compiler/lib/src/kernel/dart2js_target.dart b/pkg/compiler/lib/src/kernel/dart2js_target.dart index 385c59c24d3..d6e096aa123 100644 --- a/pkg/compiler/lib/src/kernel/dart2js_target.dart +++ b/pkg/compiler/lib/src/kernel/dart2js_target.dart @@ -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 { diff --git a/pkg/front_end/lib/src/api_unstable/modular_incremental_compilation.dart b/pkg/front_end/lib/src/api_unstable/modular_incremental_compilation.dart index 1ce4467b095..7d74da15ef9 100644 --- a/pkg/front_end/lib/src/api_unstable/modular_incremental_compilation.dart +++ b/pkg/front_end/lib/src/api_unstable/modular_incremental_compilation.dart @@ -83,7 +83,9 @@ Future 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 initializeIncrementalCompiler( options.packagesFileUri = packagesFile; options.fileSystem = fileSystem; processedOpts.clearFileSystemCache(); + options.target!.updateModularCompatibilityAs(target); } // Then read all the input summary components. diff --git a/pkg/front_end/lib/src/base/incremental_compiler.dart b/pkg/front_end/lib/src/base/incremental_compiler.dart index d995a1e4b01..3588a98ad91 100644 --- a/pkg/front_end/lib/src/base/incremental_compiler.dart +++ b/pkg/front_end/lib/src/base/incremental_compiler.dart @@ -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 currentlyCompilingLocal = _currentlyCompiling!; _currentlyCompiling = null; diff --git a/pkg/front_end/lib/src/kernel_generator_impl.dart b/pkg/front_end/lib/src/kernel_generator_impl.dart index 394619f7015..4ee685413fe 100644 --- a/pkg/front_end/lib/src/kernel_generator_impl.dart +++ b/pkg/front_end/lib/src/kernel_generator_impl.dart @@ -224,6 +224,7 @@ Future _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) { diff --git a/pkg/frontend_server/lib/compute_kernel.dart b/pkg/frontend_server/lib/compute_kernel.dart index 8aca0e1d9b5..02db28c4da7 100644 --- a/pkg/frontend_server/lib/compute_kernel.dart +++ b/pkg/frontend_server/lib/compute_kernel.dart @@ -392,7 +392,11 @@ Future 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 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) { diff --git a/pkg/frontend_server/test/compute_kernel_test.dart b/pkg/frontend_server/test/compute_kernel_test.dart new file mode 100644 index 00000000000..bb2a4fd00ad --- /dev/null +++ b/pkg/frontend_server/test/compute_kernel_test.dart @@ -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 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 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> 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 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> 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 runHelper(Future 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 _compileSummaryAndCheck( + Uri outDill, + Uri packagesFileUri, + Uri ddcOutlineUri, + Map> inputDigests, + String source, + InitializedCompilerState? previousState, { + List? 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 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" + } + ] +} +"""); +} diff --git a/pkg/kernel/lib/target/targets.dart b/pkg/kernel/lib/target/targets.dart index ce2b72727cc..2e0b21af905 100644 --- a/pkg/kernel/lib/target/targets.dart +++ b/pkg/kernel/lib/target/targets.dart @@ -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 libraries = new List.of(component.libraries);