[dmkozh]

Description

Replace the single global mutex + RandomEvictionCache with 16 sharded caches (each with its own mutex) so concurrent signature verification threads no longer block on a shared lock.

Also use the single-lookup maybeGet() in place of the exists()/get() double-lookup and use Rust verification unconditionally (an overdue cleanup).

Checklist

• Reviewed the contributing document
• Rebased on top of master (no merge commits)
• Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
• Compiles
• Ran all tests
• If change impacts performance, include supporting evidence per the performance document

[Copilot]

Pull request overview

This PR reduces lock contention in the global Ed25519 signature-verification result cache by sharding it into multiple independently-locked caches, improving throughput when signature verification occurs concurrently across threads. It also removes the protocol-gated switch to Rust ed25519-dalek verification and uses the Rust verifier unconditionally.

Changes:

• Replace the single global RandomEvictionCache + mutex with 16 sharded caches (one mutex per shard).
• Switch cache lookups from exists()/get() to maybeGet() to avoid a double hash-table lookup on hits.
• Remove the protocol-version-triggered enableRustDalekVerify() mechanism and always verify via the Rust dalek bridge.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/main/ApplicationImpl.cpp	Removes protocol-version-based enablement of Rust dalek verification during startup.
src/ledger/LedgerManagerImpl.cpp	Removes protocol-version-based enablement of Rust dalek verification when setting LCL.
src/herder/Upgrades.cpp	Removes Rust dalek enablement at the protocol version upgrade boundary.
src/crypto/test/CryptoTests.cpp	Removes explicit test-time enabling of Rust dalek verification.
src/crypto/SecretKey.h	Removes the enableRustDalekVerify() declaration and related comments.
src/crypto/SecretKey.cpp	Implements sharded verifySig cache, atomic hit/miss counters, and unconditional Rust dalek verification.

[SirTyson]

A few nits. One performance observation though. While this is an improvement, we still have a blocking dependency when we hash anything due to gKeyMutex. This actually make a pretty significant delta here, I had AI put together a benchmark test, and we got 10x better throughput when we removed this mutex (assuming all threads saturated with 100% cache hit rate). In practice it won't be that significant, but still seems non-trivial, and I assume there's other places where we hash state in a multi-threaded context.

It doesn't seem like we actually need this mutex either. It's just to protect our initial hash seed, which we just set once on startup. We do modify the seed in test contexts, but is there any reason we can't just move the mutex behind BUILD_TESTS?

[dmkozh]

One performance observation though. While this is an improvement, we still have a blocking dependency when we hash anything due to gKeyMutex.

ShortHash locking has been removed a while ago, this branch is just very old. So it shouldn't be an issue anymore.

FWIW I've also prototyped the lock-free version of cache, but it's sketchy and doesn't seem worth it based on benchmarks. Also, there was yet another global mutex, which was easily replaceable with atomics.

[Copilot]

🟡 Changes recommended

Concurrent metric drains can produce inconsistent counts, and shard selection causes hash-bucket clustering on Windows.

2 open findings

• Use upper hash bits for shard selection · New
• Prevent inconsistent hit and lookup metric flushes · New

🧠 Review effort: Balanced

[SirTyson]

Thanks, LGTM!