[PATCH] D155781: [Support] Change StringMap hash function from xxHash64 to xxh3_64bits

Fri Jul 21 22:00:25 PDT 2023

MaskRay added a comment.

In D155781#4524570 <https://reviews.llvm.org/D155781#4524570>, @erikdesjardins wrote:

> FYI, I ran this against the Rust compiler performance test suite and it didn't make a noticeable difference compared to the initial switch from DJB -> xxhash. (Which makes sense--DJB is so slow that it's hard to make an improvement that's anywhere close to that.)
> It does seem to improve an artificial benchmark with very large constant strings by ~3% locally.

Thank you for testing this patch! ~3% is still good.

> It of course still makes sense to do this, even if only to remove the old xxh64.

Exactly, to make it clear that xxh64 is only for exceptions and only granted very carefully. Generally xxh3 should be used.

> LGTM with the noted issues resolved.

I've managed to fix 10+ instances that relied on the iteration order. D155789 <https://reviews.llvm.org/D155789> is now in place (tested by [[ https://lab.llvm.org/buildbot/#/builders/54 | a bot named reverse-iteration for `llvm;clang;polly` ]]) to prevent back sliding.
This patch is now as simple as changing just the StringMap code:)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155781/new/

https://reviews.llvm.org/D155781