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

rpc: Faster sorting for get_token_largest_accounts() #35263

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

brooksprumo
Copy link
Contributor

@brooksprumo brooksprumo commented Feb 20, 2024

Problem

Please see #34584 for context.

The RPC method to get the largest token accounts, we sort everything even though we only return a max of 20.

Summary of Changes

Only sort the top 20 entries.

Note, this PR purposely does not tackle the issue of string parsing. That'll be handled in a subsequent PR.

Copy link

codecov bot commented Feb 20, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (d87e7bc) 81.6% compared to head (6f6d67c) 81.6%.
Report is 14 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master   #35263     +/-   ##
=========================================
- Coverage    81.6%    81.6%   -0.1%     
=========================================
  Files         833      833             
  Lines      224768   224825     +57     
=========================================
+ Hits       183475   183511     +36     
- Misses      41293    41314     +21     

Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

What we should probably do is expand the api to be able to use a BinaryHeap during the accounts scan (like we do for native largest accounts link). But that is a more invasive change. This simple improvement lgtm in the meantime. :shipit:

@brooksprumo brooksprumo merged commit f122e99 into solana-labs:master Feb 20, 2024
35 checks passed
@brooksprumo brooksprumo deleted the rpc/get-largest/sort branch February 20, 2024 22:55
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Feb 27, 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.

2 participants