A few mempool removal optimizations #8515

1. 
   sipa commented at 11:30 am on August 15, 2016: member

   • Make the removed and conflicted arguments to the CTxMemPool::remove* functions optionals, avoiding building full lists of those when we don’t need them.
   • Use std::vector instead of std::list for those lists, which have lower allocation overhead (std::list only has an advantage when you need frequent inserts or deletions not at the end).
   • Switch those lists to returning std::shared_ptrs instead, so no full copying is needed.
2. MarcoFalke added the label Mempool on Aug 15, 2016
3. MarcoFalke added the label Refactoring on Aug 15, 2016
4. 
   jonasschnelli commented at 11:56 am on August 15, 2016: contributor

   Using shared pointers makes much more sense. Thanks. utACK eb8104aebccd42bbd28f33ea0f2e100cccdf42f7.
5. sipa force-pushed on Aug 15, 2016
6. 
   gmaxwell commented at 11:24 pm on August 16, 2016: contributor

   @pstratem You had benchmarks showing removal was taking all the time. Care to do a comparison with this code?
7. 
   sipa commented at 10:37 am on August 25, 2016: member

   Anyone feel like benchmarking whether this has an impact on invalidateblock/reconsiderblock times (mimicking a large reorg)?
8. in src/txmempool.h: in fb464c8c56 outdated

    5@@ -6,7 +6,6 @@
    6 #ifndef BITCOIN_TXMEMPOOL_H
    7 #define BITCOIN_TXMEMPOOL_H
    8 
    9-#include <list>
   10 #include <memory>
   11 #include <set>
   

   ---

   ---

   

   luke-jr commented at 9:29 pm on August 27, 2016:

   Shouldn’t there be <vector> here?

   ---

   

   sipa commented at 11:20 pm on August 27, 2016:

   Note: <vector> does not render, unless inside .

   ---

   

   sipa commented at 11:28 pm on August 27, 2016:

   Added some missing includes (map for std::map, utility for std::pair, vector for std::vector, string for std::string). I’m sure we have many more cases of these missing though.
9. in src/txmempool.cpp: in 17df3fdc83 outdated

   557                 indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash);
   558                 if (it2 != mapTx.end())
   559                     continue;
   560                 const CCoins *coins = pcoins->AccessCoins(txin.prevout.hash);
   561-		if (nCheckFrequency != 0) assert(coins);
   562+                if (nCheckFrequency != 0) assert(coins);
   

   ---

   ---

   

   luke-jr commented at 9:32 pm on August 27, 2016:

   What changed here?

   ---

   

   rebroad commented at 2:09 am on August 30, 2016:

   a space was missing
10. in src/txmempool.cpp: in fb464c8c56 outdated

    559                 indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash);
    560                 if (it2 != mapTx.end())
    561                     continue;
    562                 const CCoins *coins = pcoins->AccessCoins(txin.prevout.hash);
    563-		if (nCheckFrequency != 0) assert(coins);
    564+                if (nCheckFrequency != 0) assert(coins);
    

    ---

    ---

    

    sipa commented at 10:13 pm on August 27, 2016:

    Incorrect indentation.
11. 
    luke-jr commented at 10:20 pm on August 27, 2016: member

    ACK

    Benchmarked c072b8fd95cd4fa84f08189a0cd8b173ea2dbb8e vs c072b8fd95cd4fa84f08189a0cd8b173ea2dbb8e + fb464c8c5670e94bd429786e7c8985c4100da811 with testnet blocks 925000-925617 (000000000000020c778212bbe3cdd2ad7f2d102ff4c3ae39515b8008c170d9f6) with no measurable real-time performance difference (~1m10s invalidate, ~30s reconsider)

12. sipa force-pushed on Sep 9, 2016
13. 
    sipa commented at 4:29 pm on September 9, 2016: member

    Rebased.
14. 
    laanwj commented at 8:58 pm on October 18, 2016: member

    utACK, needs rebase again though
15. sipa force-pushed on Oct 20, 2016
16. 
    sipa commented at 6:16 pm on October 20, 2016: member

    Rebased.
17. 
    TheBlueMatt commented at 5:49 pm on October 21, 2016: member

    utACK 6ee761c83e3eeea8c685f789758232a1c4325f99.
18. Bypass removeRecursive in removeForReorg f48211b700
19. Make removed and conflicted arguments optional to remove 51f278329d
20. Return shared_ptr<CTransaction> from mempool removes 4100499db4
21. Add some missing includes 0334430b39
22. sipa force-pushed on Oct 22, 2016
23. laanwj merged this on Oct 25, 2016
24. laanwj closed this on Oct 25, 2016

    ---

25. laanwj referenced this in commit 9bdf5269f8 on Oct 25, 2016
26. codablock referenced this in commit 24c0193c35 on Sep 19, 2017
27. codablock referenced this in commit 5bcd438b13 on Jan 13, 2018
28. gladcow referenced this in commit b98c102202 on Mar 2, 2018
29. gladcow referenced this in commit f68c64c6bb on Mar 13, 2018
30. gladcow referenced this in commit d66aa72841 on Mar 14, 2018
31. gladcow referenced this in commit afa025b964 on Mar 15, 2018
32. gladcow referenced this in commit 5c27de4fd7 on Mar 15, 2018
33. gladcow referenced this in commit 88a4e0c2a7 on Mar 15, 2018
34. gladcow referenced this in commit ada8cdadd8 on Mar 15, 2018
35. gladcow referenced this in commit fc8be4768e on Mar 24, 2018
36. gladcow referenced this in commit bc47ef3db2 on Apr 4, 2018
37. UdjinM6 referenced this in commit bc45a2f87a on Apr 11, 2018
38. andvgal referenced this in commit b0467d226b on Jan 6, 2019
39. andvgal referenced this in commit fd5c50bc2b on Jan 6, 2019
40. CryptoCentric referenced this in commit f35d8264cc on Feb 15, 2019
41. CryptoCentric referenced this in commit dd3fd51204 on Feb 28, 2019
42. furszy referenced this in commit 1f010c0969 on Jan 23, 2021
43. DrahtBot locked this on Sep 8, 2021

Contributors
sipa jonasschnelli gmaxwell luke-jr rebroad laanwj TheBlueMatt

Labels
Refactoring Mempool