From 795aa1309c4a2a004891176c06467a080fa13372 Mon Sep 17 00:00:00 2001
From: mwilliams <mwilliams@fb.com>
Date: Mon, 10 Jun 2013 08:58:36 -0700
Subject: [PATCH] Cleanup lifetime management for translation units

For PhpFiles, there was a mixed management system, where references from
translated code were held until the code was made unreachable, while
references from interpreted code were held for the duration of the
current request. Under the new scheme, PhpFiles are always treadmilled.
They are owned by the FileRepository, and so need to be ref-counted
because the FileRepository can have the same PhpFile under multiple
paths. But we don't ref-count the *uses* of PhpFiles anymore.

Classes still need to be ref-counted, but as soon as their Unit
goes away, most of their internals can be freed. We just need to
hold onto them if derived classes are referencing them. Even in
that case, the next time we try to instantiate the derived class,
we can kill any that reference this Class.

There were also lots of holes where references were not dropped,
or owned data structures were not destroyed. eg a Class's methods
were not destroyed when the Class was destroyed; there were
several paths where an entry was erased from the file map, but the
corresponding PhpFile was not decRef'd - or worse, a null entry
was left in the map (something we had asserts to check for).

This tries to make the handling more consistent.
---
 hphp/runtime/base/execution_context.cpp |  14 +-
 hphp/runtime/base/file_repository.cpp   |  61 ++++---
 hphp/runtime/base/file_repository.h     |  15 +-
 hphp/runtime/base/object_data.cpp       |   8 +-
 hphp/runtime/ext/ext_reflection.cpp     |  13 +-
 hphp/runtime/ext/ext_spl.cpp            |   2 +-
 hphp/runtime/vm/bytecode.cpp            |   6 -
 hphp/runtime/vm/class.cpp               | 208 ++++++++++++++++--------
 hphp/runtime/vm/class.h                 |  45 ++++-
 hphp/runtime/vm/fixed_string_map.cpp    |   8 +-
 hphp/runtime/vm/fixed_string_map.h      |   4 +-
 hphp/runtime/vm/func.cpp                |   3 +
 hphp/runtime/vm/indexed_string_map.h    |   7 +
 hphp/runtime/vm/jit/codegen.cpp         |   4 +
 hphp/runtime/vm/jit/translator-x64.cpp  |   2 -
 hphp/runtime/vm/jit/translator.cpp      |  51 +-----
 hphp/runtime/vm/jit/translator.h        |   3 +-
 hphp/runtime/vm/treadmill.cpp           |  10 --
 hphp/runtime/vm/treadmill.h             |   9 +-
 hphp/runtime/vm/unit.cpp                |  37 ++---
 20 files changed, 286 insertions(+), 224 deletions(-)

diff --git a/hphp/runtime/base/execution_context.cpp b/hphp/runtime/base/execution_context.cpp
index f4c5a73be..0d0d02409 100644
--- a/hphp/runtime/base/execution_context.cpp
+++ b/hphp/runtime/base/execution_context.cpp
@@ -117,19 +117,7 @@ VMExecutionContext::~VMExecutionContext() {
        it != m_constInfo.end(); ++it) {
     delete it->second;
   }
