Record deferred accesses in kernel world impacts and use it for splitting only
under a flag.
This reapplies commit 70e1517d985e234ffcb1df637dc650eb1cd1386c, 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 <sra@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c3a62fd..681e1c6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -21,6 +21,64 @@
### Tool Changes
+#### dart2js
+
+* We fixed a bug in how deferred constructor calls were incorrectly not
+ marked as deferred. The old behavior didn't cause breakages, but was imprecise
+ and pushed more code to the main output unit.
+
+* A new deferred split algorithm implementation was added.
+
+ This implementation fixes a soundness bug and addresses performance issues of
+ the previous implementation, because of that it can have a visible impact
+ on apps. In particular,
+
+ * We fixed a performance issue which was introduced when we migrated to the
+ Common front-end. On large apps, the fix can cut down 2/3 of the time
+ spent on this task.
+
+ * We fixed a bug in how inferred types were miscategorized (#35311). The old
+ behavior was unsound and could produce broken programs. The fix may cause
+ more code to be pulled into the main output unit.
+
+ This shows up frequently when returning deferred values from closures
+ since the closure's inferred return type is the deferred type.
+ For example, if you have:
+
+ ```dart
+ () async {
+ await deferred_prefix.loadLibrary();
+ return new deferred_prefix.Foo();
+ }
+ ```
+
+ The closure's return type is `Future<Foo>`. The old implementation defers
+ `Foo`, and incorrectly makes the return type `Future<dynamic>`. This may
+ break in places where the correct type is expected.
+
+ The new implementation will not defer `Foo`, and will place it in the main
+ output unit. If your intent is to defer it, then you need to ensure the
+ return type is not inferred to be `Foo`. For example, you can do so by
+ changing the code to a named closure with a declared type, or by ensuring
+ that the return expression has the type you want, like:
+
+ ```dart
+ () async {
+ await deferred_prefix.loadLibrary();
+ return new deferred_prefix.Foo() as dynamic;
+ }
+ ```
+
+ * Because the new implementation might require you to inspect and fix
+ your app, we exposed two temporary flags:
+
+ * `--report-invalid-deferred-types`: when provided, we will run
+ both the old and new algorithm and report where the issue was
+ detected.
+
+ * `--new-deferred-split`: enables the new algorithm.
+
+
#### Pub
#### Linter
diff --git a/pkg/compiler/lib/src/commandline_options.dart b/pkg/compiler/lib/src/commandline_options.dart
index 391cdd9..a3cf93f 100644
--- a/pkg/compiler/lib/src/commandline_options.dart
+++ b/pkg/compiler/lib/src/commandline_options.dart
@@ -78,6 +78,10 @@
static const String serverMode = '--server-mode';
+ static const String newDeferredSplit = '--new-deferred-split';
+ static const String reportInvalidInferredDeferredTypes =
+ '--report-invalid-deferred-types';
+
/// Flag for a combination of flags for 'production' mode.
static const String benchmarkingProduction = '--benchmarking-production';
diff --git a/pkg/compiler/lib/src/compiler.dart b/pkg/compiler/lib/src/compiler.dart
index 85cc18e..1c04d76 100644
--- a/pkg/compiler/lib/src/compiler.dart
+++ b/pkg/compiler/lib/src/compiler.dart
@@ -67,10 +67,9 @@
/// Options provided from command-line arguments.
final CompilerOptions options;
- /**
- * If true, stop compilation after type inference is complete. Used for
- * debugging and testing purposes only.
- */
+ // These internal flags are used to stop compilation after a specific phase.
+ // Used only for debugging and testing purposes only.
+ bool stopAfterClosedWorld = false;
bool stopAfterTypeInference = false;
/// Output provider from user of Compiler API.
@@ -251,6 +250,7 @@
generateJavaScriptCode(results);
} else {
KernelResult result = await kernelLoader.load(uri);
+ reporter.log("Kernel load complete");
if (result == null) return;
if (compilationFailed && !options.generateCodeWithCompileTimeErrors) {
return;
@@ -390,6 +390,7 @@
selfTask.measureSubtask("compileFromKernel", () {
JClosedWorld closedWorld = selfTask.measureSubtask("computeClosedWorld",
() => computeClosedWorld(rootLibraryUri, libraries));
+ if (stopAfterClosedWorld) return;
if (closedWorld != null) {
GlobalTypeInferenceResults globalInferenceResults =
performGlobalTypeInference(closedWorld);
diff --git a/pkg/compiler/lib/src/dart2js.dart b/pkg/compiler/lib/src/dart2js.dart
index dfab97f..e13dae6 100644
--- a/pkg/compiler/lib/src/dart2js.dart
+++ b/pkg/compiler/lib/src/dart2js.dart
@@ -377,6 +377,8 @@
new OptionHandler(Flags.disableRtiOptimization, passThrough),
new OptionHandler(Flags.terse, passThrough),
new OptionHandler('--deferred-map=.+', passThrough),
+ new OptionHandler(Flags.newDeferredSplit, passThrough),
+ new OptionHandler(Flags.reportInvalidInferredDeferredTypes, passThrough),
new OptionHandler(Flags.dumpInfo, passThrough),
new OptionHandler('--disallow-unsafe-eval', ignoreOption),
new OptionHandler(Option.showPackageWarnings, passThrough),
diff --git a/pkg/compiler/lib/src/deferred_load.dart b/pkg/compiler/lib/src/deferred_load.dart
index 8aafc91..d2fd134 100644
--- a/pkg/compiler/lib/src/deferred_load.dart
+++ b/pkg/compiler/lib/src/deferred_load.dart
@@ -130,6 +130,9 @@
final Compiler compiler;
bool get disableProgramSplit => compiler.options.disableProgramSplit;
+ bool get newDeferredSplit => compiler.options.newDeferredSplit;
+ bool get reportInvalidInferredDeferredTypes =>
+ compiler.options.reportInvalidInferredDeferredTypes;
DeferredLoadTask(this.compiler) : super(compiler.measurer) {
_mainOutputUnit = new OutputUnit(true, 'main', new Set<ImportEntity>());
@@ -193,7 +196,7 @@
void addLiveInstanceMember(MemberEntity member) {
if (!compiler.resolutionWorldBuilder.isMemberUsed(member)) return;
if (!member.isInstanceMember) return;
- dependencies.members.add(member);
+ dependencies.addMember(member);
_collectDirectMemberDependencies(member, dependencies);
}
@@ -202,7 +205,7 @@
elementEnvironment.forEachSupertype(cls, (InterfaceType type) {
_collectTypeDependencies(type, dependencies);
});
- dependencies.classes.add(cls);
+ dependencies.addClass(cls);
}
/// Finds all elements and constants that [element] depends directly on.
@@ -216,7 +219,7 @@
elementEnvironment.getFunctionType(element), dependencies);
}
if (element.isStatic || element.isTopLevel || element.isConstructor) {
- dependencies.members.add(element);
+ dependencies.addMember(element);
_collectDirectMemberDependencies(element, dependencies);
}
if (element is ConstructorEntity && element.isGenerativeConstructor) {
@@ -243,21 +246,26 @@
/// Recursively collects all the dependencies of [type].
void _collectTypeDependencies(DartType type, Dependencies dependencies) {
- // TODO(het): we would like to separate out types that are only needed for
- // rti from types that are needed for their members.
if (type is FunctionType) {
_collectFunctionTypeDependencies(type, dependencies);
} else if (type is TypedefType) {
- type.typeArguments
- .forEach((t) => _collectTypeDependencies(t, dependencies));
+ _collectTypeArgumentDependencies(type.typeArguments, dependencies);
_collectTypeDependencies(type.unaliased, dependencies);
} else if (type is InterfaceType) {
- type.typeArguments
- .forEach((t) => _collectTypeDependencies(t, dependencies));
- dependencies.classes.add(type.element);
+ _collectTypeArgumentDependencies(type.typeArguments, dependencies);
+ // TODO(sigmund): when we are able to split classes from types in our
+ // runtime-type representation, this should track type.element as a type
+ // dependency instead.
+ dependencies.addClass(type.element);
}
}
+ void _collectTypeArgumentDependencies(
+ Iterable<DartType> typeArguments, Dependencies dependencies) {
+ if (typeArguments == null) return;
+ typeArguments.forEach((t) => _collectTypeDependencies(t, dependencies));
+ }
+
void _collectFunctionTypeDependencies(
FunctionType type, Dependencies dependencies) {
for (FunctionTypeVariable typeVariable in type.typeVariables) {
@@ -285,7 +293,7 @@
new WorldImpactVisitorImpl(visitStaticUse: (StaticUse staticUse) {
Entity usedEntity = staticUse.element;
if (usedEntity is MemberEntity) {
- dependencies.members.add(usedEntity);
+ dependencies.addMember(usedEntity, staticUse.deferredImport);
} else {
assert(usedEntity is KLocalFunction,
failedAt(usedEntity, "Unexpected static use $staticUse."));
@@ -298,19 +306,23 @@
switch (staticUse.kind) {
case StaticUseKind.CONSTRUCTOR_INVOKE:
case StaticUseKind.CONST_CONSTRUCTOR_INVOKE:
- _collectTypeDependencies(staticUse.type, dependencies);
+ // The receiver type of generative constructors is a dependency of
+ // the constructor (handled by `addMember` above) and not a
+ // dependency at the call site.
+ // Factory methods, on the other hand, are like static methods so
+ // the target type is not relevant.
+ // TODO(johnniwinther): Use rti need data to skip unneeded type
+ // arguments.
+ _collectTypeArgumentDependencies(
+ staticUse.type.typeArguments, dependencies);
break;
case StaticUseKind.INVOKE:
case StaticUseKind.CLOSURE_CALL:
case StaticUseKind.DIRECT_INVOKE:
// TODO(johnniwinther): Use rti need data to skip unneeded type
// arguments.
- List<DartType> typeArguments = staticUse.typeArguments;
- if (typeArguments != null) {
- for (DartType typeArgument in typeArguments) {
- _collectTypeDependencies(typeArgument, dependencies);
- }
- }
+ _collectTypeArgumentDependencies(
+ staticUse.typeArguments, dependencies);
break;
default:
}
@@ -320,7 +332,8 @@
case TypeUseKind.TYPE_LITERAL:
if (type.isInterfaceType) {
InterfaceType interface = type;
- dependencies.classes.add(interface.element);
+ dependencies.addClass(
+ interface.element, typeUse.deferredImport);
}
break;
case TypeUseKind.INSTANTIATION:
@@ -352,12 +365,8 @@
}, visitDynamicUse: (DynamicUse dynamicUse) {
// TODO(johnniwinther): Use rti need data to skip unneeded type
// arguments.
- List<DartType> typeArguments = dynamicUse.typeArguments;
- if (typeArguments != null) {
- for (DartType typeArgument in typeArguments) {
- _collectTypeDependencies(typeArgument, dependencies);
- }
- }
+ _collectTypeArgumentDependencies(
+ dynamicUse.typeArguments, dependencies);
}),
DeferredLoadTask.IMPACT_USE);
}
@@ -428,7 +437,8 @@
Dependencies dependencies = new Dependencies();
_collectAllElementsAndConstantsResolvedFromClass(element, dependencies);
LibraryEntity library = element.library;
- _processDependencies(library, dependencies, oldSet, newSet, queue);
+ _processDependencies(
+ library, dependencies, oldSet, newSet, queue, element);
} else {
queue.addClass(element, newSet);
}
@@ -455,7 +465,8 @@
_collectAllElementsAndConstantsResolvedFromMember(element, dependencies);
LibraryEntity library = element.library;
- _processDependencies(library, dependencies, oldSet, newSet, queue);
+ _processDependencies(
+ library, dependencies, oldSet, newSet, queue, element);
} else {
queue.addMember(element, newSet);
}
@@ -487,60 +498,111 @@
/// same nodes we have already seen.
_shouldAddDeferredDependency(ImportSet newSet) => newSet.length <= 1;
+ void _fixDependencyInfo(DependencyInfo info, List<ImportEntity> imports,
+ String prefix, String name, Spannable context) {
+ var isDeferred = _isExplicitlyDeferred(imports);
+ if (isDeferred) {
+ if (!newDeferredSplit) {
+ info.isDeferred = true;
+ info.imports = imports;
+ }
+ if (reportInvalidInferredDeferredTypes) {
+ reporter.reportErrorMessage(context, MessageKind.GENERIC, {
+ 'text': "$prefix '$name' is deferred but appears to be inferred as"
+ " a return type or a type parameter (dartbug.com/35311)."
+ });
+ }
+ }
+ }
+
+ // The following 3 methods are used to check whether the new deferred split
+ // algorithm and the old one match. Because of a soundness bug in the old
+ // algorithm the new algorithm can pull in a lot of code to the main output
+ // unit. This logic detects it and will make it easier for us to migrate code
+ // off it incrementally.
+ // Note: we only expect discrepancies on class-dependency-info due to how
+ // inferred types expose deferred types in type-variables and return types
+ // (Issue #35311). We added the other two methods to test our transition, but
+ // we don't expect to detect any mismatches there.
+ //
+ // TODO(sigmund): delete once the new implementation is on by default.
+ void _fixClassDependencyInfo(DependencyInfo info, ClassEntity cls,
+ LibraryEntity library, Spannable context) {
+ if (info.isDeferred) return;
+ if (newDeferredSplit && !reportInvalidInferredDeferredTypes) return;
+ var imports = classImportsTo(cls, library);
+ _fixDependencyInfo(info, imports, "Class", cls.name, context);
+ }
+
+ void _fixMemberDependencyInfo(DependencyInfo info, MemberEntity member,
+ LibraryEntity library, Spannable context) {
+ if (info.isDeferred || compiler.options.newDeferredSplit) return;
+ var imports = memberImportsTo(member, library);
+ _fixDependencyInfo(info, imports, "Member", member.name, context);
+ }
+
+ void _fixConstantDependencyInfo(DependencyInfo info, ConstantValue constant,
+ LibraryEntity library, Spannable context) {
+ if (info.isDeferred || compiler.options.newDeferredSplit) return;
+ if (constant is TypeConstantValue) {
+ var type = constant.representedType;
+ if (type is InterfaceType) {
+ var imports = classImportsTo(type.element, library);
+ _fixDependencyInfo(
+ info, imports, "Class (in constant) ", type.element.name, context);
+ } else if (type is TypedefType) {
+ var imports = typedefImportsTo(type.element, library);
+ _fixDependencyInfo(
+ info, imports, "Typedef ", type.element.name, context);
+ }
+ }
+ }
+
void _processDependencies(LibraryEntity library, Dependencies dependencies,
- ImportSet oldSet, ImportSet newSet, WorkQueue queue) {
- for (ClassEntity cls in dependencies.classes) {
- Iterable<ImportEntity> imports = classImportsTo(cls, library);
- if (_isExplicitlyDeferred(imports)) {
+ ImportSet oldSet, ImportSet newSet, WorkQueue queue, Spannable context) {
+ dependencies.classes.forEach((ClassEntity cls, DependencyInfo info) {
+ _fixClassDependencyInfo(info, cls, library, context);
+ if (info.isDeferred) {
if (_shouldAddDeferredDependency(newSet)) {
- for (ImportEntity deferredImport in imports) {
+ for (ImportEntity deferredImport in info.imports) {
queue.addClass(cls, importSets.singleton(deferredImport));
}
}
} else {
_updateClassRecursive(cls, oldSet, newSet, queue);
}
- }
+ });
- for (MemberEntity member in dependencies.members) {
- Iterable<ImportEntity> imports = memberImportsTo(member, library);
- if (_isExplicitlyDeferred(imports)) {
+ dependencies.members.forEach((MemberEntity member, DependencyInfo info) {
+ _fixMemberDependencyInfo(info, member, library, context);
+ if (info.isDeferred) {
if (_shouldAddDeferredDependency(newSet)) {
- for (ImportEntity deferredImport in imports) {
+ for (ImportEntity deferredImport in info.imports) {
queue.addMember(member, importSets.singleton(deferredImport));
}
}
} else {
_updateMemberRecursive(member, oldSet, newSet, queue);
}
- }
+ });
for (Local localFunction in dependencies.localFunctions) {
_updateLocalFunction(localFunction, oldSet, newSet);
}
- for (ConstantValue dependency in dependencies.constants) {
- if (dependency is TypeConstantValue) {
- var type = dependency.representedType;
- var imports = const <ImportEntity>[];
- if (type is InterfaceType) {
- imports = classImportsTo(type.element, library);
- } else if (type is TypedefType) {
- imports = typedefImportsTo(type.element, library);
- }
- if (_isExplicitlyDeferred(imports)) {
- if (_shouldAddDeferredDependency(newSet)) {
- for (ImportEntity deferredImport in imports) {
- queue.addConstant(
- dependency, importSets.singleton(deferredImport));
- }
+ dependencies.constants
+ .forEach((ConstantValue constant, DependencyInfo info) {
+ _fixConstantDependencyInfo(info, constant, library, context);
+ if (info.isDeferred) {
+ if (_shouldAddDeferredDependency(newSet)) {
+ for (ImportEntity deferredImport in info.imports) {
+ queue.addConstant(constant, importSets.singleton(deferredImport));
}
- continue;
}
+ } else {
+ _updateConstantRecursive(constant, oldSet, newSet, queue);
}
-
- _updateConstantRecursive(dependency, oldSet, newSet, queue);
- }
+ });
}
/// Adds extra dependencies coming from mirror usage.
@@ -1488,8 +1550,36 @@
}
class Dependencies {
- final Set<ClassEntity> classes = new Set<ClassEntity>();
- final Set<MemberEntity> members = new Set<MemberEntity>();
+ final Map<ClassEntity, DependencyInfo> classes = {};
+ final Map<MemberEntity, DependencyInfo> members = {};
final Set<Local> localFunctions = new Set<Local>();
- final Set<ConstantValue> constants = new Set<ConstantValue>();
+ final Map<ConstantValue, DependencyInfo> constants = {};
+
+ void addClass(ClassEntity cls, [ImportEntity import]) {
+ (classes[cls] ??= new DependencyInfo()).registerImport(import);
+ }
+
+ void addMember(MemberEntity m, [ImportEntity import]) {
+ (members[m] ??= new DependencyInfo()).registerImport(import);
+ }
+
+ void addConstant(ConstantValue c, [ImportEntity import]) {
+ (constants[c] ??= new DependencyInfo()).registerImport(import);
+ }
+}
+
+class DependencyInfo {
+ bool isDeferred = true;
+ List<ImportEntity> imports;
+
+ registerImport(ImportEntity import) {
+ if (!isDeferred) return;
+ // A null import represents a direct non-deferred dependency.
+ if (import != null) {
+ (imports ??= []).add(import);
+ } else {
+ imports = null;
+ isDeferred = false;
+ }
+ }
}
diff --git a/pkg/compiler/lib/src/ir/util.dart b/pkg/compiler/lib/src/ir/util.dart
index fb6ba1e..fb57fed 100644
--- a/pkg/compiler/lib/src/ir/util.dart
+++ b/pkg/compiler/lib/src/ir/util.dart
@@ -111,3 +111,34 @@
}
return null;
}
+
+/// Check whether [node] is immediately guarded by a
+/// [ir.CheckLibraryIsLoaded], and hence the node is a deferred access.
+ir.LibraryDependency getDeferredImport(ir.TreeNode node) {
+ // Note: this code relies on the CFE generating the code as we expect it here.
+ // If one day we optimize away redundant CheckLibraryIsLoaded instructions,
+ // we'd need to derive this information directly from the CFE (See #35005),
+ ir.TreeNode parent = node.parent;
+
+ // TODO(sigmund): remove when CFE generates the correct tree (#35320). For
+ // instance, it currently generates
+ //
+ // let _ = check(prefix) in (prefix::field.property)
+ //
+ // instead of:
+ //
+ // (let _ = check(prefix) in prefix::field).property
+ if (node is ir.StaticGet) {
+ while (parent is ir.PropertyGet || parent is ir.MethodInvocation) {
+ parent = parent.parent;
+ }
+ }
+
+ if (parent is ir.Let) {
+ var initializer = parent.variable.initializer;
+ if (initializer is ir.CheckLibraryIsLoaded) {
+ return initializer.import;
+ }
+ }
+ return null;
+}
diff --git a/pkg/compiler/lib/src/kernel/deferred_load.dart b/pkg/compiler/lib/src/kernel/deferred_load.dart
index 0abcdb3..ecdbd9f 100644
--- a/pkg/compiler/lib/src/kernel/deferred_load.dart
+++ b/pkg/compiler/lib/src/kernel/deferred_load.dart
@@ -11,6 +11,7 @@
import '../constants/values.dart';
import '../deferred_load.dart';
import '../elements/entities.dart';
+import '../ir/util.dart';
import 'element_map.dart';
class KernelDeferredLoadTask extends DeferredLoadTask {
@@ -25,6 +26,7 @@
return measureSubtask('find-imports', () {
List<ImportEntity> imports = [];
ir.Library source = _elementMap.getLibraryNode(library);
+ if (!source.dependencies.any((d) => d.isDeferred)) return const [];
for (ir.LibraryDependency dependency in source.dependencies) {
if (dependency.isExport) continue;
if (!_isVisible(dependency.combinators, nodeName)) continue;
@@ -55,7 +57,11 @@
Iterable<ImportEntity> memberImportsTo(
Entity element, LibraryEntity library) {
ir.Member node = _elementMap.getMemberNode(element);
- return _findImportsTo(node, node.name.name, node.enclosingLibrary, library);
+ return _findImportsTo(
+ node is ir.Constructor ? node.enclosingClass : node,
+ node is ir.Constructor ? node.enclosingClass.name : node.name.name,
+ node.enclosingLibrary,
+ library);
}
@override
@@ -138,7 +144,8 @@
ConstantValue constant =
elementMap.getConstantValue(node, requireConstant: required);
if (constant != null) {
- dependencies.constants.add(constant);
+ dependencies.addConstant(
+ constant, elementMap.getImport(getDeferredImport(node)));
}
}
diff --git a/pkg/compiler/lib/src/kernel/element_map_impl.dart b/pkg/compiler/lib/src/kernel/element_map_impl.dart
index 3db40e0..abc103e 100644
--- a/pkg/compiler/lib/src/kernel/element_map_impl.dart
+++ b/pkg/compiler/lib/src/kernel/element_map_impl.dart
@@ -707,6 +707,7 @@
}
ImportEntity getImport(ir.LibraryDependency node) {
+ if (node == null) return null;
ir.Library library = node.parent;
KLibraryData data = libraries.getData(getLibraryInternal(library));
return data.imports[node];
diff --git a/pkg/compiler/lib/src/kernel/kernel_impact.dart b/pkg/compiler/lib/src/kernel/kernel_impact.dart
index a38cd9e..249418d 100644
--- a/pkg/compiler/lib/src/kernel/kernel_impact.dart
+++ b/pkg/compiler/lib/src/kernel/kernel_impact.dart
@@ -252,10 +252,12 @@
InterfaceType type = elementMap.createInterfaceType(
target.enclosingClass, node.arguments.types);
CallStructure callStructure = elementMap.getCallStructure(node.arguments);
+ ImportEntity deferredImport = elementMap.getImport(getDeferredImport(node));
impactBuilder.registerStaticUse(isConst
- ? new StaticUse.constConstructorInvoke(constructor, callStructure, type)
+ ? new StaticUse.constConstructorInvoke(
+ constructor, callStructure, type, deferredImport)
: new StaticUse.typedConstructorInvoke(
- constructor, callStructure, type));
+ constructor, callStructure, type, deferredImport));
if (type.typeArguments.any((DartType type) => !type.isDynamic)) {
impactBuilder.registerFeature(Feature.TYPE_VARIABLE_BOUNDS_CHECK);
}
@@ -321,8 +323,13 @@
_handleExtractTypeArguments(node, target, typeArguments);
return;
}
+ ImportEntity deferredImport =
+ elementMap.getImport(getDeferredImport(node));
impactBuilder.registerStaticUse(new StaticUse.staticInvoke(
- target, elementMap.getCallStructure(node.arguments), typeArguments));
+ target,
+ elementMap.getCallStructure(node.arguments),
+ typeArguments,
+ deferredImport));
}
switch (elementMap.getForeignKind(node)) {
case ForeignKind.JS:
@@ -382,17 +389,20 @@
ir.Member target = node.target;
if (target is ir.Procedure && target.kind == ir.ProcedureKind.Method) {
FunctionEntity method = elementMap.getMethod(target);
- impactBuilder.registerStaticUse(new StaticUse.staticTearOff(method));
+ impactBuilder.registerStaticUse(new StaticUse.staticTearOff(
+ method, elementMap.getImport(getDeferredImport(node))));
} else {
MemberEntity member = elementMap.getMember(target);
- impactBuilder.registerStaticUse(new StaticUse.staticGet(member));
+ impactBuilder.registerStaticUse(new StaticUse.staticGet(
+ member, elementMap.getImport(getDeferredImport(node))));
}
}
@override
void handleStaticSet(ir.StaticSet node, ir.DartType valueType) {
MemberEntity member = elementMap.getMember(node.target);
- impactBuilder.registerStaticUse(new StaticUse.staticSet(member));
+ impactBuilder.registerStaticUse(new StaticUse.staticSet(
+ member, elementMap.getImport(getDeferredImport(node))));
}
void handleSuperInvocation(ir.Name name, ir.Node arguments) {
@@ -725,8 +735,9 @@
@override
void handleTypeLiteral(ir.TypeLiteral node) {
- impactBuilder.registerTypeUse(
- new TypeUse.typeLiteral(elementMap.getDartType(node.type)));
+ ImportEntity deferredImport = elementMap.getImport(getDeferredImport(node));
+ impactBuilder.registerTypeUse(new TypeUse.typeLiteral(
+ elementMap.getDartType(node.type), deferredImport));
if (node.type is ir.FunctionType) {
ir.FunctionType functionType = node.type;
assert(functionType.typedef != null);
diff --git a/pkg/compiler/lib/src/options.dart b/pkg/compiler/lib/src/options.dart
index ed6fcc8..7b10c51 100644
--- a/pkg/compiler/lib/src/options.dart
+++ b/pkg/compiler/lib/src/options.dart
@@ -95,6 +95,25 @@
/// libraries are subdivided.
Uri deferredMapUri;
+ /// Whether to apply the new deferred split fixes. The fixes improve on
+ /// performance and fix a soundness issue with inferred types. The latter will
+ /// move more code to the main output unit, because of that we are not
+ /// enabling the feature by default right away.
+ ///
+ /// When [reportInvalidInferredDeferredTypes] shows no errors, we expect this
+ /// flag to produce the same or better results than the current unsound
+ /// implementation.
+ bool newDeferredSplit = false;
+
+ /// Show errors when a deferred type is inferred as a return type of a closure
+ /// or in a type parameter. Those cases cause the compiler today to behave
+ /// unsoundly by putting the code in a deferred output unit. In the future
+ /// when [newDeferredSplit] is on by default, those cases will be treated
+ /// soundly and will cause more code to be moved to the main output unit.
+ ///
+ /// This flag is presented to help developers find and fix the affected code.
+ bool reportInvalidInferredDeferredTypes = false;
+
/// Whether to disable inlining during the backend optimizations.
// TODO(sigmund): negate, so all flags are positive
bool disableInlining = false;
@@ -282,6 +301,9 @@
_extractStringOption(options, '--build-id=', _UNDETERMINED_BUILD_ID)
..compileForServer = _hasOption(options, Flags.serverMode)
..deferredMapUri = _extractUriOption(options, '--deferred-map=')
+ ..newDeferredSplit = _hasOption(options, Flags.newDeferredSplit)
+ ..reportInvalidInferredDeferredTypes =
+ _hasOption(options, Flags.reportInvalidInferredDeferredTypes)
..fatalWarnings = _hasOption(options, Flags.fatalWarnings)
..terseDiagnostics = _hasOption(options, Flags.terse)
..suppressWarnings = _hasOption(options, Flags.suppressWarnings)
diff --git a/pkg/compiler/lib/src/universe/use.dart b/pkg/compiler/lib/src/universe/use.dart
index a5e2b41..1336828 100644
--- a/pkg/compiler/lib/src/universe/use.dart
+++ b/pkg/compiler/lib/src/universe/use.dart
@@ -173,12 +173,22 @@
final int hashCode;
final InterfaceType type;
final CallStructure callStructure;
+ final ImportEntity deferredImport;
StaticUse.internal(Entity element, this.kind,
- {this.type, this.callStructure, typeArgumentsHash: 0})
+ {this.type,
+ this.callStructure,
+ this.deferredImport,
+ typeArgumentsHash: 0})
: this.element = element,
- this.hashCode = Hashing.objectsHash(
- element, kind, type, typeArgumentsHash, callStructure);
+ this.hashCode = Hashing.listHash([
+ element,
+ kind,
+ type,
+ typeArgumentsHash,
+ callStructure,
+ deferredImport
+ ]);
/// Short textual representation use for testing.
String get shortText {
@@ -232,30 +242,33 @@
/// [callStructure].
factory StaticUse.staticInvoke(
FunctionEntity element, CallStructure callStructure,
- [List<DartType> typeArguments]) {
+ [List<DartType> typeArguments, ImportEntity deferredImport]) {
assert(
element.isStatic || element.isTopLevel,
failedAt(
element,
"Static invoke element $element must be a top-level "
"or static method."));
- return new GenericStaticUse(
- element, StaticUseKind.INVOKE, callStructure, typeArguments);
+ return new GenericStaticUse(element, StaticUseKind.INVOKE, callStructure,
+ typeArguments, deferredImport);
}
/// Closurization of a static or top-level function [element].
- factory StaticUse.staticTearOff(FunctionEntity element) {
+ factory StaticUse.staticTearOff(FunctionEntity element,
+ [ImportEntity deferredImport]) {
assert(
element.isStatic || element.isTopLevel,
failedAt(
element,
"Static tear-off element $element must be a top-level "
"or static method."));
- return new StaticUse.internal(element, StaticUseKind.STATIC_TEAR_OFF);
+ return new StaticUse.internal(element, StaticUseKind.STATIC_TEAR_OFF,
+ deferredImport: deferredImport);
}
/// Read access of a static or top-level field or getter [element].
- factory StaticUse.staticGet(MemberEntity element) {
+ factory StaticUse.staticGet(MemberEntity element,
+ [ImportEntity deferredImport]) {
assert(
element.isStatic || element.isTopLevel,
failedAt(
@@ -266,11 +279,13 @@
element.isField || element.isGetter,
failedAt(element,
"Static get element $element must be a field or a getter."));
- return new StaticUse.internal(element, StaticUseKind.GET);
+ return new StaticUse.internal(element, StaticUseKind.GET,
+ deferredImport: deferredImport);
}
/// Write access of a static or top-level field or setter [element].
- factory StaticUse.staticSet(MemberEntity element) {
+ factory StaticUse.staticSet(MemberEntity element,
+ [ImportEntity deferredImport]) {
assert(
element.isStatic || element.isTopLevel,
failedAt(
@@ -281,7 +296,8 @@
element.isField || element.isSetter,
failedAt(element,
"Static set element $element must be a field or a setter."));
- return new StaticUse.internal(element, StaticUseKind.SET);
+ return new StaticUse.internal(element, StaticUseKind.SET,
+ deferredImport: deferredImport);
}
/// Invocation of the lazy initializer for a static or top-level field
@@ -432,8 +448,11 @@
/// Constructor invocation of [element] with the given [callStructure] on
/// [type].
- factory StaticUse.typedConstructorInvoke(ConstructorEntity element,
- CallStructure callStructure, InterfaceType type) {
+ factory StaticUse.typedConstructorInvoke(
+ ConstructorEntity element,
+ CallStructure callStructure,
+ InterfaceType type,
+ ImportEntity deferredImport) {
assert(type != null,
failedAt(element, "No type provided for constructor invocation."));
assert(
@@ -443,13 +462,18 @@
"Typed constructor invocation element $element "
"must be a constructor."));
return new StaticUse.internal(element, StaticUseKind.CONSTRUCTOR_INVOKE,
- type: type, callStructure: callStructure);
+ type: type,
+ callStructure: callStructure,
+ deferredImport: deferredImport);
}
/// Constant constructor invocation of [element] with the given
/// [callStructure] on [type].
- factory StaticUse.constConstructorInvoke(ConstructorEntity element,
- CallStructure callStructure, InterfaceType type) {
+ factory StaticUse.constConstructorInvoke(
+ ConstructorEntity element,
+ CallStructure callStructure,
+ InterfaceType type,
+ ImportEntity deferredImport) {
assert(type != null,
failedAt(element, "No type provided for constructor invocation."));
assert(
@@ -460,7 +484,9 @@
"must be a constructor."));
return new StaticUse.internal(
element, StaticUseKind.CONST_CONSTRUCTOR_INVOKE,
- type: type, callStructure: callStructure);
+ type: type,
+ callStructure: callStructure,
+ deferredImport: deferredImport);
}
/// Constructor redirection to [element] on [type].
@@ -563,9 +589,11 @@
final List<DartType> typeArguments;
GenericStaticUse(Entity entity, StaticUseKind kind,
- CallStructure callStructure, this.typeArguments)
+ CallStructure callStructure, this.typeArguments,
+ [ImportEntity deferredImport])
: super.internal(entity, kind,
callStructure: callStructure,
+ deferredImport: deferredImport,
typeArgumentsHash: Hashing.listHash(typeArguments)) {
assert(
(callStructure?.typeArgumentCount ?? 0) == (typeArguments?.length ?? 0),
@@ -599,11 +627,12 @@
final DartType type;
final TypeUseKind kind;
final int hashCode;
+ final ImportEntity deferredImport;
- TypeUse.internal(DartType type, TypeUseKind kind)
+ TypeUse.internal(DartType type, TypeUseKind kind, [this.deferredImport])
: this.type = type,
this.kind = kind,
- this.hashCode = Hashing.objectHash(type, Hashing.objectHash(kind));
+ this.hashCode = Hashing.objectsHash(type, kind, deferredImport);
/// Short textual representation use for testing.
String get shortText {
@@ -678,8 +707,8 @@
}
/// [type] used as a type literal, like `foo() => T;`.
- factory TypeUse.typeLiteral(DartType type) {
- return new TypeUse.internal(type, TypeUseKind.TYPE_LITERAL);
+ factory TypeUse.typeLiteral(DartType type, ImportEntity deferredImport) {
+ return new TypeUse.internal(type, TypeUseKind.TYPE_LITERAL, deferredImport);
}
/// [type] used in an instantiation, like `new T();`.
diff --git a/tests/compiler/dart2js/deferred_loading/data/static_separate.dart b/tests/compiler/dart2js/deferred_loading/data/static_separate.dart
index ac5b27d..1f5a958 100644
--- a/tests/compiler/dart2js/deferred_loading/data/static_separate.dart
+++ b/tests/compiler/dart2js/deferred_loading/data/static_separate.dart
@@ -15,7 +15,9 @@
/*element: main:OutputUnit(main, {})*/
void main() {
asyncStart();
- Expect.throws(/*OutputUnit(main, {})*/ () => new lib1.C());
+ Expect.throws(/*OutputUnit(main, {})*/ () {
+ new lib1.C();
+ });
lib1.loadLibrary().then(/*OutputUnit(main, {})*/ (_) {
lib2.loadLibrary().then(/*OutputUnit(main, {})*/ (_) {
print("HERE");
diff --git a/tests/compiler/dart2js/deferred_loading/libs/deferred_overlapping_lib1.dart b/tests/compiler/dart2js/deferred_loading/libs/deferred_overlapping_lib1.dart
index 7587f4e..194e27d 100644
--- a/tests/compiler/dart2js/deferred_loading/libs/deferred_overlapping_lib1.dart
+++ b/tests/compiler/dart2js/deferred_loading/libs/deferred_overlapping_lib1.dart
@@ -4,6 +4,6 @@
import "deferred_overlapping_lib3.dart";
-/*class: C1:OutputUnit(1, {lib1})*/
-/*element: C1.:OutputUnit(1, {lib1})*/
+/*class: C1:OutputUnit(2, {lib1})*/
+/*element: C1.:OutputUnit(2, {lib1})*/
class C1 extends C3 {}
diff --git a/tests/compiler/dart2js/deferred_loading/libs/deferred_overlapping_lib3.dart b/tests/compiler/dart2js/deferred_loading/libs/deferred_overlapping_lib3.dart
index 5447ff9..5167cc6 100644
--- a/tests/compiler/dart2js/deferred_loading/libs/deferred_overlapping_lib3.dart
+++ b/tests/compiler/dart2js/deferred_loading/libs/deferred_overlapping_lib3.dart
@@ -2,6 +2,6 @@
// 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.
-/*class: C3:OutputUnit(2, {lib1, lib2})*/
-/*element: C3.:OutputUnit(2, {lib1, lib2})*/
+/*class: C3:OutputUnit(1, {lib1, lib2})*/
+/*element: C3.:OutputUnit(1, {lib1, lib2})*/
class C3 {}
diff --git a/tests/compiler/dart2js/model/enqueuer_test.dart b/tests/compiler/dart2js/model/enqueuer_test.dart
index 60c3fe6..8a8a6a6 100644
--- a/tests/compiler/dart2js/model/enqueuer_test.dart
+++ b/tests/compiler/dart2js/model/enqueuer_test.dart
@@ -177,8 +177,8 @@
elementEnvironment.lookupConstructor(cls, '');
InterfaceType type = elementEnvironment.getRawType(cls);
WorldImpact impact = new WorldImpactBuilderImpl()
- ..registerStaticUse(new StaticUse.typedConstructorInvoke(
- constructor, constructor.parameterStructure.callStructure, type));
+ ..registerStaticUse(new StaticUse.typedConstructorInvoke(constructor,
+ constructor.parameterStructure.callStructure, type, null));
enqueuer.applyImpact(impact);
}
