RPC: Add reserve member function to UniValue and use it in blockToJSON function #31179

pull ismaelsadeeq wants to merge 3 commits into bitcoin:master from ismaelsadeeq:10-2024-add-reserve-to-univalue changing 5 files +33 −3
  1. ismaelsadeeq commented at 5:11 pm on October 29, 2024: member

    This PR is motivated by #30495 (comment), It adds a reserve member function to UniValue and applies it within the blockToJSON function to pre-allocate memory, minimizing reallocation’s.

    On master:

    ns/op op/s err% total benchmark
    190,342 5,254 2.3% 0.01 BlockToJsonVerbose1
    34,812,292 28.73 1.0% 0.38 BlockToJsonVerbose2
    34,457,167 29.02 1.0% 0.38 BlockToJsonVerbose3

    On this PR:

    ns/op op/s err% total benchmark
    172,278 5,805 0.7% 0.01 BlockToJsonVerbose1
    33,720,584 29.66 0.4% 0.37 BlockToJsonVerbose2
    33,884,417 29.51 1.2% 0.38 BlockToJsonVerbose3
  2. DrahtBot commented at 5:11 pm on October 29, 2024: contributor

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

    Code Coverage & Benchmarks

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

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK maflcko, Eunovo, l0rinc, achow101
    Concept ACK andrewtoth

    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 RPC/REST/ZMQ on Oct 29, 2024
  4. andrewtoth commented at 5:27 pm on October 29, 2024: contributor
    Concept ACK
  5. andrewtoth commented at 7:13 pm on October 29, 2024: contributor
    I think we can take this further and reserve for VOBJ types instead of just VARR. The VOBJ uses both values and keys, so we can reserve both. We can count how many times we do pushKV and reserve that amount for both keys and values.
  6. kennet31526 approved
  7. andrewtoth commented at 0:53 am on October 30, 2024: contributor
    I believe you can get more fine grained benchmarks for this using the rpc_blockchain benchmarks.
  8. maflcko commented at 6:38 am on October 30, 2024: member
    In theory you could also add UniValue to VectorLikeClasses ( https://clang.llvm.org/extra/clang-tidy/checks/performance/inefficient-vector-operation.html#cmdoption-arg-VectorLikeClasses), but this is probably best done in a follow-up.
  9. ismaelsadeeq force-pushed on Oct 30, 2024
  10. ismaelsadeeq commented at 7:23 am on October 30, 2024: member

    I think we can take this further and reserve for VOBJ types instead of just VARR. The VOBJ type uses both values and keys, so we can reserve for both.

    Done in the latest push, see diff, but I am reserving keys only when typ is of VOBJ.

    We can count how many times we use pushKV and reserve that amount for both keys and values.

    +1, this approach is possible but is it maintainable? Manually counting each pushKVs means updating the value thats being passed to .reserve every time we add another element, will be better to reserve for size’s that are determined at runtime or at least use a constant.

    Is there a constant for the number of items in a block so that I can reserve for results in blockheaderToJSON?

  11. ismaelsadeeq commented at 7:32 am on October 30, 2024: member

    I believe you can get more fine-grained benchmarks for this using the rpc_blockchain benchmarks.

    Yes, I can, but I’ll need to add verbosity levels 1 and 2, where we can see some performance improvements.

     0diff --git a/src/bench/rpc_blockchain.cpp b/src/bench/rpc_blockchain.cpp
 1index 7e3e2d8e48a..6d431685e66 100644
 2--- a/src/bench/rpc_blockchain.cpp
 3+++ b/src/bench/rpc_blockchain.cpp
 4@@ -45,7 +45,25 @@ struct TestBlockAndIndex {
 5 
 6 } // namespace
 7 
 8-static void BlockToJsonVerbose(benchmark::Bench& bench)
 9+static void BlockToJsonVerbose1(benchmark::Bench& bench)
10+{
11+    TestBlockAndIndex data;
12+    bench.run([&] {
13+        auto univalue = blockToJSON(data.testing_setup->m_node.chainman->m_blockman, data.block, data.blockindex, data.blockindex, TxVerbosity::SHOW_TXID);
14+        ankerl::nanobench::doNotOptimizeAway(univalue);
15+    });
16+}
17+
18+static void BlockToJsonVerbose2(benchmark::Bench& bench)
19+{
20+    TestBlockAndIndex data;
21+    bench.run([&] {
22+        auto univalue = blockToJSON(data.testing_setup->m_node.chainman->m_blockman, data.block, data.blockindex, data.blockindex, TxVerbosity::SHOW_DETAILS);
23+        ankerl::nanobench::doNotOptimizeAway(univalue);
24+    });
25+}
26+
27+static void BlockToJsonVerbose3(benchmark::Bench& bench)
28 {
29     TestBlockAndIndex data;
30     bench.run([&] {
31@@ -54,7 +72,9 @@ static void BlockToJsonVerbose(benchmark::Bench& bench)
32     });
33 }
34 
35-BENCHMARK(BlockToJsonVerbose, benchmark::PriorityLevel::HIGH);
36+BENCHMARK(BlockToJsonVerbose1, benchmark::PriorityLevel::HIGH);
37+BENCHMARK(BlockToJsonVerbose2, benchmark::PriorityLevel::HIGH);
38+BENCHMARK(BlockToJsonVerbose3, benchmark::PriorityLevel::HIGH);
39 
40 static void BlockToJsonVerboseWrite(benchmark::Bench& bench)
41 {

    Benchmark Results

    On master:

    ns/op op/s err% total benchmark
    190,342 5,254 2.3% 0.01 BlockToJsonVerbose1
    34,812,292 28.73 1.0% 0.38 BlockToJsonVerbose2
    34,457,167 29.02 1.0% 0.38 BlockToJsonVerbose3

    On this PR:

    ns/op op/s err% total benchmark
    172,278 5,805 0.7% 0.01 BlockToJsonVerbose1
    33,720,584 29.66 0.4% 0.37 BlockToJsonVerbose2
    33,884,417 29.51 1.2% 0.38 BlockToJsonVerbose3
  12. andrewtoth commented at 1:28 pm on October 30, 2024: contributor

    this approach is possible but is it maintainable?

    Hmm, yeah this would end up with reserve(<magic number>) littered throughout the json serialization. Not ideal.

    I took a quick look through most of the block parsing, and many of the reservations we would do would be <5, so unlikely to be a big benefit.

    The entry in TxToUniv could have up to 12 kv-pairs stored, and in blockheaderToJSON it could have 14. These would also be reserving two vectors so might have some visible benefit.

    Also, the witness stack array inside TxToUniv is a VARR that could be reserved with a parameter resolved at runtime.

  13. ismaelsadeeq force-pushed on Nov 1, 2024
  14. ismaelsadeeq commented at 9:56 pm on November 1, 2024: member

    Also, the witness stack array inside TxToUniv is a VARR that could be reserved with a parameter resolved at runtime.

    Thanks, I’ve added this in the latest push.

  15. Eunovo commented at 6:06 am on November 17, 2024: contributor

    I believe you can get more fine-grained benchmarks for this using the rpc_blockchain benchmarks.

    Yes, I can, but I’ll need to add verbosity levels 1 and 2, where we can see some performance improvements.

    Show diff Benchmark Results

    On master:

    ns/op op/s err% total benchmark 190,342 5,254 2.3% 0.01 BlockToJsonVerbose1 34,812,292 28.73 1.0% 0.38 BlockToJsonVerbose2 34,457,167 29.02 1.0% 0.38 BlockToJsonVerbose3 On this PR:

    ns/op op/s err% total benchmark 172,278 5,805 0.7% 0.01 BlockToJsonVerbose1 33,720,584 29.66 0.4% 0.37 BlockToJsonVerbose2 33,884,417 29.51 1.2% 0.38 BlockToJsonVerbose3 @ismaelsadeeq, could you commit the diff you’ve provided and include the benchmark results in the commit message? Then, rebase your current changes on top, rerun the benchmarks for https://github.com/bitcoin/bitcoin/pull/31179/commits/952463ccdc2baeacb527c66732fd184cbcbd53ba and https://github.com/bitcoin/bitcoin/pull/31179/commits/28e3392d11355b1160dc1a7a5557081728a02840, and update their commit messages with the results.

    This way I can just go through your commits, run the benchmarks and compare with your results.

  16. l0rinc commented at 8:37 pm on December 27, 2024: contributor

    @ismaelsadeeq, I ran your benchmarks from #31179 (comment), but I only see a 7% speedup in the TxVerbosity::SHOW_TXID bench - while the rest were exactly the same as before. But if it’s that important, we could easily get a 30% speedup as well there by deduplicating the GetSerializeSize calls:

    0static inline int64_t GetBlockWeight(const size_t size_no_witness, const size_t size_with_witness)
1{
2    return size_no_witness * (WITNESS_SCALE_FACTOR - 1) + size_with_witness;
3}
4static inline int64_t GetBlockWeight(const CBlock& block)
5{
6    const size_t size_no_witness{GetSerializeSize(TX_NO_WITNESS(block))};
7    const size_t size_with_witness{GetSerializeSize(TX_WITH_WITNESS(block))};
8    return GetBlockWeight(size_no_witness, size_with_witness);
9}

    …

     0UniValue blockToJSON(BlockManager& blockman, const CBlock& block, const CBlockIndex& tip, const CBlockIndex& blockindex, TxVerbosity verbosity)
 1{
 2    UniValue result = blockheaderToJSON(tip, blockindex);
 3
 4    size_t size_no_witness{GetSerializeSize(TX_NO_WITNESS(block))};
 5    size_t size_with_witness{GetSerializeSize(TX_WITH_WITNESS(block))};
 6    result.pushKV("strippedsize", (int)size_no_witness);
 7    result.pushKV("size", (int)size_with_witness);
 8    result.pushKV("weight", (int)GetBlockWeight(size_no_witness, size_with_witness));
 9    UniValue txs(UniValue::VARR);
10    txs.reserve(block.vtx.size());
11...

    Or if you need TxVerbosity::SHOW_DETAILS and TxVerbosity::SHOW_DETAILS_AND_PREVOUT, they can be sped up by ~20% by reviving the abandoned (for lack of interest) #29473 (edited link).

    And if BlockToJsonVerboseWrite needs a boost, #31144 will speed up by ~14%.

  17. l0rinc commented at 12:59 pm on December 30, 2024: contributor
    @ismaelsadeeq #20999 by @martinus also contains a few optimization attempts (a lot of them were already applied in other PRs since, but a few of the changes can still be done). If RPC speed is important to you, the above suggestions can result in a measurable improvements.
  18. ismaelsadeeq force-pushed on Dec 30, 2024
  19. ismaelsadeeq commented at 10:15 pm on December 30, 2024: member

    Thanks for taking a look @Eunovo @l0rinc

    could you commit the diff you’ve provided and include the benchmark results in the commit message?

    I’ve added 8d4823b383be4f8cd0152176a4f67b447d29549 For the benchmarks and instead update the PR title with the bench results.

    I ran your benchmarks from #31179 (comment), but I only see a 7% speedup in the TxVerbosity::SHOW_TXID bench - while the rest were exactly the same as before.

    I did that locally and the result is the same as well

    But if it’s that important, we could easily get a 30% speedup as well there by deduplicating the GetSerializeSize calls:

    I’ll be happy to review that as a PR or might attempt it myself.

    Or if you need TxVerbosity::SHOW_DETAILS and TxVerbosity::SHOW_DETAILS_AND_PREVOUT, they can be sped up by ~20% by reviving the abandoned (for lack of interest) #30035.

    Interesting I will take a look at that, it seems there are some unaddressed comments by @TheCharlatan at that PR

    And if BlockToJsonVerboseWrite needs a boost, #31144 will speed up by ~14%.

    I will take a look.

    @martinus also contains a few optimization attempts (a lot of them were already applied in other PRs since, but a few of the changes can still be done). If RPC speed is important to you, the above suggestions can result in a measurable improvements.

    RPC speed is important not just to me but to all users. For now, I’ve been experimenting with using libbitcoinkernel, and it seems to be more performant than the RPC.

    I’ll look into it further.

    All those PRs offer improvements that are beneficial to users. I also think having this reservation ability is important to avoid the reallocation overhead. @l0rinc?

  20. l0rinc commented at 10:45 pm on December 30, 2024: contributor
    I can implement my suggestions in separate PRs if you say the RPC speedups would be welcome
  21. in src/bench/rpc_blockchain.cpp:48 in f1ec37cc3c outdated 
    44@@ -45,16 +45,33 @@ struct TestBlockAndIndex {
45 
46 } // namespace
47 
48-static void BlockToJsonVerbose(benchmark::Bench& bench)
49+static void BlockToJson(benchmark::Bench& bench, TxVerbosity verbosity_level)
    l0rinc commented at 12:57 pm on December 31, 2024:

    :+1: for extracting the common part

    nit: in blockToJSON this is simply called verbosity

    ismaelsadeeq commented at 2:58 pm on January 2, 2025:
    Done, thanks
  22. in src/core_write.cpp:210 in f1ec37cc3c outdated 
    203@@ -203,6 +204,7 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry
204         }
205         if (!tx.vin[i].scriptWitness.IsNull()) {
206             UniValue txinwitness(UniValue::VARR);
207+            txinwitness.reserve(tx.vin[i].scriptWitness.stack.size());
208             for (const auto& item : tx.vin[i].scriptWitness.stack) {
209                 txinwitness.push_back(HexStr(item));
210             }
    l0rinc commented at 1:44 pm on December 31, 2024:

    we’re repeating the same request 3 times, might as well extract it:

    0        if (auto witness{tx.vin[i].scriptWitness}; !witness.IsNull()) {
1            UniValue txinwitness(UniValue::VARR);
2            txinwitness.reserve(witness.stack.size());
3            for (const auto& item : witness.stack) {
4                txinwitness.push_back(HexStr(item));
5            }
6            in.pushKV("txinwitness", std::move(txinwitness));
7        }
    ismaelsadeeq commented at 2:58 pm on January 2, 2025:
    good idea, pushed. Thanks
  23. l0rinc commented at 3:24 pm on December 31, 2024: contributor
    Approach ACK There are a LOT of other places where we will be able to use this - we should do them in follow-up PRs
  24. in src/bench/rpc_blockchain.cpp:57 in f1ec37cc3c outdated 
    55         ankerl::nanobench::doNotOptimizeAway(univalue);
56     });
57 }
58 
59-BENCHMARK(BlockToJsonVerbose, benchmark::PriorityLevel::HIGH);
60+static void BlockToJsonVerbose1(benchmark::Bench& bench)
    l0rinc commented at 3:28 pm on December 31, 2024:

    the real RPC also reads the block from disk, we could bench that as well - instead of assuming it’s already in memory (this will also enable additional optimization opportunities).

    Edit: note that this will reduce the difference between before/after state a lot, but the measurements will reflect a real call more.

    ismaelsadeeq commented at 3:01 pm on January 2, 2025:

    I think this is benching just blockToJSON . But it’s a good idea to write a bench for ReadRawBlockFromDisk as well.

    I will leave that as an idea for a follow-up PR.

    l0rinc commented at 5:15 pm on January 2, 2025:

    I will leave that as an idea for a follow-up PR.

    The problem is that the goal of this PR was to speed up the RPC. But if we benchmark that instead of just a subset of the task, i.e.:

     0diff --git a/src/bench/rpc_blockchain.cpp b/src/bench/rpc_blockchain.cpp
 1--- a/src/bench/rpc_blockchain.cpp	(revision bf5c569898d0297de010102a623bf52009607ed8)
 2+++ b/src/bench/rpc_blockchain.cpp	(date 1735836623994)
 3@@ -24,7 +24,6 @@
 4 namespace {
 5 
 6 struct TestBlockAndIndex {
 7-    const std::unique_ptr<const TestingSetup> testing_setup{MakeNoLogFileContext<const TestingSetup>(ChainType::MAIN)};
 8     CBlock block{};
 9     uint256 blockHash{};
10     CBlockIndex blockindex{};
11@@ -47,9 +46,10 @@
12 
13 static void BlockToJson(benchmark::Bench& bench, TxVerbosity verbosity)
14 {
15-    TestBlockAndIndex data;
16+    const std::unique_ptr<const TestingSetup> testing_setup{MakeNoLogFileContext<const TestingSetup>(ChainType::MAIN)};
17     bench.run([&] {
18-        auto univalue = blockToJSON(data.testing_setup->m_node.chainman->m_blockman, data.block, data.blockindex, data.blockindex, verbosity);
19+        TestBlockAndIndex data{};
20+        auto univalue = blockToJSON(testing_setup->m_node.chainman->m_blockman, data.block, data.blockindex, data.blockindex, verbosity);
21         ankerl::nanobench::doNotOptimizeAway(univalue);
22     });
23 }
24@@ -75,8 +75,9 @@
25 
26 static void BlockToJsonVerboseWrite(benchmark::Bench& bench)
27 {
28+    const std::unique_ptr<const TestingSetup> testing_setup{MakeNoLogFileContext<const TestingSetup>(ChainType::MAIN)};
29     TestBlockAndIndex data;
30-    auto univalue = blockToJSON(data.testing_setup->m_node.chainman->m_blockman, data.block, data.blockindex, data.blockindex, TxVerbosity::SHOW_DETAILS_AND_PREVOUT);
31+    auto univalue = blockToJSON(testing_setup->m_node.chainman->m_blockman, data.block, data.blockindex, data.blockindex, TxVerbosity::SHOW_DETAILS_AND_PREVOUT);
32     bench.run([&] {
33         auto str = univalue.write();
34         ankerl::nanobench::doNotOptimizeAway(str);

    via

    build/src/bench/bench_bitcoin -filter=‘BlockToJsonVerbosity.*’ -min-time=10000

    unfortunately it reveals:

    Before:

    ns/op op/s err% total benchmark
    1,317,881.88 758.79 0.3% 10.99 BlockToJsonVerbosity1
    26,534,215.46 37.69 0.3% 10.95 BlockToJsonVerbosity2
    26,486,601.35 37.75 0.1% 10.95 BlockToJsonVerbosity3

    After:

    ns/op op/s err% total benchmark
    1,313,779.11 761.16 0.6% 11.00 BlockToJsonVerbosity1
    26,128,170.14 38.27 0.2% 11.02 BlockToJsonVerbosity2
    26,143,108.35 38.25 0.2% 11.03 BlockToJsonVerbosity3

    i.e. barely any speedup for the RPC.

    ismaelsadeeq commented at 2:05 pm on January 8, 2025:

    The problem is that the goal of this PR was to speed up the RPC.

    I’ve updated the description and the title to limit the usage to just blockToJSON

    l0rinc commented at 4:26 pm on January 8, 2025:
    My concern isn’t with the description (I think reserving space can be a valuable change), but that it doesn’t really help the user calling this RPC, right?
    ismaelsadeeq commented at 4:34 pm on January 8, 2025:
    Yes end to end bench it’s not significant. I see it locally as well.
    l0rinc commented at 4:36 pm on January 8, 2025:
    I would be ok with it if we make the benchmarks representative - or not claim that this improved the RPC (I already have the follow-up PR locally using reserve for all other cases, I just want to be honest about it not being a measurable optimization)
    ismaelsadeeq commented at 6:20 pm on January 8, 2025:
    Ahh I see, Initially I was under the impression that it did, but I was wrong hence I updated the title and description
  25. ismaelsadeeq force-pushed on Jan 2, 2025
  26. Eunovo approved
  27. Eunovo commented at 10:14 am on January 6, 2025: contributor

    Tested ACK https://github.com/bitcoin/bitcoin/pull/31179/commits/bf5c569898d0297de010102a623bf52009607ed8

    Confirmed slight improvement in BlockToJson bench. Before:

    0|               ns/op |                op/s |    err% |          ins/op |          cyc/op |    IPC |         bra/op |   miss% |     total | benchmark
1|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
2|          438,695.00 |            2,279.49 |    1.3% |    3,818,825.00 |    1,495,260.00 |  2.554 |     682,779.00 |    0.8% |      0.01 | `BlockToJsonVerbosity1`
3|       45,518,930.00 |               21.97 |    0.3% |  354,868,701.00 |  154,939,608.00 |  2.290 |  60,169,811.00 |    1.5% |      0.51 | `BlockToJsonVerbosity2`
4|       45,463,793.00 |               22.00 |    0.2% |  354,866,371.00 |  154,811,098.00 |  2.292 |  60,168,983.00 |    1.4% |      0.51 | `BlockToJsonVerbosity3`

    After:

    0|               ns/op |                op/s |    err% |          ins/op |          cyc/op |    IPC |         bra/op |   miss% |     total | benchmark
1|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
2|          394,595.00 |            2,534.24 |    0.2% |    3,649,536.00 |    1,344,645.33 |  2.714 |     661,067.00 |    0.7% |      0.01 | `BlockToJsonVerbosity1`
3|       44,967,386.00 |               22.24 |    0.1% |  353,693,003.00 |  153,066,486.00 |  2.311 |  59,949,244.00 |    1.4% |      0.51 | `BlockToJsonVerbosity2`
4|       44,831,936.00 |               22.31 |    0.5% |  353,691,496.00 |  152,611,234.00 |  2.318 |  59,948,519.00 |    1.4% |      0.51 | `BlockToJsonVerbosity3`
  28. DrahtBot requested review from l0rinc on Jan 6, 2025
  29. DrahtBot requested review from andrewtoth on Jan 6, 2025
  30. ismaelsadeeq renamed this:
    RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC
    RPC: Add reserve member function to `UniValue` and use it in `blockToJSON` function
    on Jan 8, 2025
  31. in src/core_write.cpp:205 in bf5c569898 outdated 
    201@@ -201,9 +202,10 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry
202             o.pushKV("hex", HexStr(txin.scriptSig));
203             in.pushKV("scriptSig", std::move(o));
204         }
205-        if (!tx.vin[i].scriptWitness.IsNull()) {
206+        if (auto witness{tx.vin[i].scriptWitness}; !witness.IsNull()) {
    maflcko commented at 3:50 pm on January 8, 2025:
    This creates a copy of the witness stack. Previously it did not, so it seems like a pessimisation.
  32. maflcko commented at 3:53 pm on January 8, 2025: member

    Is this worth it, when the real end-to-end speedup is less than a percent, according to #31179 (review)?

    Edit: I see you had an end-to-end benchmark in the initial description, but it was replaced by the micro-benchmark. I guess the block in the micro-benchmark is too small to make a noticeable difference?

  33. ismaelsadeeq commented at 4:20 pm on January 8, 2025: member

    Is this worth it, when the real end-to-end speedup is less than a percent, according to #31179 (review)?

    I think it is useful to have 1749aef52af8ea5f436e5b26dc7281fce73d1436 We can use it in places like bf5c569898d0297de010102a623bf52009607ed8 and maybe more like @l0rinc mentioned #31179#pullrequestreview-2526455183.

    Edit: I see you had an end-to-end benchmark in the initial description, but it was replaced by the micro-benchmark. I guess the block in the micro-benchmark is too small to make a noticeable difference?

    Yeah the first run of the end to end benchmark shows a noticeable difference but then subsequent runs were very slight, so the performance increase due to the reallocation is not much, hence I limit the scope of the change to just adding the reservation feature and using it in blockToJSON.

  34. ismaelsadeeq force-pushed on Jan 8, 2025
  35. DrahtBot added the label Needs rebase on Jan 22, 2025
  36. bench: support benching all verbosity of `BlockToJson` bd461195f4
  37. UniValue: add reserve member function 
    - Only reserve keys when the typ is of `VOBJ`.
    6a506d5c37
  38. rpc: reserve space for `UniValue` variables in `blockToJSON` 
    - Reserving space avoid reallocation, this provide noticeable
  performance increase in verbosity 1.
    5d82d92aff
  39. ismaelsadeeq force-pushed on Jan 22, 2025
  40. DrahtBot removed the label Needs rebase on Jan 22, 2025
  41. luke-jr referenced this in commit 1c894b355e on Feb 26, 2025
  42. luke-jr referenced this in commit 5dcab5edf5 on Feb 26, 2025
  43. DrahtBot commented at 2:02 am on July 22, 2025: contributor

    There hasn’t been much activity lately. What is the status here?

    Finding reviewers may take time. However, if the patch is no longer relevant, please close this pull request. If the author lost interest or time to work on this, please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.

  44. ismaelsadeeq requested review from maflcko on Jul 22, 2025
  45. ismaelsadeeq requested review from Eunovo on Jul 22, 2025
  46. maflcko commented at 6:32 pm on July 22, 2025: member

    review ACK 5d82d92aff7c11ce17ee809c060e37f73a8a687a 🐀

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK 5d82d92aff7c11ce17ee809c060e37f73a8a687a 🐀
3qjQpXl4a54Be7NspYxvMwo7sXYk4VexhjMNKMam19QanyHVtiG9qYMoopQZyaTcZap5LpDA8M8N8ev9ZA2M5Bw==
  48. l0rinc commented at 7:35 pm on July 23, 2025: contributor

    While I don’t think this is a measurable optimization (i.e. RPCs won’t get faster because of it), it does make sense to reduce useless work and memory fragmentation. We’re reserving the right amounts - and can do other such cases (e.g. https://github.com/bitcoin/bitcoin/blob/master/src/rpc/rawtransaction.cpp#L1246) in follow-ups.

    0rm -rfd build && cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release && cmake --build build -j$(nproc) && build/bin/bench_bitcoin -filter='BlockToJsonVerbosity.*'

    Before (bd461195f4b3a3c53366c8f197e7701836cab053):

    ns/op op/s err% total benchmark
    120,922.57 8,269.75 2.0% 0.01 BlockToJsonVerbosity1
    19,956,458.00 50.11 0.3% 0.22 BlockToJsonVerbosity2
    20,062,458.00 49.84 0.8% 0.22 BlockToJsonVerbosity3

    After (5d82d92aff7c11ce17ee809c060e37f73a8a687a):

    ns/op op/s err% total benchmark
    103,754.20 9,638.16 1.3% 0.01 BlockToJsonVerbosity1
    19,978,000.00 50.06 0.6% 0.22 BlockToJsonVerbosity2
    20,106,791.00 49.73 0.5% 0.22 BlockToJsonVerbosity3

    ACK 5d82d92aff7c11ce17ee809c060e37f73a8a687a

  49. achow101 commented at 8:11 pm on July 23, 2025: member
    ACK 5d82d92aff7c11ce17ee809c060e37f73a8a687a
  50. achow101 merged this on Jul 23, 2025
  51. achow101 closed this on Jul 23, 2025
  52. ismaelsadeeq deleted the branch on Jul 23, 2025


ismaelsadeeq DrahtBot andrewtoth kennet31526 maflcko Eunovo l0rinc achow101


andrewtoth l0rinc maflcko

Labels
RPC/REST/ZMQ

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: 2025-10-31 09:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me