Replaces ReadAccountMapEntry in read_index_for_accessor_or_load_slow() #35220
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #35220 +/- ##
=========================================
- Coverage 81.6% 81.6% -0.1%
=========================================
Files 833 833
Lines 224765 224776 +11
=========================================
- Hits 183480 183476 -4
- Misses 41285 41300 +15 |
brooksprumo
force-pushed
the
self-ref/2/read_index_for_accessor_or_load_slow
branch
2 times, most recently
from
2e99fc1 to
f57b729
Compare
brooksprumo
changed the title
brooksprumo
commented
Feb 19, 2024
Comment on lines
-5104
to
-5118
| // `lock` is dropped here rather pretty quickly with clone_in_lock = false, | ||
| // so the entry could be raced for mutation by other subsystems, | ||
| // before we actually provision an account data for caller's use from now on. | ||
| // This is traded for less contention and resultant performance, introducing fair amount of | ||
| // delicate handling in retry_to_get_account_accessor() below ;) | ||
| // you're warned! |
There was a problem hiding this comment.
There's a lot of comments that I removed in this function. I feel like some is out-of-date and is fine to remove, but some may still be useful... Any thoughts on what should stay and how/where to put it?
There was a problem hiding this comment.
it is hard to fit them in. They are outdated in a few ways. clone_in_lock is straightforward to understand. I'm ok with removing the comments. The code is simpler.
HaoranYi
reviewed
Feb 22, 2024
brooksprumo
force-pushed
the
self-ref/2/read_index_for_accessor_or_load_slow
branch
from
f57b729 to
ddce2f2
Compare
|
I rebased and force-pushed this branch to pull in #35307. |
HaoranYi
approved these changes
Feb 26, 2024
brooksprumo
deleted the
self-ref/2/read_index_for_accessor_or_load_slow
branch
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
See #34786 for background.
We want to limit the use of
ReadAccountMapEntry, from AccountsIndex, everywhere. Ultimately removing it once there are no more uses.When reading the index for an account accessor, we currently use
ReadAccountMapEntryviaAccountsIndex::get(). But we can do it other ways withoutReadAccountMapEntry.Summary of Changes
Adds
get_with_and_then()and uses it inread_index_for_accessor_or_load_slow()