Skip to content

Commit 741f3c3

Browse files
CoreyKaylorCoreyKaylor
committed
Incorporate Claude / Seb feedback for MDBValue optimizations
1 parent 8ebf7f8 commit 741f3c3

9 files changed

Lines changed: 114 additions & 66 deletions

‎src/LightningDB.Tests/ComparerTests.cs‎

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System;
2-
using System.Runtime.InteropServices;
32
using LightningDB.Comparers;
43
using Shouldly;
54

@@ -85,19 +84,19 @@ public void signed_integer_comparer_sorts_int32_with_negatives_first()
8584
using var cursor = txn.CreateCursor(db);
8685

8786
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
88-
MemoryMarshal.Read<int>(cursor.GetCurrent().key.AsSpan()).ShouldBe(-50);
87+
cursor.GetCurrent().key.Read<int>().ShouldBe(-50);
8988

9089
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
91-
MemoryMarshal.Read<int>(cursor.GetCurrent().key.AsSpan()).ShouldBe(-10);
90+
cursor.GetCurrent().key.Read<int>().ShouldBe(-10);
9291

9392
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
94-
MemoryMarshal.Read<int>(cursor.GetCurrent().key.AsSpan()).ShouldBe(0);
93+
cursor.GetCurrent().key.Read<int>().ShouldBe(0);
9594

9695
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
97-
MemoryMarshal.Read<int>(cursor.GetCurrent().key.AsSpan()).ShouldBe(50);
96+
cursor.GetCurrent().key.Read<int>().ShouldBe(50);
9897

9998
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
100-
MemoryMarshal.Read<int>(cursor.GetCurrent().key.AsSpan()).ShouldBe(100);
99+
cursor.GetCurrent().key.Read<int>().ShouldBe(100);
101100

102101
cursor.Next().Item1.ShouldBe(MDBResultCode.NotFound);
103102
}
@@ -120,13 +119,13 @@ public void signed_integer_comparer_sorts_int64_with_negatives_first()
120119
using var cursor = txn.CreateCursor(db);
121120

122121
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
123-
MemoryMarshal.Read<long>(cursor.GetCurrent().key.AsSpan()).ShouldBe(-10L);
122+
cursor.GetCurrent().key.Read<long>().ShouldBe(-10L);
124123

125124
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
126-
MemoryMarshal.Read<long>(cursor.GetCurrent().key.AsSpan()).ShouldBe(0L);
125+
cursor.GetCurrent().key.Read<long>().ShouldBe(0L);
127126

128127
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
129-
MemoryMarshal.Read<long>(cursor.GetCurrent().key.AsSpan()).ShouldBe(50L);
128+
cursor.GetCurrent().key.Read<long>().ShouldBe(50L);
130129

131130
cursor.Next().Item1.ShouldBe(MDBResultCode.NotFound);
132131
}
@@ -149,13 +148,13 @@ public void reverse_signed_integer_comparer_sorts_descending()
149148
using var cursor = txn.CreateCursor(db);
150149

151150
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
152-
MemoryMarshal.Read<int>(cursor.GetCurrent().key.AsSpan()).ShouldBe(50);
151+
cursor.GetCurrent().key.Read<int>().ShouldBe(50);
153152

154153
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
155-
MemoryMarshal.Read<int>(cursor.GetCurrent().key.AsSpan()).ShouldBe(0);
154+
cursor.GetCurrent().key.Read<int>().ShouldBe(0);
156155

157156
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
158-
MemoryMarshal.Read<int>(cursor.GetCurrent().key.AsSpan()).ShouldBe(-10);
157+
cursor.GetCurrent().key.Read<int>().ShouldBe(-10);
159158

160159
cursor.Next().Item1.ShouldBe(MDBResultCode.NotFound);
161160
}
@@ -179,16 +178,16 @@ public void unsigned_integer_comparer_sorts_uint32()
179178
using var cursor = txn.CreateCursor(db);
180179

181180
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
182-
MemoryMarshal.Read<uint>(cursor.GetCurrent().key.AsSpan()).ShouldBe(0u);
181+
cursor.GetCurrent().key.Read<uint>().ShouldBe(0u);
183182

184183
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
185-
MemoryMarshal.Read<uint>(cursor.GetCurrent().key.AsSpan()).ShouldBe(50u);
184+
cursor.GetCurrent().key.Read<uint>().ShouldBe(50u);
186185

187186
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
188-
MemoryMarshal.Read<uint>(cursor.GetCurrent().key.AsSpan()).ShouldBe(100u);
187+
cursor.GetCurrent().key.Read<uint>().ShouldBe(100u);
189188

190189
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
191-
MemoryMarshal.Read<uint>(cursor.GetCurrent().key.AsSpan()).ShouldBe(uint.MaxValue);
190+
cursor.GetCurrent().key.Read<uint>().ShouldBe(uint.MaxValue);
192191

