Merging this PR will not alter performance

✅ 70 untouched benchmarks
⏩ 129 skipped benchmarks¹

_{Comparing felix/021726-build (f447aa2) with main (fcf3784)}

Looks like clang-tidy is getting errors with an intrinsics header when parsing src/workerd/tests/bench-tools.h 🙄
This is happening since we are using clang-tidy-21 while still using LLVM19 otherwise and LLVM19 headers try to use an MMX builtin that has been removed in LLVM 21. This seems like too small of a nuisance to justify updating to (and requiring) LLVM21, I'll just disable header parsing for that target and add a comment for now.

LGTM

This should fix #5977 by ensuring both places where we compute SQL-related hashes use hardware acceleration.

Should/can we add a test for this?

We should – @FlorentCollin said that he could write a test for this, the only wrinkle is that it will merely cause a "HashIndex detected hash table inconsistency" warning if there was a regression. In the future, we should have some way to have errors in tests cause an exception instead of just a log, for doing this in capnproto we might add a KJ_DASSERT (that way it would not just affect tests but be limited to debug builds and thus not affect release builds, hopefully good enough).

Can't we set global copt via .bazelrc?

Does this change cover the capnp dependency? I think it's important that it be compiled with consistent flags since hash codes could be passed between workerd code and capnp code.

Can't we set global copt via .bazelrc?

Does this change cover the capnp dependency? I think it's important that it be compiled with consistent flags since hash codes could be passed between workerd code and capnp code.

Unfortunately, we can't do that in a way that works in all edge cases – .bazelrc gives us no way to determine the cpu architecture, so unless we require the user to pass flags like --config=x64 or --config=arm64 we can't enable something for all targets (we could set that in CI, but expecting users to pass that flag or add it to their ~/.bazelrc seems onerous). One way to enforce this is using a custom Bazel C++ toolchain (as we do internally), where we can easily set flags based on the architecture, but that would mean adding a lot more configuration here. Maybe @mikea has some other ideas

This does not include capnproto – that's why I would prefer to always use the same hash function and fall back to using a software implementation, while having mixed compile flags is not ideal it is something that commonly happens when using precompiled libraries that do not enable ISA extensions beyond the x64 baseline (e.g. in apt packages). We'd want to avoid it, but capnproto should be able to handle it gracefully.

We need to figure out how to compile all of workerd -- including capnproto -- with the same flags.

Mixing implementations between translation units is not the right answer, even if they are designed to be equivalent. It's fragile and it's an ODR violation.

If we need to just force these flags on in the capnp bazel build, that's better than what we have now.

We need to figure out how to compile all of workerd -- including capnproto -- with the same flags.

Mixing implementations between translation units is not the right answer, even if they are designed to be equivalent. It's fragile and it's an ODR violation.

If we need to just force these flags on in the capnp bazel build, that's better than what we have now.

We can enable the required flags for all capnproto code by introducing a kj_cc_library macro and using that instead of plain cc_library, similarly to what we do with wd_cc_library in workerd. By using a flag, we can also make it possible to opt out if desired.
That would just cover capnproto (and not all of workerd), but we can also add a poisoning mechanism based on preprocessor defines so that compilation will fail if code that depends on KJ is being compiled with the Bazel flag on but without having the hardware extensions enabled. Does that work for you?