SkTArray movable and swap for move only elements.

include/private/SkTArray.h

Index: include/private/SkTArray.h
diff --git a/include/private/SkTArray.h b/include/private/SkTArray.h
index d12ab1af05e54c8cf98674c7ca45b625a2923846..043784f093a2169ce5d640ea11c020eee2d8d1ac 100644
--- a/include/private/SkTArray.h
+++ b/include/private/SkTArray.h
@@ -38,14 +38,20 @@ public:
      * elements.
      */
     explicit SkTArray(int reserveCount) {
-        this->init(NULL, 0, NULL, reserveCount);
+        this->init(0, NULL, reserveCount);
     }
 
     /**
      * Copies one array to another. The new array will be heap allocated.
      */
-    explicit SkTArray(const SkTArray& array) {
-        this->init(array.fItemArray, array.fCount, NULL, 0);
+    explicit SkTArray(const SkTArray& that) {
+        this->init(that.fCount, NULL, 0);
+        this->copy(that.fItemArray);
+    }
+    explicit SkTArray(SkTArray&& that) {
+        this->init(that.fCount, NULL, 0);
+        that.move(fMemArray);
+        that.fCount = 0;
     }
 
     /**
@@ -54,20 +60,32 @@ public:
      * when you really want the (void*, int) version.
      */
     SkTArray(const T* array, int count) {
-        this->init(array, count, NULL, 0);
+        this->init(count, NULL, 0);
+        this->copy(array);
     }
 
     /**
      * assign copy of array to this
      */
-    SkTArray& operator =(const SkTArray& array) {
+    SkTArray& operator =(const SkTArray& that) {
+        for (int i = 0; i < fCount; ++i) {
+            fItemArray[i].~T();
+        }
+        fCount = 0;
+        this->checkRealloc(that.count());
+        fCount = that.count();
+        this->copy(that.fItemArray);
+        return *this;
+    }
+    SkTArray& operator =(SkTArray&& that) {
         for (int i = 0; i < fCount; ++i) {
             fItemArray[i].~T();
         }
         fCount = 0;
-        this->checkRealloc((int)array.count());
-        fCount = array.count();
-        this->copy(static_cast<const T*>(array.fMemArray));
+        this->checkRealloc(that.count());
+        fCount = that.count();
+        that.move(fMemArray);
+        that.fCount = 0;
         return *this;
     }
 
@@ -93,7 +111,7 @@ public:
         for (int i = 0; i < fCount; ++i) {
             fItemArray[i].~T();
         }
-        // set fCount to 0 before calling checkRealloc so that no copy cons. are called.
+        // Set fCount to 0 before calling checkRealloc so that no elements are moved.
         fCount = 0;
         this->checkRealloc(n);
         fCount = n;
@@ -109,8 +127,8 @@ public:
         for (int i = 0; i < fCount; ++i) {
             fItemArray[i].~T();
         }
-        int delta = count - fCount;
-        this->checkRealloc(delta);
+        fCount = 0;
+        this->checkRealloc(count);
         fCount = count;
         this->copy(array);
     }
@@ -264,9 +282,9 @@ public:
             SkTSwap(fAllocCount, that->fAllocCount);
         } else {
             // This could be more optimal...

[mtklein, 2016/04/20 15:50:44]

Still true?

[bungeman-skia, 2016/04/20 17:21:12]

It could be. We could avoid dome destructor/constructor calls on the same memory
by doing more moving. Also, if only one is using the pre-allocated array, it
would be better to detach from the one that isn't, move from the pre-allocated
to the non, then move from the detached to the pre-allocated. I think this means
one less set of moves.

-            SkTArray copy(*that);
-            *that = *this;
-            *this = copy;
+            SkTArray copy(std::move(*that));
+            *that = std::move(*this);
+            *this = std::move(copy);
         }
     }
 
@@ -351,7 +369,7 @@ protected:
      */
     template <int N>
     SkTArray(SkAlignedSTStorage<N,T>* storage) {
-        this->init(NULL, 0, storage->get(), N);
+        this->init(0, storage->get(), N);
     }
 
     /**
@@ -361,7 +379,8 @@ protected:
      */
     template <int N>
     SkTArray(const SkTArray& array, SkAlignedSTStorage<N,T>* storage) {
-        this->init(array.fItemArray, array.fCount, storage->get(), N);
+        this->init(array.fCount, storage->get(), N);
+        this->copy(array.fItemArray);
     }
 
     /**
@@ -371,11 +390,11 @@ protected:
      */
     template <int N>
     SkTArray(const T* array, int count, SkAlignedSTStorage<N,T>* storage) {
-        this->init(array, count, storage->get(), N);
+        this->init(count, storage->get(), N);
+        this->copy(array);
     }
 
-    void init(const T* array, int count,
-              void* preAllocStorage, int preAllocOrReserveCount) {
+    void init(int count, void* preAllocStorage, int preAllocOrReserveCount) {
         SkASSERT(count >= 0);
         SkASSERT(preAllocOrReserveCount >= 0);
         fCount              = count;
@@ -391,8 +410,6 @@ protected:
             fAllocCount = SkMax32(fCount, fReserveCount);
             fMemArray = sk_malloc_throw(fAllocCount * sizeof(T));
         }
-
-        this->copy(array);
     }
 
 private:
@@ -405,7 +422,7 @@ private:
     template <bool E = MEM_COPY> SK_WHEN(E, void) move(int dst, int src) {
         memcpy(&fItemArray[dst], &fItemArray[src], sizeof(T));
     }
-    template <bool E = MEM_COPY> SK_WHEN(E, void) move(char* dst) {
+    template <bool E = MEM_COPY> SK_WHEN(E, void) move(void* dst) {
         sk_careful_memcpy(dst, fMemArray, fCount * sizeof(T));
     }
 
@@ -418,9 +435,9 @@ private:
         new (&fItemArray[dst]) T(std::move(fItemArray[src]));
         fItemArray[src].~T();
     }
-    template <bool E = MEM_COPY> SK_WHEN(!E, void) move(char* dst) {
+    template <bool E = MEM_COPY> SK_WHEN(!E, void) move(void* dst) {
         for (int i = 0; i < fCount; ++i) {
-            new (dst + sizeof(T) * i) T(std::move(fItemArray[i]));
+            new (static_cast<char*>(dst) + sizeof(T) * i) T(std::move(fItemArray[i]));
             fItemArray[i].~T();
         }
     }
@@ -453,12 +470,12 @@ private:
         if (newAllocCount != fAllocCount) {
 
             fAllocCount = newAllocCount;
-            char* newMemArray;
+            void* newMemArray;
 
             if (fAllocCount == fReserveCount && fPreAllocMemArray) {
-                newMemArray = (char*) fPreAllocMemArray;
+                newMemArray = fPreAllocMemArray;
             } else {
-                newMemArray = (char*) sk_malloc_throw(fAllocCount*sizeof(T));
+                newMemArray = sk_malloc_throw(fAllocCount*sizeof(T));
             }
 
             this->move(newMemArray);