193192
cursor.Next().Item1.ShouldBe(MDBResultCode.NotFound);
194193
}
@@ -211,13 +210,13 @@ public void unsigned_integer_comparer_sorts_uint64()
211210
using var cursor = txn.CreateCursor(db);
212211

213212
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
214-
MemoryMarshal.Read<ulong>(cursor.GetCurrent().key.AsSpan()).ShouldBe(0UL);
213+
cursor.GetCurrent().key.Read<ulong>().ShouldBe(0UL);
215214

216215
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
217-
MemoryMarshal.Read<ulong>(cursor.GetCurrent().key.AsSpan()).ShouldBe(50UL);
216+
cursor.GetCurrent().key.Read<ulong>().ShouldBe(50UL);
218217

219218
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
220-
MemoryMarshal.Read<ulong>(cursor.GetCurrent().key.AsSpan()).ShouldBe(ulong.MaxValue);
219+
cursor.GetCurrent().key.Read<ulong>().ShouldBe(ulong.MaxValue);
221220

222221
cursor.Next().Item1.ShouldBe(MDBResultCode.NotFound);
223222
}
@@ -240,13 +239,13 @@ public void reverse_unsigned_integer_comparer_sorts_descending()
240239
using var cursor = txn.CreateCursor(db);
241240

242241
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
243-
MemoryMarshal.Read<uint>(cursor.GetCurrent().key.AsSpan()).ShouldBe(100u);
242+
cursor.GetCurrent().key.Read<uint>().ShouldBe(100u);
244243

245244
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
246-
MemoryMarshal.Read<uint>(cursor.GetCurrent().key.AsSpan()).ShouldBe(50u);
245+
cursor.GetCurrent().key.Read<uint>().ShouldBe(50u);
247246

248247
cursor.Next().Item1.ShouldBe(MDBResultCode.Success);
249-
MemoryMarshal.Read<uint>(cursor.GetCurrent().key.AsSpan()).ShouldBe(0u);
248+
cursor.GetCurrent().key.Read<uint>().ShouldBe(0u);
250249

251250
cursor.Next().Item1.ShouldBe(MDBResultCode.NotFound);
252251
}

‎src/LightningDB.Tests/CursorTests.cs‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -275,10 +275,10 @@ public void should_get_both_range()
275275
var result = c.GetBothRange(key, values[1]);
276276
result.ShouldBe(MDBResultCode.Success);
277277
var current = c.GetCurrent();
278-
current.value.CopyToNewArray().ShouldBe(values[1]);
279-
}, DatabaseOpenFlags.DuplicatesFixed | DatabaseOpenFlags.Create);
278+
current.value.Read<int>().ShouldBe(2);
279+
}, DatabaseOpenFlags.DuplicatesFixed | DatabaseOpenFlags.Create);
280280
}
281-
281+
282282
public void should_get_both_range_with_span()
283283
{
284284
using var env = CreateEnvironment();
@@ -290,10 +290,10 @@ public void should_get_both_range_with_span()
290290
var result = c.GetBothRange(key, values[1].AsSpan());
291291
result.ShouldBe(MDBResultCode.Success);
292292
var current = c.GetCurrent();
293-
current.value.CopyToNewArray().ShouldBe(values[1]);
294-
}, DatabaseOpenFlags.DuplicatesFixed | DatabaseOpenFlags.Create);
293+
current.value.Read<int>().ShouldBe(2);
294+
}, DatabaseOpenFlags.DuplicatesFixed | DatabaseOpenFlags.Create);
295295
}
296-
296+
297297
public void should_move_to_first_duplicate()
298298
{
299299
using var env = CreateEnvironment();
@@ -306,10 +306,10 @@ public void should_move_to_first_duplicate()
306306
result.ShouldBe(MDBResultCode.Success);
307307
var dupResult = c.FirstDuplicate();
308308
dupResult.resultCode.ShouldBe(MDBResultCode.Success);
309-
dupResult.value.CopyToNewArray().ShouldBe(values[0]);
310-
}, DatabaseOpenFlags.DuplicatesFixed | DatabaseOpenFlags.Create);
309+
dupResult.value.Read<int>().ShouldBe(1);
310+
}, DatabaseOpenFlags.DuplicatesFixed | DatabaseOpenFlags.Create);
311311
}
312-
312+
313313
public void should_move_to_last_duplicate()
314314
{
315315
using var env = CreateEnvironment();
@@ -321,8 +321,8 @@ public void should_move_to_last_duplicate()
321321
c.Set(key);
322322
var result = c.LastDuplicate();
323323
result.resultCode.ShouldBe(MDBResultCode.Success);
324-
result.value.CopyToNewArray().ShouldBe(values[4]);
325-
}, DatabaseOpenFlags.DuplicatesFixed | DatabaseOpenFlags.Create);
324+
result.value.Read<int>().ShouldBe(5);
325+
}, DatabaseOpenFlags.DuplicatesFixed | DatabaseOpenFlags.Create);
326326
}
327327

