bugfix: correct DisconnectedBlockTransactions memory usage

With `queuedTx` owning the `CTransactionRef` shared ptrs, they (and
the managed objects) are entirely allocated on the heap. In
`DisconnectedBlockTransactions::DynamicMemoryUsage`, we account for
the 2 pointers that make up the shared_ptr, but not for the managed
object (CTransaction) or the control block.

Prior to this commit, by calculating the `RecursiveDynamicUsage` on
a `CTransaction` whenever modifying `cachedInnerUsage`, we account
for the dynamic usage of the `CTransaction`, i.e. the `vins` and
`vouts` vectors, but we do not account for the `CTransaction`
object itself, nor for the `CTransactionRef` control block.

This means prior to this commit, `DynamicMemoryUsage` underestimates
dynamic memory usage by not including the `CTransaction` objects and
the shared ptr control blocks.

Fix this by calculating `RecursiveDynamicUsage` on the
`CTransactionRef` instead of the `CTransaction` whenever modifying
`cachedInnerUsage`.
This commit is contained in:
stickies-v 2023-10-18 11:38:45 +01:00 committed by ismaelsadeeq
parent f4254e2098
commit b2d0447964
2 changed files with 4 additions and 5 deletions

View file

@ -34,7 +34,7 @@ std::vector<CTransactionRef> DisconnectedBlockTransactions::LimitMemoryUsage()
while (!queuedTx.empty() && DynamicMemoryUsage() > m_max_mem_usage) {
evicted.emplace_back(queuedTx.front());
cachedInnerUsage -= RecursiveDynamicUsage(*queuedTx.front());
cachedInnerUsage -= RecursiveDynamicUsage(queuedTx.front());
iters_by_txid.erase(queuedTx.front()->GetHash());
queuedTx.pop_front();
}
@ -53,7 +53,7 @@ size_t DisconnectedBlockTransactions::DynamicMemoryUsage() const
auto it = queuedTx.insert(queuedTx.end(), *block_it);
auto [_, inserted] = iters_by_txid.emplace((*block_it)->GetHash(), it);
assert(inserted); // callers may never pass multiple transactions with the same txid
cachedInnerUsage += RecursiveDynamicUsage(**block_it);
cachedInnerUsage += RecursiveDynamicUsage(*block_it);
}
return LimitMemoryUsage();
}
@ -69,7 +69,7 @@ void DisconnectedBlockTransactions::removeForBlock(const std::vector<CTransactio
if (iter != iters_by_txid.end()) {
auto list_iter = iter->second;
iters_by_txid.erase(iter);
cachedInnerUsage -= RecursiveDynamicUsage(**list_iter);
cachedInnerUsage -= RecursiveDynamicUsage(*list_iter);
queuedTx.erase(list_iter);
}
}

View file

@ -36,8 +36,7 @@ static const unsigned int MAX_DISCONNECTED_TX_POOL_BYTES{20'000'000};
*/
class DisconnectedBlockTransactions {
private:
/** Cached dynamic memory usage for the CTransactions (memory for the shared pointers is
* included in the container calculations). */
/** Cached dynamic memory usage for the `CTransactionRef`s */
uint64_t cachedInnerUsage = 0;
const size_t m_max_mem_usage;
std::list<CTransactionRef> queuedTx;