Revert "clean up SharedMap"

hphp/runtime/base/concurrent_shared_store.cpp

@@ -253,7 +253,7 @@ SharedVariant* ConcurrentTableSharedStore::unserialize(CStrRef key,
     VariableUnserializer vu(sval->sAddr, sval->getSerializedSize(), sType);
     Variant v;
     v.unserialize(&vu);
-    sval->var = new SharedVariant(v, sval->isSerializedObj());
+    sval->var = SharedVariant::Create(v, sval->isSerializedObj());
     stats_on_add(key.get(), sval, 0, true, true); // delayed prime
     return sval->var;
   } catch (Exception &e) {
@@ -520,7 +520,7 @@ bool ConcurrentTableSharedStore::constructPrime(CStrRef v, KeyValuePair& item,
       return false;
     }
   }
-  item.value = new SharedVariant(v, serialized);
+  item.value = SharedVariant::Create(v, serialized);
   return true;
 }
 
@@ -538,7 +538,7 @@ bool ConcurrentTableSharedStore::constructPrime(CVarRef v,
       return false;
     }
   }
-  item.value = new SharedVariant(v, false);
+  item.value = SharedVariant::Create(v, false);
   return true;
 }
 

hphp/runtime/base/concurrent_shared_store.h

@@ -39,7 +39,7 @@ namespace HPHP {
 
 class ConcurrentTableSharedStore : public SharedStore {
 public:
-  explicit ConcurrentTableSharedStore(int id)
+  ConcurrentTableSharedStore(int id)
     : SharedStore(id), m_lockingFlag(false), m_purgeCounter(0) {}
 
   virtual int size() {
@@ -63,7 +63,7 @@ public:
 
 protected:
   virtual SharedVariant* construct(CVarRef v) {
-    return new SharedVariant(v, false);
+    return SharedVariant::Create(v, false);
   }
 
   struct charHashCompare {

hphp/runtime/base/immutable_map.cpp

@@ -39,7 +39,11 @@ ImmutableMap* ImmutableMap::Create(ArrayData* arr,
 
   try {
     for (ArrayIter it(arr); !it.end(); it.next()) {
-      ret->add(ret->m.m_num, it.first(), it.secondRef(), unserializeObj);
+      SharedVariant* key = SharedVariant::Create(it.first(), false, true,
+                                                 unserializeObj);
+      SharedVariant* val = SharedVariant::Create(it.secondRef(), false, true,
+                                                 unserializeObj);
+      ret->add(ret->m.m_num, key, val);
       ++ret->m.m_num;
     }
   } catch (...) {
@@ -54,62 +58,36 @@ HOT_FUNC
 void ImmutableMap::Destroy(ImmutableMap* map) {
   Bucket* buckets = map->buckets();
   for (int i = 0; i < map->m.m_num; i++) {
-    (*(SharedVariant*)&buckets[i].val).~SharedVariant();
+    delete buckets[i].key;
+    delete buckets[i].val;
   }
   free(map);
 }
 
 HOT_FUNC
-void ImmutableMap::addVal(int pos, int hash_pos,
-                          CVarRef val, bool unserializeObj) {
+void ImmutableMap::add(int pos, SharedVariant *key, SharedVariant *val) {
   // NOTE: no check on duplication because we assume the original array has no
   // duplication
   Bucket* bucket = buckets() + pos;
-  new (&bucket->val) SharedVariant(val, false, true, unserializeObj);
+  bucket->key = key;
+  bucket->val = val;
+  int hash_pos =
+    (key->is(KindOfInt64) ?
+     key->intData() : key->getStringData()->hash()) & m.m_capacity_mask;
+
   int& hp = hash()[hash_pos];
   bucket->next = hp;
   hp = pos;
 }
 
-HOT_FUNC
-void ImmutableMap::add(int pos, CVarRef key, CVarRef val, bool unserializeObj) {
-  int64_t ikey;
-  StringData* skey;
-  int32_t hash;
-  Bucket* b = buckets() + pos;
-
-  switch (key.getType()) {
-    case KindOfInt64: {
-      hash = ikey = key.toInt64();
-      b->setIntKey(ikey);
-      break;
-    }
-    case KindOfString: {
-      skey = StringData::GetStaticString(key.getStringData());
-      goto static_case;
-    }
-    case KindOfStaticString: {
-      skey = key.getStringData();
-static_case:
-      hash = skey->hash();
-      b->setStrKey(skey, hash);
-      break;
-    }
-    default: not_reached();
-  }
-  addVal(pos, hash & m.m_capacity_mask, val, unserializeObj);
-}
-
-#define STR_HASH(x)   (int32_t(x) | 0x80000000)
-
 HOT_FUNC
 int ImmutableMap::indexOf(const StringData* key) {
-  strhash_t h = STR_HASH(key->hash());
+  strhash_t h = key->hash();
   int bucket = hash()[h & m.m_capacity_mask];
   Bucket* b = buckets();
   while (bucket != -1) {
-    Bucket* cand = &b[bucket];
-    if (cand->hash() == h && (cand->skey == key || key->same(cand->skey))) {
+    if (!b[bucket].key->is(KindOfInt64) &&
+        key->same(b[bucket].key->getStringData())) {
       return bucket;
     }
     bucket = b[bucket].next;
@@ -122,7 +100,8 @@ int ImmutableMap::indexOf(int64_t key) {
   int bucket = hash()[key & m.m_capacity_mask];
   Bucket* b = buckets();
   while (bucket != -1) {
-    if (b[bucket].hasIntKey() && key == b[bucket].ikey) {
+    if (b[bucket].key->is(KindOfInt64) &&
+        key == b[bucket].key->intData()) {
       return bucket;
     }
     bucket = b[bucket].next;

hphp/runtime/base/immutable_map.h

@@ -19,7 +19,6 @@
 #define incl_HPHP_IMMUTABLE_MAP_H_
 
 #include "hphp/runtime/base/types.h"
-#include "hphp/runtime/base/shared_variant.h"
 #include "hphp/util/lock.h"
 #include "hphp/util/hash.h"
 #include "hphp/util/atomic.h"
@@ -27,6 +26,7 @@
 namespace HPHP {
 ///////////////////////////////////////////////////////////////////////////////
 
+class SharedVariant;
 /**
  * an immutable map is a php-style array that can take strings and
  * ints as keys. the map also stores the order in which the elements
@@ -38,19 +38,14 @@ public:
   int indexOf(const StringData* key);
   int indexOf(int64_t key);
 
-  Variant getKey(int index) {
+  SharedVariant* getKeyIndex(int index) {
     assert(index < size());
-    Bucket* b = &buckets()[index];
-    if (b->hasIntKey()) {
-      return b->ikey;
-    } else {
-      return b->skey;
-    }
+    return buckets()[index].key;
   }
 
-  SharedVariant* getValue(int index) {
+  SharedVariant* getValIndex(int index) {
     assert(index < size());
-    return (SharedVariant*)&buckets()[index].val;
+    return buckets()[index].val;
   }
 
   unsigned size() const {
@@ -71,43 +66,18 @@ public:
   static ImmutableMap* Create(ArrayData* arr,
                               bool unserializeObj);
   static void Destroy(ImmutableMap* im);
+private:
+  ImmutableMap() {}
+  ~ImmutableMap() {}
+  void add(int pos, SharedVariant *key, SharedVariant *val);
 
   struct Bucket {
     /** index of the next bucket, or -1 if the end of a chain */
     int next;
-    /* similar to HphpArray::Elm */
-    union {
-      int64_t     ikey;
-      StringData* skey;
-    };
-    // cannot declare SharedVariant here because of cyclic header
-    // includes
-    TypedValueAux val;
-    bool hasStrKey() const {
-      return val.hash() != 0;
-    }
-    bool hasIntKey() const {
-      return val.hash() == 0;
-    }
-    int32_t hash() const {
-      return val.hash();
-    }
-    void setStrKey(StringData* k, strhash_t h) {
-      skey = k;
-      val.hash() = int32_t(h) | 0x80000000;
-    }
-    void setIntKey(int64_t k) {
-      ikey = k;
-      val.hash() = 0;
-    }
+    /** the value of this bucket */
+    SharedVariant *key;
+    SharedVariant *val;
   };
-
-private:
-  ImmutableMap() {}
-  ~ImmutableMap() {}
-  void addVal(int pos, int hash_pos, CVarRef val, bool unserializeObj);
-  void add(int pos, CVarRef key, CVarRef val, bool unserializeObj);
-
   /** index of the beginning of each hash chain */
   int *hash() const { return (int*)(this + 1); }
   /** buckets, stored in index order */

hphp/runtime/base/shared_map.cpp

@@ -28,16 +28,17 @@ IMPLEMENT_SMART_ALLOCATION_HOT(SharedMap);
 ///////////////////////////////////////////////////////////////////////////////
 HOT_FUNC
 CVarRef SharedMap::getValueRef(ssize_t pos) const {
-  SharedVariant *sv = getValueImpl(pos);
+  SharedVariant *sv = m_arr->getValue(pos);
   DataType t = sv->getType();
   if (!IS_REFCOUNTED_TYPE(t)) return sv->asCVarRef();
   if (LIKELY(m_localCache != nullptr)) {
-    assert(unsigned(pos) < size());
+    assert(unsigned(pos) < m_arr->arrCap());
     TypedValue* tv = &m_localCache[pos];
     if (tv->m_type != KindOfUninit) return tvAsCVarRef(tv);
   } else {
     static_assert(KindOfUninit == 0, "must be 0 since we use smart_calloc");
-    m_localCache = (TypedValue*) smart_calloc(size(), sizeof(TypedValue));
+    unsigned cap = m_arr->arrCap();
+    m_localCache = (TypedValue*) smart_calloc(cap, sizeof(TypedValue));
   }
   TypedValue* tv = &m_localCache[pos];
   tvAsVariant(tv) = sv->toLocal();
@@ -52,7 +53,7 @@ CVarRef SharedMap::GetValueRef(const ArrayData* ad, ssize_t pos) {
 HOT_FUNC
 SharedMap::~SharedMap() {
   if (m_localCache) {
-    for (TypedValue* tv = m_localCache, *end = tv + size();
+    for (TypedValue* tv = m_localCache, *end = tv + m_arr->arrCap();
          tv < end; ++tv) {
       tvRefcountedDecRef(tv);
     }
@@ -75,39 +76,32 @@ inline const SharedMap* SharedMap::asSharedMap(const ArrayData* ad) {
   return static_cast<const SharedMap*>(ad);
 }
 
-ssize_t SharedMap::getIndex(const StringData* k) const {
-  if (isVector()) return -1;
-  return m_map->indexOf(k);
-}
-
-ssize_t SharedMap::getIndex(int64_t k) const {
-  if (isVector()) {
-    if (k < 0 || (size_t)k >= m_vec->m_size) return -1;
-    return k;
-  }
-  return m_map->indexOf(k);
-}
-
 bool SharedMap::IsVectorData(const ArrayData* ad) {
   auto a = asSharedMap(ad);
-  if (a->isVector()) return true;
   const auto n = a->size();
   for (ssize_t i = 0; i < n; i++) {
-    if (a->m_map->indexOf(i) != i) return false;
+    if (a->getIndex(i) != i) return false;
   }
   return true;
 }
 
 bool SharedMap::ExistsStr(const ArrayData* ad, const StringData* k) {
   auto a = asSharedMap(ad);
-  if (a->isVector()) return false;
-  return a->m_map->indexOf(k) != -1;
+  return a->getIndex(k) != -1;
 }
 
 bool SharedMap::ExistsInt(const ArrayData* ad, int64_t k) {
   return asSharedMap(ad)->getIndex(k) != -1;
 }
 
+ssize_t SharedMap::getIndex(int64_t k) const {
+  return m_arr->getIndex(k);
+}
+
+ssize_t SharedMap::getIndex(const StringData* k) const {
+  return m_arr->getIndex(k);
+}
+
 /* if a2 is modified copy of a1 (i.e. != a1), then release a1 and return a2 */
 inline ArrayData* releaseIfCopied(ArrayData* a1, ArrayData* a2) {
   if (a1 != a2) a1->release();
@@ -212,7 +206,8 @@ ArrayData *SharedMap::Prepend(ArrayData* ad, CVarRef v, bool copy) {
 }
 
 ArrayData *SharedMap::Escalate(const ArrayData* ad) {
-  auto ret = asSharedMap(ad)->loadElems();
+  auto smap = asSharedMap(ad);
+  auto ret = smap->m_arr->loadElems(*smap);
   assert(!ret->isStatic());
   return ret;
 }
@@ -233,22 +228,17 @@ TypedValue* SharedMap::NvGetStr(const ArrayData* ad, const StringData* key) {
 
 void SharedMap::NvGetKey(const ArrayData* ad, TypedValue* out, ssize_t pos) {
   auto a = asSharedMap(ad);
-  if (a->isVector()) {
-    out->m_data.num = pos;
-    out->m_type = KindOfInt64;
-  } else {
-    Variant k = a->m_map->getKey(pos);
-    TypedValue* tv = k.asTypedValue();
-    // copy w/out clobbering out->_count.
-    out->m_type = tv->m_type;
-    out->m_data.num = tv->m_data.num;
-    if (tv->m_type != KindOfInt64) out->m_data.pstr->incRefCount();
-  }
+  Variant k = a->m_arr->getKey(pos);
+  TypedValue* tv = k.asTypedValue();
+  // copy w/out clobbering out->_count.
+  out->m_type = tv->m_type;
+  out->m_data.num = tv->m_data.num;
+  if (tv->m_type != KindOfInt64) out->m_data.pstr->incRefCount();
 }
 
 ArrayData* SharedMap::EscalateForSort(ArrayData* ad) {
   auto a = asSharedMap(ad);
-  auto ret = a->loadElems(true /* mapInit */);
+  auto ret = a->m_arr->loadElems(*a, true /* mapInit */);
   assert(!ret->isStatic());
   return ret;
 }
@@ -286,33 +276,9 @@ bool SharedMap::AdvanceFullPos(ArrayData* ad, FullPos& fp) {
   return false;
 }
 
-ArrayData* SharedMap::loadElems(bool mapInit /* = false */) const {
-  uint count = size();
-  bool isVec = isVector();
-
-  auto ai =
-    mapInit ? ArrayInit(count, ArrayInit::mapInit) :
-    isVec ? ArrayInit(count, ArrayInit::vectorInit) :
-    ArrayInit(count);
-
-  if (isVec) {
-    for (uint i = 0; i < count; i++) {
-      ai.set(getValueRef(i));
-    }
-  } else {
-    for (uint i = 0; i < count; i++) {
-      ai.add(m_map->getKey(i), getValueRef(i),
-             true);
-    }
-  }
-  ArrayData* elems = ai.create();
-  if (elems->isStatic()) elems = elems->copy();
-  return elems;
-}
-
 void SharedMap::getChildren(std::vector<TypedValue *> &out) {
   if (m_localCache) {
-    TypedValue *localCacheEnd = m_localCache + size();
+    TypedValue *localCacheEnd = m_localCache + m_size;
     for (TypedValue *tv = m_localCache;
          tv < localCacheEnd;
          ++tv) {

hphp/runtime/base/shared_map.h

@@ -33,11 +33,10 @@ namespace HPHP {
 class SharedMap : public ArrayData {
 public:
   explicit SharedMap(SharedVariant* source)
-      : ArrayData(kSharedKind)
-      , m_localCache(nullptr) {
-    m_map      = source->getMap();
-    m_isVector = source->getIsVector();
-    m_size = isVector() ? m_vec->m_size : m_map->size();
+    : ArrayData(kSharedKind)
+    , m_arr(source)
+    , m_localCache(nullptr) {
+    m_size = m_arr->arrSize();
   }
 
   ~SharedMap();
@@ -54,8 +53,8 @@ public:
   using ArrayData::addLval;
   using ArrayData::remove;
 
-  SharedVariant* getValueImpl(ssize_t pos) const {
-    return isVector() ? m_vec->getValue(pos) : m_map->getValue(pos);
+  Variant getKey(ssize_t pos) const {
+    return m_arr->getKey(pos);
   }
 
   CVarRef getValueRef(ssize_t pos) const;
@@ -120,25 +119,16 @@ public:
   static ArrayData* Escalate(const ArrayData*);
   static ArrayData* EscalateForSort(ArrayData*);
 
-  ArrayData* loadElems(bool mapInit = false) const;
-
 private:
   ssize_t getIndex(int64_t k) const;
   ssize_t getIndex(const StringData* k) const;
   static SharedMap* asSharedMap(ArrayData* ad);
   static const SharedMap* asSharedMap(const ArrayData* ad);
 
-private:
-  bool m_isVector;
-  union {
-    ImmutableMap* m_map;
-    VectorData*   m_vec;
-  };
-  mutable TypedValue* m_localCache;
-  bool isVector() const { return m_isVector; }
-
 public:
   void getChildren(std::vector<TypedValue *> &out);
+  SharedVariant *m_arr;
+  mutable TypedValue* m_localCache;
 };
 
 ///////////////////////////////////////////////////////////////////////////////

hphp/runtime/base/shared_variant.cpp

@@ -29,6 +29,7 @@ SharedVariant::SharedVariant(CVarRef source, bool serialized,
                              bool unserializeObj /* = false */)
   : m_flags(0) {
   assert(!serialized || source.isString());
+  m_count = 1;
   m_type = source.getType();
   switch (m_type) {
   case KindOfBoolean:
@@ -91,7 +92,9 @@ StringCase:
         setIsVector();
         m_data.vec = new (arr->size()) VectorData();
         for (ArrayIter it(arr); !it.end(); it.next()) {
-          m_data.vec->add(it.secondRef(), unserializeObj);
+          SharedVariant* val = Create(it.secondRef(), false, true,
+                                      unserializeObj);
+          m_data.vec->vals()[m_data.vec->m_size++] = val;
         }
       } else {
         m_data.map = ImmutableMap::Create(arr, unserializeObj);
@@ -244,6 +247,88 @@ SharedVariant::~SharedVariant() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+HOT_FUNC
+int SharedVariant::getIndex(const StringData* key) {
+  assert(is(KindOfArray));
+  if (getIsVector()) return -1;
+  return m_data.map->indexOf(key);
+}
+
+int SharedVariant::getIndex(int64_t key) {
+  assert(is(KindOfArray));
+  if (getIsVector()) {
+    if (key < 0 || (size_t) key >= m_data.vec->m_size) return -1;
+    return key;
+  }
+  return m_data.map->indexOf(key);
+}
+
+Variant SharedVariant::getKey(ssize_t pos) const {
+  assert(is(KindOfArray));
+  if (getIsVector()) {
+    assert(pos < (ssize_t) m_data.vec->m_size);
+    return pos;
+  }
+  return m_data.map->getKeyIndex(pos)->toLocal();
+}
+
+HOT_FUNC
+SharedVariant* SharedVariant::getValue(ssize_t pos) const {
+  assert(is(KindOfArray));
+  if (getIsVector()) {
+    assert(pos < (ssize_t) m_data.vec->m_size);
+    return m_data.vec->vals()[pos];
+  }
+  return m_data.map->getValIndex(pos);
+}
+
+ArrayData* SharedVariant::loadElems(const SharedMap &sharedMap,
+                                    bool mapInit /* = false */) {
+  assert(is(KindOfArray));
+  uint count = arrSize();
+  bool isVector = getIsVector();
+
+  auto ai =
+    mapInit ? ArrayInit(count, ArrayInit::mapInit) :
+    isVector ? ArrayInit(count, ArrayInit::vectorInit) :
+    ArrayInit(count);
+
+  if (isVector) {
+    for (uint i = 0; i < count; i++) {
+      ai.set(sharedMap.getValueRef(i));
+    }
+  } else {
+    for (uint i = 0; i < count; i++) {
+      ai.add(m_data.map->getKeyIndex(i)->toLocal(), sharedMap.getValueRef(i),
+             true);
+    }
+  }
+  ArrayData* elems = ai.create();
+  if (elems->isStatic()) elems = elems->copy();
+  return elems;
+}
+
+int SharedVariant::countReachable() const {
+  int count = 1;
+  if (getType() == KindOfArray) {
+    int size = arrSize();
+    if (!getIsVector()) {
+      count += size; // for keys
+    }
+    for (int i = 0; i < size; i++) {
+      SharedVariant* p = getValue(i);
+      count += p->countReachable(); // for values
+    }
+  }
+  return count;
+}
+
+SharedVariant *SharedVariant::Create
+(CVarRef source, bool serialized, bool inner /* = false */,
+ bool unserializeObj /* = false*/) {
+  return new SharedVariant(source, serialized, inner, unserializeObj);
+}
+
 SharedVariant* SharedVariant::convertObj(CVarRef var) {
   if (!var.is(KindOfObject) || getObjAttempted()) {
     return nullptr;
@@ -283,14 +368,14 @@ int32_t SharedVariant::getSpaceUsage() const {
       size += sizeof(VectorData) +
               sizeof(SharedVariant*) * m_data.vec->m_size;
       for (size_t i = 0; i < m_data.vec->m_size; i++) {
-        size += m_data.vec->getValue(i)->getSpaceUsage();
+        size += m_data.vec->vals()[i]->getSpaceUsage();
       }
     } else {
       ImmutableMap *map = m_data.map;
       size += map->getStructSize();
       for (int i = 0; i < map->size(); i++) {
-        size += sizeof(int64_t);
-        size += map->getValue(i)->getSpaceUsage();
+        size += map->getKeyIndex(i)->getSpaceUsage();
+        size += map->getValIndex(i)->getSpaceUsage();
       }
     }
     break;
@@ -337,7 +422,7 @@ void SharedVariant::getStats(SharedVariantStats *stats) const {
       stats->dataTotalSize = sizeof(SharedVariant) + sizeof(VectorData);
       stats->dataTotalSize += sizeof(SharedVariant*) * m_data.vec->m_size;
       for (size_t i = 0; i < m_data.vec->m_size; i++) {
-        SharedVariant *v = m_data.vec->getValue(i);
+        SharedVariant *v = m_data.vec->vals()[i];
         SharedVariantStats childStats;
         v->getStats(&childStats);
         stats->addChildStats(&childStats);
@@ -347,10 +432,9 @@ void SharedVariant::getStats(SharedVariantStats *stats) const {
       stats->dataTotalSize = sizeof(SharedVariant) + map->getStructSize();
       for (int i = 0; i < map->size(); i++) {
         SharedVariantStats childStats;
-        // for key
-        childStats.dataSize = childStats.dataTotalSize = sizeof(int64_t);
+        map->getKeyIndex(i)->getStats(&childStats);
         stats->addChildStats(&childStats);
-        map->getValue(i)->getStats(&childStats);
+        map->getValIndex(i)->getStats(&childStats);
         stats->addChildStats(&childStats);
       }
     }

hphp/runtime/base/shared_variant.h

@@ -52,6 +52,11 @@ public:
                 bool unserializeObj = false);
   ~SharedVariant();
 
+  // Create will do the wrapped check before creating a SharedVariant
+  static SharedVariant* Create(CVarRef source, bool serialized,
+                               bool inner = false,
+                               bool unserializeObj = false);
+
   bool is(DataType d) const { return m_type == d; }
   DataType getType() const { return (DataType)m_type; }
   CVarRef asCVarRef() const {
@@ -83,6 +88,28 @@ public:
     return m_data.str->hash();
   }
 
+  size_t arrSize() const {
+    assert(is(KindOfArray));
+    if (getIsVector()) return m_data.vec->m_size;
+    return m_data.map->size();
+  }
+
+  size_t arrCap() const {
+    assert(is(KindOfArray));
+    if (getIsVector()) return m_data.vec->m_size;
+    return m_data.map->capacity();
+  }
+
+  int getIndex(int64_t key);
+  int getIndex(const StringData* key);
+
+  ArrayData* loadElems(const SharedMap &sharedMap,
+                       bool mapInit = false);
+
+  Variant getKey(ssize_t pos) const;
+
+  SharedVariant* getValue(ssize_t pos) const;
+
   // implementing LeakDetectable
   void dump(std::string &out);
 
@@ -97,7 +124,34 @@ public:
   SharedVariant *convertObj(CVarRef var);
   bool isUnserializedObj() { return getIsObj(); }
 
+  int countReachable() const;
+
 private:
+  class VectorData {
+  public:
+    union {
+      size_t m_size;
+      SharedVariant* m_align_dummy;
+    };
+
+    VectorData() : m_size(0) {}
+
+    ~VectorData() {
+      SharedVariant** v = vals();
+      for (size_t i = 0; i < m_size; i++) {
+        delete v[i];
+      }
+    }
+    SharedVariant** vals() { return (SharedVariant**)(this + 1); }
+    void *operator new(size_t sz, int num) {
+      assert(sz == sizeof(VectorData));
+      return malloc(sizeof(VectorData) + num * sizeof(SharedVariant*));
+    }
+    void operator delete(void* ptr) { free(ptr); }
+    // just to keep the compiler happy; used if the constructor throws
+    void operator delete(void* ptr, int num) { free(ptr); }
+  };
+
   /*
    * 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
@@ -159,6 +213,7 @@ private:
   void setSerializedArray() { m_flags |= SerializedArray;}
   void clearSerializedArray() { m_flags &= ~SerializedArray;}
 
+  bool getIsVector() const { return (bool)(m_flags & IsVector);}
   void setIsVector() { m_flags |= IsVector;}
   void clearIsVector() { m_flags &= ~IsVector;}
 
@@ -169,44 +224,6 @@ private:
   bool getObjAttempted() const { return (bool)(m_flags & ObjAttempted);}
   void setObjAttempted() { m_flags |= ObjAttempted;}
   void clearObjAttempted() { m_flags &= ~ObjAttempted;}
-
-public:
-  bool getIsVector() const { return (bool)(m_flags & IsVector);}
-  ImmutableMap* getMap() const { return m_data.map; }
-};
-
-/* VectorData is used when when all the keys are integers
- * in the sequential range 0 to n-1. */
-class VectorData {
-public:
-  union {
-    size_t m_size;
-    SharedVariant* m_align_dummy;
-  };
-
-  VectorData() : m_size(0) {}
-
-  ~VectorData() {
-    SharedVariant* v = vals();
-    for (size_t i = 0; i < m_size; i++) {
-      v[i].~SharedVariant();
-    }
-  }
-  SharedVariant* getValue(ssize_t pos) {
-    return &((SharedVariant *)(this + 1))[pos];
-  }
-  SharedVariant* vals() { return (SharedVariant*)(this + 1); }
-  void *operator new(size_t sz, int num) {
-    assert(sz == sizeof(VectorData));
-    return malloc(sizeof(VectorData) + num * sizeof(SharedVariant));
-  }
-  void add(CVarRef val, bool unserializeObj) {
-    /* placement new */
-    new (&vals()[m_size++]) SharedVariant(val, false, true, unserializeObj);
-  }
-  void operator delete(void* ptr) { free(ptr); }
-  // just to keep the compiler happy; used if the constructor throws
-  void operator delete(void* ptr, int num) { free(ptr); }
 };
 
 class SharedVariantStats {