Revert "Treadmill for shared variants"

hphp/runtime/base/array_data-defs.h

@@ -354,6 +354,10 @@ inline ArrayData* ArrayData::merge(const ArrayData* elms, bool copy) {
   return g_array_funcs.merge[m_kind](this, elms, copy);
 }
 
+inline SharedVariant* ArrayData::getSharedVariant() {
+  return g_array_funcs.getSharedVariant[m_kind](this);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 }
 

hphp/runtime/base/array_data.cpp

@@ -321,6 +321,11 @@ extern const ArrayFunctions g_array_funcs = {
     &SharedMap::Escalate,
     &ArrayData::Escalate,
     &PolicyArray::Escalate },
+  // getSharedVariant
+  { &ArrayData::GetSharedVariant, &ArrayData::GetSharedVariant,
+    &SharedMap::GetSharedVariant,
+    &ArrayData::GetSharedVariant,
+    &ArrayData::GetSharedVariant },
 };
 
 ///////////////////////////////////////////////////////////////////////////////

hphp/runtime/base/array_data.h

@@ -199,6 +199,9 @@ public:
    */
   bool isVectorData() const;
 
+  static SharedVariant *GetSharedVariant(const ArrayData* ad) { return nullptr; }
+  SharedVariant* getSharedVariant();
+
   /**
    * Whether or not this array has a referenced Variant or Object appearing
    * twice. This is mainly for APC to decide whether to serialize an array.
@@ -595,6 +598,7 @@ struct ArrayFunctions {
   void (*renumber[NK])(ArrayData*);
   void (*onSetEvalScalar[NK])(ArrayData*);
   ArrayData* (*escalate[NK])(const ArrayData*);
+  SharedVariant* (*getSharedVariant[NK])(const ArrayData*);
 };
 
 extern const ArrayFunctions g_array_funcs;

hphp/runtime/base/concurrent_shared_store.cpp

@@ -107,7 +107,7 @@ bool ConcurrentTableSharedStore::clear() {
   for (Map::iterator iter = m_vars.begin(); iter != m_vars.end();
        ++iter) {
     if (iter->second.inMem()) {
-      g_vmContext->enqueueSharedVar(iter->second.var);
+      iter->second.var->decRef();
     }
     free((void *)iter->first);
   }
@@ -133,7 +133,7 @@ bool ConcurrentTableSharedStore::eraseImpl(CStrRef key, bool expired) {
     }
     if (acc->second.inMem()) {
       stats_on_delete(key.get(), &acc->second, expired);
-      g_vmContext->enqueueSharedVar(acc->second.var);
+      acc->second.var->decRef();
     } else {
       assert(acc->second.inFile());
       assert(acc->second.expiry == 0);
@@ -216,7 +216,7 @@ bool ConcurrentTableSharedStore::handlePromoteObj(CStrRef key,
     if (!m_vars.find(acc, key.data())) {
       // There is a chance another thread deletes the key when this thread is
       // converting the object. In that case, we just bail
-      delete converted;
+      converted->decRef();
       return false;
     }
     // A write lock was acquired during find
@@ -228,10 +228,10 @@ bool ConcurrentTableSharedStore::handlePromoteObj(CStrRef key,
       int64_t ttl = sval->expiry ? sval->expiry - time(nullptr) : 0;
       stats_on_update(key.get(), sval, converted, ttl);
       sval->var = converted;
-      g_vmContext->enqueueSharedVar(sv);
+      sv->decRef();
       return true;
     }
-    delete converted;
+    converted->decRef();
   }
   return false;
 }
@@ -296,6 +296,8 @@ bool ConcurrentTableSharedStore::get(CStrRef key, Variant &value) {
         }
 
         if (RuntimeOption::ApcAllowObj && svar->is(KindOfObject)) {
+          // Hold ref here for later promoting the object
+          svar->incRef();
           promoteObj = true;
         }
         value = svar->toLocal();
@@ -312,6 +314,8 @@ bool ConcurrentTableSharedStore::get(CStrRef key, Variant &value) {
 
   if (promoteObj)  {
     handlePromoteObj(key, svar, value);
+    // release the extra ref
+    svar->decRef();
   }
   return true;
 }
@@ -341,7 +345,7 @@ int64_t ConcurrentTableSharedStore::inc(CStrRef key, int64_t step, bool &found)
       if (!sval->expired()) {
         ret = get_int64_value(sval) + step;
         SharedVariant *svar = construct(Variant(ret));
-        g_vmContext->enqueueSharedVar(sval->var);
+        sval->var->decRef();
         sval->var = svar;
         found = true;
         log_apc(std_apc_hit);
@@ -362,7 +366,7 @@ bool ConcurrentTableSharedStore::cas(CStrRef key, int64_t old, int64_t val) {
       sval = &acc->second;
       if (!sval->expired() && get_int64_value(sval) == old) {
         SharedVariant *var = construct(Variant(val));
-        g_vmContext->enqueueSharedVar(sval->var);
+        sval->var->decRef();
         sval->var = var;
         success = true;
         log_apc(std_apc_cas);
@@ -437,7 +441,7 @@ bool ConcurrentTableSharedStore::store(CStrRef key, CVarRef value, int64_t ttl,
         if (sval->inMem()) {
           stats_on_update(key.get(), sval, svar,
                           adjust_ttl(ttl, overwritePrime));
-          g_vmContext->enqueueSharedVar(sval->var);
+          sval->var->decRef();
           update = true;
         } else {
           // mark the inFile copy invalid since we are updating the key
@@ -445,7 +449,7 @@ bool ConcurrentTableSharedStore::store(CStrRef key, CVarRef value, int64_t ttl,
           sval->sSize = 0;
         }
       } else {
-        delete svar;
+        svar->decRef();
         return false;
       }
     }

hphp/runtime/base/execution_context.h

@@ -411,8 +411,6 @@ private:
   DECLARE_DBG_SETTING
 };
 
-typedef std::vector<SharedVariant*> SVarVector;
-
 class VMExecutionContext : public BaseExecutionContext {
 public:
   VMExecutionContext();
@@ -439,12 +437,8 @@ public:
     ActRec* fp, const Func* origFunc, const Func* genFunc,
     c_Continuation* cont);
   void pushLocalsAndIterators(const HPHP::Func* f, int nparams = 0);
-  void enqueueSharedVar(SharedVariant* var);
 
 private:
-  SVarVector m_freedSvars;
-  void treadmillSharedVars();
-
   enum class VectorLeaveCode {
     ConsumeAll,
     LeaveLast

hphp/runtime/base/immutable_map.cpp

@@ -25,7 +25,8 @@ namespace HPHP {
 
 HOT_FUNC
 ImmutableMap* ImmutableMap::Create(ArrayData* arr,
-                                   bool unserializeObj) {
+                                   bool unserializeObj,
+                                   bool &shouldCache) {
   int num = arr->size();
   int cap = num > 2 ? Util::roundUpToPowerOfTwo(num) : 2;
 
@@ -43,6 +44,7 @@ ImmutableMap* ImmutableMap::Create(ArrayData* arr,
                                                  unserializeObj);
       SharedVariant* val = SharedVariant::Create(it.secondRef(), false, true,
                                                  unserializeObj);
+      if (val->m_shouldCache) shouldCache = true;
       ret->add(ret->m.m_num, key, val);
       ++ret->m.m_num;
     }
@@ -58,8 +60,8 @@ HOT_FUNC
 void ImmutableMap::Destroy(ImmutableMap* map) {
   Bucket* buckets = map->buckets();
   for (int i = 0; i < map->m.m_num; i++) {
-    delete buckets[i].key;
-    delete buckets[i].val;
+    buckets[i].key->decRef();
+    buckets[i].val->decRef();
   }
   free(map);
 }

hphp/runtime/base/immutable_map.h

@@ -64,7 +64,8 @@ public:
   }
 
   static ImmutableMap* Create(ArrayData* arr,
-                              bool unserializeObj);
+                              bool unserializeObj,
+                              bool& shouldCache);
   static void Destroy(ImmutableMap* im);
 private:
   ImmutableMap() {}

hphp/runtime/base/immutable_obj.cpp

@@ -83,7 +83,7 @@ ImmutableObj::~ImmutableObj() {
   if (m_props) {
     for (int i = 0; i < m_propCount; i++) {
       m_props[i].name->destruct();
-      if (m_props[i].val) delete m_props[i].val;
+      if (m_props[i].val) m_props[i].val->decRef();
     }
     free(m_props);
   }

hphp/runtime/base/memory_manager.cpp

@@ -180,6 +180,7 @@ MemoryManager::MemoryManager() : m_front(0), m_limit(0),
   m_stats.maxBytes = INT64_MAX;
   // make the circular-lists empty.
   m_sweep.next = m_sweep.prev = &m_sweep;
+  m_strings.next = m_strings.prev = &m_strings;
 }
 
 void MemoryManager::resetStats() {
@@ -232,6 +233,7 @@ struct SmallNode {
 typedef std::vector<char*>::const_iterator SlabIter;
 
 void MemoryManager::rollback() {
+  StringData::sweepAll();
   for (unsigned int i = 0, n = m_smartAllocators.size(); i < n; i++) {
     m_smartAllocators[i]->clear();
   }

hphp/runtime/base/memory_manager.h

@@ -353,6 +353,7 @@ private:
   char *m_front, *m_limit;
   GarbageList m_smartfree[kNumSizes];
   SweepNode m_sweep;   // oversize smart_malloc'd blocks
+  SweepNode m_strings; // in-place node is head of circular list
   MemoryUsageStats m_stats;
   bool m_enabled;
 

hphp/runtime/base/policy_array.h

@@ -342,6 +342,10 @@ public:
    */
   static bool IsVectorData(const ArrayData*);
 
