Update array_unshift to support Vector and Set

This diff updates the implementation of array_unshift to support Vector
and Set for the first parameter. A very simple scheme was used to make
Vector and Set support adding elements to the front similar to what
HphpArray currently does. I leave improving the data structures' perf
when adding elements to the front to a subsequent diff.

Along the way, I fixed the behavior of array_unshift() to match PHP 5.5
in the case where the first parameter is not an array, Vector, or Set.

Reviewed By: @elgenie

Differential Revision: D1139932

hphp/runtime/ext/ext_array.cpp

@@ -711,43 +711,84 @@ Variant f_array_sum(CVarRef array) {
   }
 }
 
-int64_t f_array_unshift(int _argc, VRefParam array, CVarRef var, CArrRef _argv /* = null_array */) {
-  if (array.toArray()->isVectorData()) {
-    if (!_argv.empty()) {
-      for (ssize_t pos = _argv->iter_end(); pos != ArrayData::invalid_index;
-        pos = _argv->iter_rewind(pos)) {
-        array.prepend(_argv->getValueRef(pos));
-      }
-    }
-    array.prepend(var);
-  } else {
-    {
-      Array newArray;
-      newArray.append(var);
+Variant f_array_unshift(int _argc, VRefParam array, CVarRef var, CArrRef _argv /* = null_array */) {
+  const auto* cell_array = array->asCell();
+  if (UNLIKELY(!isContainer(*cell_array))) {
+    raise_warning("%s() expects parameter 1 to be an array, Vector, or Set",
+                  __FUNCTION__+2 /* remove the "f_" prefix */);
+    return uninit_null();
+  }
+  if (cell_array->m_type == KindOfArray) {
+    if (array.toArray()->isVectorData()) {
       if (!_argv.empty()) {
-        for (ssize_t pos = _argv->iter_begin();
-             pos != ArrayData::invalid_index;
-             pos = _argv->iter_advance(pos)) {
-          newArray.append(_argv->getValueRef(pos));
+        for (ssize_t pos = _argv->iter_end(); pos != ArrayData::invalid_index;
+          pos = _argv->iter_rewind(pos)) {
+          array.prepend(_argv->getValueRef(pos));
         }
       }
-      for (ArrayIter iter(array.toArray()); iter; ++iter) {
-        Variant key(iter.first());
-        CVarRef value(iter.secondRef());
-        if (key.isInteger()) {
-          newArray.appendWithRef(value);
-        } else {
-          newArray.setWithRef(key, value, true);
+      array.prepend(var);
+    } else {
+      {
+        Array newArray;
+        newArray.append(var);
+        if (!_argv.empty()) {
+          for (ssize_t pos = _argv->iter_begin();
+               pos != ArrayData::invalid_index;
+               pos = _argv->iter_advance(pos)) {
+            newArray.append(_argv->getValueRef(pos));
+          }
         }
+        for (ArrayIter iter(array.toArray()); iter; ++iter) {
+          Variant key(iter.first());
+          CVarRef value(iter.secondRef());
+          if (key.isInteger()) {
+            newArray.appendWithRef(value);
+          } else {
+            newArray.setWithRef(key, value, true);
+          }
+        }
+        array = newArray;
+      }
+      // Reset the array's internal pointer
+      if (array.is(KindOfArray)) {
+        f_reset(array);
       }
-      array = newArray;
     }
-    // Reset the array's internal pointer
-    if (array.is(KindOfArray)) {
-      f_reset(array);
+    return array.toArray().size();
+  }
+  // Handle collections
+  assert(cell_array->m_type == KindOfObject);
+  auto* obj = cell_array->m_data.pobj;
+  assert(obj->isCollection());
+  switch (obj->getCollectionType()) {
+    case Collection::VectorType: {
+      auto* vec = static_cast<c_Vector*>(obj);
+      if (!_argv.empty()) {
+        for (ssize_t pos = _argv->iter_end(); pos != ArrayData::invalid_index;
+             pos = _argv->iter_rewind(pos)) {
+          vec->addFront(cvarToCell(&_argv->getValueRef(pos)));
+        }
+      }
+      vec->addFront(cvarToCell(&var));
+      return vec->size();
+    }
+    case Collection::SetType: {
+      auto* st = static_cast<c_Set*>(obj);
+      if (!_argv.empty()) {
+        for (ssize_t pos = _argv->iter_end(); pos != ArrayData::invalid_index;
+             pos = _argv->iter_rewind(pos)) {
+          st->addFront(cvarToCell(&_argv->getValueRef(pos)));
+        }
+      }
+      st->addFront(cvarToCell(&var));
+      return st->size();
+    }
+    default: {
+      raise_warning("%s() expects parameter 1 to be an array, Vector, or Set",
+                    __FUNCTION__+2 /* remove the "f_" prefix */);
+      return uninit_null();
     }
   }
-  return array.toArray().size();
 }
 
 Variant f_array_values(CVarRef input) {

hphp/runtime/ext/ext_array.h

@@ -96,7 +96,7 @@ Variant f_array_splice(VRefParam input, int offset,
 Variant f_array_sum(CVarRef array);
 Variant f_array_unique(CVarRef array, int sort_flags = 2);
 
-int64_t f_array_unshift(int _argc, VRefParam array, CVarRef var, CArrRef _argv = null_array);
+Variant f_array_unshift(int _argc, VRefParam array, CVarRef var, CArrRef _argv = null_array);
 
 Variant f_array_values(CVarRef input);
 bool f_array_walk_recursive(VRefParam input, CVarRef funcname,

hphp/runtime/ext/ext_collections.cpp

@@ -430,6 +430,18 @@ void BaseVector::grow() {
   m_data = (TypedValue*)smart_realloc(m_data, m_capacity * sizeof(TypedValue));
 }
 
+void BaseVector::addFront(TypedValue* val) {
+  assert(val->m_type != KindOfRef);
+  ++m_version;
+  mutate();
+  if (m_capacity <= m_size) {
+    grow();
+  }
+  memmove(m_data+1, m_data, m_size * sizeof(TypedValue));
+  cellDup(*val, m_data[0]);
+  ++m_size;
+}
+
 void BaseVector::reserve(int64_t sz) {
   if (sz <= 0) return;
 
@@ -2916,17 +2928,16 @@ void BaseSet::add(int64_t h) {
   assert(p);
   if (validPos(*p)) {
     // When there is a conflict, the add() API is supposed to replace the
-    // existing element with the new element. However since Sets currently
-    // only support integer and string elements, there is no way user code
-    // can really tell whether the existing element was replaced or not so
-    // for efficiency we do nothing.
+    // existing element with the new element in place. However since Sets
+    // currently only support integer and string elements, there is no way
+    // user code can really tell whether the existing element was replaced
+    // so for efficiency we do nothing.
     return;
   }
   if (UNLIKELY(isFull())) {
     makeRoom();
     p = findForInsert(h);
   }
-  assert(p);
   auto& e = allocElm(p);
   e.setInt(h);
   ++m_version;
@@ -2943,12 +2954,68 @@ void BaseSet::add(StringData *key) {
     makeRoom();
     p = findForInsert(key, h);
   }
-  assert(p);
   auto& e = allocElm(p);
   e.setStr(key, h);
   ++m_version;
 }
 
+BaseSet::Elm& BaseSet::allocElmFront(int32_t* ei) {
+  assert(ei && !validPos(*ei) && m_size <= m_used && m_used < m_cap);
+  // Move the existing elements to make element slot 0 available.
+  memmove(data() + 1, data(), m_used * sizeof(Elm));
+  ++m_used;
+  // Update the hashtable to reflect the fact that everything was moved
+  // over one position
+  auto* hash = hashTab();
+  auto* hashEnd = hash + hashSize();
+  for (; hash != hashEnd; ++hash) {
+    if (validPos(*hash)) {
+      ++(*hash);
+    }
+  }
+  // Set the hash entry we found to point to element slot 0.
+  (*ei) = 0;
+  // Store the value into element slot 0.
+  ++m_size;
+  return data()[0];
+}
+
+void BaseSet::addFront(int64_t h) {
+  auto* p = findForInsert(h);
+  assert(p);
+  if (validPos(*p)) {
+    // When there is a conflict, the addFront() API is supposed to replace
+    // the existing element with the new element in place. However since
+    // Sets currently only support integer and string elements, there is
+    // no way user code can really tell whether the existing element was
+    // replaced so for efficiency we do nothing.
+    return;
+  }
+  if (UNLIKELY(isFull())) {
+    makeRoom();
+    p = findForInsert(h);
+  }
+  auto& e = allocElmFront(p);
+  e.setInt(h);
+  ++m_version;
+}
+
+void BaseSet::addFront(StringData *key) {
+  strhash_t h = key->hash();
+  auto* p = findForInsert(key, h);
+  assert(p);
+  if (validPos(*p)) {
+    return;
+  }
+  if (UNLIKELY(isFull())) {
+    makeRoom();
+    p = findForInsert(key, h);
+  }
+  auto& e = allocElmFront(p);
+  e.setStr(key, h);
+  ++m_version;
+}
+
 void BaseSet::throwOOB(int64_t val) {
   throwIntOOB(val);
 }

hphp/runtime/ext/ext_collections.h

@@ -255,6 +255,8 @@ class BaseVector : public ExtCollectionObjectData {
     return offsetof(BaseVector, m_frozenCopy);
   }
 
+  void addFront(TypedValue* val);
+
  protected:
 
   explicit BaseVector(Class* cls);
@@ -784,7 +786,7 @@ class BaseMap : public ExtCollectionObjectData {
   void compactIfNecessary();
 
   BaseMap::Elm& allocElm(int32_t* ei) {
-    assert(!validPos(*ei) && m_size <= m_used && m_used < m_cap);
+    assert(ei && !validPos(*ei) && m_size <= m_used && m_used < m_cap);
     size_t i = m_used;
     (*ei) = i;
     m_used = i + 1;
@@ -1233,7 +1235,7 @@ class BaseSet : public ExtCollectionObjectData {
   void compact();
 
   BaseSet::Elm& allocElm(int32_t* ei) {
-    assert(!validPos(*ei) && m_size <= m_used && m_used < m_cap);
+    assert(ei && !validPos(*ei) && m_size <= m_used && m_used < m_cap);
     size_t i = m_used;
     (*ei) = i;
     m_used = i + 1;
@@ -1241,6 +1243,8 @@ class BaseSet : public ExtCollectionObjectData {
     return data()[i];
   }
 
+  BaseSet::Elm& allocElmFront(int32_t* ei);
+
  public:
   ssize_t iter_begin() const {
     Elm* p = data();
@@ -1301,6 +1305,18 @@ class BaseSet : public ExtCollectionObjectData {
   void add(int64_t h);
   void add(StringData* key);
 
+  void addFront(TypedValue* val) {
+    if (val->m_type == KindOfInt64) {
+      addFront(val->m_data.num);
+    } else if (IS_STRING_TYPE(val->m_type)) {
+      addFront(val->m_data.pstr);
+    } else {
+      throwBadValueType();
+    }
+  }
+  void addFront(int64_t h);
+  void addFront(StringData* key);
+
   void remove(int64_t key) {
     ++m_version;
     auto* p = findForInsert(key);

hphp/system/idl/array.idl.json

@@ -797,7 +797,7 @@
                 "VariableArguments"
             ],
             "return": {
-                "type": "Int64",
+                "type": "Variant",
                 "desc": "Returns the new number of elements in the array."
             },
             "args": [

hphp/test/slow/collection_classes/array-unshift.php

@@ -0,0 +1,30 @@
+<?hh
+function main() {
+  $x1 = array();
+  var_dump(array_unshift($x1, 1));
+  var_dump(array_unshift($x1, 'b', 'c'));
+  var_dump($x1);
+  $x2 = array('d');
+  var_dump(array_unshift($x2, 'e', 'f', 2, 'f'));
+  var_dump($x2);
+  $x3 = Vector {};
+  var_dump(array_unshift($x3, 1));
+  var_dump(array_unshift($x3, 'b', 'c'));
+  var_dump($x3);
+  $x4 = Vector {'d'};
+  var_dump(array_unshift($x4, 'e', 'f', 2, 'f'));
+  var_dump($x4);
+  $x5 = Set {};
+  var_dump(array_unshift($x5, 1));
+  var_dump(array_unshift($x5, 'b', 'c'));
+  var_dump($x5);
+  $x6 = Set {'d'};
+  var_dump(array_unshift($x6, 'e', 'f', 2, 'f'));
+  var_dump($x6);
+  $x7 = Set {};
+  var_dump(array_unshift($x7, 'h', 3, 'j'));
+  var_dump(array_unshift($x7, 3, 'j'));
+  var_dump($x7);
+}
+main();
+

hphp/test/slow/collection_classes/array-unshift.php.expect

@@ -0,0 +1,67 @@
+int(1)
+int(3)
+array(3) {
+  [0]=>
+  string(1) "b"
+  [1]=>
+  string(1) "c"
+  [2]=>
+  int(1)
+}
+int(5)
+array(5) {
+  [0]=>
+  string(1) "e"
+  [1]=>
+  string(1) "f"
+  [2]=>
+  int(2)
+  [3]=>
+  string(1) "f"
+  [4]=>
+  string(1) "d"
+}
+int(1)
+int(3)
+object(HH\Vector)#1 (3) {
+  [0]=>
+  string(1) "b"
+  [1]=>
+  string(1) "c"
+  [2]=>
+  int(1)
+}
+int(5)
+object(HH\Vector)#2 (5) {
+  [0]=>
+  string(1) "e"
+  [1]=>
+  string(1) "f"
+  [2]=>
+  int(2)
+  [3]=>
+  string(1) "f"
+  [4]=>
+  string(1) "d"
+}
+int(1)
+int(3)
+object(HH\Set)#3 (3) {
+  string(1) "b"
+  string(1) "c"
+  int(1)
+}
+int(4)
+object(HH\Set)#4 (4) {
+  string(1) "e"
+  int(2)
+  string(1) "f"
+  string(1) "d"
+}
+int(3)
+int(3)
+object(HH\Set)#5 (3) {
+  string(1) "h"
+  int(3)
+  string(1) "j"
+}