-  // decRef all of the PhpFiles in m_evaledFiles. Any PhpFile whose refcount
-  // reaches zero will be destroyed. Currently each PhpFile "owns" its Unit,
-  // so when a PhpFile is destroyed it will free its Unit as well.
-  for (EvaledFilesMap::iterator it = m_evaledFiles.begin();
-       it != m_evaledFiles.end();) {
-    EvaledFilesMap::iterator current = it;
-    ++it;
-    StringData* sd = current->first;
-    Eval::PhpFile* efile = current->second;
-    efile->decRefAndDelete();
-    m_evaledFiles.erase(current);
-    decRefStr(sd);
-  }
+
   // Discard all units that were created via create_function().
   for (EvaledUnitsVec::iterator it = m_createdFuncs.begin();
        it != m_createdFuncs.end(); ++it) {
diff --git a/hphp/runtime/base/file_repository.cpp b/hphp/runtime/base/file_repository.cpp
index 1bda485eb..def44be6e 100644
--- a/hphp/runtime/base/file_repository.cpp
+++ b/hphp/runtime/base/file_repository.cpp
@@ -28,6 +28,9 @@
 #include "hphp/runtime/vm/pendq.h"
 #include "hphp/runtime/vm/repo.h"
 #include "hphp/runtime/vm/runtime.h"
+#include "hphp/runtime/vm/treadmill.h"
+
+#include "folly/ScopeGuard.h"
 
 using std::endl;
 
@@ -75,8 +78,18 @@ int PhpFile::decRef(int n) {
 }
 
 void PhpFile::decRefAndDelete() {
+  class FileInvalidationTrigger : public Treadmill::WorkItem {
+    Eval::PhpFile* m_f;
+   public:
+    FileInvalidationTrigger(Eval::PhpFile* f) : m_f(f) { }
+    virtual void operator()() {
+      FileRepository::onDelete(m_f);
+    }
+  };
+
   if (decRef() == 0) {
-    FileRepository::onDelete(this);
+    if (RuntimeOption::EvalJit) Transl::Translator::Get()->invalidateFile(this);
+    Treadmill::WorkItem::enqueue(new FileInvalidationTrigger(this));
   }
 }
 
@@ -117,7 +130,7 @@ bool FileRepository::fileDump(const char *filename) {
   return true;
 }
 
-void FileRepository::onDelete(PhpFile *f) {
+void FileRepository::onDelete(PhpFile* f) {
   assert(f->getRef() == 0);
   if (md5Enabled()) {
     WriteLock lock(s_md5Lock);
@@ -155,41 +168,54 @@ PhpFile *FileRepository::checkoutFile(StringData *rname,
     if (s_files.find(acc, name.get()) && !acc->second->isChanged(s)) {
       TRACE(1, "FR fast path hit %s\n", rname->data());
       ret = acc->second->getPhpFile();
-      ret->incRef();
       return ret;
     }
   }
 
   TRACE(1, "FR fast path miss: %s\n", rname->data());
   const StringData *n = StringData::GetStaticString(name.get());
+
+  PhpFile* toKill = nullptr;
+  SCOPE_EXIT {
+    // run this after acc is destroyed (and its lock released)
+    if (toKill) toKill->decRefAndDelete();
+  };
   ParsedFilesMap::accessor acc;
   bool isNew = s_files.insert(acc, n);
-  assert(isNew || acc->second); // We don't leave null entries around.
-  bool isChanged = !isNew && acc->second->isChanged(s);
+  PhpFileWrapper* old = acc->second;
+  SCOPE_EXIT {
+    // run this just before acc is released
+    if (old && old != acc->second) {
+      if (old->getPhpFile() != acc->second->getPhpFile()) {
+        toKill = old->getPhpFile();
+      }
+      delete old;
+    }
+    if (!acc->second) s_files.erase(acc);
+  };
+
+  assert(isNew || old); // We don't leave null entries around.
+  bool isChanged = !isNew && old->isChanged(s);
 
   if (isNew || isChanged) {
     if (!readFile(n, s, fileInfo)) {
-      // Be sure to get rid of the new reference to it.
-      s_files.erase(acc);
       TRACE(1, "File disappeared between stat and FR::readNewFile: %s\n",
             rname->data());
       return nullptr;
     }
     ret = fileInfo.m_phpFile;
-    if (isChanged && ret == acc->second->getPhpFile()) {
+    if (isChanged && ret == old->getPhpFile()) {
       // The file changed but had the same contents.
       if (debug && md5Enabled()) {
         ReadLock lock(s_md5Lock);
         assert(s_md5Files.find(ret->getMd5())->second == ret);
       }
-      ret->incRef();
       return ret;
     }
   } else if (!isNew) {
     // Somebody might have loaded the file between when we dropped
     // our read lock and got the write lock
-    ret = acc->second->getPhpFile();
-    ret->incRef();
+    ret = old->getPhpFile();
     return ret;
   }
 
@@ -203,10 +229,7 @@ PhpFile *FileRepository::checkoutFile(StringData *rname,
       raise_error("Tried to parse %s in repo authoritative mode", n->data());
     }
     ret = parseFile(n->data(), fileInfo);
-    if (!ret) {
-      s_files.erase(acc);
-      return nullptr;
-    }
+    if (!ret) return nullptr;
   }
   assert(ret != nullptr);
 
@@ -215,22 +238,18 @@ PhpFile *FileRepository::checkoutFile(StringData *rname,
     ret->incRef();
     ret->setId(Transl::TargetCache::allocBit());
   } else {
-    PhpFile *f = acc->second->getPhpFile();
+    PhpFile *f = old->getPhpFile();
     if (f != ret) {
       ret->setId(f->getId());
-      Transl::Translator::Get()->invalidateFile(f); // f has changed
+      ret->incRef();
     }
-    f->decRefAndDelete();
-    delete acc->second;
     acc->second = new PhpFileWrapper(s, ret);
-    ret->incRef();
   }
 
   if (md5Enabled()) {
     WriteLock lock(s_md5Lock);
     s_md5Files[ret->getMd5()] = ret;
   }
-  ret->incRef();
   return ret;
 }
 
diff --git a/hphp/runtime/base/file_repository.h b/hphp/runtime/base/file_repository.h
index f5fb376ac..c252e7f44 100644
--- a/hphp/runtime/base/file_repository.h
+++ b/hphp/runtime/base/file_repository.h
@@ -121,7 +121,20 @@ typedef RankedCHM<const StringData*, HPHP::Eval::PhpFileWrapper*,
 typedef hphp_hash_map<std::string, PhpFile*, string_hash> Md5FileMap;
 
 /**
- * FileRepository is global.
+ * FileRepository tracks all the Units that are currently live in this session.
+ *
+ * Currently live means that its been loaded at least once, and we haven't
+ * noticed a change to the underlying file yet.
+ *
+ * FileRepository contains a chm from file-paths to PhpFileWrappers.
+ * The PhpFileWrapper is owned by the chm entry, and refers to a refCounted
+ * PhpFile (the refCount is the number of times it appears in the
+ * FileRepository in total - symlinks can cause a PhpFile to appear more than
+ * once). The PhpFile refers to, and owns a Unit.
+ *
+ * When the last reference to a PhpFile goes away, the Unit and its contents
+ * can no longer be reached by new requests, but existing requests could still
+ * be referring to it, so the Unit is freed via TreadMill.
  */
 class FileRepository {
 public:
diff --git a/hphp/runtime/base/object_data.cpp b/hphp/runtime/base/object_data.cpp
index 6af8c08da..653031b91 100644
--- a/hphp/runtime/base/object_data.cpp
+++ b/hphp/runtime/base/object_data.cpp
@@ -363,12 +363,10 @@ void ObjectData::o_getArray(Array& props, bool pubOnly /* = false */) const {
   const Class* cls = m_cls;
   do {
     getProps(cls, pubOnly, cls->m_preClass.get(), props, inserted);
-    auto& usedTraits = cls->m_usedTraits;
-    for (unsigned t = 0; t < usedTraits.size(); t++) {
-      const ClassPtr& trait = usedTraits[t];
+    for (auto const& trait : cls->m_usedTraits) {
       getProps(cls, pubOnly, trait->m_preClass.get(), props, inserted);
     }
-    cls = cls->m_parent.get();
+    cls = cls->parent();
   } while (cls);
 
   // Iterate over dynamic properties and insert {name --> prop} pairs.
@@ -419,7 +417,7 @@ Array ObjectData::o_toIterArray(CStrRef context,
         }
       }
     }
-    klass = klass->m_parent.get();
+    klass = klass->parent();
   }
 
   // Now get dynamic properties.
diff --git a/hphp/runtime/ext/ext_reflection.cpp b/hphp/runtime/ext/ext_reflection.cpp
index d500696a0..b0e818be6 100644
--- a/hphp/runtime/ext/ext_reflection.cpp
+++ b/hphp/runtime/ext/ext_reflection.cpp
@@ -759,14 +759,11 @@ Array f_hphp_get_class_info(CVarRef name) {
   ret.set(s_extension, empty_string);
   ret.set(s_parent,    cls->parentRef());
 
-  typedef vector<ClassPtr> ClassVec;
   // interfaces
   {
     Array arr = Array::Create();
-    const ClassVec &interfaces = cls->declInterfaces();
-    for (ClassVec::const_iterator it = interfaces.begin(),
-           end = interfaces.end(); it != end; ++it) {
-      arr.set(it->get()->nameRef(), VarNR(1));
+    for (auto const& interface : cls->declInterfaces()) {
+      arr.set(interface->nameRef(), VarNR(1));
     }
     ret.set(s_interfaces, VarNR(arr));
   }
@@ -774,10 +771,8 @@ Array f_hphp_get_class_info(CVarRef name) {
   // traits
   {
     Array arr = Array::Create();
-    const ClassVec &traits = cls->usedTraits();
-    for (ClassVec::const_iterator it = traits.begin(),
-           end = traits.end(); it != end; ++it) {
-      arr.set(it->get()->nameRef(), VarNR(1));
+    for (auto const& trait : cls->usedTraits()) {
+      arr.set(trait->nameRef(), VarNR(1));
     }
     ret.set(s_traits, VarNR(arr));
   }
diff --git a/hphp/runtime/ext/ext_spl.cpp b/hphp/runtime/ext/ext_spl.cpp
index 551bc5da9..2011860b5 100644
--- a/hphp/runtime/ext/ext_spl.cpp
+++ b/hphp/runtime/ext/ext_spl.cpp
@@ -195,7 +195,7 @@ Variant f_class_uses(CVarRef obj, bool autoload /* = true */) {
   }
   Array ret(Array::Create());
   for (auto& elem : cls->usedTraits()) {
-    auto& traitName = elem.get()->nameRef();
+    auto& traitName = elem->nameRef();
     ret.set(traitName, traitName);
   }
   return ret;
diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp
index d153fcdb3..e0ff3340d 100644
--- a/hphp/runtime/vm/bytecode.cpp
+++ b/hphp/runtime/vm/bytecode.cpp
@@ -2287,7 +2287,6 @@ HPHP::Eval::PhpFile* VMExecutionContext::lookupPhpFile(StringData* path,
     // We found it! Return the unit.
     efile = it->second;
     initial = false;
-    if (!initial_opt) efile->incRef();
     return efile;
   }
   // We didn't find it, so try the realpath.
@@ -2309,9 +2308,7 @@ HPHP::Eval::PhpFile* VMExecutionContext::lookupPhpFile(StringData* path,
           efile = it->second;
           m_evaledFiles[spath.get()] = efile;
           spath.get()->incRefCount();
-          efile->incRef();
           initial = false;
-          if (!initial_opt) efile->incRef();
           return efile;
         }
       }
@@ -2320,7 +2317,6 @@ HPHP::Eval::PhpFile* VMExecutionContext::lookupPhpFile(StringData* path,
   // This file hasn't been included yet, so we need to parse the file
   efile = HPHP::Eval::FileRepository::checkoutFile(
     hasRealpath ? rpath.get() : spath.get(), s);
-  assert(!efile || efile->getRef() > 0);
   if (efile && initial_opt) {
     // if initial_opt is not set, this shouldnt be recorded as a
     // per request fetch of the file.
@@ -2335,7 +2331,6 @@ HPHP::Eval::PhpFile* VMExecutionContext::lookupPhpFile(StringData* path,
     if (hasRealpath) {
       m_evaledFiles[rpath.get()] = efile;
       rpath.get()->incRefCount();
-      efile->incRef();
     }
     DEBUGGER_ATTACHED_ONLY(phpDebuggerFileLoadHook(efile));
   }
@@ -2393,7 +2388,6 @@ HPHP::Eval::PhpFile* VMExecutionContext::lookupIncludeRoot(StringData* path,
   EvaledFilesMap::const_iterator it = m_evaledFiles.find(absPath.get());
   if (it != m_evaledFiles.end()) {
     if (initial) *initial = false;
-    if (!initial) it->second->incRef();
     return it->second;
   }
 
diff --git a/hphp/runtime/vm/class.cpp b/hphp/runtime/vm/class.cpp
index 713b8cdaa..17a678022 100644
--- a/hphp/runtime/vm/class.cpp
+++ b/hphp/runtime/vm/class.cpp
@@ -195,11 +195,11 @@ void PreClass::prettyPrint(std::ostream &out) const {
 //=============================================================================
 // Class.
 
-ClassPtr Class::newClass(PreClass* preClass, Class* parent) {
+Class* Class::newClass(PreClass* preClass, Class* parent) {
   unsigned classVecLen = (parent != nullptr) ? parent->m_classVecLen+1 : 1;
   void* mem = Util::low_malloc(sizeForNClasses(classVecLen));
   try {
-    return ClassPtr(new (mem) Class(preClass, parent, classVecLen));
+    return new (mem) Class(preClass, parent, classVecLen);
   } catch (...) {
     Util::low_free(mem);
     throw;
@@ -207,7 +207,7 @@ ClassPtr Class::newClass(PreClass* preClass, Class* parent) {
 }
 
 Class::Class(PreClass* preClass, Class* parent, unsigned classVecLen)
-  : m_preClass(PreClassPtr(preClass)), m_parent(ClassPtr(parent)),
+  : m_preClass(PreClassPtr(preClass)), m_parent(parent),
     m_traitsBeginIdx(0), m_traitsEndIdx(0), m_clsInfo(nullptr),
     m_builtinPropSize(0), m_classVecLen(classVecLen), m_cachedOffset(0),
     m_propDataCache(-1), m_propSDataCache(-1), m_InstanceCtor(nullptr),
@@ -224,26 +224,99 @@ Class::Class(PreClass* preClass, Class* parent, unsigned classVecLen)
   setClassVec();
 }
 
-void Class::atomicRelease() {
-  if (m_cachedOffset != 0u) {
-    /*
-      m_cachedOffset is initialied to 0, and is only set
-      when the Class is put on the list, so we only have to
-      remove this node if its NOT 0.
-      Since we're about to remove it, reset to 0 so we know
-      its safe to kill the node during the delayed Treadmill
-      callback.
-    */
-    m_cachedOffset = 0u;
-    PreClass* pcls = m_preClass.get();
-    {
-      Lock l(Unit::s_classesMutex);
-      pcls->namedEntity()->removeClass(this);
+Class::~Class() {
+  releaseRefs();
+
+  auto methods = methodRange();
+  while (!methods.empty()) {
+    Func* meth = methods.popFront();
+    if (meth) Func::destroy(meth);
+  }
+}
+
+void Class::releaseRefs() {
+  /*
+   * We have to be careful here.
+   * We want to free up as much as possible as early as possible, but
+   * some of our methods may actually belong to our parent
+   * This means we can't destroy *our* Funcs until our refCount
+   * hits zero (ie when Class::~Class gets called), because there
+   * could be a child class which hasn't yet been destroyed, which
+   * will need to inspect them. Also, we need to inspect the Funcs
+   * now (while we still have a references to parent) to determine
+   * which ones we will eventually need to free.
+   * Similarly, if any of our func's belong to a parent class, we
+   * can't free the parent, because one of our children could also
+   * have a reference to those func's (and its only reference to
+   * our parent is via this class).
+   */
+  auto methods = mutableMethodRange();
+  bool okToReleaseParent = true;
+  while (!methods.empty()) {
+    Func*& meth = methods.popFront();
+    if (meth /* releaseRefs can be called more than once */ &&
+        meth->cls() != this &&
+        ((meth->attrs() & AttrPrivate) || !meth->hasStaticLocals())) {
+      meth = nullptr;
+      okToReleaseParent = false;
     }
-    Treadmill::WorkItem::enqueue(new Treadmill::FreeClassTrigger(this));
-    return;
   }
 
+  if (okToReleaseParent) {
+    m_parent.reset();
+  }
+  m_declInterfaces.clear();
+  m_usedTraits.clear();
+}
+
+namespace {
+
+class FreeClassTrigger : public Treadmill::WorkItem {
+  TRACE_SET_MOD(treadmill);
+  Class* m_cls;
+ public:
+  explicit FreeClassTrigger(Class* cls) : m_cls(cls) {
+    TRACE(3, "FreeClassTrigger @ %p, cls %p\n", this, m_cls);
+  }
+  void operator()() {
+    TRACE(3, "FreeClassTrigger: Firing @ %p , cls %p\n", this, m_cls);
+    if (!m_cls->decAtomicCount()) {
+      m_cls->atomicRelease();
+    }
+  }
+};
+
+}
+
+void Class::destroy() {
+  /*
+   * If we were never put on NamedEntity::classList, or
+   * we've already been destroy'd, there's nothing to do
+   */
+  if (!m_cachedOffset) return;
+
+  Lock l(Unit::s_classesMutex);
+  // Need to recheck now we have the lock
+  if (!m_cachedOffset) return;
+  // Only do this once.
+  m_cachedOffset = 0;
+
+  PreClass* pcls = m_preClass.get();
+  pcls->namedEntity()->removeClass(this);
+  /*
+   * Regardless of refCount, this Class is now unusable.
+   * Release what we can immediately, to allow dependent
+   * classes to be freed.
+   * Needs to be under the lock, because multiple threads
+   * could call destroy
+   */
+  releaseRefs();
+  Treadmill::WorkItem::enqueue(new FreeClassTrigger(this));
+}
+
+void Class::atomicRelease() {
+  assert(!m_cachedOffset);
+  assert(!getCount());
   this->~Class();
   Util::low_free(this);
 }
@@ -262,16 +335,13 @@ bool Class::verifyPersistent() const {
       !Transl::TargetCache::isPersistentHandle(m_parent->m_cachedOffset)) {
     return false;
   }
-  for (size_t i = 0, nInterfaces = m_declInterfaces.size();
-       i < nInterfaces; ++i) {
-    Class* declInterface = m_declInterfaces[i].get();
+  for (auto const& declInterface : m_declInterfaces) {
     if (!Transl::TargetCache::isPersistentHandle(
           declInterface->m_cachedOffset)) {
       return false;
     }
   }
-  for (size_t i = 0; i < m_usedTraits.size(); i++) {
-    Class* usedTrait = m_usedTraits[i].get();
+  for (auto const& usedTrait : m_usedTraits) {
     if (!Transl::TargetCache::isPersistentHandle(
           usedTrait->m_cachedOffset)) {
       return false;
@@ -429,11 +499,15 @@ Class::Avail Class::avail(Class*& parent, bool tryAutoload /*=false*/) const {
         return Avail::Fail;
       }
     }
-    if (parent != ourParent) return Avail::False;
+    if (parent != ourParent) {
+      if (UNLIKELY(ourParent->isZombie())) {
+        const_cast<Class*>(this)->destroy();
+      }
+      return Avail::False;
+    }
   }
-  for (size_t i = 0, nInterfaces = m_declInterfaces.size();
-       i < nInterfaces; ++i) {
-    Class* declInterface = m_declInterfaces[i].get();
+  for (auto const& di : m_declInterfaces) {
+    Class* declInterface = di.get();
     PreClass *pint = declInterface->m_preClass.get();
     Class* interface = Unit::getClass(pint->namedEntity(), pint->name(),
                                       tryAutoload);
@@ -442,11 +516,14 @@ Class::Avail Class::avail(Class*& parent, bool tryAutoload /*=false*/) const {
         parent = declInterface;
         return Avail::Fail;
       }
+      if (UNLIKELY(declInterface->isZombie())) {
+        const_cast<Class*>(this)->destroy();
+      }
       return Avail::False;
     }
   }
-  for (size_t i = 0; i < m_usedTraits.size(); i++) {
-    Class* usedTrait = m_usedTraits[i].get();
+  for (auto const& ut : m_usedTraits) {
+    Class* usedTrait = ut.get();
     PreClass* ptrait = usedTrait->m_preClass.get();
     Class* trait = Unit::getClass(ptrait->namedEntity(), ptrait->name(),
                                   tryAutoload);
@@ -455,6 +532,9 @@ Class::Avail Class::avail(Class*& parent, bool tryAutoload /*=false*/) const {
         parent = usedTrait;
         return Avail::Fail;
       }
+      if (UNLIKELY(usedTrait->isZombie())) {
+        const_cast<Class*>(this)->destroy();
+      }
       return Avail::False;
     }
   }
@@ -1020,26 +1100,26 @@ void Class::applyTraitPrecRule(const PreClass::TraitPrecRule& rule,
   }
 }
 
-ClassPtr Class::findSingleTraitWithMethod(const StringData* methName) {
+Class* Class::findSingleTraitWithMethod(const StringData* methName) {
   // Note: m_methods includes methods from parents / traits recursively
-  ClassPtr traitCls = ClassPtr();
-  for (size_t t = 0; t < m_usedTraits.size(); t++) {
-    if (m_usedTraits[t]->m_methods.contains(methName)) {
-      if (traitCls.get() != nullptr) { // more than one trait contains method
-        return ClassPtr();
+  Class* traitCls = nullptr;
+  for (auto const& t : m_usedTraits) {
+    if (t->m_methods.contains(methName)) {
+      if (traitCls != nullptr) { // more than one trait contains method
+        return nullptr;
       }
-      traitCls = m_usedTraits[t];
+      traitCls = t.get();
     }
   }
   return traitCls;
 }
 
 void Class::setImportTraitMethodModifiers(TraitMethodList& methList,
-                                          ClassPtr         traitCls,
+                                          Class*           traitCls,
                                           Attr             modifiers) {
   for (TraitMethodList::iterator iter = methList.begin();
        iter != methList.end(); iter++) {
-    if (iter->m_trait.get() == traitCls.get()) {
+    if (iter->m_trait == traitCls) {
       iter->m_modifiers = modifiers;
       return;
     }
@@ -1065,14 +1145,14 @@ void Class::applyTraitAliasRule(const PreClass::TraitAliasRule& rule,
   const StringData* origMethName = rule.getOrigMethodName();
   const StringData* newMethName  = rule.getNewMethodName();
 
-  ClassPtr traitCls;
+  Class* traitCls = nullptr;
   if (traitName->empty()) {
     traitCls = findSingleTraitWithMethod(origMethName);
   } else {
     traitCls = Unit::loadClass(traitName);
   }
 
-  if (!traitCls.get() || (!(traitCls->attrs() & AttrTrait))) {
+  if (!traitCls || (!(traitCls->attrs() & AttrTrait))) {
     raise_error("unknown trait '%s'", traitName->data());
   }
 
@@ -1115,7 +1195,6 @@ void Class::applyTraitRules(MethodToTraitListMap& importMethToTraitMap) {
 void Class::importTraitMethod(const TraitMethod&  traitMethod,
                               const StringData*   methName,
                               MethodMap::Builder& builder) {
-  ClassPtr trait     = traitMethod.m_trait;
   Func*    method    = traitMethod.m_method;
   Attr     modifiers = traitMethod.m_modifiers;
 
@@ -1218,8 +1297,8 @@ void Class::importTraitMethods(MethodMap::Builder& builder) {
   MethodToTraitListMap importMethToTraitMap;
 
   // 1. Find all methods to be imported
-  for (size_t t = 0; t < m_usedTraits.size(); t++) {
-    ClassPtr trait = m_usedTraits[t];
+  for (auto const& t : m_usedTraits) {
+    Class* trait = t.get();
     for (Slot i = 0; i < trait->m_methods.size(); ++i) {
       Func* method = trait->m_methods[i];
       const StringData* methName = method->name();
@@ -1460,8 +1539,7 @@ void Class::setConstants() {
   }
 
   // Copy in interface constants.
-  for (std::vector<ClassPtr>::const_iterator it = m_declInterfaces.begin();
-       it != m_declInterfaces.end(); ++it) {
+  for (auto it = m_declInterfaces.begin(); it != m_declInterfaces.end(); ++it) {
     for (Slot slot = 0; slot < (*it)->m_constants.size(); ++slot) {
       const Const& iConst = (*it)->m_constants[slot];
 
@@ -1763,7 +1841,7 @@ bool Class::compatibleTraitPropInit(TypedValue& tv1, TypedValue& tv2) {
   }
 }
 
-void Class::importTraitInstanceProp(ClassPtr    trait,
+void Class::importTraitInstanceProp(Class*      trait,
                                     Prop&       traitProp,
                                     TypedValue& traitPropVal,
                                     PropMap::Builder& curPropMap) {
@@ -1793,7 +1871,7 @@ void Class::importTraitInstanceProp(ClassPtr    trait,
   }
 }
 
-void Class::importTraitStaticProp(ClassPtr trait,
+void Class::importTraitStaticProp(Class*   trait,
                                   SProp&   traitProp,
                                   PropMap::Builder& curPropMap,
                                   SPropMap::Builder& curSPropMap) {
@@ -1836,8 +1914,8 @@ void Class::importTraitStaticProp(ClassPtr trait,
 void Class::importTraitProps(PropMap::Builder& curPropMap,
                              SPropMap::Builder& curSPropMap) {
   if (attrs() & AttrNoExpandTrait) return;
-  for (size_t t = 0; t < m_usedTraits.size(); t++) {
-    ClassPtr trait = m_usedTraits[t];
+  for (auto& t : m_usedTraits) {
+    Class* trait = t.get();
 
     // instance properties
     for (Slot p = 0; p < trait->m_declProperties.size(); p++) {
@@ -1859,7 +1937,7 @@ void Class::importTraitProps(PropMap::Builder& curPropMap,
 void Class::addTraitPropInitializers(bool staticProps) {
   if (attrs() & AttrNoExpandTrait) return;
   for (unsigned t = 0; t < m_usedTraits.size(); t++) {
-    ClassPtr trait = m_usedTraits[t];
+    Class* trait = m_usedTraits[t].get();
     InitVec& traitInitVec = staticProps ? trait->m_sinitVec : trait->m_pinitVec;
     InitVec& thisInitVec  = staticProps ? m_sinitVec : m_pinitVec;
     // Insert trait's 86[ps]init into the current class, avoiding repetitions.
@@ -1974,17 +2052,17 @@ void Class::setInterfaces() {
   for (PreClass::InterfaceVec::const_iterator it =
          m_preClass->interfaces().begin();
        it != m_preClass->interfaces().end(); ++it) {
-    auto cp = ClassPtr(Unit::loadClass(*it));
-    if (cp.get() == nullptr) {
+    Class* cp = Unit::loadClass(*it);
+    if (cp == nullptr) {
       raise_error("Undefined interface: %s", (*it)->data());
     }
     if (!(cp->attrs() & AttrInterface)) {
       raise_error("%s cannot implement %s - it is not an interface",
                   m_preClass->name()->data(), cp->name()->data());
     }
-    m_declInterfaces.push_back(cp);
+    m_declInterfaces.push_back(ClassPtr(cp));
     if (interfacesBuilder.find(cp->name()) == interfacesBuilder.end()) {
-      interfacesBuilder.add(cp->name(), cp.get());
+      interfacesBuilder.add(cp->name(), cp);
     }
     int size = cp->m_interfaces.size();
     for (int i = 0; i < size; i++) {
@@ -2004,8 +2082,8 @@ void Class::setUsedTraits() {
   for (PreClass::UsedTraitVec::const_iterator
        it = m_preClass->usedTraits().begin();
        it != m_preClass->usedTraits().end(); it++) {
-    auto classPtr = ClassPtr(Unit::loadClass(*it));
-    if (classPtr.get() == nullptr) {
+    Class* classPtr = Unit::loadClass(*it);
+    if (classPtr == nullptr) {
       raise_error("Trait '%s' not found", (*it)->data());
     }
     if (!(classPtr->attrs() & AttrTrait)) {
@@ -2013,14 +2091,14 @@ void Class::setUsedTraits() {
                   m_preClass->name()->data(),
                   classPtr->name()->data());
     }
-    m_usedTraits.push_back(classPtr);
+    m_usedTraits.push_back(ClassPtr(classPtr));
   }
 }
 
 void Class::setClassVec() {
   if (m_classVecLen > 1) {
     assert(m_parent.get() != nullptr);
-    memcpy(m_classVec, m_parent.get()->m_classVec,
+    memcpy(m_classVec, m_parent->m_classVec,
            (m_classVecLen-1) * sizeof(Class*));
   }
   m_classVec[m_classVecLen-1] = this;
@@ -2041,12 +2119,12 @@ void Class::setInstanceBitsImpl() {
 
   InstanceBits bits;
   bits.set(0);
-  auto setBits = [&](ClassPtr& c) {
+  auto setBits = [&](Class* c) {
     if (setParents) c->setInstanceBitsAndParents();
     bits |= c->m_instanceBits;
   };
-  if (m_parent.get()) setBits(m_parent);
-  std::for_each(m_declInterfaces.begin(), m_declInterfaces.end(), setBits);
+  if (m_parent.get()) setBits(m_parent.get());
+  for (auto& di : m_declInterfaces) setBits(di.get());
 
   unsigned bit;
   if (mapGet(s_instanceBits, m_preClass->name(), &bit)) {
@@ -2082,9 +2160,9 @@ void Class::getMethodNames(const Class* ctx, HphpArray* methods) const {
     }
     methods->set(const_cast<StringData*>(func->name()), true_varNR, false);
   }
-  if (m_parent.get()) m_parent.get()->getMethodNames(ctx, methods);
+  if (m_parent.get()) m_parent->getMethodNames(ctx, methods);
   for (int i = 0, sz = m_declInterfaces.size(); i < sz; i++) {
-    m_declInterfaces[i].get()->getMethodNames(ctx, methods);
+    m_declInterfaces[i]->getMethodNames(ctx, methods);
   }
 }
 
diff --git a/hphp/runtime/vm/class.h b/hphp/runtime/vm/class.h
index 16dd0cd06..24c4a76ab 100644
--- a/hphp/runtime/vm/class.h
+++ b/hphp/runtime/vm/class.h
@@ -446,9 +446,40 @@ public:
 
 public:
   // Call newClass() instead of directly calling new.
-  static ClassPtr newClass(PreClass* preClass, Class* parent);
+  static Class* newClass(PreClass* preClass, Class* parent);
   Class(PreClass* preClass, Class* parent, unsigned classVecLen);
+  ~Class();
+  /*
+   * destroy() is called when a Class becomes unreachable. This may happen
+   * before its refCount hits zero, because it is referred to by
+   * its NamedEntity, any derived classes, and (for interfaces) and Class
+   * that implents it, and (for traits) any class that uses it.
+   * Such referring classes must also be logically dead at the time
+   * destroy is called, but we dont have back pointers to find them,
+   * so instead we leave the Class in a zombie state. When we try to
+   * instantiate one of its referrers, we will notice that it depends
+   * on a zombie, and destroy *that*, releasing its refernce to this
+   * Class.
+   */
+  void destroy();
+  /*
+   * atomicRelease() is called when the (atomic) refCount hits zero.
+   * The class is completely dead at this point, and its memory is
+   * freed immediately.
+   */
   void atomicRelease();
+  /*
+   * releaseRefs() is called when a Class is put into the zombie state,
+   * to free any references to child classes, interfaces and traits
+   * Its safe to call multiple times, so is also called from the destructor
+   * (in case we bypassed the zombie state).
+   */
+  void releaseRefs();
+  /*
+   * isZombie() returns true if this class has been logically destroyed,
+   * but needed to be preserved due to outstanding references.
+   */
+  bool isZombie() const { return !m_cachedOffset; }
 
   static size_t sizeForNClasses(unsigned nClasses) {
     return offsetof(Class, m_classVec) + (sizeof(Class*) * nClasses);
@@ -693,10 +724,10 @@ private:
   static std::atomic<bool> s_instanceBitsInit;
 
   struct TraitMethod {
-    ClassPtr          m_trait;
+    Class*            m_trait;
     Func*             m_method;
     Attr              m_modifiers;
-    TraitMethod(ClassPtr trait, Func* method, Attr modifiers) :
+    TraitMethod(Class* trait, Func* method, Attr modifiers) :
         m_trait(trait), m_method(method), m_modifiers(modifiers) { }
   };
 
@@ -718,9 +749,9 @@ private:
   void importTraitMethod(const TraitMethod&  traitMethod,
                          const StringData*   methName,
                          MethodMap::Builder& curMethodMap);
-  ClassPtr findSingleTraitWithMethod(const StringData* methName);
+  Class* findSingleTraitWithMethod(const StringData* methName);
   void setImportTraitMethodModifiers(TraitMethodList& methList,
-                                     ClassPtr         traitCls,
+                                     Class*           traitCls,
                                      Attr             modifiers);
   void importTraitMethods(MethodMap::Builder& curMethodMap);
   void addTraitPropInitializers(bool staticProps);
@@ -731,11 +762,11 @@ private:
                            MethodToTraitListMap& importMethToTraitMap);
   void importTraitProps(PropMap::Builder& curPropMap,
                         SPropMap::Builder& curSPropMap);
-  void importTraitInstanceProp(ClassPtr    trait,
+  void importTraitInstanceProp(Class*      trait,
                                Prop&       traitProp,
                                TypedValue& traitPropVal,
                                PropMap::Builder& curPropMap);
-  void importTraitStaticProp(ClassPtr trait,
+  void importTraitStaticProp(Class*   trait,
                              SProp&   traitProp,
                              PropMap::Builder& curPropMap,
                              SPropMap::Builder& curSPropMap);
diff --git a/hphp/runtime/vm/fixed_string_map.cpp b/hphp/runtime/vm/fixed_string_map.cpp
index 9803cc07f..11ed4a9b6 100644
--- a/hphp/runtime/vm/fixed_string_map.cpp
+++ b/hphp/runtime/vm/fixed_string_map.cpp
@@ -38,10 +38,10 @@ inline bool strEqual(bool case_sensitive,
 }
 
 template <typename V, bool case_sensitive>
-FixedStringMap<V, case_sensitive>::~FixedStringMap() {
-  if (m_table != (Elm*)&null_key) {
-    free(m_table);
-  }
+void FixedStringMap<V, case_sensitive>::clear() {
+  if (m_table != (Elm*)&null_key) free(m_table);
+  m_table = nullptr;
+  m_mask = 0;
 }
 
 template <typename V, bool case_sensitive>
diff --git a/hphp/runtime/vm/fixed_string_map.h b/hphp/runtime/vm/fixed_string_map.h
index abecf9f43..fe3f41012 100644
--- a/hphp/runtime/vm/fixed_string_map.h
+++ b/hphp/runtime/vm/fixed_string_map.h
@@ -27,8 +27,8 @@ template <typename V, bool case_sensitive> class FixedStringMap {
 public:
   explicit FixedStringMap(int num) : m_table(0) { init(num); }
   FixedStringMap() : m_mask(0), m_table(0) {}
-  ~FixedStringMap();
-
+  ~FixedStringMap() { clear(); }
+  void clear();
   void init(int num);
   void add(const StringData* s, const V& v);
   V *find(const StringData* s) const;
diff --git a/hphp/runtime/vm/func.cpp b/hphp/runtime/vm/func.cpp
index 150832702..0066583af 100644
--- a/hphp/runtime/vm/func.cpp
+++ b/hphp/runtime/vm/func.cpp
@@ -238,6 +238,9 @@ void Func::destroy(Func* func) {
   if (func->isClosureBody() || func->isGeneratorFromClosure()) {
     Func** startOfFunc = (Func**) mem;
     mem = startOfFunc - 1; // move back by a pointer
+    if (Func* f = startOfFunc[-1]) {
+      Func::destroy(f);
+    }
   }
   func->~Func();
   Util::low_free(mem);
diff --git a/hphp/runtime/vm/indexed_string_map.h b/hphp/runtime/vm/indexed_string_map.h
index c1ef3ec41..870dcb772 100644
--- a/hphp/runtime/vm/indexed_string_map.h
+++ b/hphp/runtime/vm/indexed_string_map.h
@@ -49,6 +49,13 @@ struct IndexedStringMap {
     delete [] m_vec;
   }
 
+  void clear() {
+    delete [] m_vec;
+    m_vec = nullptr;
+    m_map.clear();
+    m_size = 0;
+  }
+
   /*
    * Create an IndexedStringMap from the supplied builder.  See
    * builder below.
diff --git a/hphp/runtime/vm/jit/codegen.cpp b/hphp/runtime/vm/jit/codegen.cpp
index a71fd1f0f..c22f7c25a 100644
--- a/hphp/runtime/vm/jit/codegen.cpp
+++ b/hphp/runtime/vm/jit/codegen.cpp
@@ -2347,6 +2347,10 @@ void CodeGenerator::cgLdRetAddr(IRInstruction* inst) {
 
 void checkFrame(ActRec* fp, Cell* sp, bool checkLocals) {
   const Func* func = fp->m_func;
+  func->validate();
+  if (func->cls()) {
+    assert(!func->cls()->isZombie());
+  }
   if (fp->hasVarEnv()) {
     assert(fp->getVarEnv()->getCfp() == fp);
   }
diff --git a/hphp/runtime/vm/jit/translator-x64.cpp b/hphp/runtime/vm/jit/translator-x64.cpp
index 9096ef0b2..592911bca 100644
--- a/hphp/runtime/vm/jit/translator-x64.cpp
+++ b/hphp/runtime/vm/jit/translator-x64.cpp
@@ -4047,6 +4047,4 @@ void TranslatorX64::invalidateSrcKey(SrcKey sk) {
 
 } // HPHP::Transl
 
-TRACE_SET_MOD(tx64);
-
 } // HPHP
diff --git a/hphp/runtime/vm/jit/translator.cpp b/hphp/runtime/vm/jit/translator.cpp
index a33a1aaf0..477ca33b7 100644
--- a/hphp/runtime/vm/jit/translator.cpp
+++ b/hphp/runtime/vm/jit/translator.cpp
@@ -4005,16 +4005,6 @@ void Translator::setTransCounter(TransID transId, uint64_t value) {
 
 namespace {
 
-struct DeferredFileInvalidate : public DeferredWorkItem {
-  Eval::PhpFile* m_f;
-  explicit DeferredFileInvalidate(Eval::PhpFile* f) : m_f(f) {
-    TRACE(2, "DeferredFileInvalidate @ %p, m_f %p\n", this, m_f); }
-  void operator()() {
-    TRACE(2, "DeferredFileInvalidate: Firing @ %p , m_f %p\n", this, m_f);
-    tx64->invalidateFileWork(m_f);
-  }
-};
-
 struct DeferredPathInvalidate : public DeferredWorkItem {
   const std::string m_path;
   explicit DeferredPathInvalidate(const std::string& path) : m_path(path) {
@@ -4031,49 +4021,14 @@ struct DeferredPathInvalidate : public DeferredWorkItem {
      * this lookup; since the path has changed, the file we'll get out is
      * going to be some new file, not the old file that needs invalidation.
      */
-    UNUSED Eval::PhpFile* f =
-      g_vmContext->lookupPhpFile(spath.get(), "");
-    // We don't keep around the extra ref.
-    if (f) f->decRefAndDelete();
+    (void)g_vmContext->lookupPhpFile(spath.get(), "");
   }
 };
 
 }
 
-void Translator::invalidateFileWork(Eval::PhpFile* f) {
-  class FileInvalidationTrigger : public Treadmill::WorkItem {
-    Eval::PhpFile* m_f;
-    int m_nRefs;
-  public:
-    FileInvalidationTrigger(Eval::PhpFile* f, int n) : m_f(f), m_nRefs(n) { }
-    virtual void operator()() {
-      if (m_f->decRef(m_nRefs) == 0) {
-        Eval::FileRepository::onDelete(m_f);
-      }
-    }
-  };
-  size_t nSmashed = tx64->m_srcDB.invalidateCode(f);
-  if (nSmashed) {
-    // The srcDB found an entry for this file. The entry's dependency
-    // on this file was counted as a reference, and the code is no longer
-    // reachable. We need to wait until the last outstanding request
-    // drains to know that we can really remove the reference.
-    Treadmill::WorkItem::enqueue(new FileInvalidationTrigger(f, nSmashed));
-  }
-}
-
-bool Translator::invalidateFile(Eval::PhpFile* f) {
-  // This is called from high rank, but we'll need the write lease to
-  // invalidate code.
-  if (!RuntimeOption::EvalJit) return false;
-  assert(f != nullptr);
-  PendQ::defer(new DeferredFileInvalidate(f));
-  return true;
-}
-
-void invalidatePath(const std::string& path) {
-  TRACE(1, "invalidatePath: abspath %s\n", path.c_str());
-  PendQ::defer(new DeferredPathInvalidate(path));
+void Translator::invalidateFile(Eval::PhpFile* f) {
+  m_srcDB.invalidateCode(f);
 }
 
 static const char *transKindStr[] = {
diff --git a/hphp/runtime/vm/jit/translator.h b/hphp/runtime/vm/jit/translator.h
index 413bd5a0b..02168d517 100644
--- a/hphp/runtime/vm/jit/translator.h
+++ b/hphp/runtime/vm/jit/translator.h
@@ -959,8 +959,7 @@ public:
   }
 
   // Async hook for file modifications.
-  bool invalidateFile(Eval::PhpFile* f);
-  void invalidateFileWork(Eval::PhpFile* f);
+  void invalidateFile(Eval::PhpFile* f);
 
   // Start a new translation space. Returns true IFF this thread created
   // a new space.
diff --git a/hphp/runtime/vm/treadmill.cpp b/hphp/runtime/vm/treadmill.cpp
index 0c16d647e..d300f40cf 100644
--- a/hphp/runtime/vm/treadmill.cpp
+++ b/hphp/runtime/vm/treadmill.cpp
@@ -26,7 +26,6 @@
 #include "hphp/util/base.h"
 #include "hphp/util/rank.h"
 #include "hphp/runtime/base/macros.h"
-#include "hphp/runtime/base/complex_types.h"
 #include "hphp/runtime/vm/jit/translator-x64.h"
 
 namespace HPHP {  namespace Treadmill {
@@ -141,15 +140,6 @@ void FreeMemoryTrigger::operator()() {
   free(m_ptr);
 }
 
-FreeClassTrigger::FreeClassTrigger(Class* cls) : m_cls(cls) {
-  TRACE(3, "FreeClassTrigger @ %p, cls %p\n", this, m_cls);
-}
-
-void FreeClassTrigger::operator()() {
-  TRACE(3, "FreeClassTrigger: Firing @ %p , cls %p\n", this, m_cls);
-  m_cls->atomicRelease();
-}
-
 void deferredFree(void* p) {
   WorkItem::enqueue(new FreeMemoryTrigger(p));
 }
diff --git a/hphp/runtime/vm/treadmill.h b/hphp/runtime/vm/treadmill.h
index 7f630b6af..49eabe728 100644
--- a/hphp/runtime/vm/treadmill.h
+++ b/hphp/runtime/vm/treadmill.h
@@ -59,14 +59,7 @@ class WorkItem {
 class FreeMemoryTrigger : public WorkItem {
   void* m_ptr;
  public:
-  FreeMemoryTrigger(void* ptr);
-  virtual void operator()();
-};
-
-class FreeClassTrigger : public Treadmill::WorkItem {
-  Class* m_cls;
- public:
-  FreeClassTrigger(Class* cls);
+  explicit FreeMemoryTrigger(void* ptr);
   virtual void operator()();
 };
 
diff --git a/hphp/runtime/vm/unit.cpp b/hphp/runtime/vm/unit.cpp
index 941143f05..7c79f28fa 100644
--- a/hphp/runtime/vm/unit.cpp
+++ b/hphp/runtime/vm/unit.cpp
@@ -413,9 +413,7 @@ Unit::~Unit() {
       Class* cur = cls;
       cls = cls->m_nextClass;
       if (cur->preClass() == pcls) {
-        if (!cur->decAtomicCount()) {
-          cur->atomicRelease();
-        }
+        cur->destroy();
       }
     }
   }
@@ -559,14 +557,15 @@ Class* Unit::defClass(const PreClass* preClass,
     // Search for a compatible extant class.  Searching from most to least
     // recently created may have better locality than alternative search orders.
     // In addition, its the only simple way to make this work lock free...
-    for (Class* class_ = top; class_ != nullptr; class_ = class_->m_nextClass) {
-      if (class_->preClass() != preClass) continue;
-
-      Class::Avail avail = class_->avail(parent, failIsFatal /*tryAutoload*/);
+    for (Class* class_ = top; class_ != nullptr; ) {
+      Class* cur = class_;
+      class_ = class_->m_nextClass;
+      if (cur->preClass() != preClass) continue;
+      Class::Avail avail = cur->avail(parent, failIsFatal /*tryAutoload*/);
       if (LIKELY(avail == Class::Avail::True)) {
-        class_->setCached();
-        DEBUGGER_ATTACHED_ONLY(phpDebuggerDefClassHook(class_));
-        return class_;
+        cur->setCached();
+        DEBUGGER_ATTACHED_ONLY(phpDebuggerDefClassHook(cur));
+        return cur;
       }
       if (avail == Class::Avail::Fail) {
         if (failIsFatal) {
@@ -597,9 +596,6 @@ Class* Unit::defClass(const PreClass* preClass,
     }
     Lock l(Unit::s_classesMutex);
 
-    /*
-      We could re-enter via Unit::getClass() or class_->avail().
-    */
     if (UNLIKELY(top != nameList->clsList())) {
       top = nameList->clsList();
       continue;
@@ -610,24 +606,25 @@ Class* Unit::defClass(const PreClass* preClass,
     }
     newClass->m_cachedOffset = nameList->m_cachedClassOffset;
 
+    newClass.get()->incAtomicCount();
+    newClass.get()->setCached();
     if (Class::s_instanceBitsInit.load(std::memory_order_acquire)) {
-      // If the instance bitmap has already been set up, we can just initialize
-      // our new class's bits and add ourselves to the class list normally.
+      // If the instance bitmap has already been set up, we can just
+      // initialize our new class's bits and add ourselves to the class
+      // list normally.
       newClass->setInstanceBits();
       nameList->pushClass(newClass.get());
     } else {
       // Otherwise, we have to grab the read lock. If the map has been
-      // initialized since we checked, initialize the bits normally. If not, we
-      // must add the new class to the class list before dropping the lock to
-      // ensure its bits are initialized when the time comes.
+      // initialized since we checked, initialize the bits normally. If not,
+      // we must add the new class to the class list before dropping the lock
+      // to ensure its bits are initialized when the time comes.
       ReadLock l(Class::s_instanceBitsLock);
       if (Class::s_instanceBitsInit.load(std::memory_order_acquire)) {
         newClass->setInstanceBits();
       }
       nameList->pushClass(newClass.get());
     }
-    newClass.get()->incAtomicCount();
-    newClass.get()->setCached();
     DEBUGGER_ATTACHED_ONLY(phpDebuggerDefClassHook(newClass.get()));
     return newClass.get();
   }