Skip to content

Commit

Permalink
Optimize SHAMapItem and leverage new slab allocator: (#4218)
Browse files Browse the repository at this point in the history
The `SHAMapItem` class contains a variable-sized buffer that
holds the serialized data associated with a particular item
inside a `SHAMap`.

Prior to this commit, the buffer for the serialized data was
allocated separately. Coupled with the fact that most instances
of `SHAMapItem` were wrapped around a `std::shared_ptr` meant
that an instantiation might result in up to three separate
memory allocations.

This commit switches away from `std::shared_ptr` for `SHAMapItem`
and uses `boost::intrusive_ptr` instead, allowing the reference
count for an instance to live inside the instance itself. Coupled
with using a slab-based allocator to optimize memory allocation
for the most commonly sized buffers, the net result is significant
memory savings. In testing, the reduction in memory usage hovers
between 400MB and 650MB. Other scenarios might result in larger
savings.

In performance testing with NFTs, this commit reduces memory size by
about 15% sustained over long duration.

Commit 2 of 3 in #4218.
  • Loading branch information
nbougalis authored and intelliot committed Apr 11, 2023
1 parent b7f588b commit c3acbce
Show file tree
Hide file tree
Showing 36 changed files with 293 additions and 178 deletions.
11 changes: 6 additions & 5 deletions src/ripple/app/consensus/RCLConsensus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ RCLConsensus::Adaptor::share(RCLCxTx const& tx)
if (app_.getHashRouter().shouldRelay(tx.id()))
{
JLOG(j_.debug()) << "Relaying disputed tx " << tx.id();
auto const slice = tx.tx_.slice();
auto const slice = tx.tx_->slice();
protocol::TMTransaction msg;
msg.set_rawtransaction(slice.data(), slice.size());
msg.set_status(protocol::tsNEW);
Expand Down Expand Up @@ -325,7 +325,7 @@ RCLConsensus::Adaptor::onClose(
tx.first->add(s);
initialSet->addItem(
SHAMapNodeType::tnTRANSACTION_NM,
SHAMapItem(tx.first->getTransactionID(), s.slice()));
make_shamapitem(tx.first->getTransactionID(), s.slice()));
}

// Add pseudo-transactions to the set
Expand Down Expand Up @@ -369,7 +369,8 @@ RCLConsensus::Adaptor::onClose(
RCLCensorshipDetector<TxID, LedgerIndex>::TxIDSeqVec proposed;

initialSet->visitLeaves(
[&proposed, seq](std::shared_ptr<SHAMapItem const> const& item) {
[&proposed,
seq](boost::intrusive_ptr<SHAMapItem const> const& item) {
proposed.emplace_back(item->key(), seq);
});

Expand Down Expand Up @@ -529,7 +530,7 @@ RCLConsensus::Adaptor::doAccept(
std::vector<TxID> accepted;

result.txns.map_->visitLeaves(
[&accepted](std::shared_ptr<SHAMapItem const> const& item) {
[&accepted](boost::intrusive_ptr<SHAMapItem const> const& item) {
accepted.push_back(item->key());
});

Expand Down Expand Up @@ -604,7 +605,7 @@ RCLConsensus::Adaptor::doAccept(
<< "Test applying disputed transaction that did"
<< " not get in " << dispute.tx().id();

SerialIter sit(dispute.tx().tx_.slice());
SerialIter sit(dispute.tx().tx_->slice());
auto txn = std::make_shared<STTx const>(sit);

// Disputed pseudo-transactions that were not accepted
Expand Down
11 changes: 5 additions & 6 deletions src/ripple/app/consensus/RCLCxTx.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,19 @@ class RCLCxTx
@param txn The transaction to wrap
*/
RCLCxTx(SHAMapItem const& txn) : tx_{txn}
RCLCxTx(boost::intrusive_ptr<SHAMapItem const> txn) : tx_(std::move(txn))
{
}

//! The unique identifier/hash of the transaction
ID const&
id() const
{
return tx_.key();
return tx_->key();
}

//! The SHAMapItem that represents the transaction.
SHAMapItem const tx_;
boost::intrusive_ptr<SHAMapItem const> tx_;
};

/** Represents a set of transactions in RCLConsensus.
Expand Down Expand Up @@ -90,8 +90,7 @@ class RCLTxSet
bool
insert(Tx const& t)
{
return map_->addItem(
SHAMapNodeType::tnTRANSACTION_NM, SHAMapItem{t.tx_});
return map_->addItem(SHAMapNodeType::tnTRANSACTION_NM, t.tx_);
}

/** Remove a transaction from the set.
Expand Down Expand Up @@ -145,7 +144,7 @@ class RCLTxSet
code uses the shared_ptr semantics to know whether the find
was successful and properly creates a Tx as needed.
*/
std::shared_ptr<const SHAMapItem> const&
boost::intrusive_ptr<SHAMapItem const> const&
find(Tx::ID const& entry) const
{
return map_->peekItem(entry);
Expand Down
20 changes: 9 additions & 11 deletions src/ripple/app/ledger/Ledger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,8 @@ class Ledger::sles_iter_impl : public sles_type::iter_base
sles_type::value_type
dereference() const override
{
auto const item = *iter_;
SerialIter sit(item.slice());
return std::make_shared<SLE const>(sit, item.key());
SerialIter sit(iter_->slice());
return std::make_shared<SLE const>(sit, iter_->key());
}
};

Expand Down Expand Up @@ -168,7 +167,7 @@ class Ledger::txs_iter_impl : public txs_type::iter_base
txs_type::value_type
dereference() const override
{
auto const item = *iter_;
auto const& item = *iter_;
if (metadata_)
return deserializeTxPlusMeta(item);
return {deserializeTx(item), nullptr};
Expand Down Expand Up @@ -404,8 +403,8 @@ bool
Ledger::addSLE(SLE const& sle)
{
auto const s = sle.getSerializer();
SHAMapItem item(sle.key(), s.slice());
return stateMap_->addItem(SHAMapNodeType::tnACCOUNT_STATE, std::move(item));
return stateMap_->addItem(
SHAMapNodeType::tnACCOUNT_STATE, make_shamapitem(sle.key(), s.slice()));
}

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -565,7 +564,7 @@ Ledger::rawInsert(std::shared_ptr<SLE> const& sle)
sle->add(ss);
if (!stateMap_->addGiveItem(
SHAMapNodeType::tnACCOUNT_STATE,
std::make_shared<SHAMapItem const>(sle->key(), ss.slice())))
make_shamapitem(sle->key(), ss.slice())))
LogicError("Ledger::rawInsert: key already exists");
}

Expand All @@ -576,7 +575,7 @@ Ledger::rawReplace(std::shared_ptr<SLE> const& sle)
sle->add(ss);
if (!stateMap_->updateGiveItem(
SHAMapNodeType::tnACCOUNT_STATE,
std::make_shared<SHAMapItem const>(sle->key(), ss.slice())))
make_shamapitem(sle->key(), ss.slice())))
LogicError("Ledger::rawReplace: key not found");
}

