💬 TheCharlatan commented on pull request "refactor: Make CTxMemPoolEntry non-copyable":
(https://github.com/bitcoin/bitcoin/pull/28903#issuecomment-1817123452)
Thank you @ajtowns,
Updated b9e4b4cb275e998376b0e3e68b6c22c5b1877fbb -> b8b624b36ebeb167da2f9c6d12f563f6849ccd60 ([CTxMemPoolEntryNonCopy_0](https://github.com/TheCharlatan/bitcoin/tree/CTxMemPoolEntryNonCopy_0) -> [CTxMemPoolEntryNonCopy_1](https://github.com/TheCharlatan/bitcoin/tree/CTxMemPoolEntryNonCopy_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/CTxMemPoolEntryNonCopy_0..CTxMemPoolEntryNonCopy_1))
* Applied @ajtowns' [suggestion](https://github.com/bitcoin/bitcoin/pu
...
(https://github.com/bitcoin/bitcoin/pull/28903#issuecomment-1817123452)
Thank you @ajtowns,
Updated b9e4b4cb275e998376b0e3e68b6c22c5b1877fbb -> b8b624b36ebeb167da2f9c6d12f563f6849ccd60 ([CTxMemPoolEntryNonCopy_0](https://github.com/TheCharlatan/bitcoin/tree/CTxMemPoolEntryNonCopy_0) -> [CTxMemPoolEntryNonCopy_1](https://github.com/TheCharlatan/bitcoin/tree/CTxMemPoolEntryNonCopy_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/CTxMemPoolEntryNonCopy_0..CTxMemPoolEntryNonCopy_1))
* Applied @ajtowns' [suggestion](https://github.com/bitcoin/bitcoin/pu
...
📝 brunoerg opened a pull request: "fuzz: compare scripts from `Expand` and `ExpandFromCache`"
(https://github.com/bitcoin/bitcoin/pull/28908)
When testing a descriptor, if it was able to `Expand`, `ExpandFromCache` should also work and scripts should be the same. This PR covers it in `TestDescriptor` for `descriptor_parse` fuzz.
(https://github.com/bitcoin/bitcoin/pull/28908)
When testing a descriptor, if it was able to `Expand`, `ExpandFromCache` should also work and scripts should be the same. This PR covers it in `TestDescriptor` for `descriptor_parse` fuzz.
💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817172322)
@martinus
> 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).
I've tested your [patch](https://github.com/hebasto/artefacts/blob/bisect-arm-OOM.1/bisect-arm-OOM/bitcoin-274a7b769afb-arm-linux-gnueabihf.tar.gz
...
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817172322)
@martinus
> 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).
I've tested your [patch](https://github.com/hebasto/artefacts/blob/bisect-arm-OOM.1/bisect-arm-OOM/bitcoin-274a7b769afb-arm-linux-gnueabihf.tar.gz
...
💬 ryanofsky commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1817233427)
PR description lists a few reasons for bumping the capnproto version, but it doesn't actually mention the original motivation, which was to fix a C++20 compiler issue: https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1778595873, https://github.com/capnproto/capnproto/pull/1618. Just wanted to note this explicitly
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1817233427)
PR description lists a few reasons for bumping the capnproto version, but it doesn't actually mention the original motivation, which was to fix a C++20 compiler issue: https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1778595873, https://github.com/capnproto/capnproto/pull/1618. Just wanted to note this explicitly
💬 martinus commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817427540)
It could be that the memory usage calculation has changed, and calculation for memory used in the CCoinsMap is now more correct than before. Previously it overestimated memory usage, and now it's more correct but for the same cache size this also leads to a bit higher memory usage
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817427540)
It could be that the memory usage calculation has changed, and calculation for memory used in the CCoinsMap is now more correct than before. Previously it overestimated memory usage, and now it's more correct but for the same cache size this also leads to a bit higher memory usage
💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817452601)
> It could be that the memory usage calculation has changed, and calculation for memory used in the CCoinsMap is now more correct than before. Previously it overestimated memory usage, and now it's more correct but for the same cache size this also leads to a bit higher memory usage
It depends on definition of "a bit higher" :) It was less than 0.9 GB of RAM. Now 2 GB of RAM plus 1 GB swap is not enough.
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817452601)
> It could be that the memory usage calculation has changed, and calculation for memory used in the CCoinsMap is now more correct than before. Previously it overestimated memory usage, and now it's more correct but for the same cache size this also leads to a bit higher memory usage
It depends on definition of "a bit higher" :) It was less than 0.9 GB of RAM. Now 2 GB of RAM plus 1 GB swap is not enough.
💬 martinus commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817453279)
"a bit higher" should be about 2% or so...
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817453279)
"a bit higher" should be about 2% or so...
💬 martinus commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817525832)
I investigated a bit with [heaptrack](https://github.com/KDE/heaptrack) and it seems there are a bit higher spikes than I have thought. It's still not too bad, previously heaptrack says peack memory is ~600MB, and with the pool it's ~700 MB. That bigger peak seems to happen in `CCoinsViewCache::BatchWrite`.
Previously, the `mapCoins.erase(it)` freed a node, and a subsequent `cacheCoins[it->first]` could use that freed memory. With the pool that is currently not possible any more, because the
...
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817525832)
I investigated a bit with [heaptrack](https://github.com/KDE/heaptrack) and it seems there are a bit higher spikes than I have thought. It's still not too bad, previously heaptrack says peack memory is ~600MB, and with the pool it's ~700 MB. That bigger peak seems to happen in `CCoinsViewCache::BatchWrite`.
Previously, the `mapCoins.erase(it)` freed a node, and a subsequent `cacheCoins[it->first]` could use that freed memory. With the pool that is currently not possible any more, because the
...
💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817526549)
From the observing the RAM usage during runtime, I would say that RAM usage increases over time until OOM.
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817526549)
From the observing the RAM usage during runtime, I would say that RAM usage increases over time until 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-1817583116)
From [IRC](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-11-17#984227;):
> 19:42 \<sipa\> and it's only with guix builds?
I also can confirm the regression for binaries that were natively compiled with GCC 11.4.0.
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817583116)
From [IRC](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-11-17#984227;):
> 19:42 \<sipa\> and it's only with guix builds?
I also can confirm the regression for binaries that were natively compiled with GCC 11.4.0.
✅ furszy closed a pull request: "wallet: group independent db writes on single batched db transaction"
(https://github.com/bitcoin/bitcoin/pull/25297)
(https://github.com/bitcoin/bitcoin/pull/25297)
💬 furszy commented on pull request "wallet: group independent db writes on single batched db transaction":
(https://github.com/bitcoin/bitcoin/pull/25297#issuecomment-1817587008)
Still relevant and quite useful but closing it until #28574 and its derivatives are merged.
(https://github.com/bitcoin/bitcoin/pull/25297#issuecomment-1817587008)
Still relevant and quite useful but closing it until #28574 and its derivatives are merged.
👍 TheCharlatan approved a pull request: "Drop CAutoFile"
(https://github.com/bitcoin/bitcoin/pull/28904#pullrequestreview-1738457555)
ACK 4eb2a9ea4b6262bec0bc7c20cb3e684ea75caf42
(https://github.com/bitcoin/bitcoin/pull/28904#pullrequestreview-1738457555)
ACK 4eb2a9ea4b6262bec0bc7c20cb3e684ea75caf42
💬 ishaanam commented on pull request "wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction":
(https://github.com/bitcoin/bitcoin/pull/25273#discussion_r1398253415)
In ebc39da8e5f72521fb4889bc1adc0a80136ce3da "wallet: Explicitly preserve transaction locktime in CreateTransaction":
The user-provided locktime and sequence values should also be set in the `sendall` `coin_control`. In the future if we want to take locktime into account when running `AvailbleCoins`, this would be very useful.
(https://github.com/bitcoin/bitcoin/pull/25273#discussion_r1398253415)
In ebc39da8e5f72521fb4889bc1adc0a80136ce3da "wallet: Explicitly preserve transaction locktime in CreateTransaction":
The user-provided locktime and sequence values should also be set in the `sendall` `coin_control`. In the future if we want to take locktime into account when running `AvailbleCoins`, this would be very useful.
💬 martinus commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817606023)
@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
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817606023)
@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
📝 vostrnad opened a pull request: "doc: Add release note for coinstatsindex in pruned mode"
(https://github.com/bitcoin/bitcoin/pull/28909)
#21726 enabled using `coinstatsindex` in pruned mode. I recently had to update one of my nodes from 23.0 because of this and noticed that this isn't mentioned in the release notes for 24.0.1.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide
...
(https://github.com/bitcoin/bitcoin/pull/28909)
#21726 enabled using `coinstatsindex` in pruned mode. I recently had to update one of my nodes from 23.0 because of this and noticed that this isn't mentioned in the release notes for 24.0.1.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide
...
💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817630948)
Is this difference in reported cache size expected:
- in master @ 3a93957a5dc97cb2fd0656d1e2451ebef57204df (pre-[PR25325](https://github.com/bitcoin/bitcoin/pull/25325)):
```
UpdateTip: new best=00000000000001bc255d63d3e82af7638566a5fbaadb6d5bdc41d0731a23d1ea height=209285 version=0x00000001 log2_work=69.067375 tx=9192843 date='2012-11-24T00:09:17Z' progress=0.010297 cache=376.0MiB(3276665txo)
Cache size (394377008) exceeds total space (394371840)
```
- in master @ 5aa0c82ccd6ceb4a141686fc
...
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817630948)
Is this difference in reported cache size expected:
- in master @ 3a93957a5dc97cb2fd0656d1e2451ebef57204df (pre-[PR25325](https://github.com/bitcoin/bitcoin/pull/25325)):
```
UpdateTip: new best=00000000000001bc255d63d3e82af7638566a5fbaadb6d5bdc41d0731a23d1ea height=209285 version=0x00000001 log2_work=69.067375 tx=9192843 date='2012-11-24T00:09:17Z' progress=0.010297 cache=376.0MiB(3276665txo)
Cache size (394377008) exceeds total space (394371840)
```
- in master @ 5aa0c82ccd6ceb4a141686fc
...
💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817631083)
> @hebasto could you try another patch? [martinus@04fb0b3](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
Tested. No behavior change. Still OOM.
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817631083)
> @hebasto could you try another patch? [martinus@04fb0b3](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
Tested. No behavior change. Still OOM.
💬 martinus commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817643679)
> Is this difference in the reported cache size expected
Yes, when the pool is used the flushes happen at different times because more transactions are cached, so the number of cached transaction at certain heights can be completely different
> Tested. No behavior change. Still OOM.
Thanks for testing! Hm, I'm out of ideas for now... I don't see anything in the code that could cause this. My only suspicion is that for some reason on ARM the freed memory from the pool is not reused, but
...
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817643679)
> Is this difference in the reported cache size expected
Yes, when the pool is used the flushes happen at different times because more transactions are cached, so the number of cached transaction at certain heights can be completely different
> Tested. No behavior change. Still OOM.
Thanks for testing! Hm, I'm out of ideas for now... I don't see anything in the code that could cause this. My only suspicion is that for some reason on ARM the freed memory from the pool is not reused, but
...
💬 hebasto commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817662547)
> > Is this difference in the reported cache size expected
>
>
>
> Yes, when the pool is used the flushes happen at different times because more transactions are cached, so the number of cached transaction at certain heights can be completely different
The point is that the former log shows the first flush. In the latter one, there were no flushes by that point at all.
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1817662547)
> > Is this difference in the reported cache size expected
>
>
>
> Yes, when the pool is used the flushes happen at different times because more transactions are cached, so the number of cached transaction at certain heights can be completely different
The point is that the former log shows the first flush. In the latter one, there were no flushes by that point at all.