From 5e603184f597e253b85ffd056eb2e5c673d791e3 Mon Sep 17 00:00:00 2001 From: kma Date: Tue, 26 Mar 2013 09:35:49 -0700 Subject: [PATCH] De-virtualize ArrayData::release. Of the four horsemen of the SmartAllocator, ArrayData was the only virtual call. This meant an extra layer of indirection when coming from the TC to allow the c++ compiler to emit its virtual call, and slightly larger callsites when using non-generic paths. While we're moving in this direction, consolidate ArrayData introspection on its type enum. isSharedMap() was previously implemented with a vtable slot, and we had no way of asking if an ArrayData was a NameValueTable. --- hphp/compiler/analysis/emitter.cpp | 2 +- hphp/runtime/base/array/array_data.cpp | 17 +++++++++++ hphp/runtime/base/array/array_data.h | 30 ++++++++++++------- hphp/runtime/base/array/array_iterator.cpp | 8 ++--- hphp/runtime/base/array/hphp_array.cpp | 2 +- hphp/runtime/base/array/hphp_array.h | 4 --- hphp/runtime/base/shared/shared_map.h | 5 +++- hphp/runtime/base/type_variant.cpp | 5 +--- hphp/runtime/vm/bytecode.cpp | 2 +- hphp/runtime/vm/name_value_table_wrapper.h | 6 ++-- hphp/runtime/vm/translator/translator-x64.cpp | 2 +- 11 files changed, 52 insertions(+), 31 deletions(-) diff --git a/hphp/compiler/analysis/emitter.cpp b/hphp/compiler/analysis/emitter.cpp index 95246a4fb..a97a02a7a 100644 --- a/hphp/compiler/analysis/emitter.cpp +++ b/hphp/compiler/analysis/emitter.cpp @@ -6750,7 +6750,7 @@ void EmitterVisitor::initScalar(TypedValue& tvVal, ExpressionPtr val) { HphpArray* va = m_staticArrays.back(); m_staticArrays.pop_back(); - assert(IsHphpArray(ArrayData::GetScalarArray(va))); + assert(ArrayData::GetScalarArray(va)->isHphpArray()); va = static_cast(ArrayData::GetScalarArray(va)); tvVal.m_data.parr = va; diff --git a/hphp/runtime/base/array/array_data.cpp b/hphp/runtime/base/array/array_data.cpp index d467f4de4..2edbb53b8 100644 --- a/hphp/runtime/base/array/array_data.cpp +++ b/hphp/runtime/base/array/array_data.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -102,6 +103,22 @@ ArrayData *ArrayData::nonSmartCopy() const { throw FatalErrorException("nonSmartCopy not implemented."); } +HOT_FUNC +void ArrayData::release() { + if (isHphpArray()) { + HphpArray* that = static_cast(this); + that->release(); + return; + } + if (isSharedMap()) { + SharedMap* that = static_cast(this); + that->release(); + return; + } + assert(m_kind == kNameValueTableWrapper); + // NameValueTableWrapper: nop. +} + /////////////////////////////////////////////////////////////////////////////// // reads diff --git a/hphp/runtime/base/array/array_data.h b/hphp/runtime/base/array/array_data.h index d9fe245d9..4618c85fb 100644 --- a/hphp/runtime/base/array/array_data.h +++ b/hphp/runtime/base/array/array_data.h @@ -40,25 +40,26 @@ class ArrayData : public Countable { // enum of possible array types, so we can guard nonvirtual // fast paths in runtime code. - enum ArrayKind { - kArrayData, - kHphpArray + enum ArrayKind : uint8_t { + kHphpArray, + kSharedMap, + kNameValueTableWrapper, }; protected: // used in subclasses but declared here. - enum AllocMode { kInline, kSmart, kMalloc }; + enum AllocMode : uint8_t { kInline, kSmart, kMalloc }; public: static const ssize_t invalid_index = -1; - ArrayData(ArrayKind kind = kArrayData, bool nonsmart = false) : + ArrayData(ArrayKind kind, bool nonsmart = false) : m_size(-1), m_pos(0), m_strongIterators(0), m_kind(kind), m_nonsmart(nonsmart) { } - ArrayData(const ArrayData *src, ArrayKind kind = kArrayData, + ArrayData(const ArrayData *src, ArrayKind kind, bool nonsmart = false) : - m_pos(src->m_pos), m_strongIterators(0), m_kind(kind), + m_pos(src->m_pos), m_strongIterators(0), m_kind(src->m_kind), m_nonsmart(nonsmart) { } @@ -94,8 +95,10 @@ class ArrayData : public Countable { /** * For SmartAllocator. + * + * NB: *Not* virtual. ArrayData knows about its only subclasses. */ - virtual void release() = 0; + void release(); /** * Whether this array has any element. @@ -143,7 +146,12 @@ class ArrayData : public Countable { /* * Specific derived class type querying operators. */ - virtual bool isSharedMap() const { return false; } + bool isHphpArray() const { return m_kind == kHphpArray; } + bool isSharedMap() const { return m_kind == kSharedMap; } + bool isNameValueTableWrapper() const { + return m_kind == kNameValueTableWrapper; + } + /* * Returns whether or not this array contains "vector-like" data. @@ -488,9 +496,9 @@ class ArrayData : public Countable { private: FullPos* m_strongIterators; // head of linked list protected: - const uint8_t m_kind; // enum ArrayKind + const ArrayKind m_kind; + AllocMode m_allocMode; const bool m_nonsmart; // never use smartalloc to allocate Elms - uint8_t m_allocMode; // enum AllocMode /* The 4 bytes of padding here are available to subclasses if their * first field is also <= 4 bytes. */ diff --git a/hphp/runtime/base/array/array_iterator.cpp b/hphp/runtime/base/array/array_iterator.cpp index a31755304..efa952919 100644 --- a/hphp/runtime/base/array/array_iterator.cpp +++ b/hphp/runtime/base/array/array_iterator.cpp @@ -787,7 +787,7 @@ HOT_FUNC int64_t new_iter_array(Iter* dest, ArrayData* ad, TypedValue* valOut) { TRACE(2, "%s: I %p, ad %p\n", __func__, dest, ad); valOut = tvToCell(valOut); - if (UNLIKELY(!IsHphpArray(ad))) { + if (UNLIKELY(!ad->isHphpArray())) { goto cold; } { @@ -823,7 +823,7 @@ int64_t new_iter_array_key(Iter* dest, ArrayData* ad, TypedValue* valOut, TRACE(2, "%s: I %p, ad %p\n", __func__, dest, ad); valOut = tvToCell(valOut); keyOut = tvToCell(keyOut); - if (UNLIKELY(!IsHphpArray(ad))) { + if (UNLIKELY(!ad->isHphpArray())) { goto cold; } { @@ -1000,7 +1000,7 @@ int64_t iter_next(Iter* iter, TypedValue* valOut) { } { const ArrayData* ad = arrIter->getArrayData(); - if (UNLIKELY(!IsHphpArray(ad))) { + if (UNLIKELY(!ad->isHphpArray())) { goto cold; } const HphpArray* arr = (HphpArray*)ad; @@ -1045,7 +1045,7 @@ int64_t iter_next_key(Iter* iter, TypedValue* valOut, TypedValue* keyOut) { } { const ArrayData* ad = arrIter->getArrayData(); - if (UNLIKELY(!IsHphpArray(ad))) { + if (UNLIKELY(!ad->isHphpArray())) { goto cold; } const HphpArray* arr = (HphpArray*)ad; diff --git a/hphp/runtime/base/array/hphp_array.cpp b/hphp/runtime/base/array/hphp_array.cpp index f9f996f06..d7aa24fea 100644 --- a/hphp/runtime/base/array/hphp_array.cpp +++ b/hphp/runtime/base/array/hphp_array.cpp @@ -1538,7 +1538,7 @@ ArrayData* HphpArray::AddNewElemC(ArrayData* a, TypedValue value) { HphpArray* h; ElmInd* ei; int64_t k; - if (LIKELY(IsHphpArray(a)) && + if (LIKELY(a->isHphpArray()) && ((h = (HphpArray*)a), LIKELY(h->m_pos >= 0)) && LIKELY(!h->isFull()) && ((k = h->m_nextKI), LIKELY(k >= 0)) && diff --git a/hphp/runtime/base/array/hphp_array.h b/hphp/runtime/base/array/hphp_array.h index 53ab6252e..0936c791e 100644 --- a/hphp/runtime/base/array/hphp_array.h +++ b/hphp/runtime/base/array/hphp_array.h @@ -514,10 +514,6 @@ public: } }; -inline bool IsHphpArray(const ArrayData* ad) { - return ad->kind() == ArrayData::kHphpArray; -} - //============================================================================= // VM runtime support functions. namespace VM { diff --git a/hphp/runtime/base/shared/shared_map.h b/hphp/runtime/base/shared/shared_map.h index 5da6b94a7..72f514bc4 100644 --- a/hphp/runtime/base/shared/shared_map.h +++ b/hphp/runtime/base/shared/shared_map.h @@ -31,7 +31,10 @@ namespace HPHP { */ class SharedMap : public ArrayData, Sweepable { public: - SharedMap(SharedVariant* source) : m_arr(source), m_localCache(nullptr) { + SharedMap(SharedVariant* source) + : ArrayData(kSharedMap) + , m_arr(source) + , m_localCache(nullptr) { source->incRef(); } diff --git a/hphp/runtime/base/type_variant.cpp b/hphp/runtime/base/type_variant.cpp index 82b3c8e13..04ba1a2a1 100644 --- a/hphp/runtime/base/type_variant.cpp +++ b/hphp/runtime/base/type_variant.cpp @@ -199,9 +199,6 @@ Variant::Variant(CVarWithRefBind v) { * and RefData classes. */ -HOT_FUNC -static void destructArray(RefData *p) { ((ArrayData *)p)->release(); } - static_assert(TYPE_TO_DESTR_IDX(KindOfString) == 0, "String destruct index"); static_assert(TYPE_TO_DESTR_IDX(KindOfArray) == 1, "Array destruct index"); static_assert(TYPE_TO_DESTR_IDX(KindOfObject) == 2, "Object destruct index"); @@ -212,7 +209,7 @@ static_assert(kDestrTableSize == 4, const RawDestructor g_destructors[] = { (RawDestructor)Util::getMethodPtr(&StringData::release), - (RawDestructor)destructArray, + (RawDestructor)Util::getMethodPtr(&ArrayData::release), (RawDestructor)Util::getMethodPtr(&ObjectData::release), (RawDestructor)Util::getMethodPtr(&RefData::release), }; diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp index d5110ca65..ab14ea613 100644 --- a/hphp/runtime/vm/bytecode.cpp +++ b/hphp/runtime/vm/bytecode.cpp @@ -2135,7 +2135,7 @@ void VMExecutionContext::invokeFunc(TypedValue* retval, // ArrayData::merge. hphpArrCopy.merge(params); arr = dynamic_cast(hphpArrCopy.get()); - assert(arr && IsHphpArray(arr)); + assert(arr && arr->isHphpArray()); } if (arr) { const int numParams = f->numParams(); diff --git a/hphp/runtime/vm/name_value_table_wrapper.h b/hphp/runtime/vm/name_value_table_wrapper.h index 03e9530fd..c856d7a3a 100644 --- a/hphp/runtime/vm/name_value_table_wrapper.h +++ b/hphp/runtime/vm/name_value_table_wrapper.h @@ -54,8 +54,9 @@ namespace HPHP { namespace VM { */ struct NameValueTableWrapper : public ArrayData { explicit NameValueTableWrapper(NameValueTable* tab) - : m_tab(tab) - {} + : ArrayData(kNameValueTableWrapper) + , m_tab(tab) + { } public: // ArrayData implementation @@ -74,7 +75,6 @@ public: // ArrayData implementation using ArrayData::addLval; using ArrayData::remove; - virtual void release() {} virtual ssize_t vsize() const; virtual Variant getKey(ssize_t pos) const; virtual Variant getValue(ssize_t pos) const; diff --git a/hphp/runtime/vm/translator/translator-x64.cpp b/hphp/runtime/vm/translator/translator-x64.cpp index 0bf22eb03..317b4fb9b 100644 --- a/hphp/runtime/vm/translator/translator-x64.cpp +++ b/hphp/runtime/vm/translator/translator-x64.cpp @@ -1004,7 +1004,7 @@ Call TranslatorX64::getDtorCall(DataType type) { case BitwiseKindOfString: return Call(getMethodPtr(&StringData::release)); case KindOfArray: - return Call(getVTableOffset(&HphpArray::release)); + return Call(getMethodPtr(&ArrayData::release)); case KindOfObject: return Call(getMethodPtr(&ObjectData::release)); case KindOfRef: