-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Optimize tree route to sync faster #3665
Conversation
|
I'm curious as to what other approaches you have tried. It seems much more memory and computation light to cache the results of queries to
The memory cache approach is clearly faster than doing nothing, but it requires keeping around a big LRU cache |
It seems we are making many queries from |
I wonder where is this pattern coming from. I imagine we only need to check tree paths or ancestry w.r.t the final block only when finalizing, and not when importing or checking each block. Does GRANDPA really need to check if each new block is descendent from the last finalized block? |
With any kind of finality, we always have to do this check. At the very least, if that block could be a new "best" block. |
|
Some explanation of where
Nodes in the tree have an implicit ancestry relationship that is given by I doubt that on Kusama the issue is being caused by GRANDPA since there are no authority changes happening. Most likely it was being caused by BABE (although that should have been improved by #3583). Regardless of any improvements on BABE having a way to speed up common ancestry queries makes sense to me. |
|
In #3586 we do |
Demi-Marie
left a comment
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.
LGTM although I am no expert on the database.
11af594 to
03b1c77
Compare
|
This is ready for another look, I introduced the crate and traits as @rphmeier suggested (although I'm not sure about the location and name). |
c5d05e1 to
5d8eb9f
Compare
andresilva
left a comment
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.
overall lgtm, minor nits.
6f2e7d8 to
8cb5b42
Compare
Fixes #3592
Summary
This PR removes some inefficiencies of
tree_routeand calling functions. Previously,tree_routewas computing the path between two blocks by loading headers from database. Now, we cache the information needed in an LRU cache in-memory.Also, many times we were using
tree_routejust to check if a block is descendant of another one. Instead, this PR introduces a functionlowest_common_ancestorthat compute descendants inO(1)(most of the time).As a side effect, this improves syncing speed for Kusama from ~2 hours to ~20 minutes (25 bps -> 35 bps in wasm, 80 bps -> 400 bps in native).
Changes:
CachedHeaderMetadatawith fields hash, number, parent and ancestor. Theancestorfield is used to quickly jump through the tree.20_000 HeaderMetadataused, by means of aLRU cache. The optimal number depends on the difference between best and finalized block numbers.CachedHeaderMetadatato traverse the tree in thetree_routefunction. Previously we were loading the whole headers from DB to do this task.lowest_common_ancestorfunction and use it inis_descendent_ofand other places, replacingtree_route. Right now, the query pattern of this function is as follows:lca(best, final), lca(best+1, final), lca(best+2, final), ..., lca(best+5000, final). By using theancestorfield ofLightHeader, the first query isO(n)and the followingsO(1).Notes:
lowest_common_ancestorby dividing the tree in sections and jumping between them, gettingO(sqrt(h))for query, but it didn't work very well in practice.