Refactor OneShotInterceptorData to prepare for modular codegen

Change-Id: I52d47c28978c0d935d07b438520728a304d311d6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/102365
Reviewed-by: Sigmund Cherem <sigmund@google.com>

pkg/compiler/lib/src/js_backend/backend.dart

@@ -663,7 +663,9 @@ class JavaScriptBackend {
   CodegenInputs onCodegenStart(JClosedWorld closedWorld) {
     RuntimeTypeTags rtiTags = const RuntimeTypeTags();
     OneShotInterceptorData oneShotInterceptorData = new OneShotInterceptorData(
-        closedWorld.interceptorData, closedWorld.commonElements);
+        closedWorld.interceptorData,
+        closedWorld.commonElements,
+        closedWorld.nativeData);
     Tracer tracer = new Tracer(closedWorld, compiler.outputProvider);
     _rtiEncoder = new RuntimeTypesEncoderImpl(
         rtiTags,

pkg/compiler/lib/src/js_backend/interceptor_data.dart

@@ -14,7 +14,7 @@ import '../js/js.dart' as jsAst;
 import '../serialization/serialization.dart';
 import '../universe/selector.dart';
 import '../world.dart' show JClosedWorld;
-import 'namer.dart';
+import 'namer.dart' show ModularNamer, suffixForGetInterceptor;
 import 'native_data.dart';
 
 abstract class InterceptorData {
@@ -354,55 +354,79 @@ class InterceptorDataBuilderImpl implements InterceptorDataBuilder {
 class OneShotInterceptorData {
   final InterceptorData _interceptorData;
   final CommonElements _commonElements;
+  final NativeData _nativeData;
 
-  OneShotInterceptorData(this._interceptorData, this._commonElements);
+  OneShotInterceptorData(
+      this._interceptorData, this._commonElements, this._nativeData);
 
-  /// A collection of selectors that must have a one shot interceptor generated.
-  final Map<jsAst.Name, Selector> _oneShotInterceptors =
-      <jsAst.Name, Selector>{};
+  Iterable<OneShotInterceptor> get oneShotInterceptors {
+    List<OneShotInterceptor> interceptors = [];
+    for (var map in _oneShotInterceptors.values) {
+      interceptors.addAll(map.values);
+    }
+    return interceptors;
+  }
 
-  Selector getOneShotInterceptorSelector(jsAst.Name name) =>
-      _oneShotInterceptors[name];
+  Map<Selector, Map<String, OneShotInterceptor>> _oneShotInterceptors = {};
 
-  Iterable<jsAst.Name> get oneShotInterceptorNames =>
-      _oneShotInterceptors.keys.toList()..sort();
-
-  /// A map of specialized versions of the [getInterceptorMethod].
+  /// A set of specialized versions of the [getInterceptorMethod].
   ///
   /// Since [getInterceptorMethod] is a hot method at runtime, we're always
   /// specializing it based on the incoming type. The keys in the map are the
   /// names of these specialized versions. Note that the generic version that
   /// contains all possible type checks is also stored in this map.
-  final Map<jsAst.Name, Set<ClassEntity>> _specializedGetInterceptors =
-      <jsAst.Name, Set<ClassEntity>>{};
+  Iterable<SpecializedGetInterceptor> get specializedGetInterceptors =>
+      _specializedGetInterceptors.values;
 
-  Iterable<jsAst.Name> get specializedGetInterceptorNames =>
-      _specializedGetInterceptors.keys.toList()..sort();
-
-  Set<ClassEntity> getSpecializedGetInterceptorsFor(jsAst.Name name) =>
-      _specializedGetInterceptors[name];
+  Map<String, SpecializedGetInterceptor> _specializedGetInterceptors = {};
 
   jsAst.Name registerOneShotInterceptor(
-      Selector selector, Namer namer, JClosedWorld closedWorld) {
+      Selector selector, ModularNamer namer, JClosedWorld closedWorld) {
+    selector = selector.toNormalized();
     Set<ClassEntity> classes =
         _interceptorData.getInterceptedClassesOn(selector.name, closedWorld);
-    jsAst.Name name = namer.nameForGetOneShotInterceptor(selector, classes);
-    if (!_oneShotInterceptors.containsKey(name)) {
-      registerSpecializedGetInterceptor(classes, namer);
-      _oneShotInterceptors[name] = selector;
-    }
-    return name;
+    String key = suffixForGetInterceptor(_commonElements, _nativeData, classes);
+    Map<String, OneShotInterceptor> interceptors =
+        _oneShotInterceptors[selector] ??= {};
+    OneShotInterceptor interceptor = interceptors.putIfAbsent(
+        key, () => new OneShotInterceptor(key, selector));
+    interceptor.classes.addAll(classes);
+    registerSpecializedGetInterceptor(classes, namer);
+    return namer.nameForGetOneShotInterceptor(selector, classes);
   }
 
   void registerSpecializedGetInterceptor(
-      Set<ClassEntity> classes, Namer namer) {
-    jsAst.Name name = namer.nameForGetInterceptor(classes);
+      Set<ClassEntity> classes, ModularNamer namer) {
     if (classes.contains(_commonElements.jsInterceptorClass)) {
       // We can't use a specialized [getInterceptorMethod], so we make
       // sure we emit the one with all checks.
-      _specializedGetInterceptors[name] = _interceptorData.interceptedClasses;
-    } else {
-      _specializedGetInterceptors[name] = classes;
+      classes = _interceptorData.interceptedClasses;
     }
+    String key = suffixForGetInterceptor(_commonElements, _nativeData, classes);
+    SpecializedGetInterceptor interceptor =
+        _specializedGetInterceptors[key] ??= new SpecializedGetInterceptor(key);
+    interceptor.classes.addAll(classes);
   }
 }
+
+class OneShotInterceptor {
+  final Selector selector;
+  final String key;
+  final Set<ClassEntity> classes = {};
+
+  OneShotInterceptor(this.key, this.selector);
+
+  @override
+  String toString() =>
+      'OneShotInterceptor(selector=$selector,key=$key,classes=$classes)';
+}
+
+class SpecializedGetInterceptor {
+  final String key;
+  final Set<ClassEntity> classes = {};
+
+  SpecializedGetInterceptor(this.key);
+
+  @override
+  String toString() => 'SpecializedGetInterceptor(key=$key,classes=$classes)';
+}

pkg/compiler/lib/src/js_emitter/interceptor_stub_generator.dart

@@ -31,7 +31,6 @@ class InterceptorStubGenerator {
   final Emitter _emitter;
   final NativeCodegenEnqueuer _nativeCodegenEnqueuer;
   final Namer _namer;
-  final OneShotInterceptorData _oneShotInterceptorData;
   final CustomElementsCodegenAnalysis _customElementsCodegenAnalysis;
   final CodegenWorld _codegenWorld;
   final JClosedWorld _closedWorld;
@@ -42,7 +41,6 @@ class InterceptorStubGenerator {
       this._emitter,
       this._nativeCodegenEnqueuer,
       this._namer,
-      this._oneShotInterceptorData,
       this._customElementsCodegenAnalysis,
       this._codegenWorld,
       this._closedWorld);
@@ -51,7 +49,10 @@ class InterceptorStubGenerator {
 
   InterceptorData get _interceptorData => _closedWorld.interceptorData;
 
-  jsAst.Expression generateGetInterceptorMethod(Set<ClassEntity> classes) {
+  jsAst.Expression generateGetInterceptorMethod(
+      SpecializedGetInterceptor interceptor) {
+    Set<ClassEntity> classes = interceptor.classes;
+
     jsAst.Expression interceptorFor(ClassEntity cls) {
       return _emitter.interceptorPrototypeAccess(cls);
     }
@@ -374,11 +375,9 @@ class InterceptorStubGenerator {
     return null;
   }
 
-  jsAst.Expression generateOneShotInterceptor(jsAst.Name name) {
-    Selector selector =
-        _oneShotInterceptorData.getOneShotInterceptorSelector(name);
-    Set<ClassEntity> classes =
-        _interceptorData.getInterceptedClassesOn(selector.name, _closedWorld);
+  jsAst.Expression generateOneShotInterceptor(OneShotInterceptor interceptor) {
+    Selector selector = interceptor.selector;
+    Set<ClassEntity> classes = interceptor.classes;
     jsAst.Name getInterceptorName = _namer.nameForGetInterceptor(classes);
 
     List<String> parameterNames = <String>[];

pkg/compiler/lib/src/js_emitter/program_builder/collector.dart

@@ -85,10 +85,9 @@ class Collector {
     // Go over specialized interceptors and then constants to know which
     // interceptors are needed.
     Set<ClassEntity> needed = new Set<ClassEntity>();
-    for (js.Name name
-        in _oneShotInterceptorData.specializedGetInterceptorNames) {
-      needed.addAll(
-          _oneShotInterceptorData.getSpecializedGetInterceptorsFor(name));
+    for (SpecializedGetInterceptor interceptor
+        in _oneShotInterceptorData.specializedGetInterceptors) {
+      needed.addAll(interceptor.classes);
     }
 
     // Add interceptors referenced by constants.

pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart

@@ -345,7 +345,6 @@ class ProgramBuilder {
         _task.emitter,
         _nativeCodegenEnqueuer,
         _namer,
-        _oneShotInterceptorData,
         _customElementsCodegenAnalysis,
         _codegenWorld,
         _closedWorld);
@@ -1002,11 +1001,10 @@ class ProgramBuilder {
   // We must evaluate these classes eagerly so that the prototype is
   // accessible.
   void _markEagerInterceptorClasses() {
-    Iterable<js.Name> names =
-        _oneShotInterceptorData.specializedGetInterceptorNames;
-    for (js.Name name in names) {
-      for (ClassEntity element
-          in _oneShotInterceptorData.getSpecializedGetInterceptorsFor(name)) {
+    Iterable<SpecializedGetInterceptor> interceptors =
+        _oneShotInterceptorData.specializedGetInterceptors;
+    for (SpecializedGetInterceptor interceptor in interceptors) {
+      for (ClassEntity element in interceptor.classes) {
         Class cls = _classes[element];
         if (cls != null) cls.isEager = true;
       }
@@ -1020,7 +1018,6 @@ class ProgramBuilder {
         _task.emitter,
         _nativeCodegenEnqueuer,
         _namer,
-        _oneShotInterceptorData,
         _customElementsCodegenAnalysis,
         _codegenWorld,
         _closedWorld);
@@ -1030,13 +1027,23 @@ class ProgramBuilder {
     // TODO(floitsch): we shouldn't update the registry in the middle of
     // generating the interceptor methods.
     Holder holder = _registry.registerHolder(holderName);
-
-    Iterable<js.Name> names =
-        _oneShotInterceptorData.specializedGetInterceptorNames;
+    List<js.Name> names = [];
+    Map<js.Name, SpecializedGetInterceptor> interceptorMap = {};
+    for (SpecializedGetInterceptor interceptor
+        in _oneShotInterceptorData.specializedGetInterceptors) {
+      js.Name name = _namer.nameForGetInterceptor(interceptor.classes);
+      names.add(name);
+      assert(
+          !interceptorMap.containsKey(name),
+          "Duplicate specialized get interceptor for $name: Existing: "
+          "${interceptorMap[name]}, new ${interceptor}.");
+      interceptorMap[name] = interceptor;
+    }
+    names.sort();
     return names.map((js.Name name) {
-      Set<ClassEntity> classes =
-          _oneShotInterceptorData.getSpecializedGetInterceptorsFor(name);
-      js.Expression code = stubGenerator.generateGetInterceptorMethod(classes);
+      SpecializedGetInterceptor interceptor = interceptorMap[name];
+      js.Expression code =
+          stubGenerator.generateGetInterceptorMethod(interceptor);
       return new StaticStubMethod(name, holder, code);
     });
   }
@@ -1116,7 +1123,6 @@ class ProgramBuilder {
         _task.emitter,
         _nativeCodegenEnqueuer,
         _namer,
-        _oneShotInterceptorData,
         _customElementsCodegenAnalysis,
         _codegenWorld,
         _closedWorld);
@@ -1126,10 +1132,24 @@ class ProgramBuilder {
     // TODO(floitsch): we shouldn't update the registry in the middle of
     // generating the interceptor methods.
     Holder holder = _registry.registerHolder(holderName);
-
-    List<js.Name> names = _oneShotInterceptorData.oneShotInterceptorNames;
+    List<js.Name> names = [];
+    Map<js.Name, OneShotInterceptor> interceptorMap = {};
+    for (OneShotInterceptor interceptor
+        in _oneShotInterceptorData.oneShotInterceptors) {
+      js.Name name = _namer.nameForGetOneShotInterceptor(
+          interceptor.selector, interceptor.classes);
+      names.add(name);
+      assert(
+          !interceptorMap.containsKey(name),
+          "Duplicate specialized get interceptor for $name: Existing: "
+          "${interceptorMap[name]}, new ${interceptor}.");
+      interceptorMap[name] = interceptor;
+    }
+    names.sort();
     return names.map((js.Name name) {
-      js.Expression code = stubGenerator.generateOneShotInterceptor(name);
+      OneShotInterceptor interceptor = interceptorMap[name];
+      js.Expression code =
+          stubGenerator.generateOneShotInterceptor(interceptor);
       return new StaticStubMethod(name, holder, code);
     });
   }

pkg/compiler/lib/src/universe/call_structure.dart

@@ -67,6 +67,15 @@ class CallStructure {
     sink.end(tag);
   }
 
+  /// Returns `true` if this call structure is normalized, that is, its named
+  /// arguments are sorted.
+  bool get isNormalized => true;
+
+  /// Returns the normalized version of this call structure.
+  ///
+  /// A [CallStructure] is normalized if its named arguments are sorted.
+  CallStructure toNormalized() => this;
+
   CallStructure withTypeArgumentCount(int typeArgumentCount) =>
       new CallStructure(argumentCount, namedArguments, typeArgumentCount);
 
@@ -185,17 +194,21 @@ class CallStructure {
   }
 }
 
-///
+/// Call structure with named arguments.
 class NamedCallStructure extends CallStructure {
   @override
   final List<String> namedArguments;
-  final List<String> _orderedNamedArguments = <String>[];
+  final List<String> _orderedNamedArguments;
 
   NamedCallStructure(
-      int argumentCount, this.namedArguments, int typeArgumentCount)
-      : super.unnamed(argumentCount, typeArgumentCount) {
-    assert(namedArguments.isNotEmpty);
-  }
+      int argumentCount, List<String> namedArguments, int typeArgumentCount)
+      : this.internal(
+            argumentCount, namedArguments, typeArgumentCount, <String>[]);
+
+  NamedCallStructure.internal(int argumentCount, this.namedArguments,
+      int typeArgumentCount, this._orderedNamedArguments)
+      : assert(namedArguments.isNotEmpty),
+        super.unnamed(argumentCount, typeArgumentCount);
 
   @override
   bool get isNamed => true;
@@ -209,6 +222,16 @@ class NamedCallStructure extends CallStructure {
   @override
   int get positionalArgumentCount => argumentCount - namedArgumentCount;
 
+  @override
+  bool get isNormalized => namedArguments == _orderedNamedArguments;
+
+  @override
+  CallStructure toNormalized() => new NamedCallStructure.internal(
+      argumentCount,
+      getOrderedNamedArguments(),
+      typeArgumentCount,
+      getOrderedNamedArguments());
+
   @override
   List<String> getOrderedNamedArguments() {
     if (!_orderedNamedArguments.isEmpty) return _orderedNamedArguments;

pkg/compiler/lib/src/universe/selector.dart

@@ -315,6 +315,15 @@ class Selector {
     return 'Selector($kind, $name, ${callStructure.structureToString()})';
   }
 
+  /// Returns the normalized version of this selector.
+  ///
+  /// A selector is normalized if its call structure is normalized.
+  // TODO(johnniwinther): Use normalized selectors as much as possible,
+  // especially where selectors are used in sets or as keys in maps.
+  Selector toNormalized() => callStructure.isNormalized
+      ? this
+      : new Selector(kind, memberName, callStructure.toNormalized());
+
   Selector toCallSelector() => new Selector.callClosureFrom(this);
 
   /// Returns the non-generic [Selector] corresponding to this selector.