💬 martinus commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817870240)
> 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

💬 TheCharlatan commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1382215744)
Mark constructors as `explicit`.

💬 TheCharlatan commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1382220405)
In commit 80ea259b0b8bb08637d1cb75f65ec342d03eae7a, I find the second sentence of the commit message confusing. Is there something missing?

💬 TheCharlatan commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1398410709)
The bool in `removeTx` is always `false` now, maybe remove it entirely?

💬 TheCharlatan commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1398414563)
Nit: Remove the unused argument names in the interface functions?

💬 TheCharlatan commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1398406098)
Nit: Doesn't really matter here, but could use list initialization.

💬 martinus commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817880300)
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

💬 sipa commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817885270)
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.

💬 furszy commented on issue "Signet mining is not possible when using descriptor wallet":
(https://github.com/bitcoin/bitcoin/issues/28911#issuecomment-1817889206)
Could you share the signet challenge?

Quick guess; you defined the block script as a multisig, and you didn't import the multisig descriptor.
The wallet will not automatically infer the multisig redeem script if you imported `pk()`/`pkh()`/`wpkh()` descriptors.

💬 hebasto commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1398422138)
@martinus

Could assessing of the `std::unordered_map::node_type` improve this heuristic?

💬 sipa commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817890856)
@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.

💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1398422857)
@hebasto Unfortunately not, I actually wanted to use this but learned that this is actually just the type of the node handle which more or less is just a pointer to the data. The actual type of the node used for allocation is not exposed by the standard. If it were, it would make the pool implementation quite a bit simpler

💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817902688)
> @hebasto could you please try this commit, I believe this should fix the issue: [martinus@72918e0](https://github.com/martinus/bitcoin/commit/72918e0d80455ece4aa96228be92246a83c8ad50)

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=37
...

💬 martinus commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817903990)
> It works.

Awesome! Should I open a PR?

💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817904314)
> Awesome! Should I open a PR?

Yes, please :)

📝 martinus opened a pull request: "coins: make sure PoolAllocator uses the correct alignment"
(https://github.com/bitcoin/bitcoin/pull/28913)
The class `CTxOut` has a member `CAmount` which is an int64_t, and on ARM 32bit int64_t are 8 byte aligned, which is larger than the pointer alignment of 4 bytes.

So for `CCoinsMap` to be able to use the pool, we need to use the alignment of the member instead of just `alignof(void*)`.

This fixes #28906 (first noted in https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1807197107)

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a
...

💬 hebasto commented on issue "26.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/28866#issuecomment-1817905395)
FWIW, the https://github.com/bitcoin/bitcoin/pull/25325 improves the UTXO cache performance noticeably. Might be tested with `bitcoind -reindex-chainstate`.

💬 martinus commented on issue "Impossible to run bitcoin on a Raspberry Pi 4 with 8GB, on Raspian 64 bit, which ships a 32 bit version of Docker (ARMHF instead of ARM64)":
(https://github.com/bitcoin/bitcoin/issues/28440#issuecomment-1817906128)
I'm pretty sure this issue is fixed in #28913 , because the logs here (`[...] cache=226.8MiB(31133550txo)`) indicate that the PoolAllocator is not used for the nodes, leading to incorrect cache size calculation.

🤔 mzumsande reviewed a pull request: "coins: make sure PoolAllocator uses the correct alignment"
(https://github.com/bitcoin/bitcoin/pull/28913#pullrequestreview-1738687795)
The pattern of `alignof(void*)` was used in 3 places in #25325 (also bench and tests) - looks like those other spots could be changed as well? (just a superficial observation, I didn't review that PR)

💬 hebasto commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817924791)
> The pattern of `alignof(void*)` was used in 3 places in #25325 (also bench and tests) - looks like those other spots could be changed as well? (just a superficial observation, I didn't review that PR)

Test doesn't fail now. Maybe replace `int` with `int64`?