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

svm: optimize filter_executable_program_accounts #4574

Closed
wants to merge 5 commits into from

Conversation

2501babe
Copy link
Member

@2501babe 2501babe commented Jan 22, 2025

Problem

presently, filter_executable_program_accounts makes an expensive accounts-db lookup for every account in every transaction batch. this used to be an unfortunate necessity because transaction account loading could not happen until the local program cache was built. a recent feature gate activation has severed that dependency, giving us freedom to improve this

Summary of Changes

initialize the AccountLoader before constructing the program cache, and change filter_executable_program_accounts to use it instead of accounts-db. this eliminates all expensive calls to account_matches_owners. this change does not increase the number of calls to get_account_shared_data because the same AccountLoader object is then used for transaction loading

we also remove the program owner from program_accounts_map. this was removed in an early version of 2.0 but had to be added back; we can now definitively remove it, since it is no longer required for owner validation in transaction loading

we also fix an incidental bug where usage counts of builtin programs were reset to zero

Notes

the initial attempt at this pr (#4553) tried to move program cache creation after account loading, creating one local program cache per transaction. however we cannot easily do this: under normal operation, config.check_program_modification_slot is false. this makes it impossible to determine whether a loaderv3 program should be tombstoned because it was modified in-entry. presently this behaves correctly as a side effect of reusing the same local program cache

in the long term we do want to move in this direction, cutting back the functionality of the local program cache into a much simpler passthrough to retrieve compiled/verified program bytecode, with extremely minimal internal state-tracking and no merge/update logic. this might be straightforward if we can simply remove check_program_modification_slot and make it behave as if it were always true, but this may require a feature gate

in the interim we may also be able to remove accounts-db lookups from replenish_program_cache and its children by using AccountLoader instead of the callback. this should not require major redesign but it is complicated by the fact that load_program_with_pubkey and load_program_accounts are required by Bank. in theory we could just impl TransactionProcessingCallback for AccountLoader but this has soundness concerns because it itself contains a TransactionProcessingCallback type and merits further discussion

@2501babe 2501babe self-assigned this Jan 22, 2025
@2501babe
Copy link
Member Author

this accomplishes its goal perfectly. unfortunately we cannot use it because it reintroduces the issue where accounts are prefetched before transaction size checks. we will need to do more extensive work on the program cache along the lines of #4553

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.

1 participant