When registering deferred constant, use canonical output unit.

pkg/compiler/lib/src/deferred_load.dart

 // Copyright (c) 2014, 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.
 
 library deferred_load;
 
 import 'common/backend_api.dart' show Backend;
 import 'common/tasks.dart' show CompilerTask;
 import 'common.dart';
 import 'compiler.dart' show Compiler;
@@ skipping 208 matching lines @@
   bool hasOnlyNonDeferredImportPaths(Element from, Element to) {
     OutputUnit outputUnitFrom = outputUnitForElement(from);
     OutputUnit outputUnitTo = outputUnitForElement(to);
     return outputUnitTo.imports.containsAll(outputUnitFrom.imports);
   }
 
   void registerConstantDeferredUse(
       DeferredConstantValue constant, PrefixElement prefix) {
     OutputUnit outputUnit = new OutputUnit();
     outputUnit.imports.add(new _DeclaredDeferredImport(prefix.deferredImport));
-    _constantToOutputUnit[constant] = outputUnit;
+
+    // Check to see if there is already a canonical output unit registered.
+    OutputUnit representative = allOutputUnits.lookup(outputUnit);

[Siggi Cherem (dart-lang), 2016/09/07 18:10:44]

I see this logic replicated in a few places, maybe we should pull it out into a
helper.

[Siggi Cherem (dart-lang), 2016/09/07 18:14:09]

reading more into this, it feels like we are doing some kind of unification in
how we use the allOutputUnits and operator==. It might not matter too much in
terms of performance, but maybe it is worth replacing this with a union-find
datastructure?

[Harry Terkelsen, 2016/09/07 23:53:50]

Done.

[Harry Terkelsen, 2016/09/07 23:53:50]

added a todo

+    if (representative == null) {
+      representative = outputUnit;
+      allOutputUnits.add(representative);
+    }
+    _constantToOutputUnit[constant] = representative;
   }
 
   /// Answers whether [element] is explicitly deferred when referred to from
   /// [library].
   bool _isExplicitlyDeferred(Element element, LibraryElement library) {
     Iterable<ImportElement> imports = _getImports(element, library);
     // If the element is not imported explicitly, it is implicitly imported
     // not deferred.
     if (imports.isEmpty) return false;
     // An element could potentially be loaded by several imports. If all of them
@@ skipping 432 matching lines @@
               if (compiler.commonElements.mirrorsLibrary != null) {
                 _addMirrorElements();
               }
 
               // Build the OutputUnits using these two maps.
               Map<Element, OutputUnit> elementToOutputUnitBuilder =
                   new Map<Element, OutputUnit>();
               Map<ConstantValue, OutputUnit> constantToOutputUnitBuilder =
                   new Map<ConstantValue, OutputUnit>();
 
+              // Add all constants that may have been registered during
+              // resolution with [registerConstantDeferredUse].

[Siggi Cherem (dart-lang), 2016/09/07 18:10:44]

would it make sense to store those somewhere else instead?

[Harry Terkelsen, 2016/09/07 23:53:50]

I wanted to do that at first, but it would mean having to differentiate between
when 'registerConstantDeferredUse' is called during resolution and when it is
called during codegen

+              constantToOutputUnitBuilder.addAll(_constantToOutputUnit);
+              _constantToOutputUnit.clear();
+
               // Reverse the mappings. For each element record an OutputUnit
               // collecting all deferred imports mapped to this element. Same
               // for constants.
               for (_DeferredImport import in _importedDeferredBy.keys) {
                 for (Element element in _importedDeferredBy[import]) {
                   // Only one file should be loaded when the program starts, so
                   // make sure that only one OutputUnit is created for
                   // [fakeMainImport].
                   if (import == _fakeMainImport) {
                     elementToOutputUnitBuilder[element] = mainOutputUnit;
@@ skipping 36 matching lines @@
                 }
                 _elementToOutputUnit[element] = representative;
               });
               constantToOutputUnitBuilder
                   .forEach((ConstantValue constant, OutputUnit outputUnit) {
                 OutputUnit representative = allOutputUnits.lookup(outputUnit);
                 if (representative == null) {
                   representative = outputUnit;
                   allOutputUnits.add(representative);
                 }
                 _constantToOutputUnit[constant] = representative;

[Siggi Cherem (dart-lang), 2016/09/07 18:10:44]

not sure if this would work, but another option would be to post-process the
_constantToOutputUnit map:

  for (var k = _constantToOutputUnit.keys.toList()) {
    var ouputUnit = _constasntToOutputUnit[constant];
    var rep = representativeOf(outputUnit);
    if (!identical(outputUnit, rep)) _constasntToOutputUnit[constant] = rep;
  }

[Harry Terkelsen, 2016/09/07 23:53:50]

the problem is that the 'registerConstantDeferredUse' can be called both before
this code and after

               });
 
               // Generate a unique name for each OutputUnit.
               _assignNamesToOutputUnits(allOutputUnits);
             }));
     // Notify the impact strategy impacts are no longer needed for deferred
     // load.
     compiler.impactStrategy.onImpactUsed(IMPACT_USE);
   }
 
@@ skipping 287 matching lines @@
 
   bool operator ==(other) {
     if (other is! _DeclaredDeferredImport) return false;
     return declaration == other.declaration;
   }
 
   int get hashCode => declaration.hashCode * 17;
 
   String toString() => '$declaration';
 }