Repository: bitcoin/bitcoin
File: src/node/caches.cpp (C++)

Code before:
#include <common/args.h>
#include <index/txindex.h>
#include <kernel/caches.h>
#include <logging.h>
#include <util/byte_conversion.h>

#include <algorithm>
#include <optional>
#include <stdexcept>
#include <string>

// Unlike for the UTXO database, for the txindex scenario the leveldb cache make
// a meaningful difference: https://github.com/bitcoin/bitcoin/pull/8273#issuecomment-229601991
//! Max memory allocated to tx index DB specific cache in bytes.
static constexpr size_t MAX_TX_INDEX_CACHE{MiBToBytes(1024)};
//! Max memory allocated to all block filter index caches combined in bytes.
static constexpr size_t MAX_FILTER_INDEX_CACHE{MiBToBytes(1024)};

namespace node {
std::optional<CacheSizes> CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
{
    int64_t db_cache = args.GetIntArg("-dbcache", DEFAULT_DB_CACHE);

    // negative values are permitted, but interpreted as zero.
    db_cache = std::max(int64_t{0}, db_cache);

    size_t total_cache = 0;
    try {
        total_cache = std::max(MiBToBytes(db_cache), MiBToBytes(MIN_DB_CACHE));
    } catch (const std::out_of_range&) {
        LogError("Cannot allocate more than %d MiB in total for db caches.", std::numeric_limits<size_t>::max() >> 20);
        return std::nullopt;
    }

    IndexCacheSizes index_sizes;
    index_sizes.tx_index = std::min(total_cache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE : 0);
    total_cache -= index_sizes.tx_index;
    index_sizes.filter_index = 0;
    if (n_indexes > 0) {
        int64_t max_cache = std::min(total_cache / 8, MAX_FILTER_INDEX_CACHE);
        index_sizes.filter_index = max_cache / n_indexes;
        total_cache -= index_sizes.filter_index * n_indexes;
    }
    return {{index_sizes, kernel::CacheSizes{total_cache}}};
}
} // namespace node


Review comment (performance):
In commit "init: Use size_t consistently for cache sizes" (82706e217f34d1f09cbd30dde6b8ae5ac0385f0a)

Code could be simplified to make it clear what is causing the new error:

<details><summary>diff</summary>
<p>

```diff
--- a/src/node/caches.cpp
+++ b/src/node/caches.cpp
@@ -25,14 +25,10 @@ static constexpr size_t MAX_FILTER_INDEX_CACHE{MiBToBytes(1024)};
 namespace node {
 std::optional<CacheSizes> CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
 {
-    int64_t db_cache = args.GetIntArg("-dbcache", DEFAULT_DB_CACHE);
-
-    // negative values are permitted, but interpreted as zero.
-    db_cache = std::max(int64_t{0}, db_cache);
-
+    const int64_t db_cache = std::max(MIN_DB_CACHE, args.GetIntArg("-dbcache", DEFAULT_DB_CACHE));
     size_t total_cache = 0;
     try {
-        total_cache = std::max(MiBToBytes(db_cache), MiBToBytes(MIN_DB_CACHE));
+        total_cache = MiBToBytes(db_cache);
     } catch (const std::out_of_range&) {
         LogError("Cannot allocate more than %d MiB in total for db caches.", std::numeric_limits<size_t>::max() >> 20);
         return std::nullopt;
```
</p>
</details>

Though I think we don't need this error, could do something even simpler like:

```c++
// Total cache size in MiB, floored by MIN_DB_CACHE
const int64_t db_cache = std::max(MIN_DB_CACHE, args.GetIntArg("-dbcache", DEFAULT_DB_CACHE));
// Total cache size in bytes, capped by max size_t value
size_t total_cache = std::min<int64_t>(db_cache, std::numeric_limits<size_t>::max() >> 20) << 20;
```

Current code or whatever you prefer seems fine though.

Code after:
#include <common/args.h>
#include <index/txindex.h>
#include <kernel/caches.h>
#include <logging.h>
#include <util/byte_units.h>

#include <algorithm>
#include <string>

// Unlike for the UTXO database, for the txindex scenario the leveldb cache make
// a meaningful difference: https://github.com/bitcoin/bitcoin/pull/8273#issuecomment-229601991
//! Max memory allocated to tx index DB specific cache in bytes.
static constexpr size_t MAX_TX_INDEX_CACHE{1024_MiB};
//! Max memory allocated to all block filter index caches combined in bytes.
static constexpr size_t MAX_FILTER_INDEX_CACHE{1024_MiB};

namespace node {
CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
{
    // Convert -dbcache from MiB units to bytes. The total cache is floored by MIN_DB_CACHE and capped by max size_t value.
    size_t total_cache{DEFAULT_DB_CACHE};
    if (std::optional<int64_t> db_cache = args.GetIntArg("-dbcache")) {
        if (*db_cache < 0) db_cache = 0;
        uint64_t db_cache_bytes = SaturatingLeftShift<uint64_t>(*db_cache, 20);
        total_cache = std::max<size_t>(MIN_DB_CACHE, std::min<uint64_t>(db_cache_bytes, std::numeric_limits<size_t>::max()));
    }

    IndexCacheSizes index_sizes;
    index_sizes.tx_index = std::min(total_cache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE : 0);
    total_cache -= index_sizes.tx_index;
    if (n_indexes > 0) {
        size_t max_cache = std::min(total_cache / 8, MAX_FILTER_INDEX_CACHE);
        index_sizes.filter_index = max_cache / n_indexes;
        total_cache -= index_sizes.filter_index * n_indexes;
    }
    return {index_sizes, kernel::CacheSizes{total_cache}};
}
} // namespace node