rust-lang / rustc-stable-hash

A stable hashing algorithm used by rustc: cross-platform, deterministic, not secure
Apache License 2.0
5 stars 3 forks source link

Introduce multi hasher support #8

Closed Urgau closed 2 months ago

Urgau commented 3 months ago

This PR introduce multi Hasher support in our StableHasher by adding a new trait ExtendedHasher that extended the hasher trait with support for short writes and custom hash type and hook it up with StableHasher.

For brevity here is the API for ExtendedHasher:

/// Extended the [`Hasher`] trait for use with [`StableHasher`].
///
/// It permits returning an arbitrary type as the [`Self::Hash`] type
/// contrary to the [`Hasher`] trait which can only return `u64`. This
/// is useful when the hasher uses a different representation.
pub trait ExtendedHasher: Hasher {
    /// Type returned by the hasher.
    type Hash;

    /// Optimized version of [`Hasher::write`] but for small write.
    fn short_write<const LEN: usize>(&mut self, bytes: [u8; LEN]);

    /// Finalization method of the hasher to return the [`Hash`].
    fn finish(self) -> Self::Hash;
}

Unresolved questions:

r? @michaelwoerister Fixes #5

michaelwoerister commented 2 months ago

Thanks, @Urgau!

Before we merge this I think we should make rustc actually use this crate instead of the one in rustc_data_structures. What do you think? It's making me a bit uneasy to keep making changes here without the feedback of them being integrated in the compiler, with respect to performance and also API design.

Urgau commented 2 months ago

I'm maintaining a branch on my rustc fork with the integration of this crate, Urgau:rustc-stable-hash.

I have opened https://github.com/rust-lang/rust/pull/127479 so we can test the perf with those changes included, as well as seeing exactly the changes required to the compiler, which are minimal (nothing major).

I think if the perf are acceptable we should merge this PR, make a release and merge the rustc PR to make use of the crate.

Urgau commented 2 months ago

@michaelwoerister perf are neutral on the rustc side. I think we can merge this PR and prepare the release.

weihanglo commented 2 months ago

Thanks people for making this happen!