Feature request: compile-time hashes for `HeaderName::from_static`

[itrofimow]

Hi! An almost complete rust newbie here (coming from C++, mostly), so please forgive my ignorance.

---

I believe it's a pretty common practice to have some company-wide standard headers for, say, tracing/authorization/etc., which are de-facto StandardHeaders in that sense. However, since they aren't actually values of that enum, hashing them calls into FNV (i assume hashing a StandardHeader only hashes its discriminant, am i right?), and that FNV usage is measurable.

For example, i took the axum implementation from TechEmpower benchmarks, and did this: https://github.com/itrofimow/FrameworkBenchmarks/pull/7/files Looking at flamegraphs, the contains_key amounts to ~3% of total CPU usage, most of it being spent in FNV, when replacing the CUSTOM_HEADERS with [ACCESS_CONTROL_ALLOW_CREDENTIALS, ACCESS_CONTROL_ALLOW_HEADERS, ACCESS_CONTROL_EXPOSE_HEADERS] (something of comparable length, basically) leads to the contains_key only taking ~0.3%.

---

Does it make sense to implement the optimization proposed, or do its drawbacks outweigh its performance benefits? I have a clear understanding of how that could be done in C++ (we actually do so in the web-framework of ours, with its HeaderMap implementation being heavily inspired by what hyper offers), and would be glad to give it a try in Rust.

[seanmonstar]

Hey there!

It could make sense, I think it would depend on what the implementation looks like. Would you be willing to discuss it here first? That could help save time before code is written :)

[itrofimow]

Thank you for the, dare i say, blazingly fast response :)

Sure. In my mind it could be implemented either via adding an Option<u64> compile_time_hash into Custom, with the hash implementation hijacked for the fast-hash case, or adding a new CustomStatic representation, which basically does the same, but is a separate type. I don't really know yet how (and where) to hijack the hash, but i assume it should be doable in somewhat straightforward way; i'd need to dive a bit deeper into the project to figure it out, which i'm ready to do.

In both cases something like https://crates.io/crates/const-fnv1a-hash would be needed, too.

[seanmonstar]

It's a fine thing to explore, but we'll want to weigh how much adding that affects performance of other parts. If it's much slower in other paths, we might not end up accepting the change.

I imagine we don't really need another dependency, the FNV algorithm is tiny and having the const fns in this repo would be fine (probably even removing the need for the fnv dependency in the first place).

[itrofimow]

I've got a draft for this, which right now apparently breaks dependent crates because of

error: to use a constant of type http::header::name::Repr<http::header::name::Custom> in a pattern, http::header::name::Repr<http::header::name::Custom> must be annotated with #[derive(PartialEq, Eq)]

Not really sure what this is about, but i guess there should be a workaround.

---

Benchmarks show a slowdown for header_name_from_static, which is expected, however other benchmarks are a surprise to me: https://pastebin.com/raw/N1Mqd7EE. Everything header_map-related became faster, some benchmarks for other map implementations became substantially slower, and i have no clue why.

These results confuse me, and i would assume I've done something completely nonsensical if not for the tests passing; could you @seanmonstar please have a glance on this? - I don't request a review yet, more of a sanity check, and whether this direction has potential.