alphadose / haxmap

Fastest and most memory efficient golang concurrent hashmap
MIT License
892 stars 45 forks source link

Attempting to use unsafe.Slice again #6

Closed cuonglm closed 1 year ago

cuonglm commented 1 year ago

In two commits:

All usages of unsafe.Slice were removed due to performance reason, without any proof. In any case, we should prefer correctness over performance, code should be done right, before thinking about speed.

However, this commit attempts to use unsafe.Slice again, and adding benchmarks. The result indicates that unsafe.Slice even slightly faster than incorrect usage of reflect.SliceHeader:

name                     old time/op    new time/op    delta
HaxMapReadsOnly-8          10.6µs ± 1%    10.2µs ± 1%  -3.72%  (p=0.000 n=9+10)
HaxMapReadsWithWrites-8    12.6µs ± 2%    12.1µs ± 1%  -4.18%  (p=0.000 n=10+9)

name                     old alloc/op   new alloc/op   delta
HaxMapReadsOnly-8           0.00B          0.00B         ~     (all equal)
HaxMapReadsWithWrites-8    1.04kB ± 3%    1.01kB ± 4%  -2.76%  (p=0.003 n=10+10)

name                     old allocs/op  new allocs/op  delta
HaxMapReadsOnly-8            0.00           0.00         ~     (all equal)
HaxMapReadsWithWrites-8       130 ± 4%       126 ± 4%  -2.93%  (p=0.004 n=10+10)

Note that the benchmark is run with perflock to prevent noisy as much as possible:

perflock -governor=70% go1.19 test -run=NONE -bench=. -benchmem -count=10 .
alphadose commented 1 year ago

@cuonglm can you paste the neofetch of your system ?

cuonglm commented 1 year ago

