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
Reviewers: #bitcoin_abc, PiRK, sdulfari
Reviewed By: #bitcoin_abc, PiRK, sdulfari
Differential Revision: https://reviews.bitcoinabc.org/D12873