📝 Raimo33 opened a pull request: "[WIP] cache: remove redundant find() call"
(https://github.com/bitcoin/bitcoin/pull/33376)
## Summary
This PR halves the number of implicit calls to `::find()` by using an optimistic approach: insert preemptively, remove O(1) if already spent.
## Motivation
Currently, the `::find()` calls that originate from `CCoinsViewCache::BatchWrite()` account for ~2.9% of total IBD time as shown by this flamegraph from issue #32832

## Benchmarks
Benchmarks can't currently measure this
...
(https://github.com/bitcoin/bitcoin/pull/33376)
## Summary
This PR halves the number of implicit calls to `::find()` by using an optimistic approach: insert preemptively, remove O(1) if already spent.
## Motivation
Currently, the `::find()` calls that originate from `CCoinsViewCache::BatchWrite()` account for ~2.9% of total IBD time as shown by this flamegraph from issue #32832

## Benchmarks
Benchmarks can't currently measure this
...
📝 justinjsd opened a pull request: "new gen"
(https://github.com/bitcoin/bitcoin/pull/33377)
<!--
*** 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 clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improve coverage are a
...
(https://github.com/bitcoin/bitcoin/pull/33377)
<!--
*** 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 clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improve coverage are a
...
✅ fanquake closed a pull request: "new gen"
(https://github.com/bitcoin/bitcoin/pull/33377)
(https://github.com/bitcoin/bitcoin/pull/33377)
💬 instagibbs commented on pull request "txgraph: randomize order of same-feerate distinct-cluster transactions":
(https://github.com/bitcoin/bitcoin/pull/33335#discussion_r2344302600)
I didn't really mean to dive into the exact numbers, but yeah, for "rare collisions".
(https://github.com/bitcoin/bitcoin/pull/33335#discussion_r2344302600)
I didn't really mean to dive into the exact numbers, but yeah, for "rare collisions".
✅ fanquake closed an issue: "Zero output not cleared"
(https://github.com/bitcoin/bitcoin/issues/33265)
(https://github.com/bitcoin/bitcoin/issues/33265)
🚀 fanquake merged a pull request: "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet"
(https://github.com/bitcoin/bitcoin/pull/33268)
(https://github.com/bitcoin/bitcoin/pull/33268)
💬 l0rinc commented on pull request "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup":
(https://github.com/bitcoin/bitcoin/pull/33324#discussion_r2344324168)
> Loading a large file
This is run on request, before anything else loads, it's not *that* large, only 160 MB memory is needed.
For reference, applying the mentioned `dbcache=4` (which isn't used here yet) still makes the node use > 1 GB memory:
<img width="1202" height="631" alt="image" src="https://github.com/user-attachments/assets/1867ba38-ba35-4ce5-86de-5bfe8add145b" />
> Here's roughly what I'm thinking: [ajtowns/bitcoin@202509-reobfus (commits)](https://github.com/ajtowns/bitco
...
(https://github.com/bitcoin/bitcoin/pull/33324#discussion_r2344324168)
> Loading a large file
This is run on request, before anything else loads, it's not *that* large, only 160 MB memory is needed.
For reference, applying the mentioned `dbcache=4` (which isn't used here yet) still makes the node use > 1 GB memory:
<img width="1202" height="631" alt="image" src="https://github.com/user-attachments/assets/1867ba38-ba35-4ce5-86de-5bfe8add145b" />
> Here's roughly what I'm thinking: [ajtowns/bitcoin@202509-reobfus (commits)](https://github.com/ajtowns/bitco
...
👍 instagibbs approved a pull request: "txgraph: randomize order of same-feerate distinct-cluster transactions"
(https://github.com/bitcoin/bitcoin/pull/33335#pullrequestreview-3216844701)
ACK 593d418137e4802bbe229707dcda5796522e2e5e
double-checked Trim benchmark just in case, looks unchanged
(https://github.com/bitcoin/bitcoin/pull/33335#pullrequestreview-3216844701)
ACK 593d418137e4802bbe229707dcda5796522e2e5e
double-checked Trim benchmark just in case, looks unchanged
💬 fanquake commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3285414401)
Backported to `30.x` in #33356.
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3285414401)
Backported to `30.x` in #33356.
💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2344380721)
I was deliberately trying to avoid that, I find that completely unreadable
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2344380721)
I was deliberately trying to avoid that, I find that completely unreadable
💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2344386717)
Good idea, thanks. Do you think we should also add the reason for the enable/disable (we'd store an enum instead of the optional bool which would provide more info on *why* signature verification was enabled or disabled)?
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2344386717)
Good idea, thanks. Do you think we should also add the reason for the enable/disable (we'd store an enum instead of the optional bool which would provide more info on *why* signature verification was enabled or disabled)?
💬 fanquake commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#issuecomment-3285583699)
Backported to `29.x` in #33344.
(https://github.com/bitcoin/bitcoin/pull/33296#issuecomment-3285583699)
Backported to `29.x` in #33344.
💬 purpleKarrot commented on pull request "[WIP] cache: remove redundant find() call":
(https://github.com/bitcoin/bitcoin/pull/33376#discussion_r2344612738)
Can this condition be checked before the insert?
(https://github.com/bitcoin/bitcoin/pull/33376#discussion_r2344612738)
Can this condition be checked before the insert?
💬 dergoegge commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2341477921)
nit
```suggestion
const CAmount AMOUNT_FEE{1000};
```
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2341477921)
nit
```suggestion
const CAmount AMOUNT_FEE{1000};
```
💬 dergoegge commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2341484545)
```suggestion
CBlock block = *info[index].block;
block.vtx.clear();
```
The headers message is a vector of blocks without transactions, so I think you should clear the copy of the block here? Although, technically this doesn't matter because there's only one header in the vector being sent, so the following txs are just ignored.
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2341484545)
```suggestion
CBlock block = *info[index].block;
block.vtx.clear();
```
The headers message is a vector of blocks without transactions, so I think you should clear the copy of the block here? Although, technically this doesn't matter because there's only one header in the vector being sent, so the following txs are just ignored.
💬 dergoegge commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2341516465)
```suggestion
CBlockHeaderAndShortTxIDs baseCmpctBlock = cmpctBlock;
```
nit: there are few instances of camel case usage here. Our convention is to use snake case (see dev notes).
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2341516465)
```suggestion
CBlockHeaderAndShortTxIDs baseCmpctBlock = cmpctBlock;
```
nit: there are few instances of camel case usage here. Our convention is to use snake case (see dev notes).
🤔 dergoegge requested changes to a pull request: "fuzz: compact block harness"
(https://github.com/bitcoin/bitcoin/pull/33300#pullrequestreview-3212403908)
Did a first pass, overall approach looks good to me
(https://github.com/bitcoin/bitcoin/pull/33300#pullrequestreview-3212403908)
Did a first pass, overall approach looks good to me
💬 Crypt-iQ commented on pull request "[29.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33344#issuecomment-3285838336)
Checked that #32646 and #33296 backports are correct and ran `p2p_compactblocks.py` just to be sure.
(https://github.com/bitcoin/bitcoin/pull/33344#issuecomment-3285838336)
Checked that #32646 and #33296 backports are correct and ran `p2p_compactblocks.py` just to be sure.
💬 Raimo33 commented on pull request "[WIP] cache: remove redundant find() call":
(https://github.com/bitcoin/bitcoin/pull/33376#discussion_r2344704754)
you mean as it was before?
(https://github.com/bitcoin/bitcoin/pull/33376#discussion_r2344704754)
you mean as it was before?
👍 theStack approved a pull request: "test/refactor: use test deque to avoid quadratic iteration"
(https://github.com/bitcoin/bitcoin/pull/33313#pullrequestreview-3217716655)
re-ACK 75e6984ec8c6fa196ad78c11f454da506d7c8ff1
(https://github.com/bitcoin/bitcoin/pull/33313#pullrequestreview-3217716655)
re-ACK 75e6984ec8c6fa196ad78c11f454da506d7c8ff1