Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds a bench for hash_account() #47

Merged
merged 1 commit into from
Mar 4, 2024

Conversation

brooksprumo
Copy link

@brooksprumo brooksprumo commented Mar 4, 2024

Problem

While tweaking the implementation for AccountsDb::hash_account_data(), I didn't have a way to benchmark my changes to see if there were any improvements.

Summary of Changes

Add a bench for hash_account().

Details

After running the benchmark, criterion displays useful information. Here's an example of the text results:

hash_account/data_size/0                                                                            
                        time:   [205.23 ns 205.41 ns 205.63 ns]
                        thrpt:  [375.66 MiB/s 376.07 MiB/s 376.39 MiB/s]
                 change:
                        time:   [+0.1869% +0.2886% +0.3974%] (p = 0.00 < 0.05)
                        thrpt:  [-0.3958% -0.2877% -0.1866%]
                        Change within noise threshold.
hash_account/data_size/165                                                                            
                        time:   [362.53 ns 362.56 ns 362.59 ns]
                        thrpt:  [647.02 MiB/s 647.07 MiB/s 647.12 MiB/s]
                 change:
                        time:   [-0.1588% -0.0725% -0.0067%] (p = 0.06 > 0.05)
                        thrpt:  [+0.0067% +0.0726% +0.1591%]
                        No change in performance detected.
hash_account/data_size/200                                                                            
                        time:   [434.96 ns 435.01 ns 435.07 ns]
                        thrpt:  [615.95 MiB/s 616.04 MiB/s 616.11 MiB/s]
                 change:
                        time:   [-0.0787% +0.0154% +0.0886%] (p = 0.74 > 0.05)
                        thrpt:  [-0.0885% -0.0154% +0.0787%]
                        No change in performance detected.
hash_account/data_size/1024                                                                             
                        time:   [1.4254 µs 1.4261 µs 1.4272 µs]
                        thrpt:  [738.39 MiB/s 738.94 MiB/s 739.29 MiB/s]
                 change:
                        time:   [-0.1822% -0.0684% +0.0270%] (p = 0.22 > 0.05)
                        thrpt:  [-0.0270% +0.0685% +0.1825%]
                        No change in performance detected.
hash_account/data_size/1048576                                                                            
                        time:   [270.04 µs 270.09 µs 270.13 µs]
                        thrpt:  [3.6154 GiB/s 3.6160 GiB/s 3.6166 GiB/s]
                 change:
                        time:   [-0.2249% -0.1810% -0.1467%] (p = 0.00 < 0.05)
                        thrpt:  [+0.1469% +0.1813% +0.2254%]
                        Change within noise threshold.
hash_account/data_size/10485760                                                                             
                        time:   [2.6661 ms 2.6673 ms 2.6685 ms]
                        thrpt:  [3.6596 GiB/s 3.6613 GiB/s 3.6629 GiB/s]
                 change:
                        time:   [+0.1661% +0.2227% +0.2755%] (p = 0.00 < 0.05)
                        thrpt:  [-0.2747% -0.2223% -0.1659%]
                        Change within noise threshold.

And here's two of the many webpages that gets produced.

200

history

Comment on lines +12 to +13
165, // the size of an spl token account
200, // the size of a stake account
Copy link
Author

Choose a reason for hiding this comment

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

Since accounts in this size range make up ~90% of all accounts on mnb, we should optimize hashing for them. In order to quantify any improvements, we gotta benchmark it!

@jeffwashington jeffwashington self-requested a review March 4, 2024 17:58
Copy link

@jeffwashington jeffwashington left a comment

Choose a reason for hiding this comment

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

lgtm

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.8%. Comparing base (3f9a7a5) to head (0bb8276).

Additional details and impacted files
@@            Coverage Diff            @@
##           master      #47     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         837      837             
  Lines      225922   225922             
=========================================
- Hits       184955   184888     -67     
- Misses      40967    41034     +67     

Copy link

@HaoranYi HaoranYi left a comment

Choose a reason for hiding this comment

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

lgtm!

@brooksprumo brooksprumo merged commit 570d1a9 into anza-xyz:master Mar 4, 2024
46 checks passed
@brooksprumo brooksprumo deleted the hash/bench branch March 4, 2024 18:59
codebender828 pushed a commit to codebender828/agave that referenced this pull request Oct 3, 2024
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.

4 participants