@cuonglm can you paste the neofetch of your system ?

            .-/+oossssoo+\-.               cuonglm@cuonglm-ThinkPad-X1-Carbon-Gen-9 
        ´:+ssssssssssssssssss+:`           ---------------------------------------- 
      -+ssssssssssssssssssyyssss+-         OS: Ubuntu 22.04.1 LTS x86_64 
    .ossssssssssssssssssdMMMNysssso.       Host: LENOVO 20XXS30800 
   /ssssssssssshdmmNNmmyNMMMMhssssss\      Kernel: 5.14.0-1048-oem 
  +ssssssssshmydMMMMMMMNddddyssssssss+     Uptime: 2 days, 17 hours, 43 mins 
 /sssssssshNMMMyhhyyyyhmNMMMNhssssssss\    Packages: 2350 (dpkg), 9 (snap) 
.ssssssssdMMMNhsssssssssshNMMMdssssssss.   Shell: zsh 5.8.1 
+sssshhhyNMMNyssssssssssssyNMMMysssssss+   Resolution: 3840x2400 
ossyNMMMNyMMhsssssssssssssshmmmhssssssso   DE: GNOME 42.2 (Wayland) 
ossyNMMMNyMMhsssssssssssssshmmmhssssssso   Theme: Yaru-dark [GTK2/3] 
+sssshhhyNMMNyssssssssssssyNMMMysssssss+   Icons: Yaru [GTK2/3] 
.ssssssssdMMMNhsssssssssshNMMMdssssssss.   Terminal: gnome-terminal 
 \sssssssshNMMMyhhyyyyhdNMMMNhssssssss/    CPU: 11th Gen Intel i7-1165G7 (8) @ 4.700GHz 
  +sssssssssdmydMMMMMMMMddddyssssssss+     GPU: Intel TigerLake-LP GT2 [Iris Xe Graphics] 
   \ssssssssssshdmNNNNmyNMMMMhssssss/      Memory: 10827MiB / 15730MiB 
    .ossssssssssssssssssdMMMNysssso.
      -+sssssssssssssssssyyyssss+-                                 
        `:+ssssssssssssssssss+:`                                   
            .-\+oossssoo+/-.
cuonglm commented 1 year ago

@alphadose I don't think the system affects the benchmark, just see how my benchmark is more stable that yours, read-write benchmark just have ± 1-2 %. You should make your system idle, or using perflock to lock CPU for running benchmarks.

alphadose commented 1 year ago

@alphadose I don't think the system affects the benchmark, just see how my benchmark is more stable that yours, read-write benchmark just have ± 1-2 %.

I disagree, golang is much more mature on amd64, my system arm64 is something which came up recently hence there is a certain degree of instability.

Although the golang developers are continuously improving the performance and stability on arm64 as seen in their latest patch notes, it seems that for my system unsafe.Slice is slower.

alphadose commented 1 year ago

there are lots of unsafe tricks such as this one from golang core https://cs.opensource.google/go/go/+/refs/tags/go1.19:src/math/unsafe.go;l=18

The linter says not using unsafe.Slice might lead to a possible misuse

but for us there is no problem because everything is allocated and deallocated on the function stack itself, nothing escapes to heap, hence we are not even challenging correctness in the first place.

Not using unsafe.Slice might lead to a possible misuse but it would make no difference in terms of correctness for our case specifically unless there is a bug with proper steps to reproduce the results

cuonglm commented 1 year ago

@alphadose I don't think the system affects the benchmark, just see how my benchmark is more stable that yours, read-write benchmark just have ± 1-2 %.

I disagree, golang is much more mature on amd64, my system arm64 is something which came up recently hence there is a certain degree of instability.

Although the golang developers are continuously improving the performance and stability on arm64 as seen in their latest patch notes, it seems that for my system unsafe.Slice is slower.

In this case, the platform is not problem, you just need to compare before and after. It does not relate to the platform.

cuonglm commented 1 year ago

there are lots of unsafe tricks such as this one from golang core https://cs.opensource.google/go/go/+/refs/tags/go1.19:src/math/unsafe.go;l=18

The linter says not using unsafe.Slice might lead to a possible misuse

but for us there is no problem because everything is allocated and deallocated on the function stack itself, nothing escapes to heap, hence we are not even challenging correctness in the first place.

Not using unsafe.Slice might lead to a possible misuse but it would make no difference in terms of correctness for our case specifically unless there is a bug with proper steps to reproduce the results

I dont see any relation in the link you paste above with the incorrect usage of unsafe.SliceHeader. Please re-read unsafe.Pointer rule 6th.

cuonglm commented 1 year ago

it seems that for my system unsafe.Slice is slower.

Note that like I said in the commit, your benchmark does not mean unsafe.Slice is slow, there're too much noise in your benchmark, which affect the benchmark result. Please lock down your CPU, only the result with less than 5% noisy would make sense.

alphadose commented 1 year ago

Note that like I said in the commit, your benchmark does not mean unsafe.Slice is slow, there're too much noise in your benchmark, which affect the benchmark result. Please lock down your CPU, only the result with less than 5% noisy would make sense.

Will perform the benchmarks and get back to you by tomorrow end

alphadose commented 1 year ago

@cuonglm the latest commit https://github.com/alphadose/haxmap/commit/eee8abe05d83c726178d921bdb498baf74a6e671 fixes all the issues

and it has a lot of performance improvement as well just by using arrays instead of slices 🥳

cuonglm commented 1 year ago

@cuonglm the latest commit https://github.com/alphadose/haxmap/commit/eee8abe05d83c726178d921bdb498baf74a6e671 fixes all the issues

and it has a lot of performance improvement as well just by using arrays instead of slices 🥳

Cool, but any proof? I still see your benchmark result have too much noise.

alphadose commented 1 year ago

Cool, but any proof? I still see your benchmark result have too much noise.

I cannot draw up cleaner benchmarks in which case you should also run the comparisons in your own environment for an additional perspective.

But theoretically the reason for being fast is golang slices have 2 pointer hops whereas arrays have 1 pointer hop for access hence it is faster. My current benchmarks although noisy seem to demonstrate this to a certain extent.