Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: switch to go-art #317

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Clement-Jean
Copy link

Tried the switch to go-art and here are the results I get in the Put/Get benchmark (which I modified to use Golang benchmark features):

goos: darwin
goarch: arm64
pkg: github.com/ByteStorage/FlyDB/db/engine
cpu: Apple M1 Max
        │   old.txt   │              new.txt               │
        │   sec/op    │   sec/op     vs base               │
Put-10    677.0n ± 1%   656.2n ± 0%  -3.07% (p=0.000 n=50)
Get-10    443.8n ± 1%   441.1n ± 1%       ~ (p=0.222 n=50)
geomean   548.1n        538.0n       -1.84%

        │  old.txt   │               new.txt               │
        │    B/op    │    B/op     vs base                 │
Put-10    526.0 ± 0%   474.0 ± 0%  -9.89% (n=50)
Get-10    304.0 ± 0%   304.0 ± 0%       ~ (p=1.000 n=50) ¹
geomean   399.9        379.6       -5.07%
¹ all samples are equal

        │  old.txt   │               new.txt                │
        │ allocs/op  │ allocs/op   vs base                  │
Put-10    14.00 ± 0%   12.00 ± 0%  -14.29% (n=50)
Get-10    8.000 ± 0%   8.000 ± 0%        ~ (p=1.000 n=50) ¹
geomean   10.58        9.798        -7.42%
¹ all samples are equal

Let me know what you think about this. Also, I can reset the benchmark to the original version if you need to. Just thought it would be a little bit more accurate to use Golang benchmarks.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for your contribution, we will promptly review your code if there are no errors and pass ci. We will merge your pull request into the master branch.

@qishenonly
Copy link
Member

Thanks for your PR! To maintain clean code structure, please reimplement this based on a new data structure rather than modifying the existing one. This helps avoid compatibility issues and simplifies future maintenance. Looking forward to your updates!

@Clement-Jean
Copy link
Author

If anyone is willing to implement this the way you want, you can expect even better performance now. Just check the latest go-art commit. On my computer the benchmark looks like this:

goos: darwin
goarch: arm64
pkg: github.com/ByteStorage/FlyDB/db/engine
cpu: Apple M1 Max
        │   old.txt   │              new.txt               │
        │   sec/op    │   sec/op     vs base               │
Put-10    690.2n ± 1%   656.1n ± 3%  -4.94% (p=0.000 n=10)
Get-10    438.3n ± 2%   418.6n ± 3%  -4.48% (p=0.001 n=10)
geomean   550.0n        524.1n       -4.71%

        │  old.txt   │               new.txt                │
        │    B/op    │    B/op     vs base                  │
Put-10    526.0 ± 0%   464.0 ± 0%  -11.79% (p=0.000 n=10)
Get-10    304.0 ± 0%   304.0 ± 0%        ~ (p=1.000 n=10) ¹
geomean   399.9        375.6        -6.08%
¹ all samples are equal

        │  old.txt   │               new.txt                │
        │ allocs/op  │ allocs/op   vs base                  │
Put-10    14.00 ± 0%   12.00 ± 0%  -14.29% (p=0.000 n=10)
Get-10    8.000 ± 0%   8.000 ± 0%        ~ (p=1.000 n=10) ¹
geomean   10.58        9.798        -7.42%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants