foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-03 09:56:38 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#25464: rpc: Reduce Univalue push_backV peak memory usage in listtransactions

Browse source 
fa8a1c0696 rpc: Fix Univalue push_backV OOM in listtransactions (MacroFake)

Pull request description:

  Related to, but not intended as a fix for #25229.

  Currently the RPC will have the same data stored thrice:

  * `UniValue ret` (memory filled by `ListTransactions`)
  * `std::vector<UniValue> vec` (constructed by calling `push_backV`)
  * `UniValue result` (the actual result, memory filled by `push_backV`)

  Fix this by filling the memory only once:

  * `std::vector<UniValue> ret` (memory filled by `ListTransactions`)
  * Pass iterators to `push_backV` instead of creating a full copy
  * Move memory into `UniValue result` instead of copying it

ACKs for top commit:
  shaavan:
    Code Review ACK fa8a1c0696

Tree-SHA512: 1c3ca40fc8497134a4141195160e4aa9fe72b3c00c5998c972b58ad0eb498ebea05013f9105bb80e7264c9db1d0e7a2032396a8a4af1f326d831fbee20f32ea3
This commit is contained in:
fanquake 2022-07-13 15:52:10 +01:00
commit 081965ccc3
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
2 changed files with 16 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
src/univalue/include/univalue.h
View file

@ -84,6 +84,8 @@ public:
bool push_back(const UniValue& val); bool push_back(const UniValue& val);
bool push_backV(const std::vector<UniValue>& vec); bool push_backV(const std::vector<UniValue>& vec);
template <class It>
bool push_backV(It first, It last);
void __pushKV(const std::string& key, const UniValue& val); void __pushKV(const std::string& key, const UniValue& val);
bool pushKV(const std::string& key, const UniValue& val); bool pushKV(const std::string& key, const UniValue& val);
@ -137,6 +139,14 @@ public:
friend const UniValue& find_value( const UniValue& obj, const std::string& name); friend const UniValue& find_value( const UniValue& obj, const std::string& name);
}; };
template <class It>
bool UniValue::push_backV(It first, It last)
{
if (typ != VARR) return false;
values.insert(values.end(), first, last);
return true;
}
enum jtokentype { enum jtokentype {
JTOK_ERR = -1, JTOK_ERR = -1,
JTOK_NONE = 0, // eof JTOK_NONE = 0, // eof

12
src/wallet/rpc/transactions.cpp
View file

@ -310,11 +310,12 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest)
* @param wtx The wallet transaction. * @param wtx The wallet transaction.
* @param nMinDepth The minimum confirmation depth. * @param nMinDepth The minimum confirmation depth.
* @param fLong Whether to include the JSON version of the transaction. * @param fLong Whether to include the JSON version of the transaction.
* @param ret The UniValue into which the result is stored. * @param ret The vector into which the result is stored.
* @param filter_ismine The "is mine" filter flags. * @param filter_ismine The "is mine" filter flags.
* @param filter_label Optional label string to filter incoming transactions. * @param filter_label Optional label string to filter incoming transactions.
*/ */
static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter_ismine, const std::string* filter_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) template <class Vec>
static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nMinDepth, bool fLong, Vec& ret, const isminefilter& filter_ismine, const std::string* filter_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{ {
CAmount nFee; CAmount nFee;
std::list<COutputEntry> listReceived; std::list<COutputEntry> listReceived;
@ -500,8 +501,7 @@ RPCHelpMan listtransactions()
if (nFrom < 0) if (nFrom < 0)
throw JSONRPCError(RPC_INVALID_PARAMETER, "Negative from"); throw JSONRPCError(RPC_INVALID_PARAMETER, "Negative from");
UniValue ret(UniValue::VARR); std::vector<UniValue> ret;
{ {
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
@ -523,9 +523,9 @@ RPCHelpMan listtransactions()
if ((nFrom + nCount) > (int)ret.size()) if ((nFrom + nCount) > (int)ret.size())
nCount = ret.size() - nFrom; nCount = ret.size() - nFrom;
const std::vector<UniValue>& txs = ret.getValues(); auto txs_rev_it{std::make_move_iterator(ret.rend())};
UniValue result{UniValue::VARR}; UniValue result{UniValue::VARR};
result.push_backV({ txs.rend() - nFrom - nCount, txs.rend() - nFrom }); // Return oldest to newest result.push_backV(txs_rev_it - nFrom - nCount, txs_rev_it - nFrom); // Return oldest to newest
return result; return result;
}, },
}; };