From 5fa9bd7b4905b462bac0454e2ed29e4535a4c6af Mon Sep 17 00:00:00 2001 From: Eugen Wissner Date: Wed, 18 Jan 2017 09:33:39 +0100 Subject: [PATCH] Rename Allocator.expand to reallocateInPlace Rename and extend Allocator.expand to reallocateInPlace. The problem is that realloc for example doesn't guarante that the shrinking of the memory block won't cause address change. So not only expanding should have "in place" version, but the shrinking as well. --- source/tanya/container/vector.d | 93 ++++++++++++++++++++------------- source/tanya/memory/allocator.d | 7 ++- source/tanya/memory/mmappool.d | 44 ++++++++-------- 3 files changed, 85 insertions(+), 59 deletions(-) diff --git a/source/tanya/container/vector.d b/source/tanya/container/vector.d index 113a9ad..b86028f 100644 --- a/source/tanya/container/vector.d +++ b/source/tanya/container/vector.d @@ -117,7 +117,7 @@ private struct Range(E) in { assert(i <= j); - assert(j < length); + assert(j <= length); } body { @@ -128,7 +128,7 @@ private struct Range(E) in { assert(i <= j); - assert(j < length); + assert(j <= length); } body { @@ -445,7 +445,7 @@ struct Vector(T) assert(!overflow); void[] buf = vector[0 .. capacity_]; - if (!allocator.expand(buf, byteSize)) + if (!allocator.reallocateInPlace(buf, byteSize)) { buf = allocator.allocate(byteSize); if (buf is null) @@ -494,11 +494,16 @@ struct Vector(T) */ void shrink(in size_t size) @trusted { - auto n = max(length, size); + if (capacity_ <= size) + { + return; + } + immutable n = max(length, size); void[] buf = vector[0 .. capacity_]; - allocator.reallocate(buf, n * T.sizeof); - vector = cast(T*) buf; - capacity_ = n; + if (allocator.reallocateInPlace(buf, n * T.sizeof)) + { + capacity_ = n; + } } /// @@ -733,16 +738,22 @@ struct Vector(T) } /// Ditto. - Range!T opIndexAssign()(auto ref T value) + Range!T opIndexAssign(T value) + { + return opSliceAssign(value, 0, length); + } + + /// Ditto. + Range!T opIndexAssign(ref T value) { return opSliceAssign(value, 0, length); } /** - * Assigns a range. The range should have the same length as the vector. + * Assigns a range or a static array. * * Params: - * R = Range type. + * R = Range type or static array length. * value = Value. * * Returns: Assigned value. @@ -750,15 +761,20 @@ struct Vector(T) * Precondition: $(D_INLINECODE length == value.length) */ Range!T opIndexAssign(R)(R value) - if ((!isInfinite!R && isInputRange!R + if (!isInfinite!R && isInputRange!R && isImplicitlyConvertible!(ElementType!R, T)) - || isStaticArray!R && is(ElementType!R == T)) { - return opSliceAssign(value, 0, length); + return opSliceAssign!R(value, 0, length); + } + + /// Ditto. + Range!T opIndexAssign(size_t R)(T[R] value) + { + return opSliceAssign!R(value, 0, length); } /// - unittest + @nogc unittest { auto v1 = Vector!int([12, 1, 7]); @@ -1073,7 +1089,7 @@ struct Vector(T) in { assert(i <= j); - assert(j <= length_); + assert(j <= length); } body { @@ -1128,7 +1144,9 @@ struct Vector(T) * Slicing assignment. * * Params: - * value = New value (single value or input range). + * R = Type of the assigned slice or length of the static array should be + * assigned. + * value = New value (single value, input range or static array). * i = Slice start. * j = Slice end. * @@ -1137,6 +1155,28 @@ struct Vector(T) * Precondition: $(D_INLINECODE i <= j && j <= length * && value.length == j - i) */ + Range!T opSliceAssign(R)(R value, in size_t i, in size_t j) + if (!isInfinite!R && isInputRange!R + && isImplicitlyConvertible!(ElementType!R, T)) + in + { + assert(i <= j); + assert(j <= length); + assert(j - i == walkLength(value)); + } + body + { + copy(value, opSlice(i, j)); + return opSlice(i, j); + } + + /// Ditto. + Range!T opSliceAssign(size_t R)(T[R] value, in size_t i, in size_t j) + { + return opSliceAssign!(T[])(value[], i, j); + } + + /// Ditto. Range!T opSliceAssign(ref T value, in size_t i, in size_t j) in { @@ -1145,7 +1185,7 @@ struct Vector(T) } body { - fill((() @trusted => vector[i .. j])(), value); + fill(opSlice(i, j), value); return opSlice(i, j); } @@ -1155,25 +1195,8 @@ struct Vector(T) return opSliceAssign(value, i, j); } - /// Ditto. - Range!T opSliceAssign(R)(R value, in size_t i, in size_t j) - if ((!isInfinite!R && isInputRange!R - && isImplicitlyConvertible!(ElementType!R, T)) - || isStaticArray!R && is(ElementType!R == T)) - in - { - assert(i <= j); - assert(j <= length); - assert(j - i == walkLength(value)); - } - body - { - copy(value, (() @trusted => vector[i .. j])()); - return opSlice(i, j); - } - /// - @safe unittest + @nogc @safe unittest { auto v1 = Vector!int([3, 3, 3]); auto v2 = Vector!int([1, 2]); diff --git a/source/tanya/memory/allocator.d b/source/tanya/memory/allocator.d index 761125d..930a934 100644 --- a/source/tanya/memory/allocator.d +++ b/source/tanya/memory/allocator.d @@ -52,7 +52,10 @@ interface Allocator bool reallocate(ref void[] p, in size_t size) shared nothrow @nogc; /** - * Expands a memory block in place. + * Reallocates a memory block in place if possible or returns + * $(D_KEYWORD false). This function cannot be used to allocate or + * deallocate memory, so if $(D_PARAM p) is $(D_KEYWORD null) or + * $(D_PARAM size) is `0`, it should return $(D_KEYWORD false). * * Params: * p = A pointer to the memory block. @@ -60,7 +63,7 @@ interface Allocator * * Returns: $(D_KEYWORD true) if successful, $(D_KEYWORD false) otherwise. */ - bool expand(ref void[] p, in size_t size) shared nothrow @nogc; + bool reallocateInPlace(ref void[] p, in size_t size) shared nothrow @nogc; } /** diff --git a/source/tanya/memory/mmappool.d b/source/tanya/memory/mmappool.d index 2bc3cfe..d7a2e5e 100644 --- a/source/tanya/memory/mmappool.d +++ b/source/tanya/memory/mmappool.d @@ -231,7 +231,10 @@ final class MmapPool : Allocator } /** - * Expands a memory block in place. + * Reallocates a memory block in place if possible or returns + * $(D_KEYWORD false). This function cannot be used to allocate or + * deallocate memory, so if $(D_PARAM p) is $(D_KEYWORD null) or + * $(D_PARAM size) is `0`, it should return $(D_KEYWORD false). * * Params: * p = A pointer to the memory block. @@ -239,16 +242,18 @@ final class MmapPool : Allocator * * Returns: $(D_KEYWORD true) if successful, $(D_KEYWORD false) otherwise. */ - bool expand(ref void[] p, in size_t size) shared nothrow @nogc + bool reallocateInPlace(ref void[] p, in size_t size) shared nothrow @nogc { - if (size <= p.length) - { - return true; - } - if (p is null) + if (p is null || size == 0) { return false; } + if (size <= p.length) + { + // Leave the block as is. + p = p.ptr[0 .. size]; + return true; + } Block block1 = cast(Block) (p.ptr - BlockEntry.sizeof); if (block1.size >= size) @@ -298,22 +303,22 @@ final class MmapPool : Allocator nothrow unittest { void[] p; - assert(!MmapPool.instance.expand(p, 5)); + assert(!MmapPool.instance.reallocateInPlace(p, 5)); assert(p is null); p = MmapPool.instance.allocate(1); auto orig = p.ptr; - assert(MmapPool.instance.expand(p, 2)); + assert(MmapPool.instance.reallocateInPlace(p, 2)); assert(p.length == 2); assert(p.ptr == orig); - assert(MmapPool.instance.expand(p, 4)); + assert(MmapPool.instance.reallocateInPlace(p, 4)); assert(p.length == 4); assert(p.ptr == orig); - assert(MmapPool.instance.expand(p, 2)); - assert(p.length == 4); + assert(MmapPool.instance.reallocateInPlace(p, 2)); + assert(p.length == 2); assert(p.ptr == orig); MmapPool.instance.deallocate(p); @@ -339,17 +344,12 @@ final class MmapPool : Allocator } return false; } - else if (size <= p.length) - { - // Leave the block as is. - p = p.ptr[0 .. size]; - return true; - } - else if (expand(p, size)) + else if (reallocateInPlace(p, size)) { return true; } - // Can't extend, allocate a new block, copy and delete the previous. + // Can't reallocate in place, allocate a new block, + // copy and delete the previous one. void[] reallocP = allocate(size); if (reallocP is null) { @@ -587,7 +587,7 @@ final class MmapPool : Allocator } // A lot of allocations/deallocations, but it is the minimum caused a -// segmentation fault because MmapPool expand moves a block wrong. +// segmentation fault because MmapPool reallocateInPlace moves a block wrong. unittest { auto a = MmapPool.instance.allocate(16); @@ -602,7 +602,7 @@ unittest MmapPool.instance.deallocate(c); a = MmapPool.instance.allocate(50); - MmapPool.instance.expand(a, 64); + MmapPool.instance.reallocateInPlace(a, 64); MmapPool.instance.deallocate(a); a = MmapPool.instance.allocate(1);