Expand All @@ -593,8 +592,7 @@ Ledger::rawTxInsert(
s.addVL(txn->peekData());
s.addVL(metaData->peekData());
if (!txMap().addGiveItem(
SHAMapNodeType::tnTRANSACTION_MD,
std::make_shared<SHAMapItem const>(key, s.slice())))
SHAMapNodeType::tnTRANSACTION_MD, make_shamapitem(key, s.slice())))
LogicError("duplicate_tx: " + to_string(key));
}

Expand All @@ -610,7 +608,7 @@ Ledger::rawTxInsertWithHash(
Serializer s(txn->getDataLength() + metaData->getDataLength() + 16);
s.addVL(txn->peekData());
s.addVL(metaData->peekData());
auto item = std::make_shared<SHAMapItem const>(key, s.slice());
auto item = make_shamapitem(key, s.slice());
auto hash = sha512Half(HashPrefix::txNode, item->slice(), item->key());
if (!txMap().addGiveItem(SHAMapNodeType::tnTRANSACTION_MD, std::move(item)))
LogicError("duplicate_tx: " + to_string(key));
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/ledger/LedgerReplayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class LedgerReplayer final
void
gotSkipList(
LedgerInfo const& info,
std::shared_ptr<SHAMapItem const> const& data);
boost::intrusive_ptr<SHAMapItem const> const& data);

