💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574385347)
nit:
```suggestion
~CoinsViewCacheAsync() override
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574685431)
I'm not sure I understand how we're actually testing this.
`NoAccessCoinsView` is designed to abort on access, so in the shortid collision scenario how does it simulate going to disk?
Shouldn't we assert here that the number of collisions coincides with the number of simulated disk reads?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574457335)
[we should](https://sonarcloud.io/project/issues?issueStatuses=OPEN%2CCONFIRMED&branch=31132-69310ec0035351e486cfb51cb66492639f6f8b47&id=aureleoules_bitcoin&open=AZrV9y4tqc704xdUcnxD&tab=why) likely init it here instead:
```suggestion
std::barrier<> m_barrier{WORKER_THREADS + 1};
```

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574535324)
```suggestion
BOOST_CHECK_EQUAL(coin.IsSpent(), spent);
```

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574934048)
I think some invalid block tests exercise this. If a block is invalid then the outputs of any txs are not added to the utxo set.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574936137)
If there is a missing new line github will show a red arrow at the end of the file.

💬 mdabdulkaderhossain144-ux commented on something "":
(https://github.com/bitcoin/bitcoin/commit/2e27bd9c3af91eb9fcc626fe65d065df0a80974d#commitcomment-171659028)
0xfdDB0e3EDbc6ffC1cF63f0c47d0Db9c9EA7EC671

💬 mdabdulkaderhossain144-ux commented on something "":
(https://github.com/bitcoin/bitcoin/commit/2e27bd9c3af91eb9fcc626fe65d065df0a80974d#commitcomment-171659046)
2e27bd9c3af91eb9fcc626fe65d065df0a80974d

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574956644)
> isn't this too small for our purposes?

I don't see why we need any randomness or larger values for these?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574968743)
Hmm not sure how we'd test this with a unit test. It surely gets exercised by fuzzing though.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574970212)
It gets called by functional tests though.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574973805)
> before/after

Not sure there are any before results we can use for this.

> can you reproduce 4 threads saturating the parallelism factor with it?

How would I know if I do that?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574979427)
That's more an implementation detail that can be found by reading the header file, no?

💬 sedited commented on pull request "refactor: Split multithreaded case out of CheckInputScripts":
(https://github.com/bitcoin/bitcoin/pull/32575#issuecomment-3593597789)
Looking at this patch, I am asking myself why we're keeping the single threaded / no checkqueue case in the first place. This seems to introduce a vector of jobs for both cases now, so maybe we could just create a queue without any workers?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574980697)
I would prefer to split out the implementation, the tests, the fuzz harness... But you prefer those all be in the same commit?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574982084)
> (i.e. to stop propagation to disk)

The parent could be called, but this is faster since we skip calling `ReallocateCache`.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574988562)
I'm not sure if we need all these deletes here? What are we gaining from this?

We need to have move assignment for it to compile. It can't automatically move the `atomic_flag`. So, it doesn't really matter if it gets called or not, we still need to define it. And, since it only happens before we start doing work, might as well make it simple and not bother moving the other fields. I don't think we need to bother reserving since we keep the capacity over many blocks. The looping is just extra
...

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574995185)
It doesn't simulate going to disk. It simulates not setting the coin in `ProcessInputInBackground` even though the base has it. Then the `if (ret->second.coin.IsSpent()) [[unlikely]] {` branch is executed in `FetchCoin` and the coin is fetched from base via `GetCoinWithoutMutating`.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2575017536)
> would basically pass (but hang) if we forget to Reset

A hanging test is treated as failure in the CI. I don't think it's necessary to do anything else here.

📝 hebasto opened a pull request: "depends, doc: Add `tcl` as build dependency for `sqlite` package"
(https://github.com/bitcoin/bitcoin/pull/33975)
Since https://github.com/bitcoin/bitcoin/pull/32655, the `sqlite` package has relied on a native C compiler being incidentally available on the system under the default names. However, this is not always the case. For example, on Ubuntu 24.04:
```
$ gmake -C depends sqlite CC=gcc-14 CXX=g++-14
gmake: Entering directory '/bitcoin/depends'
Configuring sqlite...
No installed jimsh or tclsh, building local bootstrap jimsh0
No working C compiler found. Tried cc and gcc.
gmake: *** [funcs.mk:34
...