Skip to content

Commit 1992717

Browse files
l-kramerl-kramer
authored
Merge pull request #14 from vibe-d/hashmap_alignment
Fix HashMap to work for elements with coarser alignment than platformAlignment
2 parents f6ffae2 + 7b7d650 commit 1992717

File tree

2 files changed

+210
-87
lines changed

2 files changed

+210
-87
lines changed

‎source/vibe/container/hashmap.d

+56-87
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ module vibe.container.hashmap;
99

1010
import vibe.container.seahash : seaHash;
1111
import vibe.container.internal.utilallocator;
12+
import vibe.container.internal.rctable;
1213

13-
import std.conv : emplace;
1414
import std.traits;
1515

1616

@@ -86,12 +86,6 @@ struct HashMap(TKey, TValue, Traits = DefaultHashMapTraits!TKey, Allocator = IAl
8686
alias Key = TKey;
8787
alias Value = TValue;
8888

89-
Allocator AW(Allocator a) { return a; }
90-
alias AllocatorType = AffixAllocator!(Allocator, int);
91-
static if (is(typeof(AllocatorType.instance)))
92-
alias AllocatorInstanceType = typeof(AllocatorType.instance);
93-
else alias AllocatorInstanceType = AllocatorType;
94-
9589
struct TableEntry {
9690
UnConst!Key key = Traits.clearValue;
9791
Value value;
@@ -105,42 +99,19 @@ struct HashMap(TKey, TValue, Traits = DefaultHashMapTraits!TKey, Allocator = IAl
10599
else this.value = value;
106100
}
107101
}
102+
103+
alias Table = RCTable!(TableEntry, Allocator);
104+
108105
private {
109-
TableEntry[] m_table; // NOTE: capacity is always POT
106+
Table m_table;
110107
size_t m_length;
111-
static if (!is(typeof(Allocator.instance)))
112-
AllocatorInstanceType m_allocator;
113108
bool m_resizing;
114109
}
115110

116111
static if (!is(typeof(Allocator.instance))) {
117112
this(Allocator allocator)
118113
{
119-
m_allocator = typeof(m_allocator)(AW(allocator));
120-
}
121-
}
122-
123-
~this()
124-
{
125-
int rc;
126-
try rc = m_table is null ? 1 : () @trusted { return --allocator.prefix(m_table); } ();
127-
catch (Exception e) assert(false, e.msg);
128-
129-
if (rc == 0) {
130-
clear();
131-
if (m_table.ptr !is null) () @trusted {
132-
static if (hasIndirections!TableEntry) GC.removeRange(m_table.ptr);
133-
try allocator.dispose(m_table);
134-
catch (Exception e) assert(false, e.msg);
135-
} ();
136-
}
137-
}
138-
139-
this(this)
140-
@trusted {
141-
if (m_table.ptr) {
142-
try allocator.prefix(m_table)++;
143-
catch (Exception e) assert(false, e.msg);
114+
m_table = Table(allocator);
144115
}
145116
}
146117

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

270-
private @property AllocatorInstanceType allocator()
271-
{
272-
static if (is(typeof(Allocator.instance)))
273-
return AllocatorType.instance;
274-
else {
275-
if (!m_allocator._parent) {
276-
static if (is(Allocator == IAllocator)) {
277-
try m_allocator = typeof(m_allocator)(AW(vibeThreadAllocator()));
278-
catch (Exception e) assert(false, e.msg);
279-
} else assert(false, "Allocator not initialized.");
280-
}
281-
return m_allocator;
282-
}
283-
}
284-
285241
private size_t findIndex(Key key)
286242
const {
287243
if (m_length == 0) return size_t.max;
@@ -321,24 +277,9 @@ struct HashMap(TKey, TValue, Traits = DefaultHashMapTraits!TKey, Allocator = IAl
321277

322278
private void makeUnique()
323279
@trusted {
324-
if (m_table is null) return;
325-
326-
int rc;
327-
try rc = allocator.prefix(m_table);
328-
catch (Exception e) assert(false, e.msg);
329-
if (rc > 1) {
330-
// enforce copy-on-write
331-
auto oldtable = m_table;
332-
try {
333-
m_table = allocator.makeArray!TableEntry(m_table.length);
334-
m_table[] = oldtable;
335-
allocator.prefix(oldtable)--;
336-
assert(allocator.prefix(oldtable) > 0);
337-
allocator.prefix(m_table) = 1;
338-
} catch (Exception e) {
339-
assert(false, e.msg);
340-
}
341-
}
280+
if (m_table.isUnique) return;
281+
282+
m_table = m_table.dup;
342283
}
343284

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

359300
// allocate the new array, automatically initializes with empty entries (Traits.clearValue)
360-
try {
361-
m_table = allocator.makeArray!TableEntry(new_size);
362-
allocator.prefix(m_table) = 1;
363-
} catch (Exception e) assert(false, e.msg);
364-
static if (hasIndirections!TableEntry) GC.addRange(m_table.ptr, m_table.length * TableEntry.sizeof);
365-
// perform a move operation of all non-empty elements from the old array to the new one
366-
foreach (ref el; oldtable)
367-
if (!Traits.equals(el.key, Traits.clearValue)) {
368-
auto idx = findInsertIndex(el.key);
369-
(cast(ubyte[])(&m_table[idx])[0 .. 1])[] = (cast(ubyte[])(&el)[0 .. 1])[];
370-
}
301+
m_table = m_table.createNew(new_size);
302+
303+
if (oldtable.isUnique) {
304+
// perform a move operation of all non-empty elements from the old array to the new one
305+
foreach (ref el; oldtable)
306+
if (!Traits.equals(el.key, Traits.clearValue)) {
307+
auto idx = findInsertIndex(el.key);
308+
(cast(ubyte[])(&m_table[idx])[0 .. 1])[] = (cast(ubyte[])(&el)[0 .. 1])[];
309+
}
371310

372-
// all elements have been moved to the new array, so free the old one without calling destructors
373-
int rc;
374-
try rc = oldtable is null ? 1 : --allocator.prefix(oldtable);
375-
catch (Exception e) assert(false, e.msg);
376-
if (rc == 0) {
377-
static if (hasIndirections!TableEntry) GC.removeRange(oldtable.ptr);
378-
try allocator.deallocate(oldtable);
379-
catch (Exception e) assert(false, e.msg);
311+
// free the old table without calling destructors
312+
oldtable.deallocate();
313+
} else {
314+
// perform a copy operation of all non-empty elements from the old array to the new one
315+
foreach (ref el; oldtable)
316+
if (!Traits.equals(el.key, Traits.clearValue)) {
317+
auto idx = findInsertIndex(el.key);
318+
m_table[idx] = el;
319+
}
380320
}
381321
}
382322
}
@@ -509,6 +449,35 @@ unittest { // test for proper use of constructor/post-blit/destructor
509449
assert(Test.constructedCounter == 0);
510450
}
511451

452+
unittest { // large alignment test;
453+
align(32) static struct S { int i; }
454+
455+
HashMap!(int, S)[] amaps;
456+
// NOTE: forcing new allocations to increase the likelyhood of getting a misaligned allocation from the GC
457+
foreach (i; 0 .. 100) {
458+
HashMap!(int, S) a;
459+
a[1] = S(42);
460+
a[2] = S(43);
461+
assert(a[1] == S(42));
462+
assert(a[2] == S(43));
463+
assert(cast(size_t)cast(void*)&a[1] % S.alignof == 0);
464+
assert(cast(size_t)cast(void*)&a[2] % S.alignof == 0);
465+
amaps ~= a;
466+
}
467+
468+
HashMap!(S, int)[] bmaps;
469+
foreach (i; 0 .. 100) {
470+
HashMap!(S, int) b;
471+
b[S(1)] = 42;
472+
b[S(2)] = 43;
473+
assert(b[S(1)] == 42);
474+
assert(b[S(2)] == 43);
475+
assert(cast(size_t)cast(void*)&b[S(1)] % S.alignof == 0);
476+
assert(cast(size_t)cast(void*)&b[S(2)] % S.alignof == 0);
477+
bmaps ~= b;
478+
}
479+
}
480+
512481
private template UnConst(T) {
513482
static if (is(T U == const(U))) {
514483
alias UnConst = U;

‎source/vibe/container/internal/rctable.d

+154
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
module vibe.container.internal.rctable;
2+
3+
import vibe.container.internal.utilallocator;
4+
5+
import std.traits;
6+
7+
8+
struct RCTable(T, Allocator = IAllocator) {
9+
import core.memory : GC;
10+
11+
// NOTE: AffixAllocator doesn't handle alignment correctly for the actual
12+
// payload, so we need to explicitly make the prefix alignment
13+
// consistent
14+
align(GCAllocator.alignment) struct RC { int rc; }
15+
16+
enum needManualAlignment = T.alignof > GCAllocator.alignment;
17+
18+
Allocator AW(Allocator a) { return a; }
19+
alias AllocatorType = AffixAllocator!(Allocator, RC);
20+
static if (is(typeof(AllocatorType.instance)))
21+
alias AllocatorInstanceType = typeof(AllocatorType.instance);
22+
else alias AllocatorInstanceType = AllocatorType;
23+
24+
private {
25+
static if (needManualAlignment) {
26+
T[] m_unalignedTable;
27+
}
28+
T[] m_table; // NOTE: capacity is always POT
29+
static if (!is(typeof(Allocator.instance)))
30+
AllocatorInstanceType m_allocator;
31+
}
32+
33+
static if (!is(typeof(Allocator.instance))) {
34+
this(Allocator allocator)
35+
{
36+
m_allocator = typeof(m_allocator)(AW(allocator));
37+
}
38+
}
39+
40+
this(this)
41+
@trusted {
42+
if (m_table.ptr)
43+
this.refCount++;
44+
}
45+
46+
~this()
47+
{
48+
if (m_table.ptr && --this.refCount == 0) {
49+
static if (hasIndirections!T) {
50+
if (m_table.ptr !is null) () @trusted {
51+
GC.removeRange(m_table.ptr);
52+
}();
53+
}
54+
55+
try {
56+
static if (needManualAlignment) {
57+
static if (hasElaborateDestructor!T)
58+
foreach (ref el; m_table)
59+
destroy(el);
60+
allocator.deallocate(m_unalignedTable);
61+
} else {
62+
allocator.dispose(m_table);
63+
}
64+
} catch (Exception e) assert(false, e.msg);
65+
}
66+
}
67+
68+
// Initializes the table with the given size
69+
void initialize(size_t length)
70+
{
71+
assert(!m_table.ptr);
72+
73+
try {
74+
static if (needManualAlignment) {
75+
m_unalignedTable = allocator.makeArray!T(length + 1);
76+
() @trusted {
77+
auto mem = cast(ubyte[])m_unalignedTable;
78+
mem = mem[T.alignof - cast(size_t)mem.ptr % T.alignof .. $];
79+
assert(cast(size_t)mem.ptr % T.alignof == 0);
80+
m_table = cast(T[])mem[0 .. length * T.sizeof];
81+
} ();
82+
} else {
83+
m_table = allocator.makeArray!T(length);
84+
}
85+
assert(cast(size_t)cast(void*)m_table.ptr % T.alignof == 0);
86+
this.refCount = 1;
87+
} catch (Exception e) assert(false, e.msg);
88+
static if (hasIndirections!T) GC.addRange(m_table.ptr, m_table.length * T.sizeof);
89+
}
90+
91+
/// Deallocates without running destructors
92+
void deallocate()
93+
nothrow {
94+
try {
95+
static if (needManualAlignment) {
96+
allocator.deallocate(m_unalignedTable);
97+
m_unalignedTable = null;
98+
} else{
99+
allocator.deallocate(m_table);
100+
}
101+
m_table = null;
102+
} catch (Exception e) assert(false, e.msg);
103+
}
104+
105+
// Creates a new table with the given length, using the same allocator
106+
RCTable createNew(size_t length)
107+
nothrow {
108+
static if (!is(typeof(Allocator.instance)))
109+
auto ret = RCTable(m_allocator._parent);
110+
else RCTable ret;
111+
ret.initialize(length);
112+
return ret;
113+
}
114+
115+
/// Determines whether this reference to the table is unique
116+
bool isUnique()
117+
{
118+
return m_table.ptr is null || this.refCount == 1;
119+
}
120+
121+
// duplicates all elements to a newly allocated table
122+
RCTable dup()
123+
{
124+
auto ret = createNew(m_table.length);
125+
ret.m_table[] = m_table;
126+
return ret;
127+
}
128+
129+
inout(T)[] get() inout return { return m_table; }
130+
131+
alias get this;
132+
133+
private ref int refCount()
134+
return nothrow {
135+
static if (needManualAlignment)
136+
return allocator.prefix(m_unalignedTable).rc;
137+
else return allocator.prefix(m_table).rc;
138+
}
139+
140+
private @property AllocatorInstanceType allocator()
141+
{
142+
static if (is(typeof(Allocator.instance)))
143+
return AllocatorType.instance;
144+
else {
145+
if (!m_allocator._parent) {
146+
static if (is(Allocator == IAllocator)) {
147+
try m_allocator = typeof(m_allocator)(AW(vibeThreadAllocator()));
148+
catch (Exception e) assert(false, e.msg);
149+
} else assert(false, "Allocator not initialized.");
150+
}
151+
return m_allocator;
152+
}
153+
}
154+
}

0 commit comments

Comments
 (0)