Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 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?
💬 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