refactor: use recommended type hiding on multi_index types #30194

1. 
   theuni commented at 3:05 pm on May 29, 2024: member

   Recommended by boost docs: https://www.boost.org/doc/libs/1_85_0/libs/multi_index/doc/compiler_specifics.html#type_hiding

   This significantly reduces the size of the symbol name lengths that end up in the binaries as well as in compiler warnings/errors. Otherwise there should be no functional change.

   Example before:

   0000000000000000 W unsigned long boost::multi_index::detail::hashed_index<mempoolentry_txid, SaltedTxidHasher, std::equal_to, boost::multi_index::detail::nth_layer<1, CTxMemPoolEntry, boost::multi_index::indexed_by<boost::multi_index::hashed_unique<mempoolentry_txid, SaltedTxidHasher, mpl_::na, mpl_::na>, boost::multi_index::hashed_unique<boost::multi_index::tag<index_by_wtxid, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, mempoolentry_wtxid, SaltedTxidHasher, mpl_::na>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<descendant_score, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::identity, CompareTxMemPoolEntryByDescendantScore>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<entry_time, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::identity, CompareTxMemPoolEntryByEntryTime>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<ancestor_score, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::identity, CompareTxMemPoolEntryByAncestorFee>, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, std::allocator >, boost::mpl::vector0<mpl_::na>, boost::multi_index::detail::hashed_unique_tag>::count<uint256, SaltedTxidHasher, std::equal_to >(uint256 const&, SaltedTxidHasher const&, std::equal_to const&, mpl_::bool_) const

   After:

   0000000000000000 W unsigned long boost::multi_index::detail::hashed_index<mempoolentry_txid, SaltedTxidHasher, std::equal_to, boost::multi_index::detail::nth_layer<1, CTxMemPoolEntry, CTxMemPool::CTxMemPoolEntry_Indicies, std::allocator >, boost::mpl::vector0<mpl_::na>, boost::multi_index::detail::hashed_unique_tag>::count<uint256, SaltedTxidHasher, std::equal_to >(uint256 const&, SaltedTxidHasher const&, std::equal_to const&, mpl_::bool_) const

2. 
   DrahtBot commented at 3:05 pm on May 29, 2024: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	glozow, TheCharlatan, fanquake

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   No conflicts as of last run.

3. DrahtBot added the label Refactoring on May 29, 2024
4. 
   theuni commented at 3:08 pm on May 29, 2024: member

   Note that I started by limiting BOOST_MULTI_INDEX_LIMIT_INDEXED_BY_SIZE and BOOST_MULTI_INDEX_LIMIT_TAG_SIZE as also recommended by the docs (which helped), but with the hiding applied with this commit those defines appear to be moot.
5. 
   TheCharlatan commented at 3:57 pm on May 29, 2024: contributor

   Concept ACK
6. 
   fanquake commented at 4:16 pm on May 29, 2024: member

   This significantly reduces the size of the symbol name lengths that end up in the binaries

   About 23'000 bytes worth.

7. refactor: use recommended type hiding on multi_index types

   Recommended by boost docs:
   https://www.boost.org/doc/libs/1_85_0/libs/multi_index/doc/compiler_specifics.html#type_hiding
   
   This significantly reduces the size of the symbol name lengths that end up in
   the binaries as well as in compiler warnings/errors. Otherwise there should be
   no functional change.
   
   Example before:
   0000000000000000 W unsigned long boost::multi_index::detail::hashed_index<mempoolentry_txid, SaltedTxidHasher, std::equal_to<uint256>, boost::multi_index::detail::nth_layer<1, CTxMemPoolEntry, boost::multi_index::indexed_by<boost::multi_index::hashed_unique<mempoolentry_txid, SaltedTxidHasher, mpl_::na, mpl_::na>, boost::multi_index::hashed_unique<boost::multi_index::tag<index_by_wtxid, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, mempoolentry_wtxid, SaltedTxidHasher, mpl_::na>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<descendant_score, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByDescendantScore>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<entry_time, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByEntryTime>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<ancestor_score, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByAncestorFee>, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, std::allocator<CTxMemPoolEntry> >, boost::mpl::vector0<mpl_::na>, boost::multi_index::detail::hashed_unique_tag>::count<uint256, SaltedTxidHasher, std::equal_to<uint256> >(uint256 const&, SaltedTxidHasher const&, std::equal_to<uint256> const&, mpl_::bool_<false>) const
   
   After:
   0000000000000000 W unsigned long boost::multi_index::detail::hashed_index<mempoolentry_txid, SaltedTxidHasher, std::equal_to<uint256>, boost::multi_index::detail::nth_layer<1, CTxMemPoolEntry, CTxMemPool::CTxMemPoolEntry_Indicies, std::allocator<CTxMemPoolEntry> >, boost::mpl::vector0<mpl_::na>, boost::multi_index::detail::hashed_unique_tag>::count<uint256, SaltedTxidHasher, std::equal_to<uint256> >(uint256 const&, SaltedTxidHasher const&, std::equal_to<uint256> const&, mpl_::bool_<false>) const

   a3cb309e7c
8. in src/node/miner.h:100 in a9b5ba43e8 outdated

    95@@ -96,21 +96,25 @@ struct CompareTxIterByAncestorCount {
    96     }
    97 };
    98 
    99+
   100+struct CTxMemPoolModifiedEntry_Indicies final : boost::multi_index::indexed_by<
   

   ---

   ---

   

   sipa commented at 8:24 pm on May 29, 2024:

   indices?

   ---

   

   theuni commented at 8:41 pm on May 29, 2024:

   😬
9. theuni force-pushed on May 29, 2024
10. theuni force-pushed on May 31, 2024
11. 
    theuni commented at 11:03 pm on May 31, 2024: member

    Accidentally pushed a temp branch here, ignore the noise. Still ready for review.
12. DrahtBot added the label CI failed on Jul 2, 2024
13. DrahtBot removed the label CI failed on Jul 3, 2024
14. 
    glozow commented at 12:49 pm on August 7, 2024: member

    ACK a3cb309e7c31853f272bffaa65fb6ab0a7cc4083, TIL, makes sense to me

    there should be no functional change.

    Reviewed with –color-moved=dimmed_zebra, LGTM

    This significantly reduces the size of the symbol name lengths that end up in the binaries

    According to nm -C src/bitcoind | wc -c on my machine a3cb309e7c31853f272bffaa65fb6ab0a7cc4083: 3173329 a3cb309e7c31853f272bffaa65fb6ab0a7cc4083~1: 3227490 3227490 - 3173329 = 54161 chars, and same for nm -C src/bitcoind | grep multi_index | wc -c

15. DrahtBot requested review from TheCharlatan on Aug 7, 2024
16. TheCharlatan approved
17. 
    TheCharlatan commented at 1:14 pm on August 7, 2024: contributor

    ACK a3cb309e7c31853f272bffaa65fb6ab0a7cc4083
18. fanquake approved
19. 
    fanquake commented at 4:39 pm on August 7, 2024: member

    ACK a3cb309e7c31853f272bffaa65fb6ab0a7cc4083
20. glozow merged this on Aug 7, 2024
21. glozow closed this on Aug 7, 2024

    ---

Contributors
theuni DrahtBot TheCharlatan fanquake sipa glozow

Labels
Refactoring