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

use binary search. #51

Closed
wants to merge 2 commits into from
Closed

use binary search. #51

wants to merge 2 commits into from

Conversation

jerome-laforge
Copy link

Hello,
Is it possible to use binary search for func (a *alphabet) Index(t rune) (int64, error)?

Thanks

@lithammer
Copy link
Owner

lithammer commented Feb 27, 2024

Hmm, benchmarks seems to indicate that this is slower. I guess maybe because of the small search space...?

           │   old.txt   │               new.txt               │
           │   sec/op    │   sec/op     vs base                │
UUID-8       2.965µ ± 3%   2.942µ ± 0%        ~ (p=0.085 n=10)
Encoding-8   2.428µ ± 2%   2.413µ ± 1%   -0.62% (p=0.002 n=10)
Decoding-8   368.4n ± 0%   448.5n ± 1%  +21.74% (p=0.000 n=10)
geomean      1.384µ        1.471µ        +6.28%

           │   old.txt    │                new.txt                │
           │     B/op     │     B/op      vs base                 │
UUID-8       2.588Ki ± 0%   2.588Ki ± 0%       ~ (p=1.000 n=10) ¹
Encoding-8   2.578Ki ± 0%   2.578Ki ± 0%       ~ (p=1.000 n=10)
Decoding-8     152.0 ± 0%     152.0 ± 0%       ~ (p=1.000 n=10) ¹
geomean       1020.7         1020.7       +0.00%
¹ all samples are equal

           │  old.txt   │               new.txt               │
           │ allocs/op  │ allocs/op   vs base                 │
UUID-8       98.00 ± 0%   98.00 ± 0%       ~ (p=1.000 n=10) ¹
Encoding-8   98.00 ± 1%   98.00 ± 0%       ~ (p=1.000 n=10)
Decoding-8   5.000 ± 0%   5.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean      36.35        36.35       +0.00%
¹ all samples are equal

@jerome-laforge
Copy link
Author

You are right.
Even with biggest random shortuuid list, I have slower decoding.
So we can reject this PR.

@lithammer lithammer closed this Feb 29, 2024
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