💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399059149)
Not sure, would have to benchmark this.
I have updated the functions though to make the main thread's entrance clearer.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399062889)
It is definitely safe to do, since all access would be on main thread. It is also safe to do from parallel threads if we don't write until all threads are done reading, which is what this PR does.
Prefiltering is slow though (several milliseconds) so is better to do in parallel.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399067712)
I'm not sure I understand this.
The m_txids set is computed on the main thread, and is only read from multiple threads. If we didn't do this we would try to fetch non-existent outputs from the db, which would be much slower.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399069027)
Not sure why that's problematic, we don't *have* to have perfect parallelism, it seems to me we can assume uniform distribution - it's fine if there are outliers if that makes the code simpler (which I think it should, it could even eliminate most locks, since the jobs are basically completely independent)

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399069282)
> I don't understand what this has to do with being lock free.

We may have fewer file system locks if the threads are accessing different regions

> I've updated to use semaphores instead of mutex

I will review that in more detail soon, probably next week.

💬 alexanderwiederin commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2399070131)
Think this can be removed.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399071701)
We don't really care, but it would be good to not continue doing work here if we know it's pointless. This just exits early. No validation is happening.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399074127)
This is to not enter if there is only a coinbase tx, since it has no inputs to fetch. If there were 2 txs, and the second has 1000 inputs, we would still want to enter here.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399077894)
My benchmarks so far indicate the opposite: after 3-4 threads there is no benefit to the parallelization (either on SSD or HDD). I will remeasure your new changes after you give me the 👍

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399079463)
I don't think we can do that. We need to set these here for other threads to read. These are only read from other threads, never written to. We also only read from other threads after the main thread has released the counting_semaphore, so we know the pointers are synced across the threads.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399082258)
Err, we want to throw a runtime error here to test the try/catch in the inputfetcher.

💬 alexanderwiederin commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2399082925)
remove `kernel` to align with https://github.com/bitcoin/bitcoin/pull/30595/commits/43f850c1ae9a66d8de599b81d3d78e7178c12251

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399083511)
I rewrote this part, these are gone now.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399084818)
But prefiltering would allow sorting, which should untangle the threads.
The threads will access the same files (which are more likely to be different from the files the other threads are requesting), so they may profit from cache locality if the OS supports it - that's why I suggested giving it a try.

💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-3361550525)
`8218daac0f...d026d5475e`: rebase due to conflicts

This is now a little bit simplified after https://github.com/bitcoin/bitcoin/pull/32326 was merged - no need to touch `FindNode()` anymore.

💬 maflcko commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3361550259)
> Looking around for more versions and checking [446e73c](https://github.com/bitcoin/bitcoin/commit/446e73cc0bb7cd628039eaf9b1bcc93db23b924f) it seem to me we could update a few more places:

This is about something else: The sdk and xcode version used for the depends/guix build. So it should be a separate pull request. Generally, bumping the sdk/xcode in depends is a lengthy/tedious process, so it should be done only when needed. My recommendation would be to bump it to 26.x within the next y
...

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399092048)
> We may have fewer file system locks if the threads are accessing different regions

Ok, but that is not the same as this InputFetcher construction being lock free.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399094785)
I really dislike that, will try to come up with a lock-free version later (maybe next week)

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2399100147)
I think I would prefer a less opinionated version, as long as it's still correct. No need to optimize for the consensus failure speed in my opinion, I would prefer simpler code for a change as risky as this one.

💬 l0rinc commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3361587199)
> This is about something else:

What's the reason for keeping a version in the bug issue template that we don't support?

> I don't see how this is related to this pull?

If we recommend a newer `AppleClang` version, it may not work with fuzzing, that's all.