Closed shawn-zil closed 5 days ago
@shawn-zil Do you have a benchmark or easily reproducible way of testing the vote processing speed? I'm keen on exploring options to do the caching at a slightly lower level which avoids adding even more state and complexity into Consensus
.
I'm testing a state cache in devnet this morning - https://github.com/Zilliqa/zq2/commit/61e3ae91ed75a67917cfb06c7e0466000e565939
Can you test the same change with your specific vote-processing issue?
@shawn-zil Do you have a benchmark or easily reproducible way of testing the vote processing speed? I'm keen on exploring options to do the caching at a slightly lower level which avoids adding even more state and complexity into
Consensus
.I'm testing a state cache in devnet this morning - https://github.com/Zilliqa/zq2/commit/61e3ae91ed75a67917cfb06c7e0466000e565939
Can you test the same change with your specific vote-processing issue?
@JamesHinshelwood I'm not doing anything fancy, just using timestamp.elapsed and printing it out at different points of the function to take readings.
Sorry.
Once I've deployed my change to devnet, would you mind checking the performance there too to see if it helps with your issue?
Once I've deployed my change to devnet, would you mind checking the performance there too to see if it helps with your issue?
Sure. Let me know which branch it's from, and I can add some debugging messages, redeploy to devnet
and do a comparison. I could even compare it against the changes from this branch.
Analysis of the vote handling process on local, finds that in the case of a 4-validator network, it takes about 25ms to handle a vote. Of this, ~13ms is spent in
total_weight()
and ~7ms is spent inget_stakers()
calls to the EVM. This represents about 80% of the overall vote handling time. Increasing the number of validators also increases the vote handling time (as observed earlier).This PR introduces some changes:
total_weight()
from O(n) to O(1) time, which reduced the handling time from 13ms to ~3ms by calling thegetTotalStake()
deposit contract; then adding a caching mechanism reduced this further to a negligible cost after the first call.get_stakers()
, which reduces the call cost to negligible, after the first call.This brings into the question of whether it might make sense to build a generalised caching mechanism for all EVM calls. But that might be overkill for now, and is something to think about if the need arises.
I think it's fine to merge these changes first, since they definitely provide some measure of improvement to the vote handling process. Hopefully, we can test and measure the real impact of these changes once the present devops issues have been resolved.