Skip to content

Commit 2a39f63

Browse files
sigmundchcommit-bot@chromium.org
authored andcommitted
Record deferred accesses in kernel world impacts and use it for splitting only
under a flag. This reapplies commit 70e1517, but adds a flag to gradually migrate users before enabling it by default. Patchset 1 matches the old CL Change-Id: Iaf7ee3dec8d4aa658f0b4334549b507e5a610a68 Reviewed-on: https://dart-review.googlesource.com/c/86444 Reviewed-by: Stephen Adams <[email protected]> Commit-Queue: Sigmund Cherem <[email protected]>
1 parent a848632 commit 2a39f63

File tree

15 files changed

+362
-104
lines changed

15 files changed

+362
-104
lines changed

CHANGELOG.md

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,64 @@
2121

2222
### Tool Changes
2323

24+
#### dart2js
25+
26+
* We fixed a bug in how deferred constructor calls were incorrectly not
27+
marked as deferred. The old behavior didn't cause breakages, but was imprecise
28+
and pushed more code to the main output unit.
29+
30+
* A new deferred split algorithm implementation was added.
31+
32+
This implementation fixes a soundness bug and addresses performance issues of
33+
the previous implementation, because of that it can have a visible impact
34+
on apps. In particular,
35+
36+
* We fixed a performance issue which was introduced when we migrated to the
37+
Common front-end. On large apps, the fix can cut down 2/3 of the time
38+
spent on this task.
39+
40+
* We fixed a bug in how inferred types were miscategorized (#35311). The old
41+
behavior was unsound and could produce broken programs. The fix may cause
42+
more code to be pulled into the main output unit.
43+
44+
This shows up frequently when returning deferred values from closures
45+
since the closure's inferred return type is the deferred type.
46+
For example, if you have:
47+
48+
```dart
49+
() async {
50+
await deferred_prefix.loadLibrary();
51+
return new deferred_prefix.Foo();
52+
}
53+
```
54+
55+
The closure's return type is `Future<Foo>`. The old implementation defers
56+
`Foo`, and incorrectly makes the return type `Future<dynamic>`. This may
57+
break in places where the correct type is expected.
58+
59+
The new implementation will not defer `Foo`, and will place it in the main
60+
output unit. If your intent is to defer it, then you need to ensure the
61+
return type is not inferred to be `Foo`. For example, you can do so by
62+
changing the code to a named closure with a declared type, or by ensuring
63+
that the return expression has the type you want, like:
64+
65+
```dart
66+
() async {
67+
await deferred_prefix.loadLibrary();
68+
return new deferred_prefix.Foo() as dynamic;
69+
}
70+
```
71+
72+
* Because the new implementation might require you to inspect and fix
73+
your app, we exposed two temporary flags:
74+
75+
* `--report-invalid-deferred-types`: when provided, we will run
76+
both the old and new algorithm and report where the issue was
77+
detected.
78+
79+
* `--new-deferred-split`: enables the new algorithm.
80+
81+
2482
#### Pub
2583
2684
#### Linter

pkg/compiler/lib/src/commandline_options.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ class Flags {
7878

7979
static const String serverMode = '--server-mode';
8080

81+
static const String newDeferredSplit = '--new-deferred-split';
82+
static const String reportInvalidInferredDeferredTypes =
83+
'--report-invalid-deferred-types';
84+
8185
/// Flag for a combination of flags for 'production' mode.
8286
static const String benchmarkingProduction = '--benchmarking-production';
8387

pkg/compiler/lib/src/compiler.dart

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,9 @@ abstract class Compiler {
6767
/// Options provided from command-line arguments.
6868
final CompilerOptions options;
6969

70-
/**
71-
* If true, stop compilation after type inference is complete. Used for
72-
* debugging and testing purposes only.
73-
*/
70+
// These internal flags are used to stop compilation after a specific phase.
71+
// Used only for debugging and testing purposes only.
72+
bool stopAfterClosedWorld = false;
7473
bool stopAfterTypeInference = false;
7574

7675
/// Output provider from user of Compiler API.
@@ -251,6 +250,7 @@ abstract class Compiler {
251250
generateJavaScriptCode(results);
252251
} else {
253252
KernelResult result = await kernelLoader.load(uri);
253+
reporter.log("Kernel load complete");
254254
if (result == null) return;
255255
if (compilationFailed && !options.generateCodeWithCompileTimeErrors) {
256256
return;
@@ -390,6 +390,7 @@ abstract class Compiler {
390390
selfTask.measureSubtask("compileFromKernel", () {
391391
JClosedWorld closedWorld = selfTask.measureSubtask("computeClosedWorld",
392392
() => computeClosedWorld(rootLibraryUri, libraries));
393+
if (stopAfterClosedWorld) return;
393394
if (closedWorld != null) {
394395
GlobalTypeInferenceResults globalInferenceResults =
395396
performGlobalTypeInference(closedWorld);

pkg/compiler/lib/src/dart2js.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,8 @@ Future<api.CompilationResult> compile(List<String> argv,
377377
new OptionHandler(Flags.disableRtiOptimization, passThrough),
378378
new OptionHandler(Flags.terse, passThrough),
379379
new OptionHandler('--deferred-map=.+', passThrough),
380+
new OptionHandler(Flags.newDeferredSplit, passThrough),
381+
new OptionHandler(Flags.reportInvalidInferredDeferredTypes, passThrough),
380382
new OptionHandler(Flags.dumpInfo, passThrough),
381383
new OptionHandler('--disallow-unsafe-eval', ignoreOption),
382384
new OptionHandler(Option.showPackageWarnings, passThrough),

0 commit comments

Comments
 (0)