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

Returns IntMap from select_candidates_by_total_usage() #33976

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

brooksprumo
Copy link
Contributor

@brooksprumo brooksprumo commented Nov 7, 2023

Problem

AccountsDb::select_candidates_by_total_usage() returns a HashMap of slots. Since this function (and return value) is an implementation detail of shrink, it is not part of consensus, and its key (Slot) does not need to be cryptographically secure. So we are free to change it out to be something faster.

Since the Slot itself is a sufficient unique identifier, we can use it directly as the hashing function. This is built in as part of IntMap.

Summary of Changes

Returns IntMap from select_candidates_by_total_usage()

@brooksprumo brooksprumo added the work in progress This isn't quite right yet label Nov 7, 2023
Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Merging #33976 (fc09fe8) into master (8c5b5f1) will increase coverage by 0.0%.
Report is 10 commits behind head on master.
The diff coverage is 100.0%.

❗ Current head fc09fe8 differs from pull request most recent head e67bbd6. Consider uploading reports for the commit e67bbd6 to get more accurate results

@@           Coverage Diff           @@
##           master   #33976   +/-   ##
=======================================
  Coverage    81.9%    81.9%           
=======================================
  Files         811      811           
  Lines      219354   219354           
=======================================
+ Hits       179749   179756    +7     
+ Misses      39605    39598    -7     

@brooksprumo brooksprumo removed the work in progress This isn't quite right yet label Nov 8, 2023
@brooksprumo brooksprumo marked this pull request as ready for review November 8, 2023 20:47
@brooksprumo brooksprumo self-assigned this Nov 8, 2023
Copy link
Contributor

@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 bba6ea2 into solana-labs:master Nov 8, 2023
33 checks passed
@brooksprumo brooksprumo deleted the shrink/intmap branch November 8, 2023 21:15
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.

2 participants