328328
public void all_values_for_should_only_return_matching_key_values()
@@ -364,8 +364,8 @@ public void should_move_to_next_no_duplicate()
364364
var values = PopulateMultipleCursorValues(c);
365365
var result = c.NextNoDuplicate();
366366
result.resultCode.ShouldBe(MDBResultCode.Success);
367-
result.value.CopyToNewArray().ShouldBe(values[0]);
368-
}, DatabaseOpenFlags.DuplicatesFixed | DatabaseOpenFlags.Create);
367+
result.value.Read<int>().ShouldBe(1);
368+
}, DatabaseOpenFlags.DuplicatesFixed | DatabaseOpenFlags.Create);
369369
}
370370

371371

@@ -490,7 +490,7 @@ public void should_move_to_previous_duplicate()
490490
result.resultCode.ShouldBe(MDBResultCode.Success);
491491

492492
// Verify we're at the second-to-last value
493-
result.value.CopyToNewArray().ShouldBe(values[3]); // Previous of the last (values[4])
493+
result.value.Read<int>().ShouldBe(4); // Previous of the last (values[4])
494494
}, DatabaseOpenFlags.DuplicatesFixed | DatabaseOpenFlags.Create);
495495
}
496496

‎src/LightningDB.Tests/TransactionTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ public void transaction_should_support_custom_comparer()
203203
{
204204
int Comparison(int l, int r) => l.CompareTo(r);
205205
var options = new DatabaseConfiguration {Flags = DatabaseOpenFlags.Create};
206-
int CompareWith(MDBValue l, MDBValue r) => Comparison(BitConverter.ToInt32(l.CopyToNewArray(), 0), BitConverter.ToInt32(r.CopyToNewArray(), 0));
206+
int CompareWith(MDBValue l, MDBValue r) => Comparison(l.Read<int>(), r.Read<int>());
207207
options.CompareWith(Comparer<MDBValue>.Create(new Comparison<MDBValue>((Func<MDBValue, MDBValue, int>)CompareWith)));
208208

209209
using var env = CreateEnvironment();
@@ -231,7 +231,7 @@ public void transaction_should_support_custom_comparer()
231231
var order = 0;
232232
(MDBResultCode, MDBValue, MDBValue) result;
233233
while ((result = c.Next()).Item1 == MDBResultCode.Success)
234-
BitConverter.ToInt32(result.Item2.CopyToNewArray()).ShouldBe(keysSorted[order++]);
234+
result.Item2.Read<int>().ShouldBe(keysSorted[order++]);
235235
}
236236
}
237237

@@ -243,7 +243,7 @@ public void transaction_should_support_custom_dup_sorter()
243243
env.Open();
244244
using var txn = env.BeginTransaction();
245245
var options = new DatabaseConfiguration {Flags = DatabaseOpenFlags.Create | DatabaseOpenFlags.DuplicatesFixed};
246-
int CompareWith(MDBValue l, MDBValue r) => Comparison(BitConverter.ToInt32(l.CopyToNewArray(), 0), BitConverter.ToInt32(r.CopyToNewArray(), 0));
246+
int CompareWith(MDBValue l, MDBValue r) => Comparison(l.Read<int>(), r.Read<int>());
247247
options.FindDuplicatesWith(Comparer<MDBValue>.Create(new Comparison<MDBValue>((Func<MDBValue, MDBValue, int>)CompareWith)));
248248
using var db = txn.OpenDatabase(configuration: options, closeOnDispose: true);
249249

@@ -260,7 +260,7 @@ public void transaction_should_support_custom_dup_sorter()
260260

261261
(MDBResultCode, MDBValue, MDBValue) result;
262262
while ((result = c.Next()).Item1 == MDBResultCode.Success)
263-
BitConverter.ToInt32(result.Item3.CopyToNewArray()).ShouldBe(valuesSorted[order++]);
263+
result.Item3.Read<int>().ShouldBe(valuesSorted[order++]);
264264
}
265265
}
266266
public void database_should_be_empty_after_truncate()

‎src/LightningDB/Comparers/ReverseSignedIntegerComparer.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.Runtime.InteropServices;
43

54
namespace LightningDB.Comparers;
65

@@ -20,10 +19,10 @@ public int Compare(MDBValue x, MDBValue y)
2019
var right = y.AsSpan();
2120

2221
if (left.Length == 4 && right.Length == 4)
23-
return MemoryMarshal.Read<int>(right).CompareTo(MemoryMarshal.Read<int>(left));
22+
return y.Read<int>().CompareTo(x.Read<int>());
2423

