getblock performance issue on verbosity #15925

1. 
   fanatid commented at 5:59 am on April 30, 2019: contributor

   getblock is slow on verbosity, it’s happened because LOCK(cs_main) applied for whole function including JSON generation https://github.com/bitcoin/bitcoin/blob/ef70f9b52b851c7997a9f1a0834714e3eebc1fd8/src/rpc/blockchain.cpp#L810

   I added few lines which print spent time:

   0    LOCK(cs_main);
   1    auto t1 = std::chrono::high_resolution_clock::now();
   2...
   3    auto t2 = std::chrono::high_resolution_clock::now();
   4    auto ret = blockToJSON(block, pblockindex, verbosity >= 2);
   5    auto t3 = std::chrono::high_resolution_clock::now();
   6    std::cout << "blockToJSON " << pblockindex->nHeight << ": " << std::chrono::duration_cast<std::chrono::microseconds>(t2 - t1).count() << " " << std::chrono::duration_cast<std::chrono::microseconds>(t3 - t1).count() << std::endl;
   7    return ret;
   

   and numbers looks like:

   0blockToJSON 570019: 14727 201870
   1blockToJSON 570022: 16049 225224
   2blockToJSON 570021: 33776 217398
   3blockToJSON 570020: 21754 191669
   4blockToJSON 570023: 30949 236941
   5blockToJSON 570025: 23346 223673
   6blockToJSON 570024: 18732 203002
   7blockToJSON 570026: 17467 216213
   8blockToJSON 570027: 16658 217395
   9blockToJSON 570028: 19015 219183
   

   i.e. everything before JSON generation takes 15-30ms, while JSON takes minimum 190ms and lock cs_main for all this time. As result it’s possible extract only ~5 blocks per second.

   Looking on the code I think it’s possible copy data from CBlockIndex to extra structure, so in this case cs_main can be unlocked right after GetBlockChecked. https://github.com/bitcoin/bitcoin/blob/ef70f9b52b851c7997a9f1a0834714e3eebc1fd8/src/rpc/blockchain.cpp#L828 As result getblock in verbosity mode should be much faster (in case if -rpcthreads will be adjusted too).

   Does this make sense?

2. fanquake added the label RPC/REST/ZMQ on Apr 30, 2019
3. 
   jonasschnelli commented at 6:13 am on April 30, 2019: contributor

   Reducing the usage of the cs_main lock is desirable. Though your approach “only” focuses on increasing the concurrency efficiency.

   IMO the JSON RPC interface is not made for efficiency.

   If you seek fast block fetches, you are much better off with using a) the p2p API or b) the REST API with binary or at least hex encoding.

4. 
   ryanofsky commented at 2:39 pm on April 30, 2019: member

   If you seek fast block fetches, you are much better off with using a) the p2p API or b) the REST API with binary or at least hex encoding.

   This is good advice, but it’s still interesting to know how time is being spent in the RPC, and I don’t think we’d reject a simple PR optimizing it.

5. 
   promag commented at 7:22 pm on May 2, 2019: member

   So JSON serialization takes around 10 times more than loading the block? 🙄

   Edit: I’d say getblock can be optimized for concurrency (#15932 by @MarcoFalke 👏) and for efficiency (TODO?).

6. 
   MarcoFalke commented at 7:53 pm on May 2, 2019: member

   Yeah, could be the typical o2 complexity thing with univalue?
7. 
   sipa commented at 8:04 pm on May 2, 2019: member

   Adding UniValue functions with move semantics/rvalue refs would help a lot…
8. 
   promag commented at 8:31 pm on May 2, 2019: member

   Yeah, could be the typical o2 complexity thing with univalue?

   I don’t think it’s the case here.

9. MarcoFalke closed this on May 3, 2019

   ---

10. MarcoFalke referenced this in commit 94daebf327 on May 3, 2019
11. MarcoFalke reopened this on May 3, 2019

    ---

12. 
    MarcoFalke commented at 12:14 pm on May 3, 2019: member

    Keeping this open since translating to json takes 100ms, which seems a bit extreme.
13. MarcoFalke added the label good first issue on May 3, 2019
14. MarcoFalke added the label Resource usage on May 3, 2019
15. sidhujag referenced this in commit ce64be77f2 on May 3, 2019
16. 
    jgarzik commented at 3:23 pm on May 7, 2019: contributor

    Adding UniValue functions with move semantics/rvalue refs would help a lot…

    Sounds good to me

17. 
    promag commented at 3:44 pm on May 7, 2019: member

    Just to have some idea of what’s going on with blockToJSON, this is a rough (some of the fields are commented) benchmark with block 413567. BTW, this was taken with some std::moves already implemented.

    (running a not-yet-pushed BENCHMARK(BlockToJSON, 40))

18. 
    fanatid commented at 2:33 pm on June 19, 2019: contributor

    @promag can you show you code for BENCHMARK(BlockToJSON, 40)? Thank you in advance!
19. 
    promag commented at 1:40 pm on June 24, 2019: member

    @fanatid sorry I’ve missed your comment 😞 thanks for the pull request!
20. laanwj referenced this in commit 0a6ee9797e on Jul 8, 2019
21. sidhujag referenced this in commit 6678aba111 on Jul 9, 2019
22. MarcoFalke removed the label good first issue on May 6, 2020
23. 
    MarcoFalke commented at 4:25 pm on May 6, 2020: member

    Closing this for now. Translating to json is inherently more expensive than serving the raw block, but as you realized, with the cs_main lock removed, at least it can be done in parallel.
24. MarcoFalke closed this on May 6, 2020

    ---

25. deadalnix referenced this in commit 2e7115ee28 on May 6, 2020
26. monstrobishi referenced this in commit 0ad43b700d on Sep 6, 2020
27. Munkybooty referenced this in commit 1afe02bc6c on Oct 12, 2021
28. Munkybooty referenced this in commit f6e9396aa8 on Oct 13, 2021
29. Munkybooty referenced this in commit 3c6c44c938 on Oct 14, 2021
30. Munkybooty referenced this in commit 2cd358cfcb on Nov 4, 2021
31. Munkybooty referenced this in commit da34449bb7 on Nov 4, 2021
32. vijaydasmp referenced this in commit 1393571bcb on Jan 5, 2022
33. vijaydasmp referenced this in commit 3066042c2a on Jan 23, 2022
34. DrahtBot locked this on Feb 15, 2022
35. vijaydasmp referenced this in commit 6e6ef11002 on Apr 21, 2022
36. vijaydasmp referenced this in commit aada277143 on Apr 23, 2022
37. vijaydasmp referenced this in commit 5f0fcad64b on May 10, 2022
38. vijaydasmp referenced this in commit 80a6b7a0f0 on May 19, 2022
39. vijaydasmp referenced this in commit 7689587314 on May 19, 2022
40. vijaydasmp referenced this in commit d7252f552e on May 19, 2022
41. vijaydasmp referenced this in commit 0fd13a904b on May 19, 2022
42. vijaydasmp referenced this in commit bb80360c78 on May 26, 2022
43. vijaydasmp referenced this in commit 8bbc680d85 on Jun 14, 2022
44. vijaydasmp referenced this in commit f94b675dc2 on Jun 14, 2022
45. vijaydasmp referenced this in commit 4762055516 on Jun 15, 2022
46. vijaydasmp referenced this in commit d1c4ed840d on Jun 16, 2022
47. vijaydasmp referenced this in commit d5986a1e8f on Jun 18, 2022

Contributors
fanatid jonasschnelli ryanofsky promag MarcoFalke sipa jgarzik

Labels
RPC/REST/ZMQ Resource usage