Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 martinus commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817850120)
@sipa I thought that too, it would definitely make sense. Although it would need to move the `size_t MallocUsage(size_t alloc);` into the pool too

If the size increase does not help, it could also be that ARM for some reason needs a bigger alignment for its nodes.
In any case it would be interesting to run this again with this commit: https://github.com/martinus/bitcoin/commit/eaa6aaa481c45866815d67dc80c509be15f22974 this will log each time the pool is called with an allocation size it can't
...
💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817851058)
@martinus
> If the size increase does not help, it could also be that ARM for some reason needs a bigger alignment for its nodes. In any case it would be interesting to run this again with this commit: [martinus@eaa6aaa](https://github.com/martinus/bitcoin/commit/eaa6aaa481c45866815d67dc80c509be15f22974) this will log each time the pool is called with an allocation size it can't handle, which should be very noisy.

I've already tested my own similar commit. There were no log entries for it.
💬 sipa commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817851784)
@hebasto Did the `+ 64` change change anything at all?
💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817851986)
> @hebasto Did the `+ 64` change change anything at all?

It is running...
💬 martinus commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817852874)
> 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
💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817854534)
> @hebasto Did the `+ 64` change change anything at all?

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

No behavior change. Still OOM.
💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817854605)
> > 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...
💬 maflcko commented on pull request "refactor: P2P transport without serialize version and type":
(https://github.com/bitcoin/bitcoin/pull/28892#discussion_r1398408939)
Done
📝 maflcko opened a pull request: " refactor: VectorWriter and SpanReader without nVersion "
(https://github.com/bitcoin/bitcoin/pull/28912)
The serialize version is unused, so remove it. This also allows to remove `GCS_SER_VERSION` and allows a scripted-diff to remove most of `CDataStream`.
💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817869184)
> > 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"
💬 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?