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

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.

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

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

Code Coverage & Benchmarks

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
Approach NACK	stickies-v

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35200 (node: smooth oversized dbcache warnings by l0rinc)
#34995 (ci, iwyu: Fix warnings in src/common and treat them as errors by hebasto)
#34806 (refactor: logging: Various API improvements by ajtowns)
#32427 (kernel: Replace leveldb-based BlockTreeDB with flat-file based store by sedited)
#30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)
#28690 (build: Introduce internal kernel library by sedited)
#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)
#24230 (indexes: Stop using node internal types and locking cs_main, improve sync logic 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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

only useable from command line, not configuration file -> only usable from command line, not configuration file [“useable” is misspelled]

2026-05-18 20:37:05

DrahtBot added the label CI failed on May 4, 2026

DrahtBot commented at 1:00 PM on May 4, 2026: contributor

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

<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>

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:

_SC_PHYS_PAGES: https://github.com/apple-oss-distributions/Libc/blob/71bbe350ab79eef58113991d817ccc6165061a64/include/unistd.h#L383
_SC_PAGESIZE: https://github.com/apple-oss-distributions/Libc/blob/main/include/unistd.h#L259

If it ever becomes a problem we can apply the alternative your 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!

l0rinc closed this on May 5, 2026

l0rinc reopened this on May 5, 2026

DrahtBot removed the label CI failed on May 5, 2026

l0rinc force-pushed on May 5, 2026

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

l0rinc marked this as a draft on May 7, 2026

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.

f1ca4b7dc6

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>

c4237ec166

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-

3f7b8c981c

common: cache `TryGetTotalRam()` result

Store the detected total RAM in a function-local static so startup paths do not repeat the underlying system query.

1dd684007c

node: move dbcache helpers to `node/dbcache`

Move dbcache defaults and warning helpers out of `node/caches.{h,cpp}` so cache splitting can include only the policy it needs.
Update node, Qt, and test includes for the new header, and have the Qt migration use `node::GetDefaultDBCache()`.

Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>

ba77d674d3

l0rinc renamed this:
~~node,kernel: share `dbcache` default sizing~~
kernel,node: clean up `dbcache` helpers and add kernel API
on May 18, 2026

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>

c72d26ff94

node: add `MAX_DBCACHE_BYTES`

Move the architecture-dependent `-dbcache` cap next to the other dbcache constants.
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.

f4a2436157

scripted-diff: rename dbcache byte constants

Add a `_BYTES` suffix to the min and default dbcache constants so they read as a group with `MAX_DBCACHE_BYTES`.

-BEGIN VERIFY SCRIPT-
git grep -qE '\b(MIN_DBCACHE_BYTES|DEFAULT_DBCACHE_BYTES)\b' -- src && echo "Error: renamed dbcache byte constants already exist in src" && exit 1
git grep -l \
    -e 'MIN_DB_CACHE' -e 'DEFAULT_DB_CACHE' -- src | \
    xargs perl -pi \
        -e 's/\bMIN_DB_CACHE\b/MIN_DBCACHE_BYTES/g;' \
        -e 's/\bDEFAULT_DB_CACHE\b/DEFAULT_DBCACHE_BYTES/g;'
-END VERIFY SCRIPT-

Co-authored-by: optout <13562139+optout21@users.noreply.github.com>

b5395d828a

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.

0e6ac47156

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>`.

78a6d4b616

l0rinc force-pushed on May 18, 2026

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.

l0rinc marked this as ready for review on May 18, 2026

DrahtBot added the label CI failed on May 18, 2026

DrahtBot removed the label CI failed on May 19, 2026