2524
if (left.Length == 8 && right.Length == 8)
26-
return MemoryMarshal.Read<long>(right).CompareTo(MemoryMarshal.Read<long>(left));
25+
return y.Read<long>().CompareTo(x.Read<long>());
2726

2827
return right.SequenceCompareTo(left);
2928
}

‎src/LightningDB/Comparers/ReverseUnsignedIntegerComparer.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.Runtime.InteropServices;
43

54
namespace LightningDB.Comparers;
65

@@ -20,10 +19,10 @@ public int Compare(MDBValue x, MDBValue y)
2019
var right = y.AsSpan();
2120

2221
if (left.Length == 4 && right.Length == 4)
23-
return MemoryMarshal.Read<uint>(right).CompareTo(MemoryMarshal.Read<uint>(left));
22+
return y.Read<uint>().CompareTo(x.Read<uint>());
2423

2524
if (left.Length == 8 && right.Length == 8)
26-
return MemoryMarshal.Read<ulong>(right).CompareTo(MemoryMarshal.Read<ulong>(left));
25+
return y.Read<ulong>().CompareTo(x.Read<ulong>());
2726

2827
return right.SequenceCompareTo(left);
2928
}

‎src/LightningDB/Comparers/SignedIntegerComparer.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.Runtime.InteropServices;
43

54
namespace LightningDB.Comparers;
65

@@ -21,10 +20,10 @@ public int Compare(MDBValue x, MDBValue y)
2120
var right = y.AsSpan();
2221

2322
if (left.Length == 4 && right.Length == 4)
24-
return MemoryMarshal.Read<int>(left).CompareTo(MemoryMarshal.Read<int>(right));
23+
return x.Read<int>().CompareTo(y.Read<int>());
2524

2625
if (left.Length == 8 && right.Length == 8)
27-
return MemoryMarshal.Read<long>(left).CompareTo(MemoryMarshal.Read<long>(right));
26+
return x.Read<long>().CompareTo(y.Read<long>());
2827

2928
return left.SequenceCompareTo(right);
3029
}

‎src/LightningDB/Comparers/UnsignedIntegerComparer.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.Runtime.InteropServices;
43

54
namespace LightningDB.Comparers;
65

@@ -21,10 +20,10 @@ public int Compare(MDBValue x, MDBValue y)
2120
var right = y.AsSpan();
2221

2322
if (left.Length == 4 && right.Length == 4)
24-
return MemoryMarshal.Read<uint>(left).CompareTo(MemoryMarshal.Read<uint>(right));
23+
return x.Read<uint>().CompareTo(y.Read<uint>());
2524

2625
if (left.Length == 8 && right.Length == 8)
27-
return MemoryMarshal.Read<ulong>(left).CompareTo(MemoryMarshal.Read<ulong>(right));
26+
return x.Read<ulong>().CompareTo(y.Read<ulong>());
2827

2928
return left.SequenceCompareTo(right);
3029
}

‎src/LightningDB/LightningCursor.cs‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
using System;
2-
using System.Linq;
2+
using System.Buffers;
33
using System.Runtime.CompilerServices;
44

55
using static LightningDB.Native.Lmdb;
@@ -377,10 +377,11 @@ public unsafe MDBResultCode Put(ReadOnlySpan<byte> key, ReadOnlySpan<byte> value
377377
/// <returns>Returns <see cref="MDBResultCode"/></returns>
378378
public unsafe MDBResultCode Put(byte[] key, byte[][] values)
379379
{
380-
const int StackAllocateLimit = 256;//I just made up a number, this can be much more aggressive -arc
381-
382-
var overallLength = values.Sum(arr => arr.Length);//probably allocates but boy is it handy...
380+
const int StackAllocateLimit = 256;
383381

382+
var overallLength = 0;
383+
for (var i = 0; i < values.Length; i++)
384+
overallLength += values[i].Length;
384385

385386
//the idea here is to gain some perf by stackallocating the buffer to
386387
//hold the contiguous keys
@@ -391,11 +392,16 @@ public unsafe MDBResultCode Put(byte[] key, byte[][] values)
391392
return InnerPutMultiple(contiguousValues);
392393
}
393394

394-
fixed (byte* contiguousValuesPtr = new byte[overallLength])
395+
var rentedArray = ArrayPool<byte>.Shared.Rent(overallLength);
396+
try
395397
{
396-
var contiguousValues = new Span<byte>(contiguousValuesPtr, overallLength);
398+
var contiguousValues = rentedArray.AsSpan(0, overallLength);
397399
return InnerPutMultiple(contiguousValues);
398400
}
401+
finally
402+
{
403+
ArrayPool<byte>.Shared.Return(rentedArray);
404+
}
399405

400406
//these local methods could be made static, but the compiler will emit these closures
401407
//as structs with very little overhead. Also static local functions isn't available

0 commit comments

Comments
 (0)