Optimize needNoSuchMethodHandling computation

This greatly improves the number of lookups needed to establish the relation.
For the two most prolific tests we have:

html/element_test: 44939 -> 6557
html/mirrors_js_typed_interop_test (mirrors+jsinterop!): 1638455 -> 171326

R=sigmund@google.com

Committed: a8b2bfb5eb

Review URL: https://codereview.chromium.org/2428543002 .

Reverted: 5397c5427e

pkg/compiler/lib/src/elements/common.dart

@@ -215,20 +215,22 @@ abstract class ClassElementCommon implements ClassElement {
    * When called on the implementation element both members declared in the
    * origin and the patch class are returned.
    */
-  Element lookupByName(Name memberName) {
-    return internalLookupByName(memberName, isSuperLookup: false);
+  Element lookupByName(Name memberName, {ClassElement stopAt}) {
+    return internalLookupByName(memberName,
+        isSuperLookup: false, stopAtSuperclass: stopAt);
   }
 
   Element lookupSuperByName(Name memberName) {
     return internalLookupByName(memberName, isSuperLookup: true);
   }
 
-  Element internalLookupByName(Name memberName, {bool isSuperLookup}) {
+  Element internalLookupByName(Name memberName,
+      {bool isSuperLookup, ClassElement stopAtSuperclass}) {
     String name = memberName.text;
     bool isPrivate = memberName.isPrivate;
     LibraryElement library = memberName.library;
     for (ClassElement current = isSuperLookup ? superclass : this;
-        current != null;
+        current != null && current != stopAtSuperclass;
         current = current.superclass) {
       Element member = current.lookupLocalMember(name);
       if (member == null && current.isPatched) {

pkg/compiler/lib/src/elements/elements.dart

@@ -1555,7 +1555,13 @@ abstract class ClassElement extends TypeDeclarationElement
   void reverseBackendMembers();
 
   Element lookupMember(String memberName);
-  Element lookupByName(Name memberName);
+
+  /// Looks up a class instance member declared or inherited in this class
+  /// using [memberName] to match the (private) name and getter/setter property.
+  ///
+  /// This method recursively visits superclasses until the member is found or
+  /// [stopAt] is reached.
+  Element lookupByName(Name memberName, {ClassElement stopAt});
   Element lookupSuperByName(Name memberName);
 
   Element lookupLocalMember(String memberName);

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

@@ -461,6 +461,10 @@ class ClassSet {
     return true;
   }
 
+  Iterable<ClassHierarchyNode> get subtypeNodes {
+    return _subtypes ?? const <ClassHierarchyNode>[];
+  }
+
   /// Returns an [Iterable] of the subclasses of [cls] possibly including [cls].
   ///
   /// Subclasses are included if their instantiation properties intersect with

pkg/compiler/lib/src/world.dart

@@ -728,16 +728,19 @@ class WorldImpl implements ClosedWorld, ClosedWorldRefiner, OpenWorld {
         : result.implementation == element.implementation;
   }
 
-  Element findMatchIn(ClassElement cls, Selector selector) {
+  Element findMatchIn(ClassElement cls, Selector selector,
+      {ClassElement stopAtSuperclass}) {
     // Use the [:implementation] of [cls] in case the found [element]
     // is in the patch class.
-    var result = cls.implementation.lookupByName(selector.memberName);
+    var result = cls.implementation
+        .lookupByName(selector.memberName, stopAt: stopAtSuperclass);
     return result;
   }
 
   /// Returns whether a [selector] call on an instance of [cls]
   /// will hit a method at runtime, and not go through [noSuchMethod].
-  bool hasConcreteMatch(ClassElement cls, Selector selector) {
+  bool hasConcreteMatch(ClassElement cls, Selector selector,
+      {ClassElement stopAtSuperclass}) {
     assert(invariant(cls, isInstantiated(cls),
         message: '$cls has not been instantiated.'));
     Element element = findMatchIn(cls, selector);
@@ -753,35 +756,49 @@ class WorldImpl implements ClosedWorld, ClosedWorldRefiner, OpenWorld {
   @override
   bool needsNoSuchMethod(
       ClassElement base, Selector selector, ClassQuery query) {
-    /// Returns `true` if [cls] is an instantiated class that does not have
-    /// a concrete method matching [selector].
-    bool needsNoSuchMethod(ClassElement cls) {
-      // We can skip uninstantiated subclasses.
-      if (!isInstantiated(cls)) {
+    /// Returns `true` if subclasses in the [rootNode] tree needs noSuchMethod
+    /// handling.
+    bool subclassesNeedNoSuchMethod(ClassHierarchyNode rootNode) {
+      if (!rootNode.isInstantiated) {
+        // No subclass needs noSuchMethod handling since they are all
+        // uninstantiated.
         return false;
       }
-      // We can just skip abstract classes because we know no
-      // instance of them will be created at runtime, and
-      // therefore there is no instance that will require
-      // [noSuchMethod] handling.
-      return !cls.isAbstract && !hasConcreteMatch(cls, selector);
+      ClassElement rootClass = rootNode.cls;
+      if (hasConcreteMatch(rootClass, selector)) {
+        // The root subclass has a concrete implementation so no subclass needs
+        // noSuchMethod handling.
+        return false;
+      } else if (rootNode.isDirectlyInstantiated) {
+        // The root class need noSuchMethod handling.
+        return true;
+      }
+      IterationStep result = rootNode.forEachSubclass((ClassElement subclass) {
+        if (hasConcreteMatch(subclass, selector, stopAtSuperclass: rootClass)) {
+          // Found a match - skip all subclasses.
+          return IterationStep.SKIP_SUBCLASSES;
+        } else {
+          // Stop fast - we found a need for noSuchMethod handling.
+          return IterationStep.STOP;
+        }
+      }, ClassHierarchyNode.DIRECTLY_INSTANTIATED, strict: true);
+      // We stopped fast so we need noSuchMethod handling.
+      return result == IterationStep.STOP;
     }
 
-    bool baseNeedsNoSuchMethod = needsNoSuchMethod(base);
-    if (query == ClassQuery.EXACT || baseNeedsNoSuchMethod) {
-      return baseNeedsNoSuchMethod;
-    }
-
-    Iterable<ClassElement> subclassesToCheck;
-    if (query == ClassQuery.SUBTYPE) {
-      subclassesToCheck = strictSubtypesOf(base);
+    ClassSet classSet = getClassSet(base);
+    ClassHierarchyNode node = classSet.node;
+    if (query == ClassQuery.EXACT) {
+      return node.isDirectlyInstantiated && !hasConcreteMatch(base, selector);
+    } else if (query == ClassQuery.SUBCLASS) {
+      return subclassesNeedNoSuchMethod(node);
     } else {
-      assert(query == ClassQuery.SUBCLASS);
-      subclassesToCheck = strictSubclassesOf(base);
+      if (subclassesNeedNoSuchMethod(node)) return true;
+      for (ClassHierarchyNode subtypeNode in classSet.subtypeNodes) {
+        if (subclassesNeedNoSuchMethod(subtypeNode)) return true;
+      }
+      return false;
     }
-
-    return subclassesToCheck != null &&
-        subclassesToCheck.any(needsNoSuchMethod);
   }
 
   final Compiler _compiler;

tests/compiler/dart2js/needs_no_such_method_test.dart

@@ -35,6 +35,7 @@ testClassSets() async {
   Selector foo, bar, baz;
   ClosedWorld closedWorld;
   ClassElement superclass, subclass, subtype;
+  String testMode;
 
   Future run(List<String> instantiated) async {
     StringBuffer main = new StringBuffer();
@@ -43,6 +44,7 @@ testClassSets() async {
       main.write('new $cls();');
     }
     main.write('}');
+    testMode = '$instantiated';
 
     var env = await TypeEnvironment.create(CLASSES,
         mainSource: main.toString(), useMockCompiler: false);
@@ -59,8 +61,11 @@ testClassSets() async {
   void check(ClassElement cls, ClassQuery query, Selector selector,
       bool expectedResult) {
     bool result = closedWorld.needsNoSuchMethod(cls, selector, query);
-    Expect.equals(expectedResult, result,
-        'Unexpected result for $selector in $cls ($query)');
+    Expect.equals(
+        expectedResult,
+        result,
+        'Unexpected result for $selector in $cls ($query)'
+        'for instantiations $testMode');
   }
 
   await run([]);
@@ -166,17 +171,13 @@ testClassSets() async {
   Expect.isFalse(closedWorld.isImplemented(subtype));
 
   check(superclass, ClassQuery.EXACT, foo, false);
-  // Should be false since the class is not directly instantiated:
-  check(superclass, ClassQuery.EXACT, bar, true);
-  // Should be false since the class is not directly instantiated:
-  check(superclass, ClassQuery.EXACT, baz, true);
+  check(superclass, ClassQuery.EXACT, bar, false);
+  check(superclass, ClassQuery.EXACT, baz, false);
   check(superclass, ClassQuery.SUBCLASS, foo, false);
-  // Should be false since all live subclasses have a concrete implementation:
-  check(superclass, ClassQuery.SUBCLASS, bar, true);
+  check(superclass, ClassQuery.SUBCLASS, bar, false);
   check(superclass, ClassQuery.SUBCLASS, baz, true);
   check(superclass, ClassQuery.SUBTYPE, foo, false);
-  // Should be false since all live subtypes have a concrete implementation:
-  check(superclass, ClassQuery.SUBTYPE, bar, true);
+  check(superclass, ClassQuery.SUBTYPE, bar, false);
   check(superclass, ClassQuery.SUBTYPE, baz, true);
 
   check(subclass, ClassQuery.EXACT, foo, false);
@@ -258,17 +259,13 @@ testClassSets() async {
   Expect.isTrue(closedWorld.isImplemented(subtype));
 
   check(superclass, ClassQuery.EXACT, foo, false);
-  // Should be false since the class is not directly instantiated:
-  check(superclass, ClassQuery.EXACT, bar, true);
-  // Should be false since the class is not directly instantiated:
-  check(superclass, ClassQuery.EXACT, baz, true);
+  check(superclass, ClassQuery.EXACT, bar, false);
+  check(superclass, ClassQuery.EXACT, baz, false);
   check(superclass, ClassQuery.SUBCLASS, foo, false);
-  // Should be false since all live subclasses have a concrete implementation:
-  check(superclass, ClassQuery.SUBCLASS, bar, true);
+  check(superclass, ClassQuery.SUBCLASS, bar, false);
   check(superclass, ClassQuery.SUBCLASS, baz, true);
   check(superclass, ClassQuery.SUBTYPE, foo, true);
-  // Should be false since all live subtypes have a concrete implementation:
-  check(superclass, ClassQuery.SUBTYPE, bar, true);
+  check(superclass, ClassQuery.SUBTYPE, bar, false);
   check(superclass, ClassQuery.SUBTYPE, baz, true);
 
   check(subclass, ClassQuery.EXACT, foo, false);