Serve typed blocks from recent block cache to avoid repeated deserialization#6400
Serve typed blocks from recent block cache to avoid repeated deserialization#6400incrypto32 merged 3 commits intomasterfrom
Conversation
…h crate Move EthereumJsonBlock and JSON patching utilities from chain/ethereum to graph/src/components/ethereum/ so they can be used by the store layer without circular dependencies. This prepares for typed block caching where CachedBlock::from_json() needs access to these utilities.
incrypto32
force-pushed
the
krishna/typed-block-cache
branch
2 times, most recently
from
1f360ae to
fd65e1a
Compare
incrypto32
force-pushed
the
krishna/typed-block-cache
branch
from
fd65e1a to
4ba28d6
Compare
lutter
left a comment
lutter
left a comment
There was a problem hiding this comment.
Nice! I left a few comments for additional improvements - I am totally fine merging this as-is to see how much this helps, but I think we should also do some of the suggestions as follow-on work. In particular, we should be able to get rid of deep clones of blocks; we also have a pretty big number of different block structs all over the code base; it would be nice to simplify and reduce that though that might be pretty hairy.
|
|
||
| /// Typed cached block for Ethereum. Stores the deserialized block so that | ||
| /// repeated reads from the in-memory cache avoid `serde_json::from_value()`. | ||
| #[derive(Clone, Debug)] |
There was a problem hiding this comment.
It would be nice to get rid of Clone for this and all other block structs; that's probably a bigger puzzle and requires using Arc judiciously in various data structures, i.e., something for another PR; but conceptually, blocks are readonly and it should therefore be possible to Arc them.
|
|
||
| /// Returns blocks as raw JSON. Used by callers that need the original | ||
| /// JSON representation (e.g., GraphQL block queries, CLI tools). | ||
| async fn blocks_as_json( |
There was a problem hiding this comment.
We should try and get rid of this one, but that's also for another PR
| let block = JsonBlock::new(block.ptr(), parent_hash, block.data().ok()); | ||
| self.recent_blocks_cache.insert_block(block); | ||
| let json_block = JsonBlock::new(block.ptr(), parent_hash, block.data().ok()); | ||
| self.recent_blocks_cache.insert_json_block(json_block); |
There was a problem hiding this comment.
upsert_block is only used in 2 places (apart from tests) It should be possible to tighten the type of block to a concrete type here so we don't have to go through JSON. It's kinda low priority since this code path is only used on the block ingestor, which isn't that performance critical.
There was a problem hiding this comment.
Noted. will write up an issue for a follow up pR
incrypto32
force-pushed
the
krishna/typed-block-cache
branch
2 times, most recently
from
7995872 to
9dd453e
Compare
- Return Arc<LightEthereumBlock> from into_light_block() to avoid deep clone - Move cache_block_to_block_ptr_ext into CacheBlock::to_extended_block_ptr()
incrypto32
force-pushed
the
krishna/typed-block-cache
branch
from
9dd453e to
78c63a5
Compare
…ization (#6400) * graph, chain/ethereum: Move json_patch and json_block modules to graph crate Move EthereumJsonBlock and JSON patching utilities from chain/ethereum to graph/src/components/ethereum/ so they can be used by the store layer without circular dependencies. This prepares for typed block caching where CachedBlock::from_json() needs access to these utilities. * graph: Add CachedBlock enum and typed block cache * graph, chain/ethereum, store: Address review feedback - Return Arc<LightEthereumBlock> from into_light_block() to avoid deep clone - Move cache_block_to_block_ptr_ext into CacheBlock::to_extended_block_ptr()
2 participants
No description provided.