InstanceOf optimization and Class::classof clean up

Changed the Set of all interfaces imlemented by a Class with an IndexedMap. Also changed the classof (and thus instanceof) to lookup in that map by name to see if an interface is imlemented by a class. That replaces the
previous mechanism that would walk the class hierarchy dereferencing PreClass in order to see if the interface was in the class inheritance. The new model seems to lead to better performance in terms of CPU instructions, lodas
and stores. The latest perflab also showed improved CPU time though that may be just lucky noise.
We have also removed the Class::classof (PreClass*) method which leads to a slightly cleaner code.

hphp/runtime/base/builtin_functions.cpp

@@ -302,7 +302,7 @@ vm_decode_function(CVarRef function,
       }
       // Only forward the current late bound class if it is the same or
       // a descendent of cls
-      if (fwdCls && fwdCls->classof(cls->preClass())) {
+      if (fwdCls && fwdCls->classof(cls)) {
         cls = fwdCls;
       }
     }

hphp/runtime/base/object_data.cpp

@@ -54,11 +54,6 @@ ObjectData::~ObjectData() {
   }
 }
 
-HPHP::Class*
-ObjectData::instanceof(const HPHP::PreClass* pc) const {
-  return m_cls->classof(pc);
-}
-
 bool ObjectData::instanceof(const HPHP::Class* c) const {
   return m_cls->classof(c);
 }

hphp/runtime/base/object_data.h

@@ -97,7 +97,6 @@ class ObjectData : public CountableNF {
     return offsetof(ObjectData, m_cls);
   }
   static size_t attributeOff() { return offsetof(ObjectData, o_attribute); }
