Reworked api to be more in line with native lib

CoreyKaylor commented 4 years ago

This is the first pass at the proposed cursor api changes. It changes the underlying check / throwing behavior and favors the result code. I wanted to get something up to share and get some feedback on the changes. I'm also finding that it makes the API generally much easier to work with to return the result codes IMO.

[x] I've reworked the cursor tests and added quite a few more that were historically missing, but I still need to add more.
[x] Review tests and DB / Environment / Transaction areas for areas of improvement as well.
[x] Compare the benchmarks pre and post changes, but it's hard to imagine that this doesn't slightly have an uptick, but you never know until you measure.

Fixes https://github.com/CoreyKaylor/Lightning.NET/issues/124 Fixes https://github.com/CoreyKaylor/Lightning.NET/issues/123

AlgorithmsAreCool commented 4 years ago

These changes look pretty great. Great work on the tests too!

I'll take a closer look today/tomorrow

CoreyKaylor commented 4 years ago

Read Before:


BenchmarkDotNet=v0.12.1, OS=macOS Mojave 10.14.6 (18G5033) [Darwin 18.7.0]
Intel Core i9-9980HK CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=3.1.102
  [Host]     : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT
  DefaultJob : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT

Method	OpsPerTransaction	ValueSize	KeyOrder	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
Read	1	8	Sequential	416.4 ns	8.23 ns	10.98 ns	0.0401	-	-	336 B
Read	1	64	Sequential	401.8 ns	5.69 ns	5.32 ns	0.0467	-	-	392 B
Read	1	256	Sequential	413.8 ns	8.20 ns	15.81 ns	0.0696	-	-	584 B
Read	100	8	Sequential	10,715.2 ns	212.17 ns	310.99 ns	0.4120	-	-	3504 B
Read	100	64	Sequential	12,291.0 ns	244.41 ns	334.55 ns	1.0834	-	-	9104 B
Read	100	256	Sequential	13,607.0 ns	257.71 ns	316.49 ns	3.3722	-	-	28304 B
Read	1000	8	Sequential	145,131.3 ns	2,889.44 ns	4,324.78 ns	3.6621	-	-	32304 B
Read	1000	64	Sequential	152,260.8 ns	2,953.81 ns	4,329.65 ns	10.4980	-	-	88304 B
Read	1000	256	Sequential	175,138.1 ns	3,485.96 ns	4,886.83 ns	33.4473	-	-	280304 B

Write Before:


BenchmarkDotNet=v0.12.1, OS=macOS Mojave 10.14.6 (18G5033) [Darwin 18.7.0]
Intel Core i9-9980HK CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=3.1.102
  [Host]     : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT
  DefaultJob : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT

Method	OpsPerTransaction	ValueSize	KeyOrder	Mean	Error	StdDev	Median	Gen 0	Gen 1	Gen 2	Allocated
Write	1	8	Sequential	85.67 μs	1.690 μs	3.256 μs	84.91 μs	-	-	-	304 B
Write	1	64	Sequential	84.17 μs	1.359 μs	1.061 μs	84.00 μs	-	-	-	304 B
Write	1	256	Sequential	83.56 μs	1.221 μs	1.082 μs	83.45 μs	-	-	-	304 B
Write	100	8	Sequential	133.22 μs	2.900 μs	8.506 μs	136.96 μs	-	-	-	304 B
Write	100	64	Sequential	144.44 μs	2.858 μs	4.775 μs	143.27 μs	-	-	-	304 B
Write	100	256	Sequential	165.20 μs	3.283 μs	4.602 μs	163.74 μs	-	-	-	304 B
Write	1000	8	Sequential	342.81 μs	6.802 μs	15.214 μs	337.43 μs	-	-	-	304 B
Write	1000	64	Sequential	412.54 μs	8.168 μs	22.905 μs	408.33 μs	-	-	-	304 B
Write	1000	256	Sequential	633.70 μs	26.703 μs	77.470 μs	621.84 μs	-	-	-	304 B

Read After:


BenchmarkDotNet=v0.12.1, OS=macOS Mojave 10.14.6 (18G5033) [Darwin 18.7.0]
Intel Core i9-9980HK CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=3.1.102
  [Host]     : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT
  DefaultJob : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT

Method	OpsPerTransaction	ValueSize	KeyOrder	Mean	Error	StdDev	Median	Gen 0	Gen 1	Gen 2	Allocated
Read	1	8	Sequential	387.6 ns	7.48 ns	7.69 ns	388.0 ns	0.0362	-	-	304 B
Read	1	64	Sequential	385.0 ns	5.57 ns	5.21 ns	384.5 ns	0.0362	-	-	304 B
Read	1	256	Sequential	399.0 ns	7.97 ns	16.47 ns	398.4 ns	0.0362	-	-	304 B
Read	100	8	Sequential	9,323.8 ns	170.66 ns	142.51 ns	9,304.8 ns	0.0305	-	-	304 B
Read	100	64	Sequential	10,773.6 ns	237.09 ns	660.92 ns	10,449.8 ns	0.0305	-	-	304 B
Read	100	256	Sequential	11,030.6 ns	220.35 ns	402.93 ns	10,910.9 ns	0.0305	-	-	304 B
Read	1000	8	Sequential	132,992.5 ns	2,658.94 ns	5,666.41 ns	131,295.0 ns	-	-	-	304 B
Read	1000	64	Sequential	138,365.9 ns	2,731.50 ns	6,852.79 ns	137,645.7 ns	-	-	-	304 B
Read	1000	256	Sequential	144,150.2 ns	2,873.21 ns	5,869.20 ns	144,740.7 ns	-	-	-	304 B

Read After w/CopyToNewArray:


