rpc: avoid copying unbroadcast transaction IDs just to count them #36349

pull musaHaruna wants to merge 1 commits into bitcoin:master from musaHaruna:rpc-unbroadcast-count changing 2 files +8 −1
  1. musaHaruna commented at 8:31 PM on September 26, 2026: contributor

    Follow-up to #35923.

    getmempoolinfo currently calls GetUnbroadcastTxs().size() to report unbroadcastcount. The getter returns the set by value, so every call copies its entries under the mempool lock and immediately discards the copy after reading its size.

    Fix: Add GetUnbroadcastTxCount() and use it in the RPC to read the size under the same lock without allocating a temporary set. This is a separate accessor because broadcast retry and persistence still need the transaction IDs returned by the existing GetUnbroadcastTxs().

    I also searched for similar cases nearby: the ancestor/descendant count helpers and the cluster-count path build vectors just to read their sizes, while the mining RPC uses GetDust(...).empty() to check whether any dust outputs exist. These could potentially use count-only or boolean helpers, but would require separate changes to the graph and policy code, so I left them out of this PR.

    Added a temporary bench_bitcoin benchmark on my branch, comparing both getters.

    Results below are medians from five runs on an Apple M5, using Apple clang 17/libc++ with -O2:

    Unbroadcast IDs GetUnbroadcastTxs().size() (ns/query) GetUnbroadcastTxCount()(ns/query)
    0 25.22 22.60
    1 58.99 22.52
    10 420.82 22.79
    100 4,788.22 22.45
    1,000 57,904.17 22.60
    10,000 908,399.04 22.62

    The copy cost grew with the set size, while the count accessor stayed approximately constant.

  2. rpc: avoid copying unbroadcast txids to count them fef74dd296
  3. DrahtBot added the label RPC/REST/ZMQ on Sep 26, 2026
  4. DrahtBot commented at 8:31 PM on September 26, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36349.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process.

    Type Reviewers
    ACK l0rinc

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  5. in src/txmempool.h:586 in fef74dd296
     582 | @@ -583,6 +583,13 @@ class CTxMemPool
     583 |          return m_unbroadcast_txids;
     584 |      }
     585 |  
     586 | +    /** Returns the number of transactions in the unbroadcast set */
    


    l0rinc commented at 8:25 PM on September 27, 2026:

    The comment just rewords the method name, I don't think it's useful

  6. in src/txmempool.h:579 in fef74dd296


    l0rinc commented at 8:36 PM on September 27, 2026:

    Given that this caused a confusion before:

        /** Returns a copy of the unbroadcast txid set */
    
  7. l0rinc approved
  8. l0rinc commented at 10:29 PM on September 27, 2026: contributor

    lightly tested ACK fef74dd2965b02668f8717c8a8cc4b8b24c0e604

    This avoids copying the unbroadcast set on every getmempoolinfo call. The other two callers use the txids after releasing the mempool lock, so avoiding their copies is less straightforward.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-09-28 10:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me