blockchain: Add ancestor optimization to finding Ancestor #1688
Conversation
Pull Request Test Coverage Report for Build 530293241Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
blockchain/blockindex.go
Outdated
| heightSkip := getSkipHeight(heightWalk) | ||
| heightSkipPrev := getSkipHeight(heightWalk - 1) | ||
|
|
||
| if indexWalk.ancestor != nil && |
There was a problem hiding this comment.
Quite a mouth-full of a conditional...
It also isn't immediately clear the significance (nor origin) of the -2 here.
There was a problem hiding this comment.
Got rid of everything here and changed the logic to be simpler.
idk why that -2 is there as well... Some sipa magic
|
Commenting here because I just saw @Roasbeef's tweet linking to it. FYI I implemented something similar in dcrd back in 2019 in what is imo a simpler way and should be a simple backport: |
kcalvinalvin
force-pushed
the
add-blockindex-parentskips
branch
from
52c9778 to
126d920
Compare
On startup, Ancestor call was taking a lot of time when the node was
loading the blockindex onto memory. This change speeds up the Ancestor
function significantly and speeds up the node during startup.
On testnet3 at blockheight ~2,500,000, the startup was around 30seconds
on current main and was 5 seconds with this change. Below is a benchstat
result showing the significant speedup.
goos: darwin
goarch: arm64
pkg: github.com/utreexo/utreexod/blockchain
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
Ancestor-8 120819.301µ ± 5% 7.013µ ± 19% -99.99% (p=0.000 n=10)
│ old.txt │ new.txt │
│ B/op │ B/op vs base │
Ancestor-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
│ old.txt │ new.txt │
│ allocs/op │ allocs/op vs base │
Ancestor-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
kcalvinalvin
force-pushed
the
add-blockindex-parentskips
branch
from
126d920 to
f396b3d
Compare
|
Reviving this as I noticed that the startup of a node is slowed down quite a bit by the blockindex loading. Here's a pprof of the startup blockindex loading without this PR. Here's one with the PR. Added a benchmark as well. The benchstat results were: |
This mimics the behavior of Ancestor() in bitcoind