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

util: Add rng utilities #686

Merged
merged 9 commits into from
Aug 25, 2022
Merged

util: Add rng utilities #686

merged 9 commits into from
Aug 25, 2022

Conversation

LucioFranco
Copy link
Member

This adds new PRNG utilities that only use libstd and not the external
rand crate. This change's motivation are that in tower middleware that
need PRNG don't need the complexity and vast utilities of the rand
crate.

This adds a Rng trait which abstracts the simple PRNG features tower
needs. This also provides a HasherRng which uses the RandomState
type from libstd to generate random u64 values. In addition, there is
an internal only sample_inplace which is used within the balance p2c
middleware to randomly pick a ready service. This implementation is
crate private since its quite specific to the balance implementation.

The goal of this in addition to the balance middlware getting rand
removed is for the upcoming Retry changes. The next_f64 will be used
in the jitter portion of the backoff utilities in #685.

This adds new PRNG utilities that only use libstd and not the external
`rand` crate. This change's motivation are that in tower middleware that
need PRNG don't need the complexity and vast utilities of the `rand`
crate.

This adds a `Rng` trait which abstracts the simple PRNG features tower
needs. This also provides a `HasherRng` which uses the `RandomState`
type from libstd to generate random `u64` values. In addition, there is
an internal only `sample_inplace` which is used within the balance p2c
middleware to randomly pick a ready service. This implementation is
crate private since its quite specific to the balance implementation.

The goal of this in addition to the balance middlware getting `rand`
removed is for the upcoming `Retry` changes. The `next_f64` will be used
in the jitter portion of the backoff utilities in #685.
H: BuildHasher,
{
fn next_u64(&mut self) -> u64 {
let mut hasher = self.hasher.build_hasher();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you want to build_hasher for each number. That will restart the hasher back to the beginning. I'd expect you'd build a hasher once, and then feed the counter into it over and over. Or am I wrong?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought finished consumed it, but I guess finish just takes a &self. Let me see if that makes any meaningful difference.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yea, thinking more, I could be much more wrong than right.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok I've updated it seems to work....

Copy link
Member Author

Choose a reason for hiding this comment

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

Follow up here HashMap creates a hasher for each hash so we are reverting back to that setup.

tower/src/util/rng.rs Outdated Show resolved Hide resolved
tower/src/util/rng.rs Outdated Show resolved Hide resolved
@LucioFranco LucioFranco merged commit e055826 into master Aug 25, 2022
@LucioFranco LucioFranco deleted the lucio/remove-rand branch August 25, 2022 17:06
@LucioFranco LucioFranco mentioned this pull request Aug 25, 2022
19 tasks
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.

3 participants