[PATCH] D24458: Using murmurhash2 instead of fnv

[Ed Maste via llvm-commits]

emaste added inline comments.

================
Comment at: ELF/OutputSections.cpp:1744
@@ -1696,3 +1743,3 @@
 template <class ELFT>
-void BuildIdFnv1<ELFT>::writeBuildId(ArrayRef<uint8_t> Buf) {
+void BuildIdMurmur2<ELFT>::writeBuildId(ArrayRef<uint8_t> Buf) {
   const endianness E = ELFT::TargetEndianness;
----------------
ruiu wrote:
> I'd rename this class BuildIdNonCryptoHash or something like that because I think we'd want to change the implementation again in the future. Specifically, we want to use threads to make it faster, but if we do, it is no longer a Murmur hash (because Murmur(concat(X1, X2, ..., Xn)) is different from SomeHash(Murmur(X1), Murmur(X2), ..., Murmur(Xn)).)
The same argument applies to the crypto hashes too; we may well want SomeHash(sha1(X1), sha1(X2), ..., sha1(Xn)). build-id doesn't specify exactly what parts of the file are included or how the hash is generated, and gold and bfd hash differently already.

Should we make it just `BuildIdFastHash`? It's not specifically non-crypto-ness of the hash that we're interested in, it's that it's as fast as possible.


https://reviews.llvm.org/D24458