💬 fjahr commented on pull request "#27307 follow-up: update mempool conflict tests + docs":
(https://github.com/bitcoin/bitcoin/pull/30365#discussion_r1660316415)
Linter fails because the import of `COINBASE_MATURITY` is not needed anymore (line 12).

💬 furszy commented on pull request "optimization: Moved repeated `-printpriority` fetching out of AddToBlock":
(https://github.com/bitcoin/bitcoin/pull/30324#discussion_r1660318367)
nano nit: would call it "DEFAULT_PRINT_FEERATE" as the "modified" word is internal and might change over time.

🤔 furszy reviewed a pull request: "optimization: Moved repeated `-printpriority` fetching out of AddToBlock"
(https://github.com/bitcoin/bitcoin/pull/30324#pullrequestreview-2150204768)
utACK 323ce303086

💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2198775424)
@andrewtoth

So the data structure we want has (a) an index on COutPoint and (b) a single-linked list that stores an iteration order. The issue (I think, I haven't followed in detail) is that we need the ability to delete entries found through the (a) index, and if we naively do that, the entry that (b) points to the deleted entry will have its prev pointer invalidated.

My suggestion is "shifting" the values of the map by one. `map[key]` stores the UTXO for the entry that is (b)-after `key
...

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2198787407)
@sipa thank you for the suggestion! I think it's missing the fact that we want entries in the map to possibly not belong to the linked list. So we want to clear the linked list after a non-erasing flush, but leave the map intact otherwise. I don't think that's possible with just this data structure. We would need 2 parallel maps, one with unflagged entries and one with dirty entries. I'm not sure if that would be more performant with 2 lookups for every access? I think if we have 2 maps we can a
...

💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2198788192)
@andrewtoth You can have 1 map, with 2 sentinels, each being the end of one linked list (one for flagged entries, one for unflagged ones)... but yeah, fair point, this perhaps complicates matters even more. And the performance penalty of this idea is probably not insignificant either.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660330315)
This one is problematic though https://github.com/bitcoin/bitcoin/pull/28280/files#diff-3d0856e8b7f136c588b229e0cbd3b2e2c309cd097ade0879521daba4e1bb2a33L609. That would be a big change if we couldn't get the flags.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660341669)
Done

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660341788)
Done, nice catch!

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660341870)
Updated comment.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660342034)
Mentioned in another comment, we need GetFlags or we will have to change a lot of test code in `coins_tests.cpp`.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660342135)
Added a comment in the commit message.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660342186)
Done.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660342456)
The std lib uses this pattern though, with `std::unordered_map::erase`. It mutates and returns next. Surely we should be following std lib conventions, no?

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660342534)
Updated comment.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1660345204)
Yeah, it is to reduce the ability for peers to construct cases that just happen to be worst case for the exact search order they know we're going to try.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1660349106)
Done, named as `MAX_SIMPLE_ITERATIONS`.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1660349184)
I've added comments.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1660349260)
I have no idea! Added a comment that it was found empirically.

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