+  virtual SharedVariant *getSharedVariant() const FOLLY_OVERRIDE {
+    return nullptr;
+  }
+
   /**
    * Testing whether a key exists.
    */

hphp/runtime/base/shared_map.cpp

@@ -50,6 +50,12 @@ CVarRef SharedMap::GetValueRef(const ArrayData* ad, ssize_t pos) {
   return asSharedMap(ad)->getValueRef(pos);
 }
 
+SharedVariant* SharedMap::GetSharedVariant(const ArrayData* ad) {
+  auto a = asSharedMap(ad);
+  if (a->m_arr->shouldCache()) return nullptr;
+  return a->m_arr;
+}
+
 HOT_FUNC
 SharedMap::~SharedMap() {
   if (m_localCache) {
@@ -59,6 +65,7 @@ SharedMap::~SharedMap() {
     }
     smart_free(m_localCache);
   }
+  m_arr->decRef();
 }
 
 HOT_FUNC

hphp/runtime/base/shared_map.h

@@ -30,17 +30,20 @@ namespace HPHP {
 /**
  * Wrapper for a shared memory map.
  */
-class SharedMap : public ArrayData {
+class SharedMap : public ArrayData, Sweepable {
 public:
   explicit SharedMap(SharedVariant* source)
     : ArrayData(kSharedKind)
     , m_arr(source)
     , m_localCache(nullptr) {
     m_size = m_arr->arrSize();
+    source->incRef();
   }
 
   ~SharedMap();
 
+  static SharedVariant *GetSharedVariant(const ArrayData* ad);
+
   // these using directives ensure the full set of overloaded functions
   // are visible in this class, to avoid triggering implicit conversions
   // from a CVarRef key to int64.
@@ -119,6 +122,9 @@ public:
   static ArrayData* Escalate(const ArrayData*);
   static ArrayData* EscalateForSort(ArrayData*);
 
+  // implements Sweepable.sweep()
+  void sweep() { m_arr->decRef(); }
+
 private:
   ssize_t getIndex(int64_t k) const;
   ssize_t getIndex(const StringData* k) const;

hphp/runtime/base/shared_variant.cpp

@@ -27,7 +27,7 @@ namespace HPHP {
 SharedVariant::SharedVariant(CVarRef source, bool serialized,
                              bool inner /* = false */,
                              bool unserializeObj /* = false */)
-  : m_flags(0) {
+  : m_shouldCache(false), m_flags(0) {
   assert(!serialized || source.isString());
   m_count = 1;
   m_type = source.getType();
@@ -82,6 +82,7 @@ StringCase:
         PointerSet seen;
         if (arr->hasInternalReference(seen)) {
           setSerializedArray();
+          m_shouldCache = true;
           String s = apc_serialize(source);
           m_data.str = new StringData(s.data(), s.size(), CopyMalloc);
           break;
@@ -94,10 +95,11 @@ StringCase:
         for (ArrayIter it(arr); !it.end(); it.next()) {
           SharedVariant* val = Create(it.secondRef(), false, true,
                                       unserializeObj);
+          if (val->m_shouldCache) m_shouldCache = true;
           m_data.vec->vals()[m_data.vec->m_size++] = val;
         }
       } else {
-        m_data.map = ImmutableMap::Create(arr, unserializeObj);
+        m_data.map = ImmutableMap::Create(arr, unserializeObj, m_shouldCache);
       }
       break;
     }
@@ -109,6 +111,7 @@ StringCase:
   default:
     {
       assert(source.isObject());
+      m_shouldCache = true;
       if (unserializeObj) {
         // This assumes hasInternalReference(seen, true) is false
         ImmutableObj* obj = new ImmutableObj(source.getObjectData());
@@ -326,6 +329,12 @@ int SharedVariant::countReachable() const {
 SharedVariant *SharedVariant::Create
 (CVarRef source, bool serialized, bool inner /* = false */,
  bool unserializeObj /* = false*/) {
+  SharedVariant *wrapped = source.getSharedVariant();
+  if (wrapped && !unserializeObj) {
+    wrapped->incRef();
+    // static cast should be enough
+    return (SharedVariant *)wrapped;
+  }
   return new SharedVariant(source, serialized, inner, unserializeObj);
 }
 

hphp/runtime/base/shared_variant.h

@@ -61,11 +61,29 @@ public:
   DataType getType() const { return (DataType)m_type; }
   CVarRef asCVarRef() const {
     // Must be non-refcounted types
+    assert(m_shouldCache == false);
     assert(m_flags == 0);
     assert(!IS_REFCOUNTED_TYPE(m_tv.m_type));
     return tvAsCVarRef(&m_tv);
   }
 
+  void incRef() {
+    assert(IS_REFCOUNTED_TYPE(m_type));
+    atomic_inc(m_count);
+  }
+
+  void decRef() {
+    assert(m_count);
+    if (IS_REFCOUNTED_TYPE(m_type)) {
+      if (atomic_dec(m_count) == 0) {
+        delete this;
+      }
+    } else {
+      assert(m_count == 1);
+      delete this;
+    }
+  }
+
   Variant toLocal();
 
   int64_t intData() const {
@@ -123,6 +141,7 @@ public:
 
   SharedVariant *convertObj(CVarRef var);
   bool isUnserializedObj() { return getIsObj(); }
+  bool shouldCache() const { return m_shouldCache; }
 
   int countReachable() const;
 
@@ -139,7 +158,7 @@ private:
     ~VectorData() {
       SharedVariant** v = vals();
       for (size_t i = 0; i < m_size; i++) {
-        delete v[i];
+        v[i]->decRef();
       }
     }
     SharedVariant** vals() { return (SharedVariant**)(this + 1); }
@@ -155,7 +174,7 @@ private:
   /*
    * Keep the object layout binary compatible with Variant for primitive types.
    * We want to have compile time assertion to guard it but still want to have
-   * anonymous struct. For non-refcounted types, m_flags is
+   * anonymous struct. For non-refcounted types, m_shouldCache and m_flags are
    * guaranteed to be 0, and other parts of runtime will not touch the count.
    */
 
@@ -174,20 +193,23 @@ private:
 #if PACKED_TV
       uint8_t _typePad;
       DataType m_type;
-      uint16_t m_flags;
+      bool m_shouldCache;
+      uint8_t m_flags;
       uint32_t m_count;
       SharedData m_data;
 #else
  #ifdef WORDS_BIGENDIAN
       SharedData m_data;
       uint32_t m_count;
-      uint16_t m_flags;
+      bool m_shouldCache;
+      uint8_t m_flags;
       uint16_t m_type;
  #else
       SharedData m_data;
       uint32_t m_count;
       uint16_t m_type;
-      uint16_t m_flags;
+      bool m_shouldCache;
+      uint8_t m_flags;
  #endif
 #endif
     };

hphp/runtime/base/string_data.cpp

@@ -220,6 +220,41 @@ void StringData::initLiteral(const char* data, int len) {
   assert(checkSane());
 }
 
+void StringData::enlist() {
+  assert(isShared());
+  SweepNode& head = MemoryManager::TheMemoryManager()->m_strings;
+  // insert after head
+  SweepNode* next = head.next;
+  assert(uintptr_t(next) != kMallocFreeWord);
+  m_big.node.next = next;
+  m_big.node.prev = &head;
+  next->prev = head.next = &m_big.node;
+}
+
+void StringData::delist() {
+  assert(isShared());
+  SweepNode* next = m_big.node.next;
+  SweepNode* prev = m_big.node.prev;
+  assert(uintptr_t(next) != kMallocFreeWord);
+  assert(uintptr_t(prev) != kMallocFreeWord);
+  next->prev = prev;
+  prev->next = next;
+}
+
+void StringData::sweepAll() {
+  SweepNode& head = MemoryManager::TheMemoryManager()->m_strings;
+  for (SweepNode *next, *n = head.next; n != &head; n = next) {
+    next = n->next;
+    assert(next && uintptr_t(next) != kSmartFreeWord);
+    assert(next && uintptr_t(next) != kMallocFreeWord);
+    StringData* s = (StringData*)(uintptr_t(n) -
+                                  offsetof(StringData, m_big.node));
+    assert(s->isShared());
+    s->m_big.shared->decRef();
+  }
+  head.next = head.prev = &head;
+}
+
 HOT_FUNC
 void StringData::initAttach(const char* data) {
   return initAttach(data, strlen(data));
@@ -308,11 +343,13 @@ HOT_FUNC
 StringData::StringData(SharedVariant *shared)
   : _count(0) {
   assert(shared && size_t(shared->stringLength()) <= size_t(MaxSize));
+  shared->incRef();
   m_hash = 0;
   m_len = shared->stringLength();
   m_cdata = shared->stringData();
   m_big.shared = shared;
   m_big.cap = m_len | IsShared;
+  enlist();
 }
 
 HOT_FUNC
@@ -324,6 +361,8 @@ void StringData::releaseData() {
     break;
   case IsShared:
     assert(checkSane());
+    m_big.shared->decRef();
+    delist();
     break;
   case IsSmart:
     assert(checkSane());
@@ -403,6 +442,10 @@ void StringData::append(const char *s, int len) {
     // buffer is immutable, don't modify it.
     StringSlice r = slice();
     char* newdata = smart_concat(r.ptr, r.len, s, len);
+    if (isShared()) {
+      m_big.shared->decRef();
+      delist();
+    }
     m_len = newlen;
     m_data = newdata;
     m_big.cap = newlen | IsSmart;

hphp/runtime/base/string_data.h

@@ -125,6 +125,14 @@ class StringData {
   void setRefCount(int32_t n) { _count = n;}
   bool isStatic() const { return _count == RefCountStaticValue; }
 
+  /**
+   * Get the wrapped SharedVariant.
+   */
+  SharedVariant *getSharedVariant() const {
+    if (isShared()) return m_big.shared;
+    return nullptr;
+  }
+
   static StringData *Escalate(StringData *in);
 
   /**
@@ -331,6 +339,7 @@ public:
   DECLARE_SMART_ALLOCATION(StringData);
   void dump() const;
   std::string toCPPString() const;
+  static void sweepAll();
 
   static StringData *GetStaticString(const StringData* str);
   static StringData *GetStaticString(const std::string& str);
@@ -371,8 +380,9 @@ public:
       // Calculate padding so that node, shared, and cap are pointer aligned,
       // and ensure cap overlaps the last byte of m_small.
       static const size_t kPadding = sizeof(m_small) -
-        sizeof(SharedVariant*) - sizeof(uint64_t);
+        sizeof(SweepNode) - sizeof(SharedVariant*) - sizeof(uint64_t);
       char junk[kPadding];
+      SweepNode node;
       SharedVariant *shared;
       uint64_t cap;
     } m_big;
@@ -393,6 +403,8 @@ public:
   void releaseData();
   int numericCompare(const StringData *v2) const;
   MutableSlice escalate(uint32_t cap); // change to smart-malloced string
+  void enlist();
+  void delist();
 
   strhash_t hashHelper() const NEVER_INLINE;
 

hphp/runtime/base/type_variant.cpp

@@ -2039,6 +2039,19 @@ Variant Variant::share(bool save) const {
   return false; // same as non-existent
 }
 
+SharedVariant *Variant::getSharedVariant() const {
+  if (m_type == KindOfRef) {
+    return m_data.pref->var()->getSharedVariant();
+  }
+  if (m_type == KindOfString) {
+    return m_data.pstr->getSharedVariant();
+  }
+  if (m_type == KindOfArray) {
+    return m_data.parr->getSharedVariant();
+  }
+  return nullptr;
+}
+
 void Variant::dump() const {
   VariableSerializer vs(VariableSerializer::Type::VarDump);
   String ret(vs.serialize(*this, true));

hphp/runtime/base/type_variant.h

@@ -708,9 +708,13 @@ class Variant : private TypedValue {
    */
   Variant share(bool save) const;
 
-  /*
-   * Print information about a variant to stdout.  For debugging
-   * purposes.
+  /**
+   * Get the wrapped SharedVariant, if any.
+   */
+  SharedVariant *getSharedVariant() const;
+
+  /**
+   * Memory allocator methods.
    */
   void dump() const;
 

hphp/runtime/vm/bytecode.cpp

@@ -42,11 +42,8 @@
 #include "hphp/util/trace.h"
 #include "hphp/util/debug.h"
 #include "hphp/runtime/base/stat_cache.h"
-#include "hphp/runtime/base/shared_variant.h"
 #include "hphp/runtime/vm/debug/debug.h"
-
 #include "hphp/runtime/vm/hhbc.h"
-#include "hphp/runtime/vm/treadmill.h"
 #include "hphp/runtime/vm/php_debug.h"
 #include "hphp/runtime/vm/debugger_hook.h"
 #include "hphp/runtime/vm/runtime.h"
@@ -2500,25 +2497,6 @@ VMExecutionContext::pushLocalsAndIterators(const Func* func,
   }
 }
 
-void VMExecutionContext::enqueueSharedVar(SharedVariant* svar) {
-  m_freedSvars.push_back(svar);
-}
-
-class FreedSVars : public Treadmill::WorkItem {
-  SVarVector m_svars;
-public:
-  explicit FreedSVars(SVarVector&& svars) : m_svars(std::move(svars)) {}
-  virtual void operator()() {
-    for (auto it = m_svars.begin(); it != m_svars.end(); it++) {
-      delete *it;
-    }
-  }
-};
-
-void VMExecutionContext::treadmillSharedVars() {
-  Treadmill::WorkItem::enqueue(new FreedSVars(std::move(m_freedSvars)));
-}
-
 void VMExecutionContext::destructObjects() {
   if (UNLIKELY(RuntimeOption::EnableObjDestructCall)) {
     while (!m_liveBCObjs.empty()) {
@@ -7247,7 +7225,7 @@ void VMExecutionContext::requestInit() {
 }
 
 void VMExecutionContext::requestExit() {
-  treadmillSharedVars();
+  destructObjects();
   syncGdbState();
   tx()->requestExit();
   Transl::Translator::clearTranslator();