Skip to content

Commit

Permalink
Merge pull request #14 from vibe-d/hashmap_alignment
Browse files Browse the repository at this point in the history
Fix HashMap to work for elements with coarser alignment than platformAlignment
  • Loading branch information
l-kramer authored Dec 13, 2024
2 parents f6ffae2 + 7b7d650 commit 1992717
Show file tree
Hide file tree
Showing 2 changed files with 210 additions and 87 deletions.
143 changes: 56 additions & 87 deletions source/vibe/container/hashmap.d
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ module vibe.container.hashmap;

import vibe.container.seahash : seaHash;
import vibe.container.internal.utilallocator;
import vibe.container.internal.rctable;

import std.conv : emplace;
import std.traits;


Expand Down Expand Up @@ -86,12 +86,6 @@ struct HashMap(TKey, TValue, Traits = DefaultHashMapTraits!TKey, Allocator = IAl
alias Key = TKey;
alias Value = TValue;

Allocator AW(Allocator a) { return a; }
alias AllocatorType = AffixAllocator!(Allocator, int);
static if (is(typeof(AllocatorType.instance)))
alias AllocatorInstanceType = typeof(AllocatorType.instance);
else alias AllocatorInstanceType = AllocatorType;

struct TableEntry {
UnConst!Key key = Traits.clearValue;
Value value;
Expand All @@ -105,42 +99,19 @@ struct HashMap(TKey, TValue, Traits = DefaultHashMapTraits!TKey, Allocator = IAl
else this.value = value;
}
}

alias Table = RCTable!(TableEntry, Allocator);

private {
TableEntry[] m_table; // NOTE: capacity is always POT
Table m_table;
size_t m_length;
static if (!is(typeof(Allocator.instance)))
AllocatorInstanceType m_allocator;
bool m_resizing;
}

static if (!is(typeof(Allocator.instance))) {
this(Allocator allocator)
{
m_allocator = typeof(m_allocator)(AW(allocator));
}
}

~this()
{
int rc;
try rc = m_table is null ? 1 : () @trusted { return --allocator.prefix(m_table); } ();
catch (Exception e) assert(false, e.msg);

if (rc == 0) {
clear();
if (m_table.ptr !is null) () @trusted {
static if (hasIndirections!TableEntry) GC.removeRange(m_table.ptr);
try allocator.dispose(m_table);
catch (Exception e) assert(false, e.msg);
} ();
}
}

this(this)
@trusted {
if (m_table.ptr) {
try allocator.prefix(m_table)++;
catch (Exception e) assert(false, e.msg);
m_table = Table(allocator);
}
}

Expand Down Expand Up @@ -267,21 +238,6 @@ struct HashMap(TKey, TValue, Traits = DefaultHashMapTraits!TKey, Allocator = IAl
private auto bySlot() { return m_table[].filter!((ref e) => !Traits.equals(e.key, Traits.clearValue)); }
private auto bySlot() const { return m_table[].filter!((ref e) => !Traits.equals(e.key, Traits.clearValue)); }

private @property AllocatorInstanceType allocator()
{
static if (is(typeof(Allocator.instance)))
return AllocatorType.instance;
else {
if (!m_allocator._parent) {
static if (is(Allocator == IAllocator)) {
try m_allocator = typeof(m_allocator)(AW(vibeThreadAllocator()));
catch (Exception e) assert(false, e.msg);
} else assert(false, "Allocator not initialized.");
}
return m_allocator;
}
}

private size_t findIndex(Key key)
const {
if (m_length == 0) return size_t.max;
Expand Down Expand Up @@ -321,24 +277,9 @@ struct HashMap(TKey, TValue, Traits = DefaultHashMapTraits!TKey, Allocator = IAl

private void makeUnique()
@trusted {
if (m_table is null) return;

int rc;
try rc = allocator.prefix(m_table);
catch (Exception e) assert(false, e.msg);
if (rc > 1) {
// enforce copy-on-write
auto oldtable = m_table;
try {
m_table = allocator.makeArray!TableEntry(m_table.length);
m_table[] = oldtable;
allocator.prefix(oldtable)--;
assert(allocator.prefix(oldtable) > 0);
allocator.prefix(m_table) = 1;
} catch (Exception e) {
assert(false, e.msg);
}
}
if (m_table.isUnique) return;

m_table = m_table.dup;
}

private void resize(size_t new_size)
Expand All @@ -357,26 +298,25 @@ struct HashMap(TKey, TValue, Traits = DefaultHashMapTraits!TKey, Allocator = IAl
auto oldtable = m_table;

// allocate the new array, automatically initializes with empty entries (Traits.clearValue)
try {
m_table = allocator.makeArray!TableEntry(new_size);
allocator.prefix(m_table) = 1;
} catch (Exception e) assert(false, e.msg);
static if (hasIndirections!TableEntry) GC.addRange(m_table.ptr, m_table.length * TableEntry.sizeof);
// perform a move operation of all non-empty elements from the old array to the new one
foreach (ref el; oldtable)
if (!Traits.equals(el.key, Traits.clearValue)) {
auto idx = findInsertIndex(el.key);
(cast(ubyte[])(&m_table[idx])[0 .. 1])[] = (cast(ubyte[])(&el)[0 .. 1])[];
}
m_table = m_table.createNew(new_size);

if (oldtable.isUnique) {
// perform a move operation of all non-empty elements from the old array to the new one
foreach (ref el; oldtable)
if (!Traits.equals(el.key, Traits.clearValue)) {
auto idx = findInsertIndex(el.key);
(cast(ubyte[])(&m_table[idx])[0 .. 1])[] = (cast(ubyte[])(&el)[0 .. 1])[];
}

// all elements have been moved to the new array, so free the old one without calling destructors
int rc;
try rc = oldtable is null ? 1 : --allocator.prefix(oldtable);
catch (Exception e) assert(false, e.msg);
if (rc == 0) {
static if (hasIndirections!TableEntry) GC.removeRange(oldtable.ptr);
try allocator.deallocate(oldtable);
catch (Exception e) assert(false, e.msg);
// free the old table without calling destructors
oldtable.deallocate();
} else {
// perform a copy operation of all non-empty elements from the old array to the new one
foreach (ref el; oldtable)
if (!Traits.equals(el.key, Traits.clearValue)) {
auto idx = findInsertIndex(el.key);
m_table[idx] = el;
}
}
}
}
Expand Down Expand Up @@ -509,6 +449,35 @@ unittest { // test for proper use of constructor/post-blit/destructor
assert(Test.constructedCounter == 0);
}

unittest { // large alignment test;
align(32) static struct S { int i; }

HashMap!(int, S)[] amaps;
// NOTE: forcing new allocations to increase the likelyhood of getting a misaligned allocation from the GC
foreach (i; 0 .. 100) {
HashMap!(int, S) a;
a[1] = S(42);
a[2] = S(43);
assert(a[1] == S(42));
assert(a[2] == S(43));
assert(cast(size_t)cast(void*)&a[1] % S.alignof == 0);
assert(cast(size_t)cast(void*)&a[2] % S.alignof == 0);
amaps ~= a;
}

HashMap!(S, int)[] bmaps;
foreach (i; 0 .. 100) {
HashMap!(S, int) b;
b[S(1)] = 42;
b[S(2)] = 43;
assert(b[S(1)] == 42);
assert(b[S(2)] == 43);
assert(cast(size_t)cast(void*)&b[S(1)] % S.alignof == 0);
assert(cast(size_t)cast(void*)&b[S(2)] % S.alignof == 0);
bmaps ~= b;
}
}

private template UnConst(T) {
static if (is(T U == const(U))) {
alias UnConst = U;
Expand Down
154 changes: 154 additions & 0 deletions source/vibe/container/internal/rctable.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
module vibe.container.internal.rctable;

import vibe.container.internal.utilallocator;

import std.traits;


struct RCTable(T, Allocator = IAllocator) {
import core.memory : GC;

// NOTE: AffixAllocator doesn't handle alignment correctly for the actual
// payload, so we need to explicitly make the prefix alignment
// consistent
align(GCAllocator.alignment) struct RC { int rc; }

enum needManualAlignment = T.alignof > GCAllocator.alignment;

Allocator AW(Allocator a) { return a; }
alias AllocatorType = AffixAllocator!(Allocator, RC);
static if (is(typeof(AllocatorType.instance)))
alias AllocatorInstanceType = typeof(AllocatorType.instance);
else alias AllocatorInstanceType = AllocatorType;

private {
static if (needManualAlignment) {
T[] m_unalignedTable;
}
T[] m_table; // NOTE: capacity is always POT
static if (!is(typeof(Allocator.instance)))
AllocatorInstanceType m_allocator;
}

static if (!is(typeof(Allocator.instance))) {
this(Allocator allocator)
{
m_allocator = typeof(m_allocator)(AW(allocator));
}
}

this(this)
@trusted {
if (m_table.ptr)
this.refCount++;
}

~this()
{
if (m_table.ptr && --this.refCount == 0) {
static if (hasIndirections!T) {
if (m_table.ptr !is null) () @trusted {
GC.removeRange(m_table.ptr);
}();
}

try {
static if (needManualAlignment) {
static if (hasElaborateDestructor!T)
foreach (ref el; m_table)
destroy(el);
allocator.deallocate(m_unalignedTable);
} else {
allocator.dispose(m_table);
}
} catch (Exception e) assert(false, e.msg);
}
}

// Initializes the table with the given size
void initialize(size_t length)
{
assert(!m_table.ptr);

try {
static if (needManualAlignment) {
m_unalignedTable = allocator.makeArray!T(length + 1);
() @trusted {
auto mem = cast(ubyte[])m_unalignedTable;
mem = mem[T.alignof - cast(size_t)mem.ptr % T.alignof .. $];
assert(cast(size_t)mem.ptr % T.alignof == 0);
m_table = cast(T[])mem[0 .. length * T.sizeof];
} ();
} else {
m_table = allocator.makeArray!T(length);
}
assert(cast(size_t)cast(void*)m_table.ptr % T.alignof == 0);
this.refCount = 1;
} catch (Exception e) assert(false, e.msg);
static if (hasIndirections!T) GC.addRange(m_table.ptr, m_table.length * T.sizeof);
}

/// Deallocates without running destructors
void deallocate()
nothrow {
try {
static if (needManualAlignment) {
allocator.deallocate(m_unalignedTable);
m_unalignedTable = null;
} else{
allocator.deallocate(m_table);
}
m_table = null;
} catch (Exception e) assert(false, e.msg);
}

// Creates a new table with the given length, using the same allocator
RCTable createNew(size_t length)
nothrow {
static if (!is(typeof(Allocator.instance)))
auto ret = RCTable(m_allocator._parent);
else RCTable ret;
ret.initialize(length);
return ret;
}

/// Determines whether this reference to the table is unique
bool isUnique()
{
return m_table.ptr is null || this.refCount == 1;
}

// duplicates all elements to a newly allocated table
RCTable dup()
{
auto ret = createNew(m_table.length);
ret.m_table[] = m_table;
return ret;
}

inout(T)[] get() inout return { return m_table; }

alias get this;

private ref int refCount()
return nothrow {
static if (needManualAlignment)
return allocator.prefix(m_unalignedTable).rc;
else return allocator.prefix(m_table).rc;
}

private @property AllocatorInstanceType allocator()
{
static if (is(typeof(Allocator.instance)))
return AllocatorType.instance;
else {
if (!m_allocator._parent) {
static if (is(Allocator == IAllocator)) {
try m_allocator = typeof(m_allocator)(AW(vibeThreadAllocator()));
catch (Exception e) assert(false, e.msg);
} else assert(false, "Allocator not initialized.");
}
return m_allocator;
}
}
}

0 comments on commit 1992717

Please sign in to comment.