Skip to content

Commit 41aa8d4

Browse files
jensjohadart-scoped@luci-project-accounts.iam.gserviceaccount.com
authored andcommitted
Reapply "[kernel/cfe/etc] Split outline transformation into performOutlineTransformations and performOutlineComponentOperations"
This reverts commit 264098c. 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>
1 parent ae0fa46 commit 41aa8d4

7 files changed

Lines changed: 375 additions & 7 deletions

File tree

‎pkg/compiler/lib/src/kernel/dart2js_target.dart‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,20 @@ class Dart2jsSummaryTarget extends Dart2jsTarget with SummaryMixin {
361361
this.excludeNonSources,
362362
TargetFlags targetFlags,
363363
) : super(name, targetFlags);
364+
365+
@override
366+
bool isModularlyCompatibleWith(Target other) {
367+
if (other is! Dart2jsSummaryTarget) return false;
368+
if (excludeNonSources != other.excludeNonSources) return false;
369+
return true;
370+
}
371+
372+
@override
373+
void updateModularCompatibilityAs(Target other) {
374+
assert(other is Dart2jsSummaryTarget);
375+
sources.clear();
376+
sources.addAll((other as Dart2jsSummaryTarget).sources);
377+
}
364378
}
365379

366380
class Dart2jsConstantsBackend extends ConstantsBackend {

‎pkg/front_end/lib/src/api_unstable/modular_incremental_compilation.dart‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,9 @@ Future<InitializedCompilerState> initializeIncrementalCompiler(
8383
!equalSets(oldState.tags, tags) ||
8484
(sdkSummary != null &&
8585
(cachedSdkInput == null ||
86-
!digestsEqual(cachedSdkInput.digest, sdkDigest)))) {
86+
!digestsEqual(cachedSdkInput.digest, sdkDigest))) ||
87+
(oldState.options.target == null ||
88+
!oldState.options.target!.isModularlyCompatibleWith(target))) {
8789
// No - or immediately not correct - previous state.
8890
// We'll load a new sdk, anything loaded already will have a wrong root.
8991
workerInputCache.clear();
@@ -148,6 +150,7 @@ Future<InitializedCompilerState> initializeIncrementalCompiler(
148150
options.packagesFileUri = packagesFile;
149151
options.fileSystem = fileSystem;
150152
processedOpts.clearFileSystemCache();
153+
options.target!.updateModularCompatibilityAs(target);
151154
}
152155

153156
// Then read all the input summary components.

‎pkg/front_end/lib/src/base/incremental_compiler.dart‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
441441
);
442442
componentWithDill = buildResult.component;
443443
}
444+
// Coverage-ignore(suite): Not run.
445+
else if (componentWithDill != null) {
446+
context.options.target.performOutlineTransformations(componentWithDill);
447+
}
444448

445449
_benchmarker
446450
// Coverage-ignore(suite): Not run.
@@ -578,6 +582,11 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
578582
// about other libraries.
579583
result.metadata.addAll(componentWithDill.metadata);
580584

585+
if (outlineOnly) {
586+
// Coverage-ignore-block(suite): Not run.
587+
context.options.target.performOutlineComponentOperations(result);
588+
}
589+
581590
// We're now done. Allow any waiting compile to start.
582591
Completer<dynamic> currentlyCompilingLocal = _currentlyCompiling!;
583592
_currentlyCompiling = null;

‎pkg/front_end/lib/src/kernel_generator_impl.dart‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ Future<InternalCompilerResult> _buildInternal(
224224
// the only need we have for these transformations).
225225
if (!buildComponent) {
226226
options.target.performOutlineTransformations(trimmedSummaryComponent);
227+
options.target.performOutlineComponentOperations(trimmedSummaryComponent);
227228
options.ticker.logMs("Transformed outline");
228229
}
229230
if (serializeIfBuildingSummary) {

‎pkg/frontend_server/lib/compute_kernel.dart‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,11 @@ Future<ComputeKernelResult> computeKernel(
392392
previousState,
393393
{
394394
"target=$targetName",
395+
// trackWidgetCreation is in TargetFlags.
395396
"trackWidgetCreation=$trackWidgetCreation",
397+
// includeUnsupportedPlatformLibraryStubs is in TargetFlags.
398+
"includeUnsupportedPlatformLibraryStubs="
399+
"$includeUnsupportedPlatformLibraryStubs",
396400
"multiRootScheme=${mrfs.markerScheme}",
397401
"multiRootRoots=${mrfs.roots}",
398402
},
@@ -483,7 +487,6 @@ Future<ComputeKernelResult> computeKernel(
483487
incrementalComponent.uriToSource.clear();
484488
incrementalComponent.problemsAsJson = null;
485489
incrementalComponent.setMainMethodAndMode(null, true);
486-
target.performOutlineTransformations(incrementalComponent);
487490
makeStable(incrementalComponent);
488491
return new Future.value(
489492
fe.serializeComponent(
@@ -615,6 +618,20 @@ class DevCompilerSummaryTarget extends DevCompilerTarget with SummaryMixin {
615618
this.excludeNonSources,
616619
TargetFlags targetFlags,
617620
) : super(targetFlags);
621+
622+
@override
623+
bool isModularlyCompatibleWith(Target other) {
624+
if (other is! DevCompilerSummaryTarget) return false;
625+
if (excludeNonSources != other.excludeNonSources) return false;
626+
return true;
627+
}
628+
629+
@override
630+
void updateModularCompatibilityAs(Target other) {
631+
assert(other is DevCompilerSummaryTarget);
632+
sources.clear();
633+
sources.addAll((other as DevCompilerSummaryTarget).sources);
634+
}
618635
}
619636

620637
Uri? toUriNullable(String? uriString) {

0 commit comments

Comments
 (0)