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

Switch from (crypto-secure) rand to fastrand #12

Merged
merged 1 commit into from
Aug 8, 2021
Merged

Switch from (crypto-secure) rand to fastrand #12

merged 1 commit into from
Aug 8, 2021

Conversation

red15
Copy link
Contributor

@red15 red15 commented Jul 19, 2021

fastrand should be a lot better suited, less hassle with threads trying to optimize random selections.
did not test the threaded build tbh but should be ok.

@avinassh
Copy link
Owner

@red15 could you test, post the before and after numbers?

@red15
Copy link
Contributor Author

red15 commented Jul 25, 2021

@red15 could you test, post the before and after numbers?

I can't I just cobbled this together on a windows pc and don't have the proper tools to do the timing comparison, I did build and verify with a rust compiler that the code is valid.

@avinassh
Copy link
Owner

no problem at all @red15 :)

Let me benchmark it, post the numbers here.

@red15
Copy link
Contributor Author

red15 commented Aug 5, 2021

Ok got around to running this as intended:

make busy-rust

Master b6671ce94
real    0m11.400s | real    0m11.189s | real    0m10.717s
user    0m10.430s | user    0m10.113s | user    0m9.651s
sys      0m0.963s | sys     0m1.069s  | sys     0m1.060s
fastrand
real    0m9.683s | real    0m9.836s | real    0m9.334s
user    0m8.607s | user    0m8.835s | user    0m8.270s
sys     0m1.065s | sys     0m0.993s | sys     0m1.056s

make busy-rust-thread

Master b6671ce94
real  0m3.707s | real  0m3.966s | real  0m3.473s
user 0m11.510s | user 0m13.007s | user 0m11.566s
sys   0m1.220s | sys   0m1.111s | sys   0m1.167s
fastrand
real  0m3.459s | real 0m3.531s | real 0m2.866s
user 0m10.758s | user 0m9.882s | user 0m9.517s
sys   0m1.139s | sys  0m1.124s | sys  0m1.175s

Not a super big difference but noticeable anyway I'd say ?

@avinassh
Copy link
Owner

avinassh commented Aug 8, 2021

Any reason you just ran for busy threads, but not for actual insertions?

@avinassh
Copy link
Owner

avinassh commented Aug 8, 2021

I ran this on actual insertions, this reduced 2s further. Thank you!

@avinassh avinassh merged commit 756c3bb into avinassh:master Aug 8, 2021
@avinassh
Copy link
Owner

avinassh commented Aug 8, 2021

Sun Aug  8 13:52:32 IST 2021 [RUST] basic_batched.rs (100_000_000) inserts

real	0m32.424s
user	0m30.826s
sys	0m2.272s

Sun Aug  8 13:53:06 IST 2021 [RUST] threaded_batched.rs (100_000_000) inserts

real	0m30.094s
user	0m42.704s
sys	0m3.877s

@red15 red15 deleted the rust_fastrand_import branch August 8, 2021 11:06
Copy link
Contributor Author

@red15 red15 left a comment

Choose a reason for hiding this comment

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

Any reason you just ran for busy threads, but not for actual insertions?

Not sure how to do that? Is that in the Makefile somewhere ?

I ran this on actual insertions, this reduced 2s further. Thank you!

Good to see it matches the improvements I was seeing.

}

pub fn get_random_area_code() -> String {
let mut rng = rand::thread_rng();
format!("{:06}", rng.gen_range(0..999999))
format!("{:06}", fastrand::u32(0..999_999))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using perf record ./target/release/busy I found another optimalization that can be done, the Display::Fmt trait is being called quite often for the area code generation, changing it basically to just become

    fastrand::u32(100_000..999_999).to_string()

saves us from the rather slow Display::Fmt, I can include that change too if you like,

Some numbers of fastrand + display_fmt fix in make busy-rust

real 0m8.211s
user 0m7.208s
sys  0m0.996s

and make busy-rust-thread

real 0m2.605s
user 0m8.691s
sys  0m1.211s

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