-  Class* instanceof(const PreClass* pc) const;
   bool instanceof(const Class* c) const;
 
   bool isCollection() const {

hphp/runtime/ext/ext_class.cpp

@@ -252,11 +252,9 @@ bool f_method_exists(CVarRef class_or_object, CStrRef method_name) {
   if (!cls) return false;
   if (cls->lookupMethod(method_name.get()) != NULL) return true;
   if (cls->attrs() & AttrAbstract) {
-    const ClassSet& ifaces = cls->allInterfaces();
-    for (ClassSet::const_iterator it = ifaces.begin();
-         it != ifaces.end();
-         ++it) {
-      if ((*it)->lookupMethod(method_name.get())) return true;
+    const Class::InterfaceMap& ifaces = cls->allInterfaces();
+    for (int i = 0, size = ifaces.size(); i < size; i++) {
+      if (ifaces[i]->lookupMethod(method_name.get())) return true;
     }
   }
   return false;

hphp/runtime/ext/ext_reflection.cpp

@@ -493,12 +493,10 @@ Array f_hphp_get_method_info(CVarRef cls, CVarRef name) {
   const Func* func = c->lookupMethod(method_name.get());
   if (!func) {
     if (c->attrs() & AttrAbstract) {
-      ClassSet::const_iterator it = c->allInterfaces().begin(),
-        end = c->allInterfaces().end();
-      while (it != end) {
-        func = (*it)->lookupMethod(method_name.get());
+      const Class::InterfaceMap& ifaces = c->allInterfaces();
+      for (int i = 0, size = ifaces.size(); i < size; i++) {
+        func = ifaces[i]->lookupMethod(method_name.get());
         if (func) break;
-        ++it;
       }
     }
     if (!func) return Array();

hphp/runtime/ext/ext_spl.cpp

@@ -152,11 +152,9 @@ Variant f_class_implements(CVarRef obj, bool autoload /* = true */) {
     return false;
   }
   Array ret(Array::Create());
-  for (auto& elem : cls->allInterfaces()) {
-    // For the case where cls is an interface, we don't want to
-    // include cls in the array we return
-    if (elem == cls) continue;
-    ret.set(elem->nameRef(), elem->nameRef());
+  const Class::InterfaceMap& ifaces = cls->allInterfaces();
+  for (int i = 0, size = ifaces.size(); i < size; i++) {
+    ret.set(ifaces[i]->nameRef(), ifaces[i]->nameRef());
   }
   return ret;
 }

hphp/runtime/vm/class.cpp

@@ -771,29 +771,6 @@ Class::Avail Class::avail(Class*& parent, bool tryAutoload /*=false*/) const {
   return AvailTrue;
 }
 
-// If this Class represents the same class as 'preClass' or a descendent of
-// 'preClass', this function returns the Class* that corresponds to 'preClass'.
-// Otherwise, this function returns NULL.
-Class* Class::classof(const PreClass* preClass) const {
-  Class* class_ = const_cast<Class*>(this);
-  do {
-    if (class_->m_preClass.get() == preClass) {
-      return class_;
-    }
-    std::vector<ClassPtr>& interfaces = class_->m_declInterfaces;
-    for (unsigned i = 0; i < interfaces.size(); ++i) {
-      // Interfaces can extend arbitrarily many interfaces themselves, so
-      // search them recursively
-      Class* iclass = interfaces[i]->classof(preClass);
-      if (iclass) {
-        return iclass;
-      }
-    }
-    class_ = class_->m_parent.get();
-  } while (class_ != nullptr);
-  return nullptr;
-}
-
 void Class::initialize(TypedValue*& sProps) const {
   if (m_pinitVec.size() > 0) {
     if (getPropData() == nullptr) {
@@ -2237,9 +2214,8 @@ void Class::setInitializers() {
 //    compatible (at least as many parameters, additional parameters must have
 //    defaults), and typehints must be compatible
 void Class::checkInterfaceMethods() {
-  for (ClassSet::const_iterator it = m_allInterfaces.begin();
-       it != m_allInterfaces.end(); it++) {
-    const Class* iface = *it;
+  for (int i = 0, size = m_interfaces.size(); i < size; i++) {
+    const Class* iface = m_interfaces[i];
 
     for (size_t m = 0; m < iface->m_methods.size(); m++) {
       Func* imeth = iface->m_methods[m];
@@ -2286,12 +2262,13 @@ void Class::checkInterfaceMethods() {
 }
 
 void Class::setInterfaces() {
-  if (attrs() & AttrInterface) {
-    m_allInterfaces.insert(this);
-  }
+  InterfaceMap::Builder interfacesBuilder;
   if (m_parent.get() != nullptr) {
-    m_allInterfaces.insert(m_parent->m_allInterfaces.begin(),
-                           m_parent->m_allInterfaces.end());
+    int size = m_parent->m_interfaces.size();
+    for (int i = 0; i < size; i++) {
+      Class* interface = m_parent->m_interfaces[i];
+      interfacesBuilder.add(interface->name(), interface);
+    }
   }
   for (PreClass::InterfaceVec::const_iterator it =
          m_preClass->interfaces().begin();
@@ -2305,9 +2282,20 @@ void Class::setInterfaces() {
                   m_preClass->name()->data(), cp->name()->data());
     }
     m_declInterfaces.push_back(cp);
-    m_allInterfaces.insert(cp->m_allInterfaces.begin(),
-                           cp->m_allInterfaces.end());
+    if (interfacesBuilder.find(cp->name()) == interfacesBuilder.end()) {
+      interfacesBuilder.add(cp->name(), cp.get());
+    }
+    int size = cp->m_interfaces.size();
+    for (int i = 0; i < size; i++) {
+      Class* interface = cp->m_interfaces[i];
+      interfacesBuilder.find(interface->name());
+      if (interfacesBuilder.find(interface->name()) ==
+          interfacesBuilder.end()) {
+        interfacesBuilder.add(interface->name(), interface);
+      }
+    }
   }
+  m_interfaces.create(interfacesBuilder);
   checkInterfaceMethods();
 }
 

hphp/runtime/vm/class.h

@@ -156,8 +156,8 @@ class PreClass : public AtomicCountable {
   struct Const {
     Const() {}
     Const(PreClass* preClass, const StringData* n,
-    	  const StringData* typeConstraint, const TypedValue& val,
-          const StringData* phpCode);
+   	  const StringData* typeConstraint, const TypedValue& val,
+      const StringData* phpCode);
 
     void prettyPrint(std::ostream& out) const;
 
@@ -638,6 +638,8 @@ public:
   typedef std::vector<std::pair<const StringData*, const StringData*> >
           TraitAliasVec;
 
+  typedef IndexedStringMap<Class*,true,int> InterfaceMap;
+
 public:
   // Call newClass() instead of directly calling new.
   static ClassPtr newClass(PreClass* preClass, Class* parent);
@@ -649,7 +651,6 @@ public:
   }
 
   Avail avail(Class *&parent, bool tryAutoload = false) const;
-  Class* classof(const PreClass* preClass) const;
   bool classof(const Class* cls) const {
     /*
       If cls is an interface or trait, we're going to have
@@ -665,7 +666,8 @@ public:
       interfaces/traits).
     */
     if (UNLIKELY(cls->attrs() & (AttrInterface | AttrTrait))) {
-      return (classof(cls->m_preClass.get()) == cls);
+      return m_interfaces.lookupDefault(cls->m_preClass->name(), nullptr)
+        == cls;
     }
     if (m_classVecLen >= cls->m_classVecLen) {
       return (m_classVec[cls->m_classVecLen-1] == cls);
@@ -722,7 +724,7 @@ public:
   bool hasDeepInitProps() const { return m_hasDeepInitProps; }
   bool needInitialization() const { return m_needInitialization; }
   bool callsCustomInstanceInit() const { return m_callsCustomInstanceInit; }
-  const ClassSet& allInterfaces() const { return m_allInterfaces; }
+  const InterfaceMap& allInterfaces() const { return m_interfaces; }
   const std::vector<ClassPtr>& usedTraits() const {
     return m_usedTraits;
   }
@@ -942,8 +944,8 @@ private:
   ClassPtr m_parent;
   std::vector<ClassPtr> m_declInterfaces; // interfaces this class declares in
                                           // its "implements" clause
-  ClassSet m_allInterfaces; // all interfaces a non-abstract class deriving
-                            // from this one (including itself) must implement
+  InterfaceMap m_interfaces;
+
   std::vector<ClassPtr> m_usedTraits;
   TraitAliasVec m_traitAliases;
 

hphp/runtime/vm/translator/hopt/codegen.cpp

@@ -1700,15 +1700,9 @@ HOT_FUNC_VM static bool instanceOfHelper(const Class* objClass,
   return testClass && objClass->classof(testClass);
 }
 
-HOT_FUNC_VM static bool instanceOfHelperIFace(const Class* objClass,
-                                              const Class* testClass) {
-  return testClass && objClass->classof(testClass->preClass());
-}
-
 void CodeGenerator::cgInstanceOf(IRInstruction* inst) {
-  const bool ifaceHint = inst->getSrc(2)->getValBool();
   cgCallHelper(m_as,
-               TCA(ifaceHint ? instanceOfHelperIFace : instanceOfHelper),
+               TCA(instanceOfHelper),
                inst->getDst(),
                kNoSyncPoint,
                ArgGroup(m_regs)

hphp/runtime/vm/translator/hopt/codegen.h

@@ -257,12 +257,10 @@ private:
 
 private:
   void emitSetCc(IRInstruction*, ConditionCode);
-  void emitInstanceCheck(IRInstruction* inst, PhysReg dstReg);
   ConditionCode emitIsTypeTest(IRInstruction* inst, bool negate);
   void cgIsTypeCommon(IRInstruction* inst, bool negate);
   void cgJmpIsTypeCommon(IRInstruction* inst, bool negate);
   void cgIsTypeMemCommon(IRInstruction*, bool negate);
-  void emitInstanceCheck(IRInstruction*);
   void emitInstanceBitmaskCheck(IRInstruction*);
   void emitTraceRet(CodeGenerator::Asm& as);
   Address cgCheckStaticBit(Type type,

hphp/runtime/vm/translator/translator-x64.cpp

@@ -10129,7 +10129,7 @@ InstanceOfDSlow(const Class* cls, const Class* constraint) {
 static uint64_t
 InstanceOfDSlowInterface(const Class* cls, const Class* parent) {
   Stats::inc(Stats::Tx64_InstanceOfDInterface);
-  return parent && cls->classof(parent->preClass());
+  return parent && cls->classof(parent);
 }
 
 void