Votes cache #1852
Votes cache #1852
Conversation
| } | ||
| votes_cache_.put(msg); | ||
|
|
||
| if (not info.has_value() or not info->set_id.has_value() |
There was a problem hiding this comment.
Copy of line 953. Need to remove it
There was a problem hiding this comment.
At least info is used before the definition
There was a problem hiding this comment.
No problem, in the line 952 it might be rewritten.
There was a problem hiding this comment.
It got here accidentally. Fixed now
| @@ -295,6 +296,8 @@ namespace kagome::consensus::grandpa { | |||
| */ | |||
| void loadMissingBlocks(GrandpaContext &&grandpa_context); | |||
|
|
|||
| const size_t kVotesCacheSize | |||
There was a problem hiding this comment.
Where is the value? What it equals to? Where is a semicolon at least?
| } | ||
| return false; | ||
| } | ||
| } // namespace kagome::consensus::grandpa |
There was a problem hiding this comment.
No line break at the end. Please add
| private: | ||
| LruCache<RoundNumber, std::unordered_set<VotesCacheItem>> lru_cache_; | ||
| }; | ||
| } // namespace kagome::consensus::grandpa |
There was a problem hiding this comment.
No line break at the end. Please add
| @@ -295,6 +296,8 @@ namespace kagome::consensus::grandpa { | |||
| */ | |||
| void loadMissingBlocks(GrandpaContext &&grandpa_context); | |||
|
|
|||
| const size_t kVotesCacheSize | |||
| std::size_t operator()( | ||
| const kagome::consensus::grandpa::VotesCacheItem &item) const { | ||
| return std::hash<size_t>()(item.type) | ||
| ^ std::hash<kagome::consensus::grandpa::Id>()(item.id) |
There was a problem hiding this comment.
Please, use boost::hash_combine instead xor
| Id id; | ||
| primitives::BlockHash block_hash; | ||
|
|
||
| bool operator==(const VotesCacheItem &rhs) const { |
There was a problem hiding this comment.
auto operator<=>(const VotesCacheItem &rhs) const = default; would be enough
kamilsa
force-pushed
the
feature/votes-cache
branch
from
bdc508b to
800debc
Compare
# Conflicts: # core/consensus/grandpa/impl/grandpa_impl.cpp
|
|
||
| void put(const VoteMessage &msg); | ||
|
|
||
| bool contains(VoteMessage msg); |
There was a problem hiding this comment.
It seems the argument could be const&.
| [[relaychain.nodes]] | ||
| name = "two" | ||
|
|
||
| #[[parachains]] |
There was a problem hiding this comment.
Please either add a comment explaining when and how to use the disabled config or remove it.
There was a problem hiding this comment.
oops, sorry. Committed accidentally
kamilsa
force-pushed
the
feature/votes-cache
branch
from
9f9eb4b to
1e61de7
Compare
| const auto &peer_id = peers_list.back().second; | ||
| disconnectFromPeer(peer_id); | ||
| } | ||
| // for (; !peers_list.empty() |
There was a problem hiding this comment.
Most likely this is unintended change.
kamilsa
force-pushed
the
feature/votes-cache
branch
from
86f2dc5 to
1e61de7
Compare
Referenced issues
None
Description of the Change
Introduces vote message cache to avoid processing of the same vote twice.
Benefits
Cache hit rate 94%
Possible Drawbacks
None