💬 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.

💬 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

💬 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
...

💬 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
...

💬 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?

💬 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 ...).

💬 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.

🤔 instagibbs reviewed a pull request: "Cluster mempool 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

💬 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?

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2331041173)
Can `Assume(!m_dependencies_processed);`

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2330691874)
56999a7085f21c8d462fd5afdfbb41dd93df423f

now is a great time to document what this thing is

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2331064925)
34251141eca785578a0b2ca6d1c0932d34b2418d

I didn't try implementing but have you considered simply processing dependencies as you go in PreChecks->StageAddition? Then we don't have to track whether we've applied deps.

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2331258415)
64de3bb675e2ff3b8f0bb1de3d7680cbb2263807

this TODO is addressed in this commit

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2331325786)
64de3bb675e2ff3b8f0bb1de3d7680cbb2263807

do we need `dynamic_cast` here since we know the underlying type vs static_cast?

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2331300634)
64de3bb675e2ff3b8f0bb1de3d7680cbb2263807

take or leave suggestion: `addChunks` could take care of starting and stopping block building; block builder doesn't have to live outside of that function?

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2331366938)
266500e01e996c6c08af31880545efb5568ded8b

MAX_REPLACEMENT_CANDIDATES documentation in rbf.h should get updated, including mentions at GetEntriesForConflicts

💬 instagibbs commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#issuecomment-3268061859)
reACK https://github.com/bitcoin/bitcoin/pull/33296/commits/8b6264768030db1840041abeeaeefd6c227a2644

💬 sipa commented on issue "[IBD] Raspberry Pi: 90% CPU time for 1.5% of block processing":
(https://github.com/bitcoin/bitcoin/issues/32832#issuecomment-3268062397)
Or run the libsecp256k1 benchmarks on both to see how fast signature verification is on both systems.

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2331417443)
still prefer `main_only` to be explicitly called

💬 Crypt-iQ commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#issuecomment-3268086810)
> Maybe a simple bool flag failed_reconstruction on PartiallyDownloadedBlock would work? Feel free to ignore.

This would work, but I think we should wait until there is an in-repo fuzz harness with good coverage before trying to refactor the net_processing state or the `PartiallyDownloadedBlock` state. I think a bool `attempted_reconstruction` could replace the calls to `SetNull()` / `IsNull()`. An ideal solution would be to get rid of the unusable `PartiallyDownloadedBlock` while making sure
...