💬 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
...
(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.
(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.
(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
(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!
(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.
(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`.
(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.
(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.
(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?
(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.
(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.
(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`.
(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.
(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.
(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.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1660349319)
Done.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1660349373)
Added a comment to `Linearize` to reflect this.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1660349373)
Added a comment to `Linearize` to reflect this.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1660349427)
Done.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1660349427)
Done.
✅ maflcko closed a pull request: "WIP: Fix coinstatsindex overflow issue"
(https://github.com/bitcoin/bitcoin/pull/26426)
(https://github.com/bitcoin/bitcoin/pull/26426)
💬 maflcko commented on pull request "WIP: Fix coinstatsindex overflow issue":
(https://github.com/bitcoin/bitcoin/pull/26426#issuecomment-2199292529)
Closing for now due to inactivity for 1.5 years. #https://github.com/bitcoin/bitcoin/issues/26362 remains open and can be used for discussion. This change can be reopened or recreated as a new pull request.
(https://github.com/bitcoin/bitcoin/pull/26426#issuecomment-2199292529)
Closing for now due to inactivity for 1.5 years. #https://github.com/bitcoin/bitcoin/issues/26362 remains open and can be used for discussion. This change can be reopened or recreated as a new pull request.