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

internal/util: new UniqRands() implementation #1019

Merged
merged 1 commit into from
Apr 2, 2018

Conversation

ploxiln
Copy link
Member

@ploxiln ploxiln commented Apr 2, 2018

using the Fisher-Yates shuffle algorithm

faster and more consistent runtime when
quantity is near maxval (aka l is near n)

using the Fisher-Yates shuffle algorithm

faster and more consistent runtime when
quantity is near maxval (aka l is near n)
@ploxiln
Copy link
Member Author

ploxiln commented Apr 2, 2018

Benchmark recap. Before:

BenchmarkUniqRands5of5-4         2000000               887 ns/op
BenchmarkUniqRands20of20-4        200000              6932 ns/op
BenchmarkUniqRands20of50-4        500000              3432 ns/op

After:

BenchmarkUniqRands5of5-4         5000000               228 ns/op
BenchmarkUniqRands20of20-4       2000000               832 ns/op
BenchmarkUniqRands20of50-4       2000000               885 ns/op

@ploxiln
Copy link
Member Author

ploxiln commented Apr 2, 2018

@vearne does this look good to you?

(Notice that I kept the author name/email of your commit, but your email address in your commit is not associated with your github account.)

@vearne
Copy link
Contributor

vearne commented Apr 2, 2018

@ploxiln I have add a email and associated with my github account.

@ploxiln
Copy link
Member Author

ploxiln commented Apr 2, 2018

Cool. We don't require it, I just figured you might wonder why the commit doesn't seem to credit your github account.

@ploxiln ploxiln merged commit bd08d63 into nsqio:master Apr 2, 2018
@ploxiln ploxiln deleted the uniq_rands_v2 branch June 3, 2018 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants