Bug 1562789 - SmallPointerArray should support moves, and have an IsEmpty() helper. r=froydnj
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 02 Jul 2019 18:50:04 +0000
changeset 480995 6525ac41c35455512bc1b681df331a5b74cb75cc
parent 480994 e1cc6ea96d6cb75b33520f81e06bfedc2f0c99bd
child 480996 1f407c83db16381332d82bcfa992eea1022284ac
push id36230
push useraciure@mozilla.com
push dateWed, 03 Jul 2019 04:09:04 +0000
treeherdermozilla-central@42a9ef2a777f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1562789
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1562789 - SmallPointerArray should support moves, and have an IsEmpty() helper. r=froydnj This also implicitly deletes its copy-assignment operator and copy-constructor, which is great since it's a huge footgun. Differential Revision: https://phabricator.services.mozilla.com/D36549
mfbt/SmallPointerArray.h
mfbt/tests/TestSmallPointerArray.cpp
--- a/mfbt/SmallPointerArray.h
+++ b/mfbt/SmallPointerArray.h
@@ -5,26 +5,27 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* A vector of pointers space-optimized for a small number of elements. */
 
 #ifndef mozilla_SmallPointerArray_h
 #define mozilla_SmallPointerArray_h
 
 #include "mozilla/Assertions.h"
+#include "mozilla/PodOperations.h"
 
 #include <algorithm>
 #include <iterator>
 #include <new>
 #include <vector>
 
 namespace mozilla {
 
 // Array class for situations where a small number of NON-NULL elements (<= 2)
-// is expected, a large number of elements must be accomodated if necessary,
+// is expected, a large number of elements must be accommodated if necessary,
 // and the size of the class must be minimal. Typical vector implementations
 // will fulfill the first two requirements by simply adding inline storage
 // alongside the rest of their member variables. While this strategy works,
 // it brings unnecessary storage overhead for vectors with an expected small
 // number of elements. This class is intended to deal with that problem.
 //
 // This class is similar in performance to a vector class. Accessing its
 // elements when it has not grown over a size of 2 does not require an extra
@@ -44,16 +45,27 @@ class SmallPointerArray {
   }
 
   ~SmallPointerArray() {
     if (!first()) {
       delete maybeVector();
     }
   }
 
+  SmallPointerArray(SmallPointerArray&& aOther) {
+    PodCopy(mArray, aOther.mArray, 2);
+    aOther.mArray[0].mValue = nullptr;
+    aOther.mArray[1].mVector = nullptr;
+  }
+
+  SmallPointerArray& operator=(SmallPointerArray&& aOther) {
+    std::swap(mArray, aOther.mArray);
+    return *this;
+  }
+
   void Clear() {
     if (first()) {
       first() = nullptr;
       new (&mArray[1].mValue) std::vector<T*>*(nullptr);
       return;
     }
 
     delete maybeVector();
@@ -155,16 +167,18 @@ class SmallPointerArray {
 
     if (auto* vec = maybeVector()) {
       return vec->size();
     }
 
     return 0;
   }
 
+  bool IsEmpty() const { return Length() == 0; }
+
   T* ElementAt(size_t aIndex) const {
     MOZ_ASSERT(aIndex < Length());
     if (first()) {
       return mArray[aIndex].mValue;
     }
 
     auto* vec = maybeVector();
     MOZ_ASSERT(vec, "must have backing vector if accessing an element");
--- a/mfbt/tests/TestSmallPointerArray.cpp
+++ b/mfbt/tests/TestSmallPointerArray.cpp
@@ -187,13 +187,51 @@ void TestRangeBasedLoops() {
 
   for (void* test : testArray) {
     verification[entries++] = test;
   }
 
   MOZ_RELEASE_ASSERT(entries == 0);
 }
 
+void TestMove() {
+  using namespace mozilla;
+
+  SmallPointerArray<void> testArray;
+  testArray.AppendElement(PTR1);
+  testArray.AppendElement(PTR2);
+
+  SmallPointerArray<void> moved = std::move(testArray);
+
+  MOZ_RELEASE_ASSERT(testArray.IsEmpty());
+  MOZ_RELEASE_ASSERT(moved.Length() == 2);
+  MOZ_RELEASE_ASSERT(moved[0] == PTR1);
+  MOZ_RELEASE_ASSERT(moved[1] == PTR2);
+
+  // Heap case.
+  moved.AppendElement(PTR3);
+
+  SmallPointerArray<void> another = std::move(moved);
+
+  MOZ_RELEASE_ASSERT(testArray.IsEmpty());
+  MOZ_RELEASE_ASSERT(moved.IsEmpty());
+  MOZ_RELEASE_ASSERT(another.Length() == 3);
+  MOZ_RELEASE_ASSERT(another[0] == PTR1);
+  MOZ_RELEASE_ASSERT(another[1] == PTR2);
+  MOZ_RELEASE_ASSERT(another[2] == PTR3);
+
+  // Move assignment.
+  testArray = std::move(another);
+
+  MOZ_RELEASE_ASSERT(moved.IsEmpty());
+  MOZ_RELEASE_ASSERT(another.IsEmpty());
+  MOZ_RELEASE_ASSERT(testArray.Length() == 3);
+  MOZ_RELEASE_ASSERT(testArray[0] == PTR1);
+  MOZ_RELEASE_ASSERT(testArray[1] == PTR2);
+  MOZ_RELEASE_ASSERT(testArray[2] == PTR3);
+}
+
 int main() {
   TestArrayManipulation();
   TestRangeBasedLoops();
+  TestMove();
   return 0;
 }