Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate memory allocation from critical path: #4353

Merged
merged 1 commit into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Builds/CMake/RippledCore.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,6 @@ target_sources (rippled PRIVATE
src/ripple/nodestore/impl/DeterministicShard.cpp
src/ripple/nodestore/impl/DecodedBlob.cpp
src/ripple/nodestore/impl/DummyScheduler.cpp
src/ripple/nodestore/impl/EncodedBlob.cpp
src/ripple/nodestore/impl/ManagerImp.cpp
src/ripple/nodestore/impl/NodeObject.cpp
src/ripple/nodestore/impl/Shard.cpp
Expand Down
8 changes: 4 additions & 4 deletions src/ripple/nodestore/backend/CassandraFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ class CassandraBackend : public Backend
// confirmed persisted. Otherwise, it can become deleted
// prematurely if other copies are removed from caches.
std::shared_ptr<NodeObject> no;
NodeStore::EncodedBlob e;
std::optional<NodeStore::EncodedBlob> e;
nbougalis marked this conversation as resolved.
Show resolved Hide resolved
std::pair<void const*, std::size_t> compressed;
std::chrono::steady_clock::time_point begin;
// The data is stored in this buffer. The void* in the above member
Expand All @@ -686,10 +686,10 @@ class CassandraBackend : public Backend
std::atomic<std::uint64_t>& retries)
: backend(f), no(nobj), totalWriteRetries(retries)
{
e.prepare(no);
e.emplace(no);

compressed =
NodeStore::nodeobject_compress(e.getData(), e.getSize(), bf);
NodeStore::nodeobject_compress(e->getData(), e->getSize(), bf);
}
};

Expand Down Expand Up @@ -722,7 +722,7 @@ class CassandraBackend : public Backend
CassError rc = cass_statement_bind_bytes(
statement,
0,
static_cast<cass_byte_t const*>(data.e.getKey()),
static_cast<cass_byte_t const*>(data.e->getKey()),
keyBytes_);
if (rc != CASS_OK)
{
Expand Down
3 changes: 1 addition & 2 deletions src/ripple/nodestore/backend/NuDBFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,7 @@ class NuDBBackend : public Backend
void
do_insert(std::shared_ptr<NodeObject> const& no)
{
EncodedBlob e;
e.prepare(no);
EncodedBlob e(no);
nudb::error_code ec;
nudb::detail::buffer bf;
auto const result = nodeobject_compress(e.getData(), e.getSize(), bf);
Expand Down
4 changes: 1 addition & 3 deletions src/ripple/nodestore/backend/RocksDBFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,11 +352,9 @@ class RocksDBBackend : public Backend, public BatchWriter::Callback
assert(m_db);
rocksdb::WriteBatch wb;

EncodedBlob encoded;

for (auto const& e : batch)
{
encoded.prepare(e);
EncodedBlob encoded(e);

wb.Put(
rocksdb::Slice(
Expand Down
42 changes: 0 additions & 42 deletions src/ripple/nodestore/impl/EncodedBlob.cpp

This file was deleted.

103 changes: 85 additions & 18 deletions src/ripple/nodestore/impl/EncodedBlob.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,42 +22,109 @@

#include <ripple/basics/Buffer.h>
#include <ripple/nodestore/NodeObject.h>
#include <cstddef>
#include <boost/align/align_up.hpp>
#include <algorithm>
#include <array>
#include <cassert>
#include <cstdint>

namespace ripple {
namespace NodeStore {

/** Utility for producing flattened node objects.
@note This defines the database format of a NodeObject!
*/
// VFALCO TODO Make allocator aware and use short_alloc
struct EncodedBlob
/** Convert a NodeObject from in-memory to database format.

The (suboptimal) database format consists of:

- 8 prefix bytes which will typically be 0, but don't assume that's the
case; earlier versions of the code would use these bytes to store the
ledger index either once or twice.
- A single byte denoting the type of the object.
- The payload.

@note This class is typically instantiated on the stack, so the size of
the object does not matter as much as it normally would since the
allocation is, effectively, free.

We leverage that fact to preallocate enough memory to handle most
payloads as part of this object, eliminating the need for dynamic
allocation. As of this writing ~94% of objects require fewer than
1024 payload bytes.
*/

class EncodedBlob
{
/** The 32-byte key of the serialized object. */
std::array<std::uint8_t, 32> key_;

/** A pre-allocated buffer for the serialized object.

The buffer is large enough for the 9 byte prefix and at least
1024 more bytes. The precise size is calculated automatically
at compile time so as to avoid wasting space on padding bytes.
*/
std::array<
std::uint8_t,
boost::alignment::align_up(9 + 1024, alignof(std::uint32_t))>
payload_;

/** The size of the serialized data. */
std::uint32_t size_;

/** A pointer to the serialized data.

This may point to the pre-allocated buffer (if it is sufficiently
large) or to a dynamically allocated buffer.
*/
std::uint8_t* const ptr_;

public:
void
prepare(std::shared_ptr<NodeObject> const& object);
explicit EncodedBlob(std::shared_ptr<NodeObject> const& obj)
: size_([&obj]() {
assert(obj);

if (!obj)
throw std::runtime_error(
"EncodedBlob: unseated std::shared_ptr used.");

return obj->getData().size() + 9;
}())
, ptr_(
(size_ <= payload_.size()) ? payload_.data()
: new std::uint8_t[size_])
{
std::fill_n(ptr_, 8, std::uint8_t{0});
ptr_[8] = static_cast<std::uint8_t>(obj->getType());
std::copy_n(obj->getData().data(), obj->getData().size(), ptr_ + 9);
std::copy_n(obj->getHash().data(), obj->getHash().size(), key_.data());
}

void const*
~EncodedBlob()
{
assert(
((ptr_ == payload_.data()) && (size_ <= payload_.size())) ||
((ptr_ != payload_.data()) && (size_ > payload_.size())));

if (ptr_ != payload_.data())
delete[] ptr_;
}

[[nodiscard]] void const*
getKey() const noexcept
{
return m_key;
return static_cast<void const*>(key_.data());
}

std::size_t
[[nodiscard]] std::size_t
getSize() const noexcept
{
return m_data.size();
return size_;
}

void const*
[[nodiscard]] void const*
getData() const noexcept
{
return reinterpret_cast<void const*>(m_data.data());
return static_cast<void const*>(ptr_);
}

private:
void const* m_key;
Buffer m_data;
};

} // namespace NodeStore
Expand Down
3 changes: 1 addition & 2 deletions src/test/nodestore/Basics_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,9 @@ class NodeStoreBasic_test : public TestBase

auto batch = createPredictableBatch(numObjectsToTest, seedValue);

EncodedBlob encoded;
for (int i = 0; i < batch.size(); ++i)
{
encoded.prepare(batch[i]);
EncodedBlob encoded(batch[i]);

DecodedBlob decoded(
encoded.getKey(), encoded.getData(), encoded.getSize());
Expand Down