From fec5acd7ece89e6b5ba36c2e612628103d329dfc Mon Sep 17 00:00:00 2001 From: aravind Date: Tue, 21 May 2013 12:39:07 -0700 Subject: [PATCH] Treadmill for shared variants Avoids atomic incRefs and decRefs. StringData no longer needs to be sweepable for SharedVariants. --- hphp/runtime/base/array/array_data.h | 2 - hphp/runtime/base/array/policy_array.h | 4 -- hphp/runtime/base/execution_context.h | 6 +++ hphp/runtime/base/memory/memory_manager.cpp | 2 - hphp/runtime/base/memory/memory_manager.h | 1 - .../base/shared/concurrent_shared_store.cpp | 22 ++++------ hphp/runtime/base/shared/immutable_map.cpp | 8 ++-- hphp/runtime/base/shared/immutable_map.h | 3 +- hphp/runtime/base/shared/immutable_obj.cpp | 2 +- hphp/runtime/base/shared/shared_map.cpp | 1 - hphp/runtime/base/shared/shared_map.h | 11 +---- hphp/runtime/base/shared/shared_variant.cpp | 13 +----- hphp/runtime/base/shared/shared_variant.h | 32 +++----------- hphp/runtime/base/string_data.cpp | 43 ------------------- hphp/runtime/base/string_data.h | 14 +----- hphp/runtime/base/type_variant.cpp | 13 ------ hphp/runtime/base/type_variant.h | 5 --- hphp/runtime/vm/bytecode.cpp | 22 ++++++++++ 18 files changed, 51 insertions(+), 153 deletions(-) diff --git a/hphp/runtime/base/array/array_data.h b/hphp/runtime/base/array/array_data.h index 04d47d364..c015c61ac 100644 --- a/hphp/runtime/base/array/array_data.h +++ b/hphp/runtime/base/array/array_data.h @@ -177,8 +177,6 @@ class ArrayData : public Countable { */ virtual bool isVectorData() const; - virtual SharedVariant *getSharedVariant() const { return nullptr; } - /** * 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. diff --git a/hphp/runtime/base/array/policy_array.h b/hphp/runtime/base/array/policy_array.h index e3ced33ca..0b706cc34 100644 --- a/hphp/runtime/base/array/policy_array.h +++ b/hphp/runtime/base/array/policy_array.h @@ -374,10 +374,6 @@ public: */ virtual bool isVectorData() const FOLLY_OVERRIDE; - virtual SharedVariant *getSharedVariant() const FOLLY_OVERRIDE { - return nullptr; - } - /** * Position-based iterations. */ diff --git a/hphp/runtime/base/execution_context.h b/hphp/runtime/base/execution_context.h index 097372445..000466293 100644 --- a/hphp/runtime/base/execution_context.h +++ b/hphp/runtime/base/execution_context.h @@ -411,6 +411,8 @@ private: DECLARE_DBG_SETTING }; +typedef std::vector SVarVector; + class VMExecutionContext : public BaseExecutionContext { public: VMExecutionContext(); @@ -445,8 +447,12 @@ public: static void unpackContVarEnvLinkage(ActRec* fp); static void packContVarEnvLinkage(ActRec* fp); void pushLocalsAndIterators(const HPHP::Func* f, int nparams = 0); + void enqueueSharedVar(SharedVariant* var); private: + SVarVector m_freedSvars; + void treadmillSharedVars(); + enum VectorLeaveCode { ConsumeAll, LeaveLast diff --git a/hphp/runtime/base/memory/memory_manager.cpp b/hphp/runtime/base/memory/memory_manager.cpp index d458ac449..2105fe55e 100644 --- a/hphp/runtime/base/memory/memory_manager.cpp +++ b/hphp/runtime/base/memory/memory_manager.cpp @@ -169,7 +169,6 @@ 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() { @@ -222,7 +221,6 @@ struct SmallNode { typedef std::vector::const_iterator SlabIter; void MemoryManager::rollback() { - StringData::sweepAll(); for (unsigned int i = 0, n = m_smartAllocators.size(); i < n; i++) { m_smartAllocators[i]->clear(); } diff --git a/hphp/runtime/base/memory/memory_manager.h b/hphp/runtime/base/memory/memory_manager.h index 823dc7f21..8ea0b4058 100644 --- a/hphp/runtime/base/memory/memory_manager.h +++ b/hphp/runtime/base/memory/memory_manager.h @@ -349,7 +349,6 @@ 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; diff --git a/hphp/runtime/base/shared/concurrent_shared_store.cpp b/hphp/runtime/base/shared/concurrent_shared_store.cpp index 3a119613b..16355f1fd 100644 --- a/hphp/runtime/base/shared/concurrent_shared_store.cpp +++ b/hphp/runtime/base/shared/concurrent_shared_store.cpp @@ -106,7 +106,7 @@ bool ConcurrentTableSharedStore::clear() { for (Map::iterator iter = m_vars.begin(); iter != m_vars.end(); ++iter) { if (iter->second.inMem()) { - iter->second.var->decRef(); + g_vmContext->enqueueSharedVar(iter->second.var); } free((void *)iter->first); } @@ -132,7 +132,7 @@ bool ConcurrentTableSharedStore::eraseImpl(CStrRef key, bool expired) { } if (acc->second.inMem()) { stats_on_delete(key.get(), &acc->second, expired); - acc->second.var->decRef(); + g_vmContext->enqueueSharedVar(acc->second.var); } else { assert(acc->second.inFile()); assert(acc->second.expiry == 0); @@ -215,7 +215,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 - converted->decRef(); + delete converted; return false; } // A write lock was acquired during find @@ -227,10 +227,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; - sv->decRef(); + g_vmContext->enqueueSharedVar(sv); return true; } - converted->decRef(); + delete converted; } return false; } @@ -295,8 +295,6 @@ 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(); @@ -313,8 +311,6 @@ bool ConcurrentTableSharedStore::get(CStrRef key, Variant &value) { if (promoteObj) { handlePromoteObj(key, svar, value); - // release the extra ref - svar->decRef(); } return true; } @@ -344,7 +340,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)); - sval->var->decRef(); + g_vmContext->enqueueSharedVar(sval->var); sval->var = svar; found = true; log_apc(std_apc_hit); @@ -365,7 +361,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)); - sval->var->decRef(); + g_vmContext->enqueueSharedVar(sval->var); sval->var = var; success = true; log_apc(std_apc_cas); @@ -440,7 +436,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)); - sval->var->decRef(); + g_vmContext->enqueueSharedVar(sval->var); update = true; } else { // mark the inFile copy invalid since we are updating the key @@ -448,7 +444,7 @@ bool ConcurrentTableSharedStore::store(CStrRef key, CVarRef value, int64_t ttl, sval->sSize = 0; } } else { - svar->decRef(); + delete svar; return false; } } diff --git a/hphp/runtime/base/shared/immutable_map.cpp b/hphp/runtime/base/shared/immutable_map.cpp index e36f46c72..5f63ef71c 100644 --- a/hphp/runtime/base/shared/immutable_map.cpp +++ b/hphp/runtime/base/shared/immutable_map.cpp @@ -24,8 +24,7 @@ namespace HPHP { HOT_FUNC ImmutableMap* ImmutableMap::Create(ArrayData* arr, - bool unserializeObj, - bool &shouldCache) { + bool unserializeObj) { int num = arr->size(); int cap = num > 2 ? Util::roundUpToPowerOfTwo(num) : 2; @@ -43,7 +42,6 @@ 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; } @@ -59,8 +57,8 @@ HOT_FUNC void ImmutableMap::Destroy(ImmutableMap* map) { Bucket* buckets = map->buckets(); for (int i = 0; i < map->m.m_num; i++) { - buckets[i].key->decRef(); - buckets[i].val->decRef(); + delete buckets[i].key; + delete buckets[i].val; } free(map); } diff --git a/hphp/runtime/base/shared/immutable_map.h b/hphp/runtime/base/shared/immutable_map.h index 291b005b6..2ee3e62b0 100644 --- a/hphp/runtime/base/shared/immutable_map.h +++ b/hphp/runtime/base/shared/immutable_map.h @@ -63,8 +63,7 @@ public: } static ImmutableMap* Create(ArrayData* arr, - bool unserializeObj, - bool& shouldCache); + bool unserializeObj); static void Destroy(ImmutableMap* im); private: ImmutableMap() {} diff --git a/hphp/runtime/base/shared/immutable_obj.cpp b/hphp/runtime/base/shared/immutable_obj.cpp index 94fa26bd1..9f23bbb66 100644 --- a/hphp/runtime/base/shared/immutable_obj.cpp +++ b/hphp/runtime/base/shared/immutable_obj.cpp @@ -82,7 +82,7 @@ ImmutableObj::~ImmutableObj() { if (m_props) { for (int i = 0; i < m_propCount; i++) { m_props[i].name->destruct(); - if (m_props[i].val) m_props[i].val->decRef(); + if (m_props[i].val) delete m_props[i].val; } free(m_props); } diff --git a/hphp/runtime/base/shared/shared_map.cpp b/hphp/runtime/base/shared/shared_map.cpp index 18af36e89..ad902c793 100644 --- a/hphp/runtime/base/shared/shared_map.cpp +++ b/hphp/runtime/base/shared/shared_map.cpp @@ -54,7 +54,6 @@ SharedMap::~SharedMap() { } smart_free(m_localCache); } - m_arr->decRef(); } bool SharedMap::exists(const StringData* k) const { diff --git a/hphp/runtime/base/shared/shared_map.h b/hphp/runtime/base/shared/shared_map.h index f48aa4f99..7254f9f66 100644 --- a/hphp/runtime/base/shared/shared_map.h +++ b/hphp/runtime/base/shared/shared_map.h @@ -29,24 +29,18 @@ namespace HPHP { /** * Wrapper for a shared memory map. */ -class SharedMap : public ArrayData, Sweepable { +class SharedMap : public ArrayData { public: SharedMap(SharedVariant* source) : ArrayData(kSharedMap) , m_arr(source) , m_localCache(nullptr) { - source->incRef(); } ~SharedMap(); virtual bool isSharedMap() const { return true; } - virtual SharedVariant *getSharedVariant() const { - if (m_arr->shouldCache()) return nullptr; - return m_arr; - } - // 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. @@ -122,9 +116,6 @@ public: */ DECLARE_SMART_ALLOCATION(SharedMap); - // implements Sweepable.sweep() - void sweep() { m_arr->decRef(); } - virtual ArrayData *escalate() const; virtual ArrayData* escalateForSort(); diff --git a/hphp/runtime/base/shared/shared_variant.cpp b/hphp/runtime/base/shared/shared_variant.cpp index ca001f02f..d8b81af9c 100644 --- a/hphp/runtime/base/shared/shared_variant.cpp +++ b/hphp/runtime/base/shared/shared_variant.cpp @@ -26,7 +26,7 @@ namespace HPHP { SharedVariant::SharedVariant(CVarRef source, bool serialized, bool inner /* = false */, bool unserializeObj /* = false */) - : m_shouldCache(false), m_flags(0) { + : m_flags(0) { assert(!serialized || source.isString()); m_count = 1; m_type = source.getType(); @@ -81,7 +81,6 @@ 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,11 +93,10 @@ 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_shouldCache); + m_data.map = ImmutableMap::Create(arr, unserializeObj); } break; } @@ -110,7 +108,6 @@ StringCase: default: { assert(source.isObject()); - m_shouldCache = true; if (unserializeObj) { // This assumes hasInternalReference(seen, true) is false ImmutableObj* obj = new ImmutableObj(source.getObjectData()); @@ -328,12 +325,6 @@ 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); } diff --git a/hphp/runtime/base/shared/shared_variant.h b/hphp/runtime/base/shared/shared_variant.h index 39423d4bf..e7e8be3d7 100644 --- a/hphp/runtime/base/shared/shared_variant.h +++ b/hphp/runtime/base/shared/shared_variant.h @@ -59,29 +59,11 @@ 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 { @@ -139,7 +121,6 @@ public: SharedVariant *convertObj(CVarRef var); bool isUnserializedObj() { return getIsObj(); } - bool shouldCache() const { return m_shouldCache; } int countReachable() const; @@ -156,7 +137,7 @@ private: ~VectorData() { SharedVariant** v = vals(); for (size_t i = 0; i < m_size; i++) { - v[i]->decRef(); + delete v[i]; } } SharedVariant** vals() { return (SharedVariant**)(this + 1); } @@ -172,7 +153,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_shouldCache and m_flags are + * anonymous struct. For non-refcounted types, m_flags is * guaranteed to be 0, and other parts of runtime will not touch the count. */ @@ -191,23 +172,20 @@ private: #if PACKED_TV uint8_t _typePad; DataType m_type; - bool m_shouldCache; - uint8_t m_flags; + uint16_t m_flags; uint32_t m_count; SharedData m_data; #else #ifdef WORDS_BIGENDIAN SharedData m_data; uint32_t m_count; - bool m_shouldCache; - uint8_t m_flags; + uint16_t m_flags; uint16_t m_type; #else SharedData m_data; uint32_t m_count; uint16_t m_type; - bool m_shouldCache; - uint8_t m_flags; + uint16_t m_flags; #endif #endif }; diff --git a/hphp/runtime/base/string_data.cpp b/hphp/runtime/base/string_data.cpp index 6fa058607..49d6e052f 100644 --- a/hphp/runtime/base/string_data.cpp +++ b/hphp/runtime/base/string_data.cpp @@ -212,41 +212,6 @@ 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)); @@ -335,13 +300,11 @@ 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 @@ -353,8 +316,6 @@ void StringData::releaseData() { break; case IsShared: assert(checkSane()); - m_big.shared->decRef(); - delist(); break; case IsSmart: assert(checkSane()); @@ -434,10 +395,6 @@ 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; diff --git a/hphp/runtime/base/string_data.h b/hphp/runtime/base/string_data.h index 388c88eac..87eb8514f 100644 --- a/hphp/runtime/base/string_data.h +++ b/hphp/runtime/base/string_data.h @@ -128,14 +128,6 @@ class StringData { void setStatic() const; 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); /** @@ -344,7 +336,6 @@ public: DECLARE_SMART_ALLOCATION(StringData); void dump() const; std::string toCPPString() const; - static void sweepAll(); static StringData *FindStaticString(const StringData* str); static StringData *GetStaticString(const StringData* str); @@ -385,9 +376,8 @@ 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(SweepNode) - sizeof(SharedVariant*) - sizeof(uint64_t); + sizeof(SharedVariant*) - sizeof(uint64_t); char junk[kPadding]; - SweepNode node; SharedVariant *shared; uint64_t cap; } m_big; @@ -408,8 +398,6 @@ 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; diff --git a/hphp/runtime/base/type_variant.cpp b/hphp/runtime/base/type_variant.cpp index 415f8b3ae..fc267c31b 100644 --- a/hphp/runtime/base/type_variant.cpp +++ b/hphp/runtime/base/type_variant.cpp @@ -3187,19 +3187,6 @@ 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::VarDump); String ret(vs.serialize(*this, true)); diff --git a/hphp/runtime/base/type_variant.h b/hphp/runtime/base/type_variant.h index 9058517ac..ba52ed253 100644 --- a/hphp/runtime/base/type_variant.h +++ b/hphp/runtime/base/type_variant.h @@ -776,11 +776,6 @@ class Variant : private TypedValue { */ Variant share(bool save) const; - /** - * Get the wrapped SharedVariant, if any. - */ - SharedVariant *getSharedVariant() const; - /** * Memory allocator methods. */ diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp index e49ce1358..a3e9ecbc8 100644 --- a/hphp/runtime/vm/bytecode.cpp +++ b/hphp/runtime/vm/bytecode.cpp @@ -31,7 +31,9 @@ #include "hphp/util/trace.h" #include "hphp/util/debug.h" #include "hphp/runtime/base/stat_cache.h" +#include "hphp/runtime/base/shared/shared_variant.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" @@ -2897,6 +2899,25 @@ 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()) { @@ -7577,6 +7598,7 @@ void VMExecutionContext::requestInit() { } void VMExecutionContext::requestExit() { + treadmillSharedVars(); destructObjects(); syncGdbState(); tx64->requestExit();