feat: replaced unsafe pointer with atomic.pointer[T] for cleaner atom…#152
Conversation
guycipher
left a comment
There was a problem hiding this comment.
Was there perf done to see if this solution has benefits on throughput and latency? Comparing benchmarks even to prior. I do wonder besides that the code looks great but I’m always curious on those perf aspects.
|
I ran the default benchmarks and suprisingly there were some improvements Benchmark Comparison: unsafe.Pointer vs atomic.Pointer[T]Master Branch (unsafe.Pointer):fillseq: 149,964.85 ops/sec Atomic.Pointer Branch:fillseq: 159,338.64 ops/sec (+6.25% improvement) Performance Analysis:fillseq: +6.25% faster (sequential writes) Latency Improvements:fillseq P50: 12.5μs → 12.2μs (2.4% faster) I think that the Go compiler can optimize atomic.Pointer[T] better than manual unsafe.Pointer casts, leading to more efficient code generation. Another guess is that type-safe wrapper may improve memory layout and cache locality. I solely did this to improve code clarity, didn't think there were performance benefits associated |
|
I have no idea why the tests for windows are failing. Do you think it's related to this PR or something we need to investigate separately ? @guycipher |
|
Good stuff, yeah windows is just generally slower the scheduler and stuff I think it may be a data race, I've reran with debug mode on. When I get to my computer I will pull your code and run it locally to see. |
|
Was a data race. |
fixes issue.