💬 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
(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.
(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.
(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?
(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.
(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
(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
...
(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?
(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 :)
(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
...
(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`.
(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.
(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)
(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`?
(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`?
💬 martinus commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817927114)
> The pattern of alignof(void*) was used in 3 places
Thanks, I forgot about these... I'll change the PoolAllocator to default to the alignment of the given type, that makes much more sense and reduces the code.
> Test doesn't fail now. Maybe replace int with int64_t?
Good idea, I'll change that too
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817927114)
> The pattern of alignof(void*) was used in 3 places
Thanks, I forgot about these... I'll change the PoolAllocator to default to the alignment of the given type, that makes much more sense and reduces the code.
> Test doesn't fail now. Maybe replace int with int64_t?
Good idea, I'll change that too
💬 hebasto commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817934135)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817934135)
Concept ACK.
💬 fujicoin commented on issue "Signet mining is not possible when using descriptor wallet":
(https://github.com/bitcoin/bitcoin/issues/28911#issuecomment-1817941716)
@furszy
You can reproduce the phenomenon by following the steps below.
```
wget https://bitcoincore.org/bin/bitcoin-core-25.0/bitcoin-25.0-x86_64-linux-gnu.tar.gz
tar -xzvf bitcoin-25.0-x86_64-linux-gnu.tar.gz
cd bitcoin-25.0
mv bin/* .
git clone https://github.com/bitcoin/bitcoin.git
cd bitcoin
git checkout v25.0
cd ..
cp -r bitcoin/test/functional/test_framework .
cp bitcoin/contrib/signet/miner .
./bitcoind -regtest -daemon
./bitcoin-cli -regtest createwallet "test" false
...
(https://github.com/bitcoin/bitcoin/issues/28911#issuecomment-1817941716)
@furszy
You can reproduce the phenomenon by following the steps below.
```
wget https://bitcoincore.org/bin/bitcoin-core-25.0/bitcoin-25.0-x86_64-linux-gnu.tar.gz
tar -xzvf bitcoin-25.0-x86_64-linux-gnu.tar.gz
cd bitcoin-25.0
mv bin/* .
git clone https://github.com/bitcoin/bitcoin.git
cd bitcoin
git checkout v25.0
cd ..
cp -r bitcoin/test/functional/test_framework .
cp bitcoin/contrib/signet/miner .
./bitcoind -regtest -daemon
./bitcoin-cli -regtest createwallet "test" false
...
💬 pinheadmz commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817951572)
concept ACK
running this branch on my RPi same as https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1816281195 will update
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817951572)
concept ACK
running this branch on my RPi same as https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1816281195 will update
💬 sipa commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817953959)
utACK 486c2916a18b56d6011117077d906c1f8a81623f
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817953959)
utACK 486c2916a18b56d6011117077d906c1f8a81623f
💬 1440000bytes commented on pull request "wallet: Deniability API (Unilateral Transaction Meta-Privacy)":
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-1817954781)
@denavila I am okay with this merged for testnet or signet alleast
We want to improve privacy and see results
Thats not how things work here
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-1817954781)
@denavila I am okay with this merged for testnet or signet alleast
We want to improve privacy and see results
Thats not how things work here