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

Return loaded program entry from the cache after insert #30336

Merged
merged 2 commits into from
Feb 16, 2023

Conversation

pgarg66
Copy link
Contributor

@pgarg66 pgarg66 commented Feb 15, 2023

Problem

LoadedPrograms::insert_entry() checks if a program exists in the cache that matches deployment and effective slot number for the given program key. It currently returns false in such cases. However, it should return an existing program entry, as that is needed for deduplication in case two transaction batches load the same program simultaneously.

Summary of Changes

This PR changes the return type to an enum LoadedProgramEntry, that conveys if the entry already existed, or was newly added. Also, it carries an Arc to the valid loaded program instance.

Fixes #

@pgarg66 pgarg66 requested a review from Lichtso February 15, 2023 17:40
@pgarg66 pgarg66 force-pushed the return-loaded-program branch from 026281c to 75e90f3 Compare February 15, 2023 17:40
Lichtso
Lichtso previously approved these changes Feb 16, 2023
Copy link
Contributor

@Lichtso Lichtso left a comment

Choose a reason for hiding this comment

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

Should we name them Vacant and Occupied like the std lib does?

https://doc.rust-lang.org/std/collections/hash_map/enum.Entry.html
https://doc.rust-lang.org/std/collections/btree_map/enum.Entry.html

Although, their entry is yet to be inserted.

@pgarg66
Copy link
Contributor Author

pgarg66 commented Feb 16, 2023

Should we name them Vacant and Occupied like the std lib does?

https://doc.rust-lang.org/std/collections/hash_map/enum.Entry.html https://doc.rust-lang.org/std/collections/btree_map/enum.Entry.html

Although, their entry is yet to be inserted.

Occupied would be ok. Vacant sounds misleading. I don't have a very strong opinion though.
How about WasOccupied, and WasVacant?

@pgarg66 pgarg66 force-pushed the return-loaded-program branch from 68c8c15 to e1ce368 Compare February 16, 2023 13:49
@pgarg66 pgarg66 merged commit 86e5965 into solana-labs:master Feb 16, 2023
@pgarg66 pgarg66 deleted the return-loaded-program branch February 16, 2023 15:38
nickfrosty pushed a commit to nickfrosty/solana that referenced this pull request Mar 12, 2023
…30336)

* Return loaded program entry from the cache after insert

* update enum variant labels
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants