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

Lazy calculate hash for accounts stored in the cache #15617

Closed
wants to merge 3 commits into from

Conversation

jeffwashington
Copy link
Contributor

@jeffwashington jeffwashington commented Mar 2, 2021

Problem

While processing a slot, the same account can be written to many times.
Hashing accounts is not free and gets expensive with large data. Hashing an account that is not the final update does not appear to be necessary.

Summary of Changes

When an account is stored to the cache, don't hash the contents first - instead use the default hash. When behavior requires a hash, calculate it when necessary. Ideally, once we calculate it, it would be stored in the cache. I don't know enough yet about the race conditions possible in the cache to attempt to update the cached entry with a new hash. I can work on that if we decide this is optimization overall is useful.

Fixes #

@jeffwashington jeffwashington added the CI Pull Request is ready to enter CI label Mar 2, 2021
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Mar 2, 2021
@jeffwashington jeffwashington added the CI Pull Request is ready to enter CI label Mar 2, 2021
@jeffwashington jeffwashington force-pushed the cache_hash branch 3 times, most recently from d0038a1 to 3c59a9e Compare March 2, 2021 17:21
@jeffwashington jeffwashington added CI Pull Request is ready to enter CI and removed CI Pull Request is ready to enter CI labels Mar 2, 2021
@jeffwashington jeffwashington added CI Pull Request is ready to enter CI and removed CI Pull Request is ready to enter CI labels Mar 2, 2021
@jeffwashington jeffwashington changed the title Cache hash Lazy calculate hash for accounts stored in the cache Mar 3, 2021
@stale
Copy link

stale bot commented Mar 19, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Mar 19, 2021
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Mar 19, 2021
@stale stale bot removed the stale [bot only] Added to stale content; results in auto-close after a week. label Mar 19, 2021
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