kernel,node: clean up `dbcache` helpers and add kernel API #35205

pull l0rinc wants to merge 10 commits into bitcoin:master from l0rinc:l0rinc/share-dbcache-defaults changing 19 files +158 −87
  1. l0rinc commented at 12:09 PM on May 4, 2026: contributor

    Problem: PR #34692 kept the simplified two-tier -dbcache default from #34641: use 1024 MiB on 64-bit systems with at least 4 GiB of detected RAM, and otherwise keep 450 MiB.

    The node-side policy is still split across generic cache and system helpers, which makes it harder to see which callers use the current dbcache default. Separately, kernel still has only a fixed 450 MiB fallback and no C API for callers to provide a cache budget from outside.

    Fix: Move RAM detection to common/system_ram and move dbcache constants/helpers to node/dbcache, then route the node and Qt dbcache default users through node::GetDefaultDBCache().

    Keep kernel independent from the node/common RAM policy by preserving DEFAULT_KERNEL_CACHE as its fallback and adding a chainstate manager option setter for explicit database cache bytes.

    This is a simplified follow-up to #34641, keeping the #34692 two-tier node default unchanged. It does not reintroduce continuous RAM-aware default sizing, and keeps the remaining cleanup focused on naming, typing, and test coverage around the dbcache policy.

  2. DrahtBot commented at 12:09 PM on May 4, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35205.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK w0xlt, sedited

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #35616 (refactor: Use u64 over size_t for all cache sizes to fix a 32-bit overflow by maflcko)
    • #35200 (node: smooth oversized dbcache warnings by l0rinc)
    • #32427 (kernel: Replace leveldb-based BlockTreeDB with WAL and .dat file based store by sedited)
    • #31260 (scripted-diff: Type-safe settings retrieval by ryanofsky)
    • #30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)
    • #29700 (kernel, refactor: return error status on all fatal errors by ryanofsky)
    • #26022 (Add util::ResultPtr class by ryanofsky)
    • #25722 (refactor: Use util::Result class for wallet loading by ryanofsky)
    • #25665 (refactor: Add util::Result failure types and ability to merge result values by ryanofsky)
    • #17783 (common: Disallow calling IsArgSet() on ALLOW_LIST options by ryanofsky)
    • #17581 (refactor: Remove settings merge reverse precedence code by ryanofsky)
    • #17580 (refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky)
    • #17493 (util: Forbid ambiguous multiple assignments in config file by ryanofsky)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  3. DrahtBot added the label CI failed on May 4, 2026
  4. DrahtBot commented at 1:00 PM on May 4, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task i686, no IPC: https://github.com/bitcoin/bitcoin/actions/runs/25318144859/job/74220316972</sub> <sub>LLM reason (✨ experimental): CI failed because the Bitcoin Core Test Suite had failing CTest results—sock_tests failed.</sub>

    <details><summary>Hints</summary>

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

    </details>

  5. in src/common/system_ram.cpp:23 in 56a8728519
      18 | +std::optional<size_t> GetTotalRAM()
      19 | +{
      20 | +    [[maybe_unused]] auto clamp{[](uint64_t v) { return size_t(std::min(v, uint64_t{std::numeric_limits<size_t>::max()})); }};
      21 | +#ifdef WIN32
      22 | +    if (MEMORYSTATUSEX m{}; (m.dwLength = sizeof(m), GlobalMemoryStatusEx(&m))) return clamp(m.ullTotalPhys);
      23 | +#elif defined(__APPLE__) || \
    


    kevkevinpal commented at 2:56 PM on May 4, 2026:

    One thing I noticed here is that in Apple's man page for sysconf, _SC_PHYS_PAGES and _SC_PAGESIZE are not listed at all. There is a chance that on OSX systems they may not be defined as seen in this post.

    The intended api for RAM on __APPLE__ looks to be sysctlbyname, which is used in crc32c and src/crypto/sha256.cpp

    ./src/crc32c/src/crc32c_arm64_check.h:57:  return sysctlbyname("hw.optional.armv8_crc32", &val, &len, nullptr, 0) == 0
    ./src/crypto/sha256.cpp:673:        if (sysctlbyname("hw.optional.arm.FEAT_SHA256", &val, &len, nullptr, 0) == 0) {
    

    l0rinc commented at 6:19 PM on May 4, 2026:

    This was introduced in https://github.com/bitcoin/bitcoin/pull/33333/changes/6c720459beead5c825b354a1d5c11969b6e3a170, it's not strictly related to the change, the code is just moved here.

    It's a valid observation though: for some reason the Apple man page doesn't list these two, but they are defined in their own open-source Libc tree and are working in reality:

    If it ever becomes a problem we can apply the alternative you're suggesting.


    kevkevinpal commented at 6:46 PM on May 4, 2026:

    Awesome, they must have forgotten to update their docs.

    Sounds like a plan, hopefully it is never an issue!

  6. l0rinc closed this on May 5, 2026

  7. l0rinc reopened this on May 5, 2026

  8. DrahtBot removed the label CI failed on May 5, 2026
  9. l0rinc force-pushed on May 5, 2026
  10. stickies-v commented at 4:13 PM on May 6, 2026: contributor

    Kernel shouldn't have any dependencies on common, so I think this approach is a regression. Generally, I think we should move towards kernel having less system dependencies instead of more, so approach nack for me

    edited: my concern seems to be addressed

  11. l0rinc marked this as a draft on May 7, 2026
  12. l0rinc renamed this:
    node,kernel: share `dbcache` default sizing
    kernel,node: clean up `dbcache` helpers and add kernel API
    on May 18, 2026
  13. l0rinc force-pushed on May 18, 2026
  14. l0rinc commented at 8:36 PM on May 18, 2026: contributor

    Thanks @stickies-v, I rewrote the kernel part based on your feedback.

    The latest push no longer makes bitcoinkernel depend on common/system_ram or node/dbcache, but keeps DEFAULT_KERNEL_CACHE as the kernel fallback and adds an explicit database-cache setter to the C API, so callers can pass the cache budget from outside.

  15. l0rinc marked this as ready for review on May 18, 2026
  16. DrahtBot added the label CI failed on May 18, 2026
  17. DrahtBot removed the label CI failed on May 19, 2026
  18. sedited commented at 8:44 PM on May 20, 2026: contributor

    Concept ACK

  19. DrahtBot added the label Needs rebase on May 22, 2026
  20. l0rinc force-pushed on May 22, 2026
  21. l0rinc commented at 7:05 AM on May 22, 2026: contributor

    Rebased, ready for review again.

  22. DrahtBot removed the label Needs rebase on May 22, 2026
  23. in src/node/dbcache.h:17 in c24ed5892a
      12 | +#include <limits>
      13 | +
      14 | +//! min. -dbcache (bytes)
      15 | +static constexpr uint64_t MIN_DBCACHE_BYTES{4_MiB};
      16 | +//! -dbcache default (bytes)
      17 | +static constexpr uint64_t DEFAULT_DBCACHE_BYTES{450_MiB};
    


    sedited commented at 9:48 AM on June 8, 2026:

    Why is this decoupled from the DEFAULT_KERNEL_CACHE?


    l0rinc commented at 3:15 PM on June 18, 2026:

    Good point, this was leftover from a previous version, thanks. It had become just an alias for DEFAULT_KERNEL_CACHE, so it added another name without representing distinct node policy. I dropped it and now use DEFAULT_KERNEL_CACHE directly everywhere.

  24. in src/qt/optionsmodel.cpp:735 in c24ed5892a
     731 | @@ -732,7 +732,7 @@ void OptionsModel::checkAndMigrate()
     732 |          // see https://github.com/bitcoin/bitcoin/pull/8273
     733 |          // force people to upgrade to the new value if they are using 100MB
     734 |          if (settingsVersion < 130000 && settings.contains("nDatabaseCache") && settings.value("nDatabaseCache").toLongLong() == 100)
     735 | -            settings.setValue("nDatabaseCache", (qint64)(DEFAULT_DB_CACHE >> 20));
     736 | +            settings.setValue("nDatabaseCache", qint64(node::GetDefaultDBCache() / 1_MiB));
    


    sedited commented at 10:02 AM on June 8, 2026:

    Is changing this to GetDefaultDBCache a bug fix?


    l0rinc commented at 3:13 PM on June 18, 2026:

    Not sure it matters much - I changed it back to the fixed DEFAULT_KERNEL_CACHE fallback since it's simpler here.

  25. sedited commented at 10:03 AM on June 8, 2026: contributor

    Approach ACK

    Took me a moment to follow along, but I do think now that what you call a "policy" split is probably a good thing and the changes seem fine.

  26. w0xlt commented at 8:08 AM on June 14, 2026: contributor

    Concept ACK

  27. l0rinc force-pushed on Jun 18, 2026
  28. in src/CMakeLists.txt:221 in 3172d54152
     217 | @@ -217,6 +218,7 @@ add_library(bitcoin_node STATIC EXCLUDE_FROM_ALL
     218 |    node/blockmanager_args.cpp
     219 |    node/blockstorage.cpp
     220 |    node/caches.cpp
     221 | +  node/dbcache.cpp
    


    sedited commented at 8:09 PM on June 18, 2026:

    Nit: alphabetical ordering

  29. in src/common/system_ram.cpp:2 in 3172d54152 outdated
       0 | @@ -0,0 +1,30 @@
       1 | +// Copyright (c) 2009-2010 Satoshi Nakamoto
       2 | +// Copyright (c) 2009-present The Bitcoin Core developers
    


    sedited commented at 8:11 PM on June 18, 2026:

    I think we don't need to transfer the dates over from the file this is moved from. Does this move to the new file still make sense in the context of the current patch? Afaict this made more sense in the earlier version of the pull request.


    l0rinc commented at 9:38 PM on June 18, 2026:

    Done both, thanks

  30. sedited approved
  31. sedited commented at 8:23 PM on June 18, 2026: contributor

    ACK 3172d54152dc73633d93bc618f7891fabb650d40

  32. DrahtBot requested review from stickies-v on Jun 18, 2026
  33. l0rinc force-pushed on Jun 18, 2026
  34. in src/common/system.cpp:28 in 6d5a3bbeba


    w0xlt commented at 12:58 AM on June 19, 2026:

    Are those includes stale ?

    diff --git a/src/common/system.cpp b/src/common/system.cpp
    index 0b1f05e02c..306b17d87e 100644
    --- a/src/common/system.cpp
    +++ b/src/common/system.cpp
    @@ -25,12 +25,8 @@
     #include <malloc.h>
     #endif
     
    -#include <algorithm>
    -#include <cstddef>
    -#include <cstdint>
     #include <cstdlib>
     #include <locale>
    -#include <optional>
     #include <stdexcept>
     #include <string>
     #include <thread>
    

    l0rinc commented at 2:43 AM on June 24, 2026:

    Done, thanks

  35. in src/common/system_ram.h:2 in 6d5a3bbeba outdated
       0 | @@ -0,0 +1,17 @@
       1 | +// Copyright (c) 2009-2010 Satoshi Nakamoto
       2 | +// Copyright (c) 2009-present The Bitcoin Core developers
    


    w0xlt commented at 1:00 AM on June 19, 2026:

    micro-nit: the copyright texts are different in src/common/system_ram.h and src/common/system_ram.cpp.


    l0rinc commented at 2:43 AM on June 24, 2026:

    done, thanks

  36. in src/test/kernel/test_kernel.cpp:780 in 6d5a3bbeba outdated
     776 | @@ -776,6 +777,7 @@ BOOST_AUTO_TEST_CASE(btck_chainman_tests)
     777 |  
     778 |      ChainstateManagerOptions chainman_opts{context, PathToString(test_directory.m_directory), PathToString(test_directory.m_directory / "blocks")};
     779 |      chainman_opts.SetWorkerThreads(4);
     780 | +    chainman_opts.SetDatabaseCacheBytes(1000_MiB);
    


    w0xlt commented at 1:25 AM on June 19, 2026:

    nit: This can be ignored or left for a follow-up. The test could also assert that the configured cache budget reaches the chainstate cache split for stronger coverage.

    <details> <summary>diff</summary>

    diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
    index c26d2a98c7..36360fcdd9 100644
    --- a/src/kernel/bitcoinkernel.cpp
    +++ b/src/kernel/bitcoinkernel.cpp
    @@ -1071,6 +1071,16 @@ btck_ChainstateManager* btck_chainstate_manager_create(
         return btck_ChainstateManager::create(std::move(chainman), opts.m_context);
     }
     
    +btck_ChainstateManagerCacheSizes btck_chainstate_manager_get_cache_sizes(
    +    const btck_ChainstateManager* chainman)
    +{
    +    const auto& manager{*btck_ChainstateManager::get(chainman).m_chainman};
    +    return {
    +        .coins_db_cache_bytes = manager.m_total_coinsdb_cache,
    +        .coins_tip_cache_bytes = manager.m_total_coinstip_cache,
    +    };
    +}
    +
     const btck_BlockTreeEntry* btck_chainstate_manager_get_block_tree_entry_by_hash(const btck_ChainstateManager* chainman, const btck_BlockHash* block_hash)
     {
         auto block_index = WITH_LOCK(btck_ChainstateManager::get(chainman).m_chainman->GetMutex(),
    diff --git a/src/kernel/bitcoinkernel.h b/src/kernel/bitcoinkernel.h
    index 3c9e34fe72..02ee7a5269 100644
    --- a/src/kernel/bitcoinkernel.h
    +++ b/src/kernel/bitcoinkernel.h
    @@ -226,6 +226,12 @@ typedef struct btck_ChainstateManagerOptions btck_ChainstateManagerOptions;
      */
     typedef struct btck_ChainstateManager btck_ChainstateManager;
     
    +/** Cache sizes selected for a chainstate manager. */
    +typedef struct {
    +    size_t coins_db_cache_bytes;  //!< LevelDB cache reserved for the coins database.
    +    size_t coins_tip_cache_bytes; //!< In-memory coins cache.
    +} btck_ChainstateManagerCacheSizes;
    +
     /**
      * Opaque data structure for holding a block.
      */
    @@ -1249,6 +1255,15 @@ BITCOINKERNEL_API void btck_chainstate_manager_options_destroy(btck_ChainstateMa
     BITCOINKERNEL_API btck_ChainstateManager* BITCOINKERNEL_WARN_UNUSED_RESULT btck_chainstate_manager_create(
         const btck_ChainstateManagerOptions* chainstate_manager_options) BITCOINKERNEL_ARG_NONNULL(1);
     
    +/**
    + * [@brief](/bitcoin-bitcoin/contributor/brief/) Get the cache sizes selected for a chainstate manager.
    + *
    + * [@param](/bitcoin-bitcoin/contributor/param/)[in] chainstate_manager Non-null.
    + * [@return](/bitcoin-bitcoin/contributor/return/)                      The chainstate cache sizes in bytes.
    + */
    +BITCOINKERNEL_API btck_ChainstateManagerCacheSizes btck_chainstate_manager_get_cache_sizes(
    +    const btck_ChainstateManager* chainstate_manager) BITCOINKERNEL_ARG_NONNULL(1);
    +
     /**
      * [@brief](/bitcoin-bitcoin/contributor/brief/) Get the btck_BlockTreeEntry whose associated btck_BlockHeader has the most
      * known cumulative proof of work.
    diff --git a/src/kernel/bitcoinkernel_wrapper.h b/src/kernel/bitcoinkernel_wrapper.h
    index a2b0ceaf9c..752ff4165b 100644
    --- a/src/kernel/bitcoinkernel_wrapper.h
    +++ b/src/kernel/bitcoinkernel_wrapper.h
    @@ -1165,6 +1165,11 @@ public:
         }
     };
     
    +struct ChainstateManagerCacheSizes {
    +    size_t coins_db;
    +    size_t coins_tip;
    +};
    +
     class ChainView : public View<btck_Chain>
     {
     public:
    @@ -1302,6 +1307,12 @@ public:
         {
         }
     
    +    ChainstateManagerCacheSizes GetCacheSizes() const
    +    {
    +        const auto sizes{btck_chainstate_manager_get_cache_sizes(get())};
    +        return {sizes.coins_db_cache_bytes, sizes.coins_tip_cache_bytes};
    +    }
    +
         bool ImportBlocks(const std::span<const std::string> paths)
         {
             std::vector<const char*> c_paths;
    diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp
    index 780164cfcd..e11f1cefa5 100644
    --- a/src/test/kernel/test_kernel.cpp
    +++ b/src/test/kernel/test_kernel.cpp
    @@ -783,6 +783,9 @@ BOOST_AUTO_TEST_CASE(btck_chainman_tests)
         BOOST_CHECK(chainman_opts.SetWipeDbs(/*wipe_block_tree=*/false, /*wipe_chainstate=*/true));
         BOOST_CHECK(chainman_opts.SetWipeDbs(/*wipe_block_tree=*/false, /*wipe_chainstate=*/false));
         ChainMan chainman{context, chainman_opts};
    +    const auto cache_sizes{chainman.GetCacheSizes()};
    +    BOOST_CHECK_EQUAL(cache_sizes.coins_db, 8_MiB);
    +    BOOST_CHECK_EQUAL(cache_sizes.coins_tip, 990_MiB);
     }
     
     std::unique_ptr<ChainMan> create_chainman(TestDirectory& test_directory,
    

    </details>


    l0rinc commented at 2:42 AM on June 24, 2026:

    Thanks, given that the current PR already barely received any review I'll leave this for a follow-up.

  37. w0xlt commented at 1:25 AM on June 19, 2026: contributor

    ACK 6d5a3bbeba515d8c39e22732f116ff238b490e04

    Some non-blocking nits:

  38. DrahtBot requested review from sedited on Jun 19, 2026
  39. node, qt: use `1_MiB` for dbcache conversions
    Convert dbcache display values with `/ 1_MiB` instead of `>> 20`, so the unit is explicit before the helper split.
    Use the local Qt functional-cast style for the touched settings migration.
    83d289d640
  40. common: move `GetTotalRAM()` to system RAM files
    Move the OS-specific total RAM query out of `common/system.{cpp,h}` and into `common/system_ram.{cpp,h}`.
    This lets dbcache policy include the RAM helper without pulling in the broader system header.
    
    Co-authored-by: sipa <sipa@bitcoincore.org>
    0f9cb209a8
  41. scripted-diff: rename `GetTotalRAM` to `TryGetTotalRam`
    Use the `Try` prefix to make failed RAM detection visible at call sites.
    
    -BEGIN VERIFY SCRIPT-
    git grep -q 'TryGetTotalRam' -- src && echo "Error: TryGetTotalRam already exists in src" && exit 1
    git grep -l 'GetTotalRAM' -- src | xargs perl -pi -e 's/\bGetTotalRAM\b/TryGetTotalRam/g'
    -END VERIFY SCRIPT-
    55b16f875d
  42. common: cache `TryGetTotalRam()` result
    Store the detected total RAM in a function-local static so startup paths do not repeat the underlying system query.
    1b2a6e9b7a
  43. node: move dbcache helpers to `node/dbcache`
    Move dbcache policy and warning helpers out of `node/caches.{h,cpp}` so cache splitting can include only the dbcache policy it needs.
    Update node, Qt, and test includes for the new header.
    Keep the legacy Qt settings migration on the fixed `DEFAULT_KERNEL_CACHE` fallback while node default callers use `node::GetDefaultDBCache()`.
    
    Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
    Co-authored-by: sedited <seb.kung@gmail.com>
    f51ad8c703
  44. kernel: allow setting chainstate `dbcache`
    Expose `btck_chainstate_manager_options_set_database_cache_bytes()` so callers can supply the database cache budget instead of making `bitcoinkernel` compute node-side RAM policy.
    Keep `DEFAULT_KERNEL_CACHE` as the fallback when no override is set.
    Store the selected `kernel::CacheSizes` on `ChainstateManagerOptions`, keep the block-tree DB cache in sync, and pass the same sizes to `LoadChainstate()`.
    Add the C++ wrapper method and cover it in `test_kernel`.
    
    Co-authored-by: stickies-v <stickies-v@protonmail.com>
    f622af3ccb
  45. node: add `MAX_DBCACHE_BYTES`
    Move the architecture-dependent `-dbcache` cap next to the dbcache minimum.
    
    The effective cap is unchanged: `1 GiB` on 32-bit builds and `size_t` max elsewhere.
    Leave the storage type unchanged so the later `uint64_t` cleanup is isolated.
    0f8f64e638
  46. scripted-diff: rename `MIN_DB_CACHE`
    Add a `_BYTES` suffix to the minimum dbcache constant so it reads as a byte value next to `MAX_DBCACHE_BYTES`.
    
    -BEGIN VERIFY SCRIPT-
    git grep -q '\bMIN_DBCACHE_BYTES\b' -- src && echo "Error: renamed dbcache byte constant already exists in src" && exit 1
    git grep -l 'MIN_DB_CACHE' -- src | xargs perl -pi -e 's/\bMIN_DB_CACHE\b/MIN_DBCACHE_BYTES/g'
    -END VERIFY SCRIPT-
    
    Co-authored-by: optout <13562139+optout21@users.noreply.github.com>
    5478c72763
  47. test: require `TryGetTotalRam()` detection
    `TryGetTotalRam()` now feeds the automatic `-dbcache` default, so a detection failure should fail `system_ram_tests` instead of being skipped.
    Use `1_GiB` for the lower bound and remove the obsolete skip regex.
    b6a9e248a2
  48. node: use `uint64_t` for dbcache byte constants
    Keep dbcache byte constants in the same type as the byte-unit literals and parsed `-dbcache` byte count.
    `GetDefaultDBCache()` and cache-splitting APIs still return `size_t` at their allocation boundaries, but `CalculateDbCacheBytes()` no longer needs an explicit `std::min<uint64_t>`.
    ce12cd6a1c
  49. l0rinc force-pushed on Jun 24, 2026
  50. l0rinc commented at 2:43 AM on June 24, 2026: contributor

    rebased and applied nits. @stickies-v, I'd appreciate your reevaluation.

  51. w0xlt commented at 3:59 AM on June 24, 2026: contributor

    reACK ce12cd6a1c602658d2ef70cf9b26d7f727ef4af0

  52. sedited approved
  53. sedited commented at 8:42 AM on June 24, 2026: contributor

    ACK ce12cd6a1c602658d2ef70cf9b26d7f727ef4af0


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: 2026-07-02 04:51 UTC

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