feat(iroh-relay)!: use explicit key cache

[rklaehn]

Description

This wires up an explicit key cache to replace the implicit one that was removed in #3051.

The default for a key cache is Disabled. A disabled key cache has a size of 1 pointer and otherwise zero performance overhead.

I have removed the Default instance for both KeyCache and DerpProtocol so you don't accidentally pass the default despite having a cache available.

We use the lru crate for the cache for now. Please comment if it should be something else.

Benchmarks have shown that conversion from a [u8;32] to a VerifyingKey is relatively cheap, so the purpose of the cache is solely to validate incoming public keys.

We add a Borrow instance to PublicKey so we can use it as a cache key.

Some performance measurements:

Benchmarking validate valid ed25519 key: 3.7674 µs
Benchmarking validate invalid ed25519 key: 3.6637 µs
Benchmarking validate valid iroh ed25519 key: 67.089 ns
Benchmarking validate invalid iroh ed25519 key: 64.004 ns

So just from validating incoming keys, without cache you would be limited to ~250 000 msgs/s per thread. At a message size of 1KiB that would be 250MB/s, which is not great.

With the cache deserialization can do 14 000 000 msgs/s, which means that this is no longer a bottleneck.

Breaking Changes

• RelayConfig has new public field key_cache_capacity

Notes & open questions

• Size of the cache
• source_and_box has a PublicKey::try_from. Is that perf critical?

Change checklist

• Self-review.
• Documentation updates following the style guide, if relevant.
• Tests if relevant.
• All breaking changes documented.

[flub]

Purely a drive-by comment. But I've had another reason to add an LRU cache to iroh-relay very recently. So I think this dependency would make it here at some point.

[github-actions]

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3053/docs/iroh/

Last updated: 2024-12-16T18:57:52Z

[github-actions]

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 4f19d1c

[dignifiedquire]

I think a default of maybe 128 or so would be good, as it is very small still but will improve perf for most basic endpoint usages

[rklaehn]

I think a default of maybe 128 or so would be good, as it is very small still but will improve perf for most basic endpoint usages

I made 128 the default for client and 1024 * 1024 the default for a http server. Presumably for the server 32 megabytes (or 56 megabytes if you add the 2 ptrs that LruEntry adds and the key) is no big deal, and it will work up to 1m connections.

[dignifiedquire]

pub(crate)?

[rklaehn]

Does it matter, given this is cfg(test)?

[dignifiedquire]

I am having a hard time finding the place this is configured for a non server endpoint, can you point me to that?

[rklaehn]

I am having a hard time finding the place this is configured for a non server endpoint, can you point me to that?

It is configurable using the ClientBuilder, with a default of 128 as you suggested.

  /// Create a new [`ClientBuilder`]
  pub fn new(url: impl Into<RelayUrl>) -> Self {
      ClientBuilder {
          is_preferred: false,
          address_family_selector: None,
          is_prober: false,
          server_public_key: None,
          url: url.into(),
          protocol: Protocol::Relay,
          #[cfg(any(test, feature = "test-utils"))]
          insecure_skip_cert_verify: false,
          proxy_url: None,
          key_cache_capacity: 128,
      }
  }
...


  /// Set the capacity of the cache for public keys.
  pub fn key_cache_capacity(mut self, capacity: usize) -> Self {
      self.key_cache_capacity = capacity;
      self
  }

Is that not the right place?

[dignifiedquire]

Is that not the right place?

It is, I was just being blind