-
Notifications
You must be signed in to change notification settings - Fork 837
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
Avoid doing two validations on transactionsRoot and ReceiptsRoot during sync #5679
Conversation
…ng sync Signed-off-by: Ameziane H <[email protected]>
|
Signed-off-by: Ameziane H <[email protected]>
…hether or not we already calculated the receipts root Signed-off-by: Ameziane H <[email protected]>
...reum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetBlockBodyValidator.java
Outdated
Show resolved
Hide resolved
The performance improvements is very good, and we definitely need to avoid doing the same work twice, but I want to propose a different implementation, that avoid the modification to the data structures, and specifically avoid making them mutable like for |
Signed-off-by: Ameziane H <[email protected]>
We can discuss another implementation. My point here is to avoid any overhead in terms of performances, I thought about keeping track of what couple header/receipts was already verified, but this means we need to do another comparaison on header hash (byte array comparaison) which is expensive. |
I think the second proposition (cache) seems to be interesting. Having all of the validation result in cache can simplify the code (we just have to modify the validator and not the other parts of code). and can also optimize if we ask several times the same block (not sure if we have this case but maybe in case of a retry) |
@ahamlat is this PR one you're going to come back to or is the outcome to go with a different approach? |
Can we circle back to this PR and determine whether this is the approach we want to take? |
I think a cache based approach, like the one done for #6375 is better |
I agree with @fab-10, let me think about how to include better caching mecanism |
@ahamlat is this still relevant/in progress? |
Closing this PR, I started another one with a better implementation #7595 |
PR description
This PR enhances the Sync process by eliminating duplicate validations for transactions root and receipts root. We've noticed in CPU profiles during sync that receipts root and transactions root are calculated twice. The first time when data is retrieved from other peers, and the second time during block import when validating block body. This PR implements a mechanism to maintain a record of previously validated transactions and receipts roots, ensuring that redundant validations are avoided.
Sync time improvement (on a 4 vCPU/ 32 GiB RAM VM)
Without this PR
Checkpoint sync time : 19 hours 54 minutes
With this PR
Checkpoint sync time : 16 hours 42 minutes
CPU profiling
We can notice in the profiling screenshots that Besu does the receipts root validation only once with this PR when getting the receipts from the peers.
Before this PR
After this PR