kernel,node: clean up dbcache helpers and add kernel API#35205
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35205. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please copy-paste ConflictsReviewers, this pull request conflicts with the following ones:
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:
2026-05-18 20:37:05 |
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
l0rinc
force-pushed
the
l0rinc/share-dbcache-defaults
branch
from
eb35f8b to
892d6cb
Compare
stickies-v
left a comment
stickies-v
left a comment
There was a problem hiding this comment.
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
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.
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>
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-
Store the detected total RAM in a function-local static so startup paths do not repeat the underlying system query.
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>
l0rinc
changed the title
dbcache default sizingdbcache helpers and add kernel API
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>
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.
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>
`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.
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>`.
l0rinc
force-pushed
the
l0rinc/share-dbcache-defaults
branch
from
892d6cb to
78a6d4b
Compare
|
Thanks @stickies-v, I rewrote the kernel part based on your feedback. The latest push no longer makes |
4 participants
Problem
PR #34692 kept the simplified two-tier
-dbcachedefault from #34641: use1024 MiBon 64-bit systems with at least4 GiBof detected RAM, and otherwise keep450 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,
kernelstill has only a fixed450 MiBfallback and no C API for callers to provide a cache budget from outside.Fix
Move RAM detection to
common/system_ramand movedbcacheconstants/helpers tonode/dbcache, then route the node and Qt dbcache default users throughnode::GetDefaultDBCache().Keep
kernelindependent from the node/common RAM policy by preservingDEFAULT_KERNEL_CACHEas 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.