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

DO NOT MERGE: PoC for storing account, access_key in memory #12783

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

mooori
Copy link
Contributor

@mooori mooori commented Jan 23, 2025

Profiles (e.g. the one linked here) have shown that get_accountand get_access_key make up for a significant amount of time in verify_and_charge_transaction. This brought up the idea of storing accounts and access keys in memory.

This PR implements a hacky proof of concept to measure an upper bound of gains achievable by storing accounts and access keys in memory. Why an upper bound? Because this implementation skips work like recording storage access for state witnesses. So this change here breaks many things, but it still allows successfully running the native transfer benchmark in a single-node, single-shard localnet.

There is no chance of ever merging this PR, so I just pushed the first iteration that works, assuming improving its code quality doesn't make sense.

@mooori mooori requested a review from a team as a code owner January 23, 2025 16:47
@mooori mooori requested a review from Longarithm January 23, 2025 16:47
@mooori mooori marked this pull request as draft January 23, 2025 16:48
@mooori mooori removed the request for review from Longarithm January 23, 2025 16:48
@mooori
Copy link
Contributor Author

mooori commented Jan 24, 2025

Currently the cache is used only from one call site of verify_and_charge_transaction, which is sufficient to eliminate the most prominent get_account, get_access_key calls in profiles. I experimented with using the cache from more call sites (e.g. from validate_tx), though that leads to invalid chunks and the node banning itself as chunk producer.

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