-
Notifications
You must be signed in to change notification settings - Fork 787
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Store full votes in vote cache #4518
Store full votes in vote cache #4518
Conversation
aa64459
to
c734caf
Compare
{ | ||
break; | ||
results.push_back ({ entry.hash (), entry.tally (), entry.final_tally () }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A std::deque might be a better result container depending on how many items can be returned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expansion of a deque is cheaper than the expansion of a std::vector because it does not involve copying of the existing elements to a new memory location.
Interesting, I always assumed deque is using vector under the hood. The result set might in fact be quite large, so using deque here seems like the most reasonable option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, I'm not sure whether there is any performance penalty when sorting deque? Sorting is performed before returning this result set. I'm planning to add a small benchmarking suite soon that I used for improving performance of stat counting, I'll revisit and measure this once that is done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deque still has O(1) indexing access so a sort should have minimal impact.
Currently vote cache stores in compressed format, keeping the representative, block hash and timestamp fields, but without the signature. This modifies the behaviour to store full votes, so that upon hinted election activation those votes can be immediately rebroadcasted. In V26 the votes were rebroadcasted only if the vote arrived after election was already active, since there was no way to rebroadcast a previously seen votes without having their signature.
Another modification is that the vote cache, for each cached block hash, now stores 64 votes with the highest voting weight. This is in contrast to previous behaviour where cache stored first 128 votes (above principal rep weight) that were observed. With current live network rep distribution this should be irrelevant, but I think it's nice to have.
The total vote cache size is also reduced from a maximum of 128k hashes to 64k. The age cutoff for entries is increased from 5 minutes to 15 minutes.
Yet another behaviour change is that votes for active elections are also cached, so that if a particular election is dropped before getting confirmed and later rescheduled, it won't have to collect all the votes from scratch.
During testing (thanks @gr0vity-dev as always) we couldn't observe a significant difference between this branch and V26 in synthetic tests. However, I believe that this will at least partially improve vote propagation on live network under conditions similar to that observed during the recent stress test.