Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make FindBlockByHeight constant-time #2644

Merged
merged 1 commit into from May 30, 2013

Conversation

@sipa
Copy link
Member

commented May 12, 2013

Remove the pnext pointer in CBlockIndex, and replace it with a vBlockIndexByHeight vector (no effect on memory usage). pnext can now be replaced by vBlockIndexByHeight[nHeight+1], but FindBlockByHeight becomes constant-time.

This also means the entire mapBlockIndex structure and the block index entries in it become purely blocktree-related data, and independent from the currently active chain, potentially allowing them to be protected by separate mutexes in the future.

Make FindBlockByHeight constant-time.
Remove the pnext pointer in CBlockIndex, and replace it with a
vBlockIndexByHeight vector (no effect on memory usage). pnext can
now be replaced by vBlockIndexByHeight[nHeight+1], but
FindBlockByHeight becomes constant-time.

This also means the entire mapBlockIndex structure and the block
index entries in it become purely blocktree-related data, and
independent from the currently active chain, potentially allowing
them to be protected by separate mutexes in the future.
@BitcoinPullTester

This comment has been minimized.

Copy link

commented May 12, 2013

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/0fe8010a10bafd67f9131b2da034fb9cd7fc5024 for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@jgarzik

This comment has been minimized.

Copy link
Contributor

commented May 12, 2013

Code looks sane and ACK-worthy. The concept is similar to something I did in pynode.

Did you actually instrument "no effect on memory usage", or is that a guess? I would like to see before and after numbers proving that :)

@sipa

This comment has been minimized.

Copy link
Member Author

commented May 12, 2013

@jgarzik Just reasoning. For each CBlockIndex representing a node in the active chain, we remove 8 bytes from the CBlockIndex, and add 8 bytes to vBlockIndexByHeight. Actually, it should reduce memory usage slightly, as non-best-chain nodes also lose 8 bytes.

jgarzik pushed a commit that referenced this pull request May 30, 2013
Jeff Garzik
Merge pull request #2644 from sipa/constfindblock
Make FindBlockByHeight constant-time

@jgarzik jgarzik merged commit d397715 into bitcoin:master May 30, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.