[Bf-blender-cvs] [147863ac7cf] functions: Improve copy and move constructor of Vector

Jacques Lucke noreply at git.blender.org
Tue Sep 10 12:10:51 CEST 2019


Commit: 147863ac7cfbc9aedce8f59bab40a32c615e3d22
Author: Jacques Lucke
Date:   Tue Sep 10 11:54:03 2019 +0200
Branches: functions
https://developer.blender.org/rB147863ac7cfbc9aedce8f59bab40a32c615e3d22

Improve copy and move constructor of Vector

===================================================================

M	source/blender/blenlib/BLI_vector.hpp
M	tests/gtests/blenlib/BLI_vector_test.cc

===================================================================

diff --git a/source/blender/blenlib/BLI_vector.hpp b/source/blender/blenlib/BLI_vector.hpp
index 4c32f2c9746..1d3f10b635f 100644
--- a/source/blender/blenlib/BLI_vector.hpp
+++ b/source/blender/blenlib/BLI_vector.hpp
@@ -51,6 +51,8 @@ template<typename T, uint N = 4, typename Allocator = GuardedAllocator> class Ve
   Allocator m_allocator;
   char m_small_buffer[sizeof(T) * N];
 
+  template<typename OtherT, uint OtherN, typename OtherAllocator> friend class Vector;
+
  public:
   /**
    * Create an empty vector.
@@ -134,9 +136,15 @@ template<typename T, uint N = 4, typename Allocator = GuardedAllocator> class Ve
    * The other vector will not be changed.
    * If the other vector has less than N elements, no allocation will be made.
    */
-  Vector(const Vector &other)
+  Vector(const Vector &other) : m_allocator(other.m_allocator)
+  {
+    this->init_copy_from_other_vector(other);
+  }
+
+  template<uint OtherN>
+  Vector(const Vector<T, OtherN, Allocator> &other) : m_allocator(other.m_allocator)
   {
-    this->copy_from_other(other);
+    this->init_copy_from_other_vector(other);
   }
 
   /**
@@ -144,14 +152,47 @@ template<typename T, uint N = 4, typename Allocator = GuardedAllocator> class Ve
    * This does not do an allocation.
    * The other vector will have zero elements afterwards.
    */
-  Vector(Vector &&other)
+  template<uint OtherN>
+  Vector(Vector<T, OtherN, Allocator> &&other) : m_allocator(other.m_allocator)
   {
-    this->steal_from_other(other);
+    uint size = other.size();
+
+    if (other.is_small()) {
+      if (size <= N) {
+        /* Copy between inline buffers. */
+        m_begin = this->small_buffer();
+        m_end = m_begin + size;
+        m_capacity_end = m_begin + N;
+        uninitialized_relocate_n(other.m_begin, size, m_begin);
+      }
+      else {
+        /* Copy from inline buffer to newly allocated buffer. */
+        uint capacity = size;
+        m_begin = (T *)m_allocator.allocate_aligned(
+            sizeof(T) * capacity, std::alignment_of<T>::value, __func__);
+        m_end = m_begin + size;
+        m_capacity_end = m_begin + capacity;
+        uninitialized_relocate_n(other.m_begin, size, m_begin);
+      }
+    }
+    else {
+      /* Steal the pointer. */
+      m_begin = other.m_begin;
+      m_end = other.m_end;
+      m_capacity_end = other.m_capacity_end;
+    }
+
+    other.m_begin = other.small_buffer();
+    other.m_end = other.m_begin;
+    other.m_capacity_end = other.m_begin + OtherN;
   }
 
   ~Vector()
   {
-    this->destruct_elements_and_free_memory();
+    destruct_n(m_begin, this->size());
+    if (!this->is_small()) {
+      m_allocator.deallocate(m_begin);
+    }
   }
 
   operator ArrayRef<T>() const
@@ -170,8 +211,8 @@ template<typename T, uint N = 4, typename Allocator = GuardedAllocator> class Ve
       return *this;
     }
 
-    this->destruct_elements_and_free_memory();
-    this->copy_from_other(other);
+    this->~Vector();
+    new (this) Vector(other);
 
     return *this;
   }
@@ -182,8 +223,8 @@ template<typename T, uint N = 4, typename Allocator = GuardedAllocator> class Ve
       return *this;
     }
 
-    this->destruct_elements_and_free_memory();
-    this->steal_from_other(other);
+    this->~Vector();
+    new (this) Vector(std::move(other));
 
     return *this;
   }
@@ -214,7 +255,11 @@ template<typename T, uint N = 4, typename Allocator = GuardedAllocator> class Ve
    */
   void clear_and_make_small()
   {
-    this->destruct_elements_and_free_memory();
+    destruct_n(m_begin, this->size());
+    if (!this->is_small()) {
+      m_allocator.deallocate(m_begin);
+    }
+
     m_begin = this->small_buffer();
     m_end = m_begin;
     m_capacity_end = m_begin + N;
@@ -502,46 +547,30 @@ template<typename T, uint N = 4, typename Allocator = GuardedAllocator> class Ve
     m_capacity_end = m_begin + min_capacity;
   }
 
