Skip to content

Conversation

@rgrinberg
Copy link
Member

@rgrinberg rgrinberg commented Apr 30, 2025

This requires 3.19 to be released

@rgrinberg rgrinberg force-pushed the ps/rr/feature__vendor_blake3_mini branch 3 times, most recently from 9ad19bc to 56a3dcc Compare April 30, 2025 16:37
Signed-off-by: Rudi Grinberg <[email protected]>

<!-- ps-id: d57eeaab-ba78-46d4-b31c-1995107817df -->

Signed-off-by: Rudi Grinberg <[email protected]>
@rgrinberg rgrinberg force-pushed the ps/rr/feature__vendor_blake3_mini branch from 56a3dcc to 5846fd3 Compare May 1, 2025 10:10
@Leonidas-from-XIV
Copy link
Collaborator

While skimming the docs I learned that OCaml 5.2 started supporting BLAKE2 in its Digest module. Maybe using BLAKE2 would make more sense, then Dune could stop vendoring BLAKE2 in the future when the minimum supported version is bumped to 5.2?

@rgrinberg
Copy link
Member Author

BLAKE2 is only marginally faster than md5 unfortunately. You can see the bench results on the main page of the repo https://github.com/BLAKE3-team/BLAKE3, and I've confirmed them as well. It wouldn't be worth it to use BLAKE2

@Leonidas-from-XIV
Copy link
Collaborator

Yeah, I didn't know what your goal was for replacing MD5, because it can be either

  1. Get a safer cryptographic hash
  2. Get a faster hash

As for 1 BLAKE2 is a significant improvement, because MD5 has been outdated for many many years and just about any halfway modern hash is better. As for faster hashing, I wonder if a non-cryptographic hash like SipHash or MurmurHash wouldn't be a better choice? What are you planning to use BLAKE3 for?

@rgrinberg
Copy link
Member Author

I'd like dune to be faster at creating content digests for build artifacts.

So my goal is 2. while being limited by 1.

I have experimented with non cryptographic hashes, and while they could work it probably doesn't make sense to relax our digest guarantees if we can speed things while maintaining them.

@rgrinberg rgrinberg closed this May 29, 2025
@Leonidas-from-XIV
Copy link
Collaborator

@rgrinberg Why was this closed? 3.19 has been released, so I think this just required a rebase?

@Alizter
Copy link
Collaborator

Alizter commented May 30, 2025

@Leonidas-from-XIV it was superseded by the PR that switches to blake3.

@rgrinberg
Copy link
Member Author

Notably this PR #11735

@Leonidas-from-XIV
Copy link
Collaborator

Ah, thanks! I somehow missed that.

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.

4 participants