Skip to content

Commit 081965c

Browse files
committed
Merge bitcoin/bitcoin#25464: rpc: Reduce Univalue push_backV peak memory usage in listtransactions
fa8a1c0 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 fa8a1c0 Tree-SHA512: 1c3ca40fc8497134a4141195160e4aa9fe72b3c00c5998c972b58ad0eb498ebea05013f9105bb80e7264c9db1d0e7a2032396a8a4af1f326d831fbee20f32ea3
2 parents 1d89fc6 + fa8a1c0 commit 081965c

File tree

2 files changed

+16
-6
lines changed

2 files changed

+16
-6
lines changed

src/univalue/include/univalue.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ class UniValue {
8484

8585
bool push_back(const UniValue& val);
8686
bool push_backV(const std::vector<UniValue>& vec);
87+
template <class It>
88+
bool push_backV(It first, It last);
8789

8890
void __pushKV(const std::string& key, const UniValue& val);
8991
bool pushKV(const std::string& key, const UniValue& val);
@@ -137,6 +139,14 @@ class UniValue {
137139
friend const UniValue& find_value( const UniValue& obj, const std::string& name);
138140
};
139141

142+
template <class It>
143+
bool UniValue::push_backV(It first, It last)
144+
{
145+
if (typ != VARR) return false;
146+
values.insert(values.end(), first, last);
147+
return true;
148+
}
149+
140150
enum jtokentype {
141151
JTOK_ERR = -1,
142152
JTOK_NONE = 0, // eof

src/wallet/rpc/transactions.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -310,11 +310,12 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest)
310310
* @param wtx The wallet transaction.
311311
* @param nMinDepth The minimum confirmation depth.
312312
* @param fLong Whether to include the JSON version of the transaction.
313-
* @param ret The UniValue into which the result is stored.
313+
* @param ret The vector into which the result is stored.
314314
* @param filter_ismine The "is mine" filter flags.
315315
* @param filter_label Optional label string to filter incoming transactions.
316316
*/
317-
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)
317+
template <class Vec>
318+
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)
318319
{
319320
CAmount nFee;
320321
std::list<COutputEntry> listReceived;
@@ -500,8 +501,7 @@ RPCHelpMan listtransactions()
500501
if (nFrom < 0)
501502
throw JSONRPCError(RPC_INVALID_PARAMETER, "Negative from");
502503

503-
UniValue ret(UniValue::VARR);
504-
504+
std::vector<UniValue> ret;
505505
{
506506
LOCK(pwallet->cs_wallet);
507507

@@ -523,9 +523,9 @@ RPCHelpMan listtransactions()
523523
if ((nFrom + nCount) > (int)ret.size())
524524
nCount = ret.size() - nFrom;
525525

526-
const std::vector<UniValue>& txs = ret.getValues();
526+
auto txs_rev_it{std::make_move_iterator(ret.rend())};
527527
UniValue result{UniValue::VARR};
528-
result.push_backV({ txs.rend() - nFrom - nCount, txs.rend() - nFrom }); // Return oldest to newest
528+
result.push_backV(txs_rev_it - nFrom - nCount, txs_rev_it - nFrom); // Return oldest to newest
529529
return result;
530530
},
531531
};

0 commit comments

Comments
 (0)