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

cache polish #1039

Merged
merged 12 commits into from
Nov 5, 2024
Merged

cache polish #1039

merged 12 commits into from
Nov 5, 2024

Conversation

shunjizhan
Copy link
Collaborator

@shunjizhan shunjizhan commented Oct 31, 2024

Change

There seems to be a bug with our storage query helper, which is very random and cannot be reproduced (so far). Before we put too much bandwidth catching the tricky bug, we can use a slightly less optimized but more stable approach: classy way to query storage with apiAt.

Also:

  • fixed a bug in block author calculation.
  • added a couple new caches for common calculations

Test

All tests still passed, also added tests to make sure cached request are much faster

Performance Analysis

These analysis are mostly in theory, we can see how it actually goes by taking a snapshot of current RPC performance, and compare.

There are two main steps for a storage call:

  1. construct apiAt
  2. make the storage call

step 1

apiAt construction already has internal cache for runtimeVersion => registries, so the only repeated part is the decoration process. These local calculations should take much less time than waiting for the async call in step 2 (TBD).

We can still alleviate repeated decoration by caching blockHash => apiAt. In practice most of the calls should be targeting recent blocks, caching only 100 apiAt could already have a sufficiently great hit rate, while maintaining a low memory footprint.

step 2

We don't necessarily need to cache storage data, since the hit rate will likely be low. Each storage call queries for a specific block with very specific params, such as "nonce for address XXX at block YYY", which is not very likely to repeat.

On the other hand, cacheing blockHash => * should have a much better hit rate, such as blockHash => apiAt mentioned above. We should also cache the (very) few common queries that are heavily repeated:

  • blockNumber => blockHash
  • blockHash => blockData
  • blockHash => header (implicitly cached blockHash => blockNumber)

@shunjizhan shunjizhan marked this pull request as ready for review November 3, 2024 09:41
@shunjizhan shunjizhan requested review from xlc and zjb0807 November 3, 2024 09:41

return result as any as T;
};

Copy link
Member

Choose a reason for hiding this comment

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

#332 (comment)

Not sure if this will cause a timeout in getting Metadata. Blockscout will query the account balances of many block hashes. We can run Blockscout for a few days to test it.

@ntduan please review this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fetching metadata in parallel should already be improved polkadot-js/api#4708, but yeah we can try and see how it actually goes

@shunjizhan
Copy link
Collaborator Author

shunjizhan commented Nov 4, 2024

an ideal way would still be querying storage directly without decorating other parts, but better through api itself polkadot-js/api#6015

@shunjizhan shunjizhan merged commit 6398220 into master Nov 5, 2024
14 checks passed
@shunjizhan shunjizhan deleted the cache-polish branch November 5, 2024 06:53
Copy link

codecov bot commented Nov 5, 2024

Codecov Report

Attention: Patch coverage is 94.62366% with 5 lines in your changes missing coverage. Please review.

Project coverage is 78.04%. Comparing base (df76528) to head (b0e18c8).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
packages/eth-providers/src/base-provider.ts 95.94% 3 Missing ⚠️
packages/eth-providers/src/utils/parseBlock.ts 71.42% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1039      +/-   ##
==========================================
+ Coverage   77.92%   78.04%   +0.11%     
==========================================
  Files          54       55       +1     
  Lines        2718     2719       +1     
  Branches      635      631       -4     
==========================================
+ Hits         2118     2122       +4     
+ Misses        587      584       -3     
  Partials       13       13              
Files with missing lines Coverage Δ
packages/eth-providers/src/utils/ApiAtCache.ts 100.00% <100.00%> (ø)
packages/eth-providers/src/utils/utils.ts 83.74% <ø> (ø)
packages/eth-rpc-adapter/src/wrapped-provider.ts 94.73% <ø> (ø)
packages/eth-providers/src/utils/parseBlock.ts 95.40% <71.42%> (-2.19%) ⬇️
packages/eth-providers/src/base-provider.ts 83.61% <95.94%> (-0.29%) ⬇️

... and 1 file with indirect coverage changes

This was referenced Nov 6, 2024
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.

3 participants