BenchmarkDotNet=v0.12.1, OS=macOS Mojave 10.14.6 (18G5033) [Darwin 18.7.0]
Intel Core i9-9980HK CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=3.1.102
  [Host]     : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT
  DefaultJob : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT

Method	OpsPerTransaction	ValueSize	KeyOrder	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
Read	1	8	Sequential	408.2 ns	7.86 ns	8.41 ns	0.0401	-	-	336 B
Read	1	64	Sequential	397.8 ns	6.08 ns	5.69 ns	0.0467	-	-	392 B
Read	1	256	Sequential	404.0 ns	6.87 ns	6.43 ns	0.0696	-	-	584 B
Read	100	8	Sequential	10,442.2 ns	208.71 ns	324.93 ns	0.4120	-	-	3504 B
Read	100	64	Sequential	11,984.0 ns	230.54 ns	315.57 ns	1.0834	-	-	9104 B
Read	100	256	Sequential	13,242.6 ns	262.62 ns	312.63 ns	3.3722	-	-	28304 B
Read	1000	8	Sequential	145,204.8 ns	2,862.13 ns	6,342.28 ns	3.6621	-	-	32304 B
Read	1000	64	Sequential	151,093.4 ns	2,944.69 ns	3,724.09 ns	10.4980	-	-	88304 B
Read	1000	256	Sequential	172,407.1 ns	2,818.85 ns	2,636.76 ns	33.4473	-	-	280305 B

Write After:


BenchmarkDotNet=v0.12.1, OS=macOS Mojave 10.14.6 (18G5033) [Darwin 18.7.0]
Intel Core i9-9980HK CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=3.1.102
  [Host]     : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT
  DefaultJob : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT

Method	OpsPerTransaction	ValueSize	KeyOrder	Mean	Error	StdDev	Median	Gen 0	Gen 1	Gen 2	Allocated
Write	1	8	Sequential	85.91 μs	1.716 μs	3.049 μs	85.02 μs	-	-	-	304 B
Write	1	64	Sequential	92.55 μs	1.831 μs	4.759 μs	93.39 μs	-	-	-	304 B
Write	1	256	Sequential	89.25 μs	1.777 μs	4.744 μs	88.46 μs	-	-	-	304 B
Write	100	8	Sequential	127.75 μs	3.708 μs	10.934 μs	123.60 μs	-	-	-	304 B
Write	100	64	Sequential	145.58 μs	2.906 μs	6.964 μs	142.71 μs	-	-	-	304 B
Write	100	256	Sequential	166.00 μs	3.263 μs	5.451 μs	164.89 μs	-	-	-	304 B
Write	1000	8	Sequential	348.07 μs	6.955 μs	14.822 μs	347.61 μs	-	-	-	304 B
Write	1000	64	Sequential	408.28 μs	8.087 μs	20.438 μs	400.03 μs	-	-	-	304 B
Write	1000	256	Sequential	622.11 μs	22.651 μs	65.353 μs	621.44 μs	-	-	-	304 B

AlgorithmsAreCool commented 4 years ago

Nice buff to read perf!

CoreyKaylor commented 4 years ago

@AlgorithmsAreCool I moved some of your methods to extension methods instead. Also after reviewing the GetResult a bit more, I think having incorrectly sized buffer is likely something you would only hit when getting started with the lib and so I favored an Exception here instead. I also reworded one of your tests once I gathered what you were trying to do, let me know if I misunderstood your intentions.

I think the benchmarks show one interesting trait by changing the functions to return the result code and the MDBValue, for instances the array isn't copied or used, there's no added memory overhead. I think the results between the two approaches might be comparable perf-wise, if maybe slightly faster with the new changes but probably negligibly so.

AlgorithmsAreCool commented 4 years ago

You also fixed alot of my bad spelling 😁.

I think having incorrectly sized buffer is likely something you would only hit when getting started with the lib and so I favored an Exception here instead.

I am inclined to agree. Besides this cursor API is zero copy and more flexible. Good Call

CoreyKaylor commented 4 years ago

Read w/value.AsSpan().CopyTo(ValueBuffer):

We have a winner


BenchmarkDotNet=v0.12.1, OS=macOS Mojave 10.14.6 (18G5033) [Darwin 18.7.0]
Intel Core i9-9980HK CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=3.1.102
  [Host]     : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT
  DefaultJob : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT

Method	OpsPerTransaction	ValueSize	KeyOrder	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
Read	1	8	Sequential	379.3 ns	6.79 ns	6.35 ns	0.0362	-	-	304 B
Read	1	64	Sequential	391.3 ns	4.51 ns	4.22 ns	0.0362	-	-	304 B
Read	1	256	Sequential	404.9 ns	8.11 ns	16.38 ns	0.0362	-	-	304 B
Read	100	8	Sequential	9,833.8 ns	195.29 ns	253.94 ns	0.0305	-	-	304 B
Read	100	64	Sequential	11,154.3 ns	216.15 ns	310.00 ns	0.0305	-	-	304 B
Read	100	256	Sequential	11,526.2 ns	215.40 ns	230.48 ns	0.0305	-	-	304 B
Read	1000	8	Sequential	136,522.7 ns	2,323.72 ns	2,173.61 ns	-	-	-	307 B
Read	1000	64	Sequential	140,748.1 ns	2,794.53 ns	3,431.94 ns	-	-	-	304 B
Read	1000	256	Sequential	151,597.5 ns	2,707.89 ns	2,897.42 ns	-	-	-	304 B

AlgorithmsAreCool commented 4 years ago

@CoreyKaylor So apparently, if i don't click "Submit Review" my comments don't appear. I'm sorry i thought i left that comment last night.

But the changes look good i don't see any correctness issues.

CoreyKaylor / Lightning.NET