Several deferred loading improvements:

- fix some discrepancies between old and new deferred loading.
  * Kernel is now using the correct world impact
  * For the old FE, I'm more careful about avoiding metadata nodes when mirros
    are disabled as well.
  * Better approximate when a constant is deferred in the kernel codegen

- make 'dump()' independent of the IR, so we can easily compare them side by
  side.

- exclude primitives in kernel since we don't need to track them. I
  didn't change the old front end because it is not as easy to do. I did this
  change because kernel was processing many more constants than the old frontend
  (the old frontend skipped constants that were never evaluated, we have no way
  to do it selectively like that in the new front end).

Change-Id: I337d3fd818753125476b7390da5d900ebdc02709
Reviewed-on: https://dart-review.googlesource.com/34509
Reviewed-by: Emily Fortuna <efortuna@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>

pkg/compiler/lib/src/deferred_load.dart

@@ -95,6 +95,9 @@ abstract class DeferredLoadTask extends CompilerTask {
   /// Will be `true` if the program contains deferred libraries.
   bool isProgramSplit = false;
 
+  /// Whether mirrors have been used in the program.
+  bool _isMirrorsUsed = false;
+
   static const ImpactUseCase IMPACT_USE = const ImpactUseCase('Deferred load');
 
   /// A mapping from the name of a defer import to all the output units it
@@ -217,7 +220,9 @@ abstract class DeferredLoadTask extends CompilerTask {
       collectConstantsInBody(analyzableElement, constants);
     }
 
-    collectConstantsFromMetadata(element, constants);
+    if (_isMirrorsUsed) {
+      collectConstantsFromMetadata(element, constants);
+    }
 
     if (element is FunctionEntity) {
       _collectTypeDependencies(
@@ -630,6 +635,7 @@ abstract class DeferredLoadTask extends CompilerTask {
 
     work() {
       var queue = new WorkQueue(this.importSets);
+      _isMirrorsUsed = closedWorld.backendUsage.isMirrorsUsed;
 
       // Add `main` and their recursive dependencies to the main output unit.
       // We do this upfront to avoid wasting time visiting these elements when
@@ -650,7 +656,7 @@ abstract class DeferredLoadTask extends CompilerTask {
         element = element is ClassElement ? element.implementation : element;
         queue.addElement(element, importSets.mainSet);
       }
-      if (closedWorld.backendUsage.isMirrorsUsed) {
+      if (_isMirrorsUsed) {
         addMirrorElementsForLibrary(queue, main.library, importSets.mainSet);
       }
 
@@ -781,14 +787,37 @@ abstract class DeferredLoadTask extends CompilerTask {
     return "${outName}_$name$extension";
   }
 
+  bool ignoreEntityInDump(Entity element) => false;
+
   /// Creates a textual representation of the output unit content.
   String dump() {
     Map<OutputUnit, List<String>> elementMap = <OutputUnit, List<String>>{};
     Map<OutputUnit, List<String>> constantMap = <OutputUnit, List<String>>{};
     _elementToSet.forEach((Entity element, ImportSet importSet) {
-      elementMap.putIfAbsent(importSet.unit, () => <String>[]).add('$element');
+      if (ignoreEntityInDump(element)) return;
+      var elements = elementMap.putIfAbsent(importSet.unit, () => <String>[]);
+      var id = element.name ?? '$element';
+      if (element is MemberEntity) {
+        var cls = element.enclosingClass?.name;
+        if (cls != null) id = '$cls.$id';
+        if (element.isSetter) id = '$id=';
+        id = '$id member';
+      } else if (element is ClassEntity) {
+        id = '$id cls';
+      } else if (element is TypedefEntity) {
+        id = '$id typedef';
+      } else if (element is Local) {
+        var context = (element as dynamic).memberContext.name;
+        id = element.name == null || element.name == '' ? '<anonymous>' : id;
+        id = '$context.$id';
+        id = '$id local';
+      }
+      elements.add(id);
     });
     _constantToSet.forEach((ConstantValue value, ImportSet importSet) {
+      // Skip primitive values: they are not stored in the constant tables and
+      // if they are shared, they end up duplicated anyways across output units.
+      if (value.isPrimitive) return;
       constantMap
           .putIfAbsent(importSet.unit, () => <String>[])
           .add(value.toStructuredText());
@@ -801,9 +830,10 @@ abstract class DeferredLoadTask extends CompilerTask {
         unitText.write(' <MAIN UNIT>');
       } else {
         unitText.write(' imports:');
-        var imports = outputUnit._imports.map((i) => '${i.uri}').toList()
-          ..sort();
-        for (var i in imports) {
+        var imports = outputUnit._imports
+            .map((i) => '${i.enclosingLibrary.canonicalUri.resolveUri(i.uri)}')
+            .toList();
+        for (var i in imports..sort()) {
           unitText.write('\n   $i:');
         }
       }

pkg/compiler/lib/src/kernel/deferred_load.dart

@@ -145,16 +145,16 @@ class ConstantCollector extends ir.RecursiveVisitor {
       constants.add(elementMap.getConstantValue(node));
 
   @override
-  void visitIntLiteral(ir.IntLiteral literal) => add(literal);
+  void visitIntLiteral(ir.IntLiteral literal) {}
 
   @override
-  void visitDoubleLiteral(ir.DoubleLiteral literal) => add(literal);
+  void visitDoubleLiteral(ir.DoubleLiteral literal) {}
 
   @override
-  void visitBoolLiteral(ir.BoolLiteral literal) => add(literal);
+  void visitBoolLiteral(ir.BoolLiteral literal) {}
 
   @override
-  void visitStringLiteral(ir.StringLiteral literal) => add(literal);
+  void visitStringLiteral(ir.StringLiteral literal) {}
 
   @override
   void visitSymbolLiteral(ir.SymbolLiteral literal) => add(literal);

pkg/compiler/lib/src/kernel/kernel_strategy.dart

@@ -248,7 +248,7 @@ class KernelWorkItem implements ResolutionWorkItem {
         WorldImpact worldImpact =
             _impactTransformer.transformResolutionImpact(impact);
         if (impactCache != null) {
-          impactCache[element] = impact;
+          impactCache[element] = worldImpact;
         }
         return worldImpact;
       });

pkg/compiler/lib/src/resolution/deferred_load.dart

@@ -15,6 +15,7 @@ import '../elements/elements.dart'
         AstElement,
         AccessorElement,
         ClassElement,
+        ConstructorElement,
         Element,
         ExportElement,
         ImportElement,
@@ -145,6 +146,11 @@ class AstDeferredLoadTask extends DeferredLoadTask {
     TreeElements treeElements = element.resolvedAst.elements;
     assert(treeElements != null);
 
+    Set<ast.Node> metadataNodes = element.implementation.metadata
+        .map((m) => m.node)
+        .toSet()
+          ..addAll(element.declaration.metadata.map((m) => m.node));
+
     // TODO(johnniwinther): Add only expressions that are actually needed.
     // Currently we have some noise here: Some potential expressions are
     // seen that should never be added (for instance field initializers
@@ -154,6 +160,7 @@ class AstDeferredLoadTask extends DeferredLoadTask {
     // See dartbug.com/26406 for context.
     treeElements
         .forEachConstantNode((ast.Node node, ConstantExpression expression) {
+      if (metadataNodes.contains(node)) return;
       if (compiler.serialization.isDeserialized(element)) {
         if (!expression.isPotential) {
           // Enforce evaluation of [expression].
@@ -366,4 +373,10 @@ class AstDeferredLoadTask extends DeferredLoadTask {
     assert(result != null);
     return result;
   }
+
+  bool ignoreEntityInDump(covariant Element element) =>
+      // origin element will already be dumped.
+      element.isPatch ||
+      // redirecting factories are not visible to the kernel ir
+      element is ConstructorElement && element.isRedirectingFactory;
 }

pkg/compiler/lib/src/ssa/builder_kernel.dart

@@ -2612,7 +2612,8 @@ class KernelSsaGraphBuilder extends ir.Visitor
           // there is no prefix the old FE wouldn't treat this in any special
           // way. Also, if the prefix points to a constant in the main output
           // unit, the old FE would still generate a deferred wrapper here.
-          if (!unit.isMainOutput) {
+          if (!compiler.backend.outputUnitData
+              .hasOnlyNonDeferredImportPaths(targetElement, field)) {
             stack.add(graph.addDeferredConstant(
                 value, unit, sourceInformation, compiler, closedWorld));
           } else {

tests/compiler/dart2js/deferred_loading/deferred_loading_test.dart

@@ -18,9 +18,7 @@ import 'package:compiler/src/constants/values.dart';
 
 import 'package:kernel/ast.dart' as ir;
 
-const List<String> skipForKernel = const <String>[
-  'dont_inline_deferred_constants.dart',
-];
+const List<String> skipForKernel = const <String>[];
 
 ///  Add in options to pass to the compiler like
 /// `Flags.disableTypeInference` or `Flags.disableInlining`
@@ -97,6 +95,7 @@ void computeAstOutputUnitData(
   if (member is FieldElement && member.isConst) {
     var node = member.initializer;
     var constant = compiler.constants.getConstantValue(member.constant);
+    if (constant.isPrimitive) return;
     _registerValue(
         new NodeId(node.getBeginToken().charOffset, IdKind.node),
         outputUnitString(data.outputUnitForConstant(constant)),
@@ -136,6 +135,7 @@ void computeKernelOutputUnitData(
   if (memberNode is ir.Field && memberNode.isConst) {
     ir.Expression node = memberNode.initializer;
     ConstantValue constant = elementMap.getConstantValue(node);
+    if (constant.isPrimitive) return;
     SourceSpan span = computeSourceSpanFromTreeNode(node);
     if (node is ir.ConstructorInvocation ||
         node is ir.ListLiteral ||

tests/compiler/dart2js/deferred_loading/libs/deferred_constant1_lib3.dart

@@ -48,7 +48,7 @@ const C5 = /*OutputUnit(main, {})*/ const C(5);
 /// ---------------------------------------------------------------------------
 
 /*element: C6:OutputUnit(1, {lib2})*/
-const C6 = /*OutputUnit(1, {lib2})*/ "string6";
+const C6 = "string6";
 
 /*element: C7:OutputUnit(1, {lib2})*/
 const C7 = /*OutputUnit(1, {lib2})*/ const C(const C(7));

tests/compiler/dart2js/deferred_loading/libs/dont_inline_deferred_constants_lib1.dart

@@ -6,18 +6,27 @@ import "dont_inline_deferred_constants_main.dart" show C;
 import "dont_inline_deferred_constants_main.dart" as main;
 
 /*element: C1:OutputUnit(1, {lib1})*/
-const C1 = /*OutputUnit(1, {lib1})*/ "string1";
+const C1 = "string1";
+
+/*element: C1b:OutputUnit(1, {lib1})*/
+const C1b = /*OutputUnit(1, {lib1})*/ const C("string1");
 
 /*element: C2:OutputUnit(1, {lib1})*/
-const C2 = /*OutputUnit(1, {lib1})*/ 1010;
+const C2 = 1010;
+
+/*element: C2b:OutputUnit(1, {lib1})*/
+const C2b = /*OutputUnit(1, {lib1})*/ const C(1010);
 
 class D {
   /*element: D.C3:OutputUnit(1, {lib1})*/
-  static const C3 = /*OutputUnit(1, {lib1})*/ "string2";
+  static const C3 = "string2";
+
+  /*element: D.C3b:OutputUnit(1, {lib1})*/
+  static const C3b = /*OutputUnit(1, {lib1})*/ const C("string2");
 }
 
 /*element: C4:OutputUnit(1, {lib1})*/
-const C4 = /*OutputUnit(main, {})*/ "string4";
+const C4 = "string4";
 
 /*element: C5:OutputUnit(1, {lib1})*/
 const C5 = /*OutputUnit(main, {})*/ const C(1);

tests/compiler/dart2js/deferred_loading/libs/dont_inline_deferred_constants_lib2.dart

@@ -6,7 +6,7 @@ import "dont_inline_deferred_constants_main.dart" show C;
 import "dont_inline_deferred_constants_main.dart" as main;
 
 /*element: C4:OutputUnit(3, {lib2})*/
-const C4 = /*OutputUnit(main, {})*/ "string4";
+const C4 = "string4";
 
 /*element: C5:OutputUnit(3, {lib2})*/
 const C5 = /*OutputUnit(main, {})*/ const C(1);

tests/compiler/dart2js/deferred_loading/libs/dont_inline_deferred_constants_main.dart

@@ -6,7 +6,7 @@ import 'dont_inline_deferred_constants_lib1.dart' deferred as lib1;
 import 'dont_inline_deferred_constants_lib2.dart' deferred as lib2;
 
 /*element: c:OutputUnit(main, {})*/
-const c = /*OutputUnit(main, {})*/ "string3";
+const c = "string3";
 
 class C {
   /*element: C.p:OutputUnit(main, {})*/
@@ -26,8 +26,11 @@ void main() {
       lib1.foo();
       lib2.foo();
       print(lib1.C1);
+      print(lib1.C1b);
       print(lib1.C2);
+      print(lib1.C2b);
       print(lib1.D.C3);
+      print(lib1.D.C3b);
       print(c);
       print(lib1.C4);
       print(lib2.C4);