Skip to content

accounts-db: Use PubkeyHasher in SlotCache#7307

Merged
vadorovsky merged 1 commit intoanza-xyz:masterfrom
vadorovsky:db-slot-cache-pubkey-hash
Aug 7, 2025
Merged

accounts-db: Use PubkeyHasher in SlotCache#7307
vadorovsky merged 1 commit intoanza-xyz:masterfrom
vadorovsky:db-slot-cache-pubkey-hash

Conversation

@vadorovsky
Copy link
Copy Markdown
Member

Problem

SlotCache used pubkeys as keys, yet it used the AHash hasher.

Summary of Changes

To imporove performance of lookups, use PubkeyHasher.

Fixes #

@vadorovsky vadorovsky force-pushed the db-slot-cache-pubkey-hash branch from 6f95cc9 to 153050d Compare August 5, 2025 15:14
`SlotCache` used pubkeys as keys, yet it used the AHash hasher. To
imporove performance of lookups, use `PubkeyHasher`.
@vadorovsky vadorovsky force-pushed the db-slot-cache-pubkey-hash branch from 153050d to 91a1a47 Compare August 6, 2025 12:41
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.2%. Comparing base (c859d8b) to head (91a1a47).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7307   +/-   ##
=======================================
  Coverage    83.2%    83.2%           
=======================================
  Files         797      797           
  Lines      361114   361114           
=======================================
+ Hits       300736   300761   +25     
+ Misses      60378    60353   -25     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vadorovsky vadorovsky marked this pull request as ready for review August 6, 2025 15:33
@brooksprumo
Copy link
Copy Markdown

To imporove performance of lookups, use PubkeyHasher.

Ooh, can you share perf results from this change?

@vadorovsky
Copy link
Copy Markdown
Member Author

To imporove performance of lookups, use PubkeyHasher.

Ooh, can you share perf results from this change?

Well, I can definitely say that pubkey hasher is in general faster:

    #[test]
    fn test_hashmapz() {
        let pubkeys1: Vec<_> = (0..1_000_000).map(|_| Pubkey::new_unique()).collect();
        let pubkeys2 = pubkeys1.clone();

        let mut ahash_map: HashMap<Pubkey, (), ahash::RandomState> =
            HashMap::with_capacity_and_hasher(1_000_000, ahash::RandomState::new());
        let mut pubkeyh_map: HashMap<Pubkey, (), PubkeyHasherBuilder> =
            HashMap::with_capacity_and_hasher(1_000_000, PubkeyHasherBuilder::default());

        let (_, ahash_measure) = measure_us!(for pubkey in pubkeys1 {
            ahash_map.insert(pubkey, ());
        });
        let (_, pubkeyh_measure) = measure_us!(for pubkey in pubkeys2 {
            pubkeyh_map.insert(pubkey, ());
        });

        println!("ahash: {ahash_measure}");
        println!("pubkey hasher: {pubkeyh_measure}");
    }
running 1 test
ahash: 305600
pubkey hasher: 256180
test bank::partitioned_epoch_rewards::calculation::tests::test_hashmapz ... ok

I ran it dozens of times and the results are similar and pubkey hasher always outperforms ahash about ~15%.

Honestly I didn't catch it with profiler. I rather just thought of it when working on #6900 (where I do profiles, but the hashing itself is not the biggest bottleneck) and I started questioning the necessity of AHash in maps with pubkeys that are not really DDOS-able. The change of hasher probably contributes a tiny bit to the overall improvement.

Copy link
Copy Markdown

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@vadorovsky vadorovsky merged commit df89c74 into anza-xyz:master Aug 7, 2025
52 checks passed
@vadorovsky vadorovsky deleted the db-slot-cache-pubkey-hash branch August 7, 2025 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants