bitcoin: RAM usage regression in 26.x and master on ARM 32-bit

A regression is caused by https://github.com/bitcoin/bitcoin/pull/25325.

cc @martinus @sipa @achow101

@hebasto could you please try this commit, I believe this should fix the issue: martinus@72918e0

It works.

Here’s an excerpt from the log with the first flushing:

2023-11-19T16:17:58Z [initload] UpdateTip: new best=00000000000002711e96b4c88a8c60b97011068f0c644bf569a12bfa72b28a0d height=213239 version=0x00000002 log2_work=69.196319 tx=10230628 date='2012-12-23T00:00:41Z' progress=0.011054 cache=376.0MiB(3527856txo)
2023-11-19T16:17:58Z [initload] Cache size (394554040) exceeds total space (394371840)

The RAM usage remains as low as expected.

@martinus Thank you for the fix so much!

I’ve already tested my own similar commit. There were no log entries for it.

There should always be quite a few log lines for the map buckets, even when all works as expected

The log is flooded with “Allocate: 104 align(8), MAX_BLOCK_SIZE_BYTES=112, ALIGN_BYTES=4”

It works.

Awesome! Should I open a PR?

@hebasto Seeing your deleted comment elsewhere, I don’t think std::unordered_map::node_type is useful, as it’s just a reference to a node, it doesn’t actually contain the node data.

The log is flooded with “Allocate: 104 align(8), MAX_BLOCK_SIZE_BYTES=112, ALIGN_BYTES=4”

Ha! This is the issue right there. For whatever reason the nodes require alignment of 8 bytes, but the pool is configured to use the platform’s 4 byte instead. Thanks a lot for all the testing! I’ll try to prepare a fix

I’ve already tested my own similar commit. There were no log entries for it.

There should always be quite a few log lines for the map buckets, even when all works as expected

Testing now…

@hebasto Did the + 64 change change anything at all?

Tested https://github.com/hebasto/bitcoin/commit/80c02a9fe6607fb0a4abfca66be7be42fd744905.

No behavior change. Still OOM.

@hebasto Did the + 64 change change anything at all?

It is running… Will report soon.

Awesome! Should I open a PR?

Yes, please 😃

My suggestion would be to use @martinus’ commit above in 26.0 (if it fixes things), and then try to make sure fallback allocations get counted too in master.

TIL that on ARM 32bit an int64_t has 8 byte alignment, even when a pointer has 4 byte alignment: https://godbolt.org/z/xW3avfGef

@hebasto could you please try this commit, I believe this should fix the issue: https://github.com/martinus/bitcoin/commit/72918e0d80455ece4aa96228be92246a83c8ad50

@hebasto The guess for node size is here: https://github.com/bitcoin/bitcoin/blob/v26.0rc2/src/coins.h#L148

Can you try increasing that (say, add 64 to it), and see if that fixes things?

@hebasto could you try another patch? martinus@04fb0b3 This removes the placement new that didn’t reuse the returned pointer, which technically has always been illegal

Tested. No behavior change. Still OOM.

@hebasto could you try another patch? https://github.com/martinus/bitcoin/commit/04fb0b35cf18d5805d58595c27f027fae4428e62 This removes the placement new that didn’t reuse the returned pointer, which technically has always been illegal

This is a shot in the dark, but could this be due to memory fragmentation? The PoolResource allocates blocks of 262144 bytes, which seems quite large for such a system. Could you try this patch, or is there a way for me to try this? It reduces the allocated block size to 16K (and fixes a few tests that rely on the hardcoded size).

diff --git a/src/support/allocators/pool.h b/src/support/allocators/pool.h
index c8e70ebacff..9f93511995e 100644
--- a/src/support/allocators/pool.h
+++ b/src/support/allocators/pool.h
@@ -184,7 +184,7 @@ public:
     /**
      * Construct a new Pool Resource object, defaults to 2^18=262144 chunk size.
      */
-    PoolResource() : PoolResource(262144) {}
+    PoolResource() : PoolResource(16384) {}
 
     /**
      * Disable copy & move semantics, these are not supported for the resource.
diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
index 12dc4d1ccc4..5f1a0120aca 100644
--- a/src/test/coins_tests.cpp
+++ b/src/test/coins_tests.cpp
@@ -1088,18 +1088,18 @@ BOOST_AUTO_TEST_CASE(coins_resource_is_used)
 {
     CCoinsMapMemoryResource resource;
     PoolResourceTester::CheckAllDataAccountedFor(resource);
-
+    size_t num_elements = 16384 / 200;
     {
         CCoinsMap map{0, CCoinsMap::hasher{}, CCoinsMap::key_equal{}, &resource};
         BOOST_TEST(memusage::DynamicUsage(map) >= resource.ChunkSizeBytes());
 
-        map.reserve(1000);
+        map.reserve(num_elements);
 
         // The resource has preallocated a chunk, so we should have space for at several nodes without the need to allocate anything else.
         const auto usage_before = memusage::DynamicUsage(map);
 
         COutPoint out_point{};
-        for (size_t i = 0; i < 1000; ++i) {
+        for (size_t i = 0; i < num_elements; ++i) {
             out_point.n = i;
             map[out_point];
         }
diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp
index 7398091215c..c8f2c9b32fa 100644
--- a/src/test/validation_flush_tests.cpp
+++ b/src/test/validation_flush_tests.cpp
@@ -37,7 +37,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
     };
 
     // PoolResource defaults to 256 KiB that will be allocated, so we'll take that and make it a bit larger.
-    constexpr size_t MAX_COINS_CACHE_BYTES = 262144 + 512;
+    constexpr size_t MAX_COINS_CACHE_BYTES = 16384 + 512;
 
     // Without any coins in the cache, we shouldn't need to flush.
     BOOST_TEST(