Page MenuHomePhabricator

Remove the txHashes structure from the mempool
ClosedPublic

Authored by Fabien on Dec 12 2022, 18:05.

Details

Reviewers
PiRK
sdulfari
Group Reviewers
Restricted Project
Commits
rABC704163206ac6: Remove the txHashes structure from the mempool
Summary

This structure has been introduced as a facility for reconstructing blocks from compact blocks.
It is still unclear to me why it was added in the first place, since the mapTx mutlti_index that is indirected can be used to achieve the same reconstruction with the same complexity.

The multi_index structure introduces some overhead, but the lookup is still O(1). I wrote and ran a simple benchmark to test the loop against a vector (like txHashes) for various sizes, and here are the results on my machine:

Using N = 1000
multi_index = 2us, vector = 0us (+  inf%)
Using N = 2000
multi_index = 5us, vector = 1us (+  500%)
Using N = 5000
multi_index = 10us, vector = 3us (+333.33%)
Using N = 10000
multi_index = 21us, vector = 8us (+262.5%)
Using N = 20000
multi_index = 42us, vector = 15us (+  280%)
Using N = 50000
multi_index = 107us, vector = 50us (+  214%)
Using N = 100000
multi_index = 224us, vector = 107us (+209.35%)
Using N = 200000
multi_index = 700us, vector = 349us (+200.57%)
Using N = 500000
multi_index = 2320us, vector = 1306us (+177.64%)
Using N = 1000000
multi_index = 4515us, vector = 2798us (+161.37%)
Using N = 2000000
multi_index = 9229us, vector = 5524us (+167.07%)
Using N = 5000000
multi_index = 23796us, vector = 14042us (+169.46%)
Using N = 1000000
multi_index = 4710us, vector = 2641us (+178.34%)

As expected the overhead is more important when there are not many elements in the loop, and decreases with the sample size. Both cases are linear. The benchmark code: https://godbolt.org/z/zqWeKf31c

The benefit is that it allows for a nice code cleanup, reduced mempool memory usage and less maintenance when updating the mempool, with no change in behavior.
Note that this code already had a minor bug (was fixed recently in core#24145).

Test Plan
ninja all check-extended

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable