perf: use ahash for reserved keys set

[jstarry]

Problem

The reserved keys set introduced in #84 for SIMD-0105 uses the default cryptographically secure siphash algorithm used by HashSet. The reserved keys set implementation doesn't require a cryptographically secure hash algorithm so it should use a faster hash algorithm.

Summary of Changes

• Use ahash for the reserved keys set

Fixes #

[mergify]

If this PR represents a change to the public RPC API:

1. Make sure it includes a complementary update to rpc-client/ (example)
2. Open a follow-up PR to update the JavaScript client @solana/web3.js (example)

Thank you for keeping the RPC clients in sync with the server API @jstarry.

[jstarry]

This change is technically a breaking change given that the reserved keys set and its consumers primarily live in the sdk crate. We already broke the API in v2.0 and so maybe one could argue that a backport to v2.0 before mainnet moves over would be ok. But still far from ideal and will inevitably still break some downstream users.

@apfitzge how far away are we from being able to make a change like this to an internal transaction type which is not subject to a backwards compatibility policy?

[apfitzge]

can we just make this generic over any hasher? it doesn't really matter which hasher we use, right?

for performance in validator we can use ahash, but for client-side i think using default is fine, and being generic would probably make this not break most existing code?

[jstarry]

for performance in validator we can use ahash, but for client-side i think using default is fine, and being generic would probably make this not break most existing code?

That's not a bad idea but I don't think it will make things much better. Without generics you could be passing HashSet::default() and not notice the breaking change because it will create an ahashset but if you pass HashSet::new() you will get a compile error. With generics passing HashSet::default() would trigger a compile error because you need to specify which hash you're using, but passing HashSet::new() would be fine.

It would be nice if we could specific a default type for the generic but support for doing that on functions is getting phased out according to this error I got:

error: defaults for type parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions
   --> sdk/program/src/message/sanitized.rs:134:36
    |
134 |     pub fn try_from_legacy_message<S: BuildHasher = std::hash::RandomState>(
    |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

So I don't think there's anyway this is not going to be a breaking change

[apfitzge]

So I don't think there's anyway this is not going to be a breaking change

Got it - still would be nicer (imo) to have flexibility in hash function. ahash has been fastest I was aware of, but there could be other faster ones in the future.
If we're gonna break it, let's break it to be more flexible 😄

[apfitzge]

Thought maybe we could define a wrapper type with a default generic param for the internal map.
Rust's syntax is...questionable in such cases: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=678c5c3a33aaeed6f7e9ca63679d44ab

EDIT:

interestingly, as pointed out to me on twitter, it does work with Default::default(). but since this is user-facing, that's probably a bit odd