Use boost::unordered_map for mapBlockIndex #4838

1. 
   sipa commented at 0:21 am on September 4, 2014: member

   As some profiling by @kdomanski showed that we’re doing a very high amount of calls to uint256::CompareTo, in particular for mapBlockIndex operations. So:

   • Turn mapBlockIndex into an unordered_map, so it only needs (few) operator== calls on uint256 instead of (multiple) operator< calls.
   • Using memcmp for uint256’s operator== and operator!=.
2. checkpoints.cpp depends on main, it can use mapBlockIndex directly a0dbe433bd
3. Introduce BlockMap type for mapBlockIndex 145d5be896
4. Use boost::unordered_map for mapBlockIndex 8a41e1edd4
5. Use memcmp for uint256 equality/inequality 1e4f87f5a1
6. sipa force-pushed on Sep 4, 2014
7. 
   BitcoinPullTester commented at 9:00 am on September 4, 2014: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4838_1e4f87f5a13e34a457b537e9d13a212e6c5b754f/ 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.
8. 
   laanwj commented at 9:55 am on September 4, 2014: member

   Looks good to me, utACK
9. in src/checkpoints.cpp: in 1e4f87f5a1

   145@@ -146,7 +146,7 @@ namespace Checkpoints {
   146         return checkpoints.rbegin()->first;
   147     }
   148 
   149-    CBlockIndex* GetLastCheckpoint(const std::map<uint256, CBlockIndex*>& mapBlockIndex)
   

   ---

   ---

   

   laanwj commented at 9:57 am on September 4, 2014:

   No strong feeling on this, but I slightly prefer passing this explicitly as it documents what input is used to the function.

   ---

   

   sipa commented at 11:11 am on September 4, 2014:

   That’s not possible. Checkpoints and main are a circular dependency, only possible now because checkpoints.h doesn’t need main.h. If the type of mapBlockIndex is defined in main.h, it becomes an actual .h circular dependency.

   Better solution: move the code pieces of checkpoints to main, and the data pieces to chainparams…

   ---

   

   laanwj commented at 11:24 am on September 4, 2014:

   OK, clear. #3802 does that data move, by the way.

   ---

   

   sipa commented at 1:14 pm on September 4, 2014:

   It moves the data but does not break the cyclic dependency.
10. 
    gmaxwell commented at 9:32 pm on September 4, 2014: contributor

    I tested out most of the same change last weekend in the hope it would decrease memory usage (it didn’t). I liked the effect on the code (esp, getting rid of the explicit types for a typedef, is 99% of the work here and should be done regardless).

    ACK (with quasi-testing, I tested my nearly identical changes)

11. 
    jgarzik commented at 9:37 pm on September 4, 2014: contributor

    ut ACK
12. sipa merged this on Sep 4, 2014
13. sipa closed this on Sep 4, 2014

    ---

14. sipa referenced this in commit af9c3b0cff on Sep 4, 2014
15. DrahtBot locked this on Sep 8, 2021

Contributors
sipa BitcoinPullTester laanwj gmaxwell jgarzik