💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1659218637)
Fixed.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1659218822)
Fixed.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1659218927)
Fixed.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1659219435)
Done, named it `MAX_SIMPLE_ITERATIONS` (and used more readable number 300000).

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2197498881)
> Changing only parts of it can break the tests

I usually try to recreate every line, after I have all the changes by interactively rebasing the current state and adding small refactoring under the current commit.
E.g. these could be separate commits doing a single thing:
* renaming `bool erase` to `bool will_erase`
* changing `it->second.flags` usages to `it->second.GetFlags()` (again, without change in behavior)
* changing `it->second.flags |= ` to use `it->second.AddFlags(CCoinsCacheEn
...

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1659219716)
Done.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1659220190)
See new unit test.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1659220493)
I'd rather not. The point of `ExhaustiveCandidateFinder` is really testing the correctness of `SimpleCandidateFinder` (whose correctness may not be obvious to reviewers), so that `SimpleCandidateFinder` on its turn can be used to test `SearchCandidateFinder`. I added comments to explain that.

Both of these are done within the same fuzz test, which is perhaps confusing? I could split up the tests (an exhaustive-vs-simple test, and a simple-vs-search test).

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1659220875)
Indeed. I don't think 1 iteration matters.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1659221292)
Disallowed.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1659221511)
Done.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1659221633)
Done.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1659222412)
I was trying to explain why using 10 iterations instead of 0 made sense. I have instead just changed it to 0 iterations.

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659223824)
What prohibits us from accessing the previous ones in constant time during iteration (I haven't had the energy to delve into it properly yet)?

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659225286)
Can you find a way to separate the side-effects? I find this very confusing.

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1659225694)
Got it, thanks

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1659228321)
It means "what the **queue size** needs to be before we can process an element from the **front**".

💬 paplorinc commented on pull request "scripted-diff: Log parameter interaction not thrice":
(https://github.com/bitcoin/bitcoin/pull/30358#issuecomment-2197514809)
ACK fa1bc7c88bb720f02955dbe4e2a174b5a52f3af8

💬 TheCharlatan commented on pull request "RFC: Instanced logs":
(https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2197523046)
Re https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2197436060

Thanks for going through this. Having something like `LaunchSettings` seems like a good thing.

> Fixing the formatting of buffered logs is a bit of work, but not very complicated. It doesn't seem particularly related to the kernel, though. Would you review a PR to do that?

yes :)

> I'm not sure what you mean? There's DisconnectTestLogger(); StartLogging(); ?

Besides the incoherent naming between the two, it a
...

👍 TheCharlatan approved a pull request: "scripted-diff: Log parameter interaction not thrice"
(https://github.com/bitcoin/bitcoin/pull/30358#pullrequestreview-2148843354)
Nice, ACK fa1bc7c88bb720f02955dbe4e2a174b5a52f3af8