Skip to content

UniValue & RPC Interface: Significant performance improvements

Calin Culianu requested to merge cculianu/bitcoin-cash-node:univalue_perf_improvements into master

Summary

As discussed here: #51 (comment 305079080) in issue #51 , I have observed ways to optimize the UniValue library. The details of the motivation for the optimizations are discussed in depth in the link above, but the summary is:

  • Don't do O(N) lookups on each insert into UniValue instances that are VOBJ (dictionaries), by modifying the call site in various places that build particularly large dicts.
  • Add move && support for the various push_* methods
  • Various small nits like making some getters constexpr to have the compiler better optimize.
  • Use noexcept where it's guaranteed noexcept for more compiler help.
  • Changed some functions that seemed to (un)intentionally break copy elision / RVO for C++ compilers.
  • Got rid of some dead code in some places that was building vectors that were never used.

Test Plan

Do the below both against master and against this branch, both times by starting from a ninja clean build dir

First, build bitcoin-bench:

$ ninja src/bench/bitcoin-bench

Some of the performance benefits are huge especially on RPC calls that build large dictionaries. In particular:

$ src/bench/bitcoin-bench -filter=RpcMem.\* -evals=20

Other benchmarks that also show improvement:

$ src/bench/bitcoin-bench -filter=BlockTo.\* -evals=20
$ src/bench/bitcoin-bench -filter=Json.\* -evals=20  # (this one shows the least improvement)

Additional testing: If you have testnet synched, build this and start bitcoind, hit it with a bitcoin-cli request for things like getrawtransactions true, getrawmempool true, and getblock to make sure the results look the same. (I believe this may be redundant as the Python tests already would catch most things). I have gone over the code carefully and am sure nothing broke due to the new pushKV(..., false) calls... but if you see something, say something! :D

I did the above and also I tested this branch against Fulcrum server (synched a full testnet, hit all the RPC commands Fulcrum uses) and nothing broke.

Edited by Calin Culianu

Merge request reports