-  void copy_from_other(const Vector &other)
+  /**
+   * Initialize all properties, except for m_allocator, which has to be initialized beforehand.
+   */
+  template<uint OtherN> void init_copy_from_other_vector(const Vector<T, OtherN, Allocator> &other)
   {
+    m_allocator = other.m_allocator;
+
     uint size = other.size();
-    if (other.is_small()) {
+    uint capacity = size;
+
+    if (size <= N) {
       m_begin = this->small_buffer();
+      capacity = N;
     }
     else {
       m_begin = (T *)m_allocator.allocate_aligned(
           sizeof(T) * size, std::alignment_of<T>::value, __func__);
+      capacity = size;
     }
 
-    BLI::uninitialized_copy(other.begin(), other.end(), m_begin);
     m_end = m_begin + size;
-    m_capacity_end = m_end;
-  }
-
-  void steal_from_other(Vector &other)
-  {
-    if (other.is_small()) {
-      uninitialized_relocate_n(other.begin(), other.size(), this->small_buffer());
-      m_begin = this->small_buffer();
-    }
-    else {
-      m_begin = other.m_begin;
-    }
-
-    m_end = m_begin + other.size();
-    m_capacity_end = m_begin + other.capacity();
-
-    other.m_begin = other.small_buffer();
-    other.m_end = other.m_begin;
-    other.m_capacity_end = other.m_begin + N;
-  }
+    m_capacity_end = m_begin + capacity;
 
-  void destruct_elements_and_free_memory()
-  {
-    destruct_n(m_begin, this->size());
-    if (!this->is_small()) {
-      m_allocator.deallocate(m_begin);
-    }
+    uninitialized_copy(other.begin(), other.end(), m_begin);
   }
 };
 
diff --git a/tests/gtests/blenlib/BLI_vector_test.cc b/tests/gtests/blenlib/BLI_vector_test.cc
index ba64959235e..803e134fc70 100644
--- a/tests/gtests/blenlib/BLI_vector_test.cc
+++ b/tests/gtests/blenlib/BLI_vector_test.cc
@@ -97,6 +97,88 @@ TEST(vector, CopyConstructor)
   EXPECT_EQ(vec2[1], 2);
 }
 
+TEST(vector, CopyConstructor2)
+{
+  Vector<int, 2> vec1 = {1, 2, 3, 4};
+  Vector<int, 3> vec2(vec1);
+
+  EXPECT_EQ(vec1.size(), 4);
+  EXPECT_EQ(vec2.size(), 4);
+  EXPECT_NE(vec1.begin(), vec2.begin());
+  EXPECT_EQ(vec2[0], 1);
+  EXPECT_EQ(vec2[1], 2);
+  EXPECT_EQ(vec2[2], 3);
+  EXPECT_EQ(vec2[3], 4);
+}
+
+TEST(vector, CopyConstructor3)
+{
+  Vector<int, 20> vec1 = {1, 2, 3, 4};
+  Vector<int, 1> vec2(vec1);
+
+  EXPECT_EQ(vec1.size(), 4);
+  EXPECT_EQ(vec2.size(), 4);
+  EXPECT_NE(vec1.begin(), vec2.begin());
+  EXPECT_EQ(vec2[2], 3);
+}
+
+TEST(vector, CopyConstructor4)
+{
+  Vector<int, 5> vec1 = {1, 2, 3, 4};
+  Vector<int, 6> vec2(vec1);
+
+  EXPECT_EQ(vec1.size(), 4);
+  EXPECT_EQ(vec2.size(), 4);
+  EXPECT_NE(vec1.begin(), vec2.begin());
+  EXPECT_EQ(vec2[3], 4);
+}
+
+TEST(vector, MoveConstructor)
+{
+  IntVector vec1 = {1, 2, 3, 4};
+  IntVector vec2(std::move(vec1));
+
+  EXPECT_EQ(vec1.size(), 0);
+  EXPECT_EQ(vec2.size(), 4);
+  EXPECT_EQ(vec2[0], 1);
+  EXPECT_EQ(vec2[1], 2);
+  EXPECT_EQ(vec2[2], 3);
+  EXPECT_EQ(vec2[3], 4);
+}
+
+TEST(vector, MoveConstructor2)
+{
+  Vector<int, 2> vec1 = {1, 2, 3, 4};
+  Vector<int, 3> vec2(std::move(vec1));
+
+  EXPECT_EQ(vec1.size(), 0);
+  EXPECT_EQ(vec2.size(), 4);
+  EXPECT_EQ(vec2[0], 1);
+  EXPECT_EQ(vec2[1], 2);
+  EXPECT_EQ(vec2[2], 3);
+  EXPECT_EQ(vec2[3], 4);
+}
+
+TEST(vector, MoveConstructor3)
+{
+  Vector<int, 20> vec1 = {1, 2, 3, 4};
+  Vector<int, 1> vec2(std::move(vec1));
+
+  EXPECT_EQ(vec1.size(), 0);
+  EXPECT_EQ(vec2.size(), 4);
+  EXPECT_EQ(vec2[2], 3);
+}
+
+TEST(vector, MoveConstructor4)
+{
+  Vector<int, 5> vec1 = {1, 2, 3, 4};
+  Vector<int, 6> vec2(std::move(vec1));
+
+  EXPECT_EQ(vec1.size(), 0);
+  EXPECT_EQ(vec2.size(), 4);
+  EXPECT_EQ(vec2[3], 4);
+}
+
 TEST(vector, MoveAssignment)
 {
   IntVector vec = {1, 2};



More information about the Bf-blender-cvs mailing list