/**
* Process a ledger delta (extracted from a TMReplayDeltaResponse message)
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/ledger/TransactionMaster.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class TransactionMaster

std::shared_ptr<STTx const>
fetch(
std::shared_ptr<SHAMapItem> const& item,
boost::intrusive_ptr<SHAMapItem> const& item,
SHAMapNodeType type,
std::uint32_t uCommitLedger);

Expand Down
26 changes: 13 additions & 13 deletions src/ripple/app/ledger/impl/LedgerReplayMsgHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,15 +163,15 @@ LedgerReplayMsgHandler::processProofPathResponse(
JLOG(journal_.debug()) << "Bad message: Cannot deserialize";
return false;
}
auto item = static_cast<SHAMapLeafNode*>(node.get())->peekItem();
if (!item)

if (auto item = static_cast<SHAMapLeafNode*>(node.get())->peekItem())
{
JLOG(journal_.debug()) << "Bad message: Cannot get ShaMapItem";
return false;
replayer_.gotSkipList(info, item);
return true;
}

replayer_.gotSkipList(info, item);
return true;
JLOG(journal_.debug()) << "Bad message: Cannot get ShaMapItem";
return false;
}

protocol::TMReplayDeltaResponse
Expand Down Expand Up @@ -206,9 +206,10 @@ LedgerReplayMsgHandler::processReplayDeltaRequest(
reply.set_ledgerheader(nData.getDataPtr(), nData.getLength());
// pack transactions
auto const& txMap = ledger->txMap();
txMap.visitLeaves([&](std::shared_ptr<SHAMapItem const> const& txNode) {
reply.add_transaction(txNode->data(), txNode->size());
});
txMap.visitLeaves(
[&](boost::intrusive_ptr<SHAMapItem const> const& txNode) {
reply.add_transaction(txNode->data(), txNode->size());
});

JLOG(journal_.debug()) << "getReplayDelta for ledger " << ledgerHash
<< " txMap hash " << txMap.getHash().as_uint256();
Expand Down Expand Up @@ -264,10 +265,9 @@ LedgerReplayMsgHandler::processReplayDeltaResponse(
STObject meta(metaSit, sfMetadata);
orderedTxns.emplace(meta[sfTransactionIndex], std::move(tx));

auto item =
std::make_shared<SHAMapItem const>(tid, shaMapItemData.slice());
if (!item ||
!txMap.addGiveItem(SHAMapNodeType::tnTRANSACTION_MD, item))
if (!txMap.addGiveItem(
SHAMapNodeType::tnTRANSACTION_MD,
make_shamapitem(tid, shaMapItemData.slice())))
{
JLOG(journal_.debug()) << "Bad message: Cannot deserialize";
return false;
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/ledger/impl/LedgerReplayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ LedgerReplayer::createDeltas(std::shared_ptr<LedgerReplayTask> task)
void
LedgerReplayer::gotSkipList(
LedgerInfo const& info,
std::shared_ptr<SHAMapItem const> const& item)
boost::intrusive_ptr<SHAMapItem const> const& item)
{
std::shared_ptr<SkipListAcquire> skipList = {};
{
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/ledger/impl/SkipListAcquire.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ SkipListAcquire::pmDowncast()
void
SkipListAcquire::processData(
std::uint32_t ledgerSeq,
std::shared_ptr<SHAMapItem const> const& item)
boost::intrusive_ptr<SHAMapItem const> const& item)
{
assert(ledgerSeq != 0 && item);
ScopedLockType sl(mtx_);
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/ledger/impl/SkipListAcquire.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class SkipListAcquire final
void
processData(
std::uint32_t ledgerSeq,
std::shared_ptr<SHAMapItem const> const& item);
boost::intrusive_ptr<SHAMapItem const> const& item);

/**
* Add a callback that will be called when the skipList is ready or failed.
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/ledger/impl/TransactionMaster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ TransactionMaster::fetch(

std::shared_ptr<STTx const>
TransactionMaster::fetch(
std::shared_ptr<SHAMapItem> const& item,
boost::intrusive_ptr<SHAMapItem> const& item,
SHAMapNodeType type,
std::uint32_t uCommitLedger)
{
Expand Down
3 changes: 1 addition & 2 deletions src/ripple/app/misc/AmendmentTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,7 @@ class AmendmentTable

initialPosition->addGiveItem(
SHAMapNodeType::tnTRANSACTION_NM,
std::make_shared<SHAMapItem>(
amendTx.getTransactionID(), s.slice()));
make_shamapitem(amendTx.getTransactionID(), s.slice()));
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/misc/FeeVoteImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ FeeVoteImpl::doVoting(

if (!initialPosition->addGiveItem(
SHAMapNodeType::tnTRANSACTION_NM,
std::make_shared<SHAMapItem>(txID, s.slice())))
make_shamapitem(txID, s.slice())))
{
JLOG(journal_.warn()) << "Ledger already had fee change";
}
Expand Down
8 changes: 4 additions & 4 deletions src/ripple/app/misc/NegativeUNLVote.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <ripple/app/consensus/RCLValidations.h>
#include <ripple/app/ledger/Ledger.h>
#include <ripple/app/misc/NegativeUNLVote.h>
#include <ripple/shamap/SHAMapItem.h>

namespace ripple {

Expand Down Expand Up @@ -115,21 +116,20 @@ NegativeUNLVote::addTx(
obj.setFieldVL(sfUNLModifyValidator, vp.slice());
});

uint256 txID = negUnlTx.getTransactionID();
Serializer s;
negUnlTx.add(s);
if (!initialSet->addGiveItem(
SHAMapNodeType::tnTRANSACTION_NM,
std::make_shared<SHAMapItem>(txID, s.slice())))
make_shamapitem(negUnlTx.getTransactionID(), s.slice())))
{
JLOG(j_.warn()) << "N-UNL: ledger seq=" << seq
<< ", add ttUNL_MODIFY tx failed";
}
else
{
JLOG(j_.debug()) << "N-UNL: ledger seq=" << seq
<< ", add a ttUNL_MODIFY Tx with txID: " << txID
<< ", the validator to "
<< ", add a ttUNL_MODIFY Tx with txID: "
<< negUnlTx.getTransactionID() << ", the validator to "
<< (modify == ToDisable ? "disable: " : "re-enable: ")
<< vp;
}
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/consensus/Consensus.h
Original file line number Diff line number Diff line change
Expand Up @@ -1643,7 +1643,7 @@ Consensus<Adaptor>::createDisputes(TxSet_t const& o)
(inThisSet && result_->txns.find(txId) && !o.find(txId)) ||
(!inThisSet && !result_->txns.find(txId) && o.find(txId)));

Tx_t tx = inThisSet ? *result_->txns.find(txId) : *o.find(txId);
Tx_t tx = inThisSet ? result_->txns.find(txId) : o.find(txId);
auto txID = tx.id();

if (result_->disputes.find(txID) != result_->disputes.end())
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/net/ShardDownloader.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ three database entries upon completion.
Since downloads execute serially by design, the entries in this table always
correspond to the contents of a single file.

| Bytes | Size | Part |
| Bytes | size | Part |
|:------:|:----------:|:----:|
| 0x... | 2147483647 | 0 |
| 0x... | 2147483647 | 1 |
Expand Down
3 changes: 1 addition & 2 deletions src/ripple/nodestore/DeterministicShard.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ uint64 Appnum Application defined constant
uint16 KeySize Key size in bytes
uint64 Salt A random seed
uint64 Pepper The salt hashed
uint16 BlockSize Size of a file block in bytes
uint16 BlockSize size of a file block in bytes
uint16 LoadFactor Target fraction in 65536ths
uint8[56] Reserved Zeroes
uint8[] Reserved Zero-pad to block size
Expand Down Expand Up @@ -160,4 +160,3 @@ Iteration 0: RIPEMD160[nudb.dat] = FAE6AE84C15968B0419FDFC014931EA12A396C71
Iteration 1: RIPEMD160[nudb.key] = F96BF2722AB2EE009FFAE4A36AAFC4F220E21951
Iteration 1: RIPEMD160[nudb.dat] = FAE6AE84C15968B0419FDFC014931EA12A396C71
```

2 changes: 1 addition & 1 deletion src/ripple/nodestore/ShardSizeTuning.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Shard Size Tuning
# Shard size Tuning

The purpose of this document is to compare the sizes of shards containing
varying amounts of ledgers.
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/nodestore/impl/DecodedBlob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ DecodedBlob::DecodedBlob(void const* key, void const* value, int valueBytes)

m_success = false;
m_key = key;
// VFALCO NOTE Ledger indexes should have started at 1
// VFALCO NOTE Ledger indexes should hav e started at 1
m_objectType = hotUNKNOWN;
m_objectData = nullptr;
m_dataBytes = std::max(0, valueBytes - 9);
Expand Down
Loading

0 comments on commit c3acbce

Please sign in to comment.