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

Async LRU cache for Ethereum block data #540

Conversation

nanocryk
Copy link
Contributor

Follow-up to #479 . Moves cache into a dedicated async task managing the cache, which is requested by various RPC functions using channels.

In the original code, in the scenario of a first request starting to fetch block data: a second request requesting the same block data before the first request cached the data would lead to fetch again the block data instead of waiting for the first result.

This new code prevents to fetch the same block data multiple times, and will instead add all awaiting requesters to a pending list which will get the data once it is fetched (once).

@nanocryk nanocryk requested a review from sorpaas as a code owner December 20, 2021 14:27
Copy link
Member

@sorpaas sorpaas left a comment

Choose a reason for hiding this comment

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

I really like the change to make all RPC functions async, which is future proof, and regardless of the rest of the async LRU changes we should merge this.

@nanocryk Have you done any basic benchmarking of this branch?

@sorpaas
Copy link
Member

sorpaas commented Jan 9, 2022

@nanocryk Just wonder any update on this?

@nanocryk
Copy link
Contributor Author

@sorpaas I'm preparing a branch on top of Moonbeam and it's Frontier fork with the same change to compare the performance with the non-async code.

@nanocryk
Copy link
Contributor Author

I did a few requests and timings seems to stay pretty much the same. I'll do more tests with concurrent requests to check the async cache actually improves performances. Do you have some specific queries in mind that I should benchmark?

@sorpaas
Copy link
Member

sorpaas commented Jan 10, 2022

As long as timing stays the same I think this is good, even we just consider this to be an async refactoring. The situation I worried about is that this would unexpectedly bring performance degradation, which would be bad.

@nanocryk
Copy link
Contributor Author

Picked a few other blocks and performance seems to be mostly the same with and without the change, so looks good to me.

@sorpaas sorpaas merged commit a630bd4 into polkadot-evm:master Jan 11, 2022
abhijeetbhagat pushed a commit to web3labs/frontier that referenced this pull request Jan 11, 2023
* async block data cache

* fmt

* fmt (mix of tabs and spaces?)

* fmt (remove type annotation to stay inline)
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