Implement accurate UTXO cache size accounting #6102

pull sipa wants to merge 6 commits into bitcoin:master from sipa:accuratecache changing 9 files +243 −42
  1. sipa commented at 0:03 am on May 4, 2015: member

    This adds a basic foundation for accurate memory counting (memusage.h, with some implementation-specific assumptions, but at least the tree node and hashtable node implementations used are very generic and straightforward, so likely accurate for several systems). On the tested system at least, they are exact, ignoring memory fragmentation (tested using a single binary creating and modifying large amounts of different configurations of these data structures, and observing total resident set size afterwards). I expect this to be useful for other resource-limiting subsystems later on.

    Then, this is used to implement accurate memory usage counting for CCoins objects, and efficiently computed memory usage counting for CCoinsViewCache (using cached totals, and increments/decrements on updates through CCoinsModifier). The existing CCoinsViewCache randomized simultation unit test is extended to also verify the correctness of the cached memory usage totals. Changing any of the cached total update statements in coins.cpp breaks the unit test.

    Finally, the internal flush triggering mechanism is changed to use the memory usage mechanism rather than transaction count of pcoinsTip.

  2. sipa force-pushed on May 4, 2015
  3. laanwj added the label UTXO Db and Indexes on May 4, 2015
  4. sipa force-pushed on May 4, 2015
  5. sipa commented at 8:56 pm on May 4, 2015: member
    I added a commit which changes the block flushing policy a bit. The block index is still flushed at least once an hour (so that we don’t need to redownload hours worth of blocks after a crash), but the coins cache is only guaranteed to be flushed once per day (as the cache size allows). @sdaftuar feel like having a look?
  6. sipa force-pushed on May 5, 2015
  7. sipa force-pushed on May 5, 2015
  8. sipa force-pushed on May 5, 2015
  9. sipa force-pushed on May 5, 2015
  10. Add memusage.h 540629c6fb
  11. Keep track of memory usage in CCoinsViewCache 046392dc1d
  12. Use accurate memory for flushing decisions fc684ad8af
  13. Cache tweak and logging improvements b3ed4236be
  14. Write block index more frequently than cache flushes 67708acff9
  15. sipa force-pushed on May 12, 2015
  16. laanwj commented at 12:05 pm on May 12, 2015: member

    utACK.

    It would be really nice to be able to get the approximated in-memory size of various data structures through RPC, such as the UTXO cache size introduced here.

  17. morcos commented at 2:14 pm on May 12, 2015: member
    As mentioned on IRC, I think it would be better to still guard the block index writes with CheckDiskSpace. Something like this: a8e9baa77. Other than that, utACK. I did a light code review of the first commit and full code review of the last 4, and some light testing of the last commit.
  18. gavinandresen commented at 6:34 pm on May 12, 2015: contributor
    Code review and light testing ACK.
  19. Relocate calls to CheckDiskSpace
    Make sure we're checking disk space for block index writes and allow for pruning to happen before chainstate writes.
    86a5f4b54e
  20. sipa commented at 7:48 pm on May 12, 2015: member

    Cherry-picked @morcos’s commit.

    As a test, I reindexed the entire main chain with -dbcache=16000, and this did not result in a single database flush. Memory usage near the end was slightly lower (100 MB off on a total of 3.5 GB) than what the coin cache usage predicted. I can’t explain that difference (it should be more), but at least it errs on the safe side.

  21. in src/coins.cpp: in 86a5f4b54e
     98@@ -93,6 +99,7 @@ bool CCoinsViewCache::GetCoins(const uint256 &txid, CCoins &coins) const {
     99 CCoinsModifier CCoinsViewCache::ModifyCoins(const uint256 &txid) {
    100     assert(!hasModifier);
    101     std::pair<CCoinsMap::iterator, bool> ret = cacheCoins.insert(std::make_pair(txid, CCoinsCacheEntry()));
    102+    size_t cachedCoinUsage = 0;
    


    sdaftuar commented at 7:56 pm on May 12, 2015:
    nit: when reading the code, I found it slightly concerning that a one-character typo in this variable name below would result in code that would still compile but be wrong (since cachedCoinsUsage is a class member variable), though thankfully that mistake causes the unit test to fail.
  22. sdaftuar commented at 1:00 am on May 13, 2015: member
    I reviewed and did a little testing as well, including replicating those results on my machine, ACK.
  23. laanwj merged this on May 15, 2015
  24. laanwj closed this on May 15, 2015

  25. laanwj referenced this in commit 6fb90d8983 on May 15, 2015
  26. furszy referenced this in commit 1f9e7e45ba on May 15, 2020
  27. MarcoFalke locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-04 13:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me