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

Const-ify hashing of Key. #241

Open
tobz opened this issue Nov 1, 2021 · 2 comments
Open

Const-ify hashing of Key. #241

tobz opened this issue Nov 1, 2021 · 2 comments
Labels
C-core Component: core functionality such as traits, etc. E-complex Effort: complex. S-waiting-on-upstream Status: issue/PR requires an upstream change to either a dependency or Rust itself T-enhancement Type: enhancement.

Comments

@tobz
Copy link
Member

tobz commented Nov 1, 2021

A while back, we added support to pre-generate the hash of a Key in order to speed up its hashing performance when used with metrics_util::Registry. One part that I was never truly satisfied with was that we had to accomplish it by using atomics so that we could store the hash after construction. This was required because:

  • we had no way to do hashing in a const fn (had to be hashed on first access)
  • Key is passed around as &Key, so it has to be doable with only an immutable reference available

Since then, many features related to const fns have become stabilized. While not all of the necessary features are stabilized yet, I was able to craft a solution (a lot of unsafe code, unfortunately) that can do entirely const hashing of a key: https://gist.github.com/tobz/fec84b1ac850aeaef216608a8585c982

We should pay attention to the next few Rust releases to see what other const features get stabilized. The primary grossness of the approach taken in the gist relates to std::slice::from_raw_parts not having a stabilized const impl, so we reimplemented a lot of the union trickery that gets used to safely transmute between *const u8 and *const [u8], which provides the basis of jumping to *const str/&str. A recent PR was merged for that which should hopefully land in 1.58.0, but there might be other useful stabilizations that let us clean up more of the code.

@tobz tobz added C-core Component: core functionality such as traits, etc. E-complex Effort: complex. T-enhancement Type: enhancement. labels Nov 1, 2021
@tobz
Copy link
Member Author

tobz commented Dec 23, 2021

Checking in, the previously mentioned "recent PR" that was merged was rust-lang/rust#90377.

The tracking issue hasn't mentioned anything new, but the PR was/is still marked as being part of the 1.58.0 milestone, so hopefully that holds true. We might want to move forward with the hacky approach since it should at least be stable/reliable, even if ugly. We could eventually update ourselves to use simpler approaches once those Rust versions are released as stable.

Otherwise, we'd be waiting for 1.58.0 at a minimum and then forcing an MSRV bump.

@tobz tobz added S-future-work Status: valid as a potential future work item, but not currently prioritized S-waiting-on-upstream Status: issue/PR requires an upstream change to either a dependency or Rust itself and removed S-future-work Status: valid as a potential future work item, but not currently prioritized labels Jan 3, 2022
@tobz
Copy link
Member Author

tobz commented Jul 22, 2022

Recent movement: rust-lang/rust#97522

This stabilizes the const fn version of std::slice::from_raw_parts, which will land in 1.64. Quite a ways away, especially given our MSRV policy... but for the sake of tracking it, I'm commenting here.

See you in like 6-7 months, const hashing of keys. 🤣 😭

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-core Component: core functionality such as traits, etc. E-complex Effort: complex. S-waiting-on-upstream Status: issue/PR requires an upstream change to either a dependency or Rust itself T-enhancement Type: enhancement.
Projects
None yet
Development

No branches or pull requests

1 participant