From c84a3b6de3b83cf5d7d0fc35dd8332fedf37bd9e Mon Sep 17 00:00:00 2001 From: whitequark Date: Fri, 6 Nov 2015 14:11:11 +0300 Subject: [PATCH] Call constructors and destructors in List and IdList. This is necessary to store non-POD classes in these containers. Note that List and IdList do not use MemRealloc anymore; this is necessarily (slightly) less efficient, but is the right semantics, as you cannot just move non-POD types, e.g. std::string, around in memory. All STL containers provide the same guarantees and share the performance hit. The slowdown is mostly mitigated by moving the contained objects, so that no additional heap allocations or copies occur beyond that of the object itself. --- src/dsc.h | 42 +++++++++++++++++++++++++++++++++--------- src/solvespace.h | 1 - src/unix/unixutil.cpp | 10 ---------- src/win32/w32util.cpp | 11 +---------- 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/dsc.h b/src/dsc.h index c428435..e5e21fc 100644 --- a/src/dsc.h +++ b/src/dsc.h @@ -152,20 +152,27 @@ public: void AllocForOneMore(void) { if(n >= elemsAllocated) { elemsAllocated = (elemsAllocated + 32)*2; - elem = (T *)MemRealloc(elem, (size_t)elemsAllocated*sizeof(elem[0])); + T *newElem = (T *)MemAlloc((size_t)elemsAllocated*sizeof(elem[0])); + for(int i = 0; i < n; i++) { + new(&newElem[i]) T(std::move(elem[i])); + elem[i].~T(); + } + MemFree(elem); + elem = newElem; } } void Add(T *t) { AllocForOneMore(); - elem[n++] = *t; + new(&elem[n++]) T(*t); } void AddToBeginning(T *t) { AllocForOneMore(); - memmove(elem+1, elem, n*sizeof(elem[0])); - n++; + new(&elem[n]) T(); + std::move_backward(elem, elem + 1, elem + n + 1); elem[0] = *t; + n++; } T *First(void) { @@ -185,6 +192,8 @@ public: } void Clear(void) { + for(int i = 0; i < n; i++) + elem[i].~T(); if(elem) MemFree(elem); elem = NULL; n = elemsAllocated = 0; @@ -203,12 +212,16 @@ public: dest++; } } + for(int i = dest; i < n; i++) + elem[i].~T(); n = dest; // and elemsAllocated is untouched, because we didn't resize } void RemoveLast(int cnt) { if(n < cnt) oops(); + for(int i = n - cnt; i < n; i++) + elem[i].~T(); n -= cnt; // and elemsAllocated is untouched, same as in RemoveTagged } @@ -251,7 +264,13 @@ public: void Add(T *t) { if(n >= elemsAllocated) { elemsAllocated = (elemsAllocated + 32)*2; - elem = (T *)MemRealloc(elem, (size_t)elemsAllocated*sizeof(elem[0])); + T *newElem = (T *)MemAlloc((size_t)elemsAllocated*sizeof(elem[0])); + for(int i = 0; i < n; i++) { + new(&newElem[i]) T(std::move(elem[i])); + elem[i].~T(); + } + MemFree(elem); + elem = newElem; } int first = 0, last = n; @@ -268,9 +287,10 @@ public: oops(); } } - int i = first; - memmove(elem+i+1, elem+i, (size_t)(n-i)*sizeof(elem[0])); + int i = first; + new(&elem[n]) T(); + std::move_backward(elem + i, elem + n, elem + n + 1); elem[i] = *t; n++; } @@ -338,6 +358,8 @@ public: dest++; } } + for(int i = dest; i < n; i++) + elem[i].~T(); n = dest; // and elemsAllocated is untouched, because we didn't resize } @@ -349,7 +371,7 @@ public: void MoveSelfInto(IdList *l) { l->Clear(); - memcpy(l, this, sizeof(*this)); + *l = *this; elemsAllocated = n = 0; elem = NULL; } @@ -357,7 +379,8 @@ public: void DeepCopyInto(IdList *l) { l->Clear(); l->elem = (T *)MemAlloc(elemsAllocated * sizeof(elem[0])); - memcpy(l->elem, elem, elemsAllocated * sizeof(elem[0])); + for(int i = 0; i < n; i++) + new(&l->elem[i]) T(elem[i]); l->elemsAllocated = elemsAllocated; l->n = n; } @@ -365,6 +388,7 @@ public: void Clear(void) { for(int i = 0; i < n; i++) { elem[i].Clear(); + elem[i].~T(); } elemsAllocated = n = 0; if(elem) MemFree(elem); diff --git a/src/solvespace.h b/src/solvespace.h index 60c15e9..aafadab 100644 --- a/src/solvespace.h +++ b/src/solvespace.h @@ -256,7 +256,6 @@ float CnfThawFloat(float v, const char *name); void *AllocTemporary(size_t n); void FreeTemporary(void *p); void FreeAllTemporary(void); -void *MemRealloc(void *p, size_t n); void *MemAlloc(size_t n); void MemFree(void *p); void InitHeaps(void); diff --git a/src/unix/unixutil.cpp b/src/unix/unixutil.cpp index a12dbb7..b52d8c9 100644 --- a/src/unix/unixutil.cpp +++ b/src/unix/unixutil.cpp @@ -95,16 +95,6 @@ void FreeAllTemporary(void) Head = NULL; } -void *MemRealloc(void *p, size_t n) { - if(!p) { - return MemAlloc(n); - } - - p = realloc(p, n); - if(!p) oops(); - return p; -} - void *MemAlloc(size_t n) { void *p = malloc(n); if(!p) oops(); diff --git a/src/win32/w32util.cpp b/src/win32/w32util.cpp index d23345d..35f329e 100644 --- a/src/win32/w32util.cpp +++ b/src/win32/w32util.cpp @@ -53,15 +53,6 @@ void FreeAllTemporary(void) vl(); } -void *MemRealloc(void *p, size_t n) { - if(!p) { - return MemAlloc(n); - } - - p = HeapReAlloc(PermHeap, HEAP_NO_SERIALIZE | HEAP_ZERO_MEMORY, p, n); - if(!p) oops(); - return p; -} void *MemAlloc(size_t n) { void *p = HeapAlloc(PermHeap, HEAP_NO_SERIALIZE | HEAP_ZERO_MEMORY, n); if(!p) oops(); @@ -82,4 +73,4 @@ void InitHeaps(void) { // Create the heap that we use to store Exprs and other temp stuff. FreeAllTemporary(); } -} \ No newline at end of file +}