Remove UniValue::push_backV() and UniValue::pushKVs()
The methods for bulk-appends to UniValue arrays/objects are removed for the following performance and code quality reasons (fitting within #51):
- In practice, this codebase abuses bulk-appends to unnecessarily create intermediate objects instead of writing directly to the target object.
- The distinct capacity management logic of bulk-appends interferes with the amortised constant complexity guarantee of individual appends.
- The lack of duplicate key checks in
pushKVs()
(contrary topushKV()
) is confusing - adding duplicate key checks topushKVs()
would even introduce a severe transaction construction bug.
To preserve the functionality of call sites, the following new overloads were added to the UniValue API:
- Overloads of
setArray()
acceptingstd::vector<UniValue>
(both copy and move). - Non-const overload of
getObjectEntries()
. - Non-const overload of
getArrayValues()
.
While updating the call sites, I also made some other UniValue-related changes to improve performance.
This MR is expected to improve performance of the following RPC commands:
listtransactions
-
getaddressinfo
(usesDescribeWalletAddress
) -
validateaddress
(usesDescribeAddress
) -
createrawtransaction
,createpsbt
,walletcreatefundedpsbt
(useConstructTransaction
)
AFAIK there are no benchmarks available to measure these, but the theoretical arguments given above are convincing on their own.
Test plan: ninja bench_bitcoin check-univalue check check-functional
Edited by BigBlockIfTrue