💬 achow101 commented on pull request "clang-tidy: Disable `UndefinedBinaryOperatorResult` check in `src/ipc`":
(https://github.com/bitcoin/bitcoin/pull/33312#issuecomment-3267858690)
ACK 589b65f06c3376df4cde3fac5c1d39a2d3254920
(https://github.com/bitcoin/bitcoin/pull/33312#issuecomment-3267858690)
ACK 589b65f06c3376df4cde3fac5c1d39a2d3254920
✅ achow101 closed an issue: "ci: tidy job is producing output"
(https://github.com/bitcoin/bitcoin/issues/33256)
(https://github.com/bitcoin/bitcoin/issues/33256)
🚀 achow101 merged a pull request: "clang-tidy: Disable `UndefinedBinaryOperatorResult` check in `src/ipc`"
(https://github.com/bitcoin/bitcoin/pull/33312)
(https://github.com/bitcoin/bitcoin/pull/33312)
💬 achow101 commented on pull request "net: Add interrupt to pcp retry loop":
(https://github.com/bitcoin/bitcoin/pull/33338#issuecomment-3267870639)
ACK 188de70c86414b8b2ad5143f5c607b67686526ea
(https://github.com/bitcoin/bitcoin/pull/33338#issuecomment-3267870639)
ACK 188de70c86414b8b2ad5143f5c607b67686526ea
🚀 achow101 merged a pull request: "net: Add interrupt to pcp retry loop"
(https://github.com/bitcoin/bitcoin/pull/33338)
(https://github.com/bitcoin/bitcoin/pull/33338)
👍 hebasto approved a pull request: "ci: Checkout latest merged pulls"
(https://github.com/bitcoin/bitcoin/pull/33303#pullrequestreview-3198124781)
ACK fa8f081af31cd99155c7545643e7b10beb26714d.
(https://github.com/bitcoin/bitcoin/pull/33303#pullrequestreview-3198124781)
ACK fa8f081af31cd99155c7545643e7b10beb26714d.
💬 fjahr commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3267981848)
> Please adjust the PR description to the current approach ("switching most of the values tracked in the index from historical totals to values per block" is outdated).
Done
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3267981848)
> Please adjust the PR description to the current approach ("switching most of the values tracked in the index from historical totals to values per block" is outdated).
Done
💬 Raimo33 commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268007494)
I think I found the root cause, not tested, just speculating:
New Raspberry devices (Pi 3, Pi 4, etc.) are 64-bit CPUs. They can also be setup to run in 32bit mode, but I assume both the above benchmarks and issues used them in the default 64bit mode.
the incriminated functions: `secp256k1_fe_mul_inner` and `secp256k1_fe_sqr_inner` are notably the most expensive operations of libsecp256k1...
libsecp256k1 has various versions of these functions, compiled selectively based on the target archite
...
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268007494)
I think I found the root cause, not tested, just speculating:
New Raspberry devices (Pi 3, Pi 4, etc.) are 64-bit CPUs. They can also be setup to run in 32bit mode, but I assume both the above benchmarks and issues used them in the default 64bit mode.
the incriminated functions: `secp256k1_fe_mul_inner` and `secp256k1_fe_sqr_inner` are notably the most expensive operations of libsecp256k1...
libsecp256k1 has various versions of these functions, compiled selectively based on the target archite
...
💬 Crypt-iQ commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2331383900)
I originally decided not to, but makes sense. Done in 5e585a0fc4fd68dd7b4982054b34deae2e7aeb89
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2331383900)
I originally decided not to, but makes sense. Done in 5e585a0fc4fd68dd7b4982054b34deae2e7aeb89
💬 Crypt-iQ commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2331384232)
Thanks, done in 5e585a0fc4fd68dd7b4982054b34deae2e7aeb89
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2331384232)
Thanks, done in 5e585a0fc4fd68dd7b4982054b34deae2e7aeb89
💬 l0rinc commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268032980)
Thanks for the hint. I'm planning on investigating in more detail, but I have noticed my intuitions were off often, I try not to speculate anymore, it's why it would help if we could back these by any concrete measurements before we attempt a fix.
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268032980)
Thanks for the hint. I'm planning on investigating in more detail, but I have noticed my intuitions were off often, I try not to speculate anymore, it's why it would help if we could back these by any concrete measurements before we attempt a fix.
💬 instagibbs commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#issuecomment-3268044429)
@fjahr definitely not ideal, this PR is basically only removing the Assume crash while better state machine can be worked on
(https://github.com/bitcoin/bitcoin/pull/33296#issuecomment-3268044429)
@fjahr definitely not ideal, this PR is basically only removing the Assume crash while better state machine can be worked on
💬 Raimo33 commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268045072)
> Thanks for the hint. I'm planning on investigating in more detail, but I have noticed my intuitions were off often, I try not to speculate anymore, it's why it would help if we could back these by any concrete measurements before we attempt a fix.
Agreed, unfortunately I don't know how much evidence I can gather without having access to those machines, but will see what I can do. If I were you, I would start by checking if this holds true:
> I wouldn't be surprised if the 32bit version Pi wo
...
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268045072)
> Thanks for the hint. I'm planning on investigating in more detail, but I have noticed my intuitions were off often, I try not to speculate anymore, it's why it would help if we could back these by any concrete measurements before we attempt a fix.
Agreed, unfortunately I don't know how much evidence I can gather without having access to those machines, but will see what I can do. If I were you, I would start by checking if this holds true:
> I wouldn't be surprised if the 32bit version Pi wo
...
💬 sipa commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268045724)
Is this before or after the assumevalid point? If before, the secp256k1 operations won't even be used.
And if you are on a 64-bit ARM system, you absolutely want to use the 64-bit secp256k1 field implementations. They perform 4x fewer multiplications than the 32-bit ones. The asm optimizations compensate somewhat for that by better pipelining, but (1) can't overcome the fact that you just need far more arithmetic operations on 32-bit, and (2) the asm optimizations aren't even enabled by default
...
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268045724)
Is this before or after the assumevalid point? If before, the secp256k1 operations won't even be used.
And if you are on a 64-bit ARM system, you absolutely want to use the 64-bit secp256k1 field implementations. They perform 4x fewer multiplications than the 32-bit ones. The asm optimizations compensate somewhat for that by better pipelining, but (1) can't overcome the fact that you just need far more arithmetic operations on 32-bit, and (2) the asm optimizations aren't even enabled by default
...
💬 Raimo33 commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268049230)
> the asm optimizations aren't even enabled by default in Bitcoin Core builds.
Is there a specific reason for this?
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268049230)
> the asm optimizations aren't even enabled by default in Bitcoin Core builds.
Is there a specific reason for this?
💬 sipa commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268056766)
Historically, because they were new and unreviewed, and after that I guess we forgot about them (sorry, @laanwj ...).
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268056766)
Historically, because they were new and unreviewed, and after that I guess we forgot about them (sorry, @laanwj ...).
💬 Raimo33 commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268058223)
@l0rinc if I were you I would setup a simple benchmark to test how many cycles it takes to execute a single 64x64->128 multiplication. and compare that with your i7.
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268058223)
@l0rinc if I were you I would setup a simple benchmark to test how many cycles it takes to execute a single 64x64->128 multiplication. and compare that with your i7.
🤔 instagibbs reviewed a pull request: "Cluster mempool implementation"
(https://github.com/bitcoin/bitcoin/pull/28676#pullrequestreview-3197152141)
few comments through "initial implementation"
(https://github.com/bitcoin/bitcoin/pull/28676#pullrequestreview-3197152141)
few comments through "initial implementation"
💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2331016065)
34251141eca785578a0b2ca6d1c0932d34b2418d
this block is already public
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2331016065)
34251141eca785578a0b2ca6d1c0932d34b2418d
this block is already public
💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2330732155)
7ddce57e96aff26fb2a5abc1009e6baef24d5bab
look for some trivial coverage of this, or directly use this in GetVirtualTransactionSize?
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2330732155)
7ddce57e96aff26fb2a5abc1009e6baef24d5bab
look for some trivial coverage of this, or directly use this in GetVirtualTransactionSize?
💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2331041173)
Can `Assume(!m_dependencies_processed);`
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2331041173)
Can `Assume(!m_dependencies_processed);`