From 086ce21c0d8e7e867aac4fd2128599de535ecefe Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Wed, 30 Aug 2023 21:12:46 -0700 Subject: [PATCH] Improve `Json::Value` memory allocation for strings: The memory allocation patterns of Json::Value benefit greatly from the slabbed allocator. This commit adds a global slabbed allocator dedicated to `Json::Value`. Real-world data indicates that only 2% of allocation requests are over 72 bytes long. The remaining 98% of allocations fall into the following 3 buckets, calculated across 9,500,000,000 allocation calls: [ 1, 32]: 17% of all allocations [33, 48]: 27% of all allocations [49, 72]: 57% of all allocations This commit should result in improved performance for servers that have JSON-heavy workloads, typically those servicing RPC and WebSocket workloads, and less memory fragmentation. --- src/ripple/app/main/Main.cpp | 56 +++++++++++++++ src/ripple/app/paths/TrustLine.h | 3 +- src/ripple/basics/SlabAllocator.h | 1 + src/ripple/json/impl/json_value.cpp | 103 ++++++++++++---------------- src/ripple/json/json_value.h | 25 +++++-- 5 files changed, 123 insertions(+), 65 deletions(-) diff --git a/src/ripple/app/main/Main.cpp b/src/ripple/app/main/Main.cpp index 407931ab5..e371892e5 100644 --- a/src/ripple/app/main/Main.cpp +++ b/src/ripple/app/main/Main.cpp @@ -19,7 +19,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -27,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -344,6 +347,56 @@ runUnitTests( #endif // ENABLE_TESTS //------------------------------------------------------------------------------ +class SlabbedValueAllocator : public Json::ValueAllocator +{ + struct Buffer + { + alignas(8) char buffer_[32]; + }; + + SlabAllocatorSet slabber_; + +public: + SlabbedValueAllocator() + : slabber_({ + // clang-format off + { 0, 2 * 1024 * 1024 }, + {16, 6 * 1024 * 1024 }, + {40, 9 * 1024 * 1024 } + // clang-format on + }) + { + } + + virtual ~SlabbedValueAllocator() = default; + + char* + duplicateStringValue(const char* value, std::size_t length = 0) override + { + if (length == 0 && value != nullptr) + length = std::strlen(value); + + if (auto ret = reinterpret_cast(slabber_.allocate(length + 1))) + { + std::copy_n(value, length, ret); + ret[length] = 0; + return ret; + } + + // Fall back to the default if we can't grab memory from the slab + // allocators. + return ValueAllocator::duplicateStringValue(value, length); + } + + void + releaseStringValue(char* value) override + { + if (!slabber_.deallocate(reinterpret_cast(value))) + ValueAllocator::releaseStringValue(value); + } +}; + +//------------------------------------------------------------------------------ int run(int argc, char** argv) { @@ -520,6 +573,9 @@ run(int argc, char** argv) return 0; } + // Do this early enough so that it's useful for unit tests too: + Json::setAllocator(new SlabbedValueAllocator()); + #ifndef ENABLE_TESTS if (vm.count("unittest") || vm.count("unittest-child")) { diff --git a/src/ripple/app/paths/TrustLine.h b/src/ripple/app/paths/TrustLine.h index 6d7fcd66f..0a90fb839 100644 --- a/src/ripple/app/paths/TrustLine.h +++ b/src/ripple/app/paths/TrustLine.h @@ -27,7 +27,8 @@ #include #include -#include +#include +#include namespace ripple { diff --git a/src/ripple/basics/SlabAllocator.h b/src/ripple/basics/SlabAllocator.h index 65a565ab6..7303307dd 100644 --- a/src/ripple/basics/SlabAllocator.h +++ b/src/ripple/basics/SlabAllocator.h @@ -20,6 +20,7 @@ #ifndef RIPPLE_BASICS_SLABALLOCATOR_H_INCLUDED #define RIPPLE_BASICS_SLABALLOCATOR_H_INCLUDED +#include #include #include #include diff --git a/src/ripple/json/impl/json_value.cpp b/src/ripple/json/impl/json_value.cpp index 328c9d27a..0b9f26b4d 100644 --- a/src/ripple/json/impl/json_value.cpp +++ b/src/ripple/json/impl/json_value.cpp @@ -23,6 +23,12 @@ #include #include +#include +#include + +#include +#include + namespace Json { const Value Value::null; @@ -30,64 +36,45 @@ const Int Value::minInt = Int(~(UInt(-1) / 2)); const Int Value::maxInt = Int(UInt(-1) / 2); const UInt Value::maxUInt = UInt(-1); -class DefaultValueAllocator : public ValueAllocator + +char* +ValueAllocator::duplicateStringValue(const char* value, std::size_t length) { -public: - virtual ~DefaultValueAllocator() = default; + //@todo investigate this old optimization + // if ( !value || value[0] == 0 ) + // return 0; - char* - makeMemberName(const char* memberName) override - { - return duplicateStringValue(memberName); - } + if (length == 0 && value != nullptr) + length = strlen(value); - void - releaseMemberName(char* memberName) override - { - releaseStringValue(memberName); - } + auto ret = new char[length + 1]; - char* - duplicateStringValue(const char* value, unsigned int length = unknown) - override - { - //@todo investigate this old optimization - // if ( !value || value[0] == 0 ) - // return 0; + if (value != nullptr && length != 0) + memcpy(ret, value, length); - if (length == unknown) - length = value ? (unsigned int)strlen(value) : 0; - - char* newString = static_cast(malloc(length + 1)); - if (value) - memcpy(newString, value, length); - newString[length] = 0; - return newString; - } - - void - releaseStringValue(char* value) override - { - if (value) - free(value); - } -}; - -static ValueAllocator*& -valueAllocator() -{ - static ValueAllocator* valueAllocator = new DefaultValueAllocator; - return valueAllocator; + ret[length] = 0; + return ret; } -static struct DummyValueAllocatorInitializer +void +ValueAllocator::releaseStringValue(char* value) { - DummyValueAllocatorInitializer() - { - valueAllocator(); // ensure valueAllocator() statics are initialized - // before main(). - } -} dummyValueAllocatorInitializer; + delete[] value; +} + +/** The default allocator to use, if no custom allocator is specified. */ +static ValueAllocator defaultValueAllocator; + +/** A pointer to the allocator to use. */ +static constinit ValueAllocator* valueAllocator = &defaultValueAllocator; + +void setAllocator(ValueAllocator *allocator) +{ + assert(allocator != nullptr && valueAllocator == &defaultValueAllocator); + + if (valueAllocator == &defaultValueAllocator) + valueAllocator = allocator; +} // ////////////////////////////////////////////////////////////////// // ////////////////////////////////////////////////////////////////// @@ -106,7 +93,7 @@ Value::CZString::CZString(int index) : cstr_(0), index_(index) Value::CZString::CZString(const char* cstr, DuplicationPolicy allocate) : cstr_( - allocate == duplicate ? valueAllocator()->makeMemberName(cstr) : cstr) + allocate == duplicate ? valueAllocator->makeMemberName(cstr) : cstr) , index_(allocate) { } @@ -114,7 +101,7 @@ Value::CZString::CZString(const char* cstr, DuplicationPolicy allocate) Value::CZString::CZString(const CZString& other) : cstr_( other.index_ != noDuplication && other.cstr_ != 0 - ? valueAllocator()->makeMemberName(other.cstr_) + ? valueAllocator->makeMemberName(other.cstr_) : other.cstr_) , index_( other.cstr_ @@ -126,7 +113,7 @@ Value::CZString::CZString(const CZString& other) Value::CZString::~CZString() { if (cstr_ && index_ == duplicate) - valueAllocator()->releaseMemberName(const_cast(cstr_)); + valueAllocator->releaseMemberName(const_cast(cstr_)); } bool @@ -228,7 +215,7 @@ Value::Value(double value) : type_(realValue) Value::Value(const char* value) : type_(stringValue), allocated_(true) { - value_.string_ = valueAllocator()->duplicateStringValue(value); + value_.string_ = valueAllocator->duplicateStringValue(value); } Value::Value(std::string_view value) : type_(stringValue), allocated_(true) @@ -239,8 +226,8 @@ Value::Value(std::string_view value) : type_(stringValue), allocated_(true) Value::Value(std::string const& value) : type_(stringValue), allocated_(true) { - value_.string_ = valueAllocator()->duplicateStringValue( - value.c_str(), (unsigned int)value.length()); + value_.string_ = valueAllocator->duplicateStringValue( + value.c_str(), value.length()); } Value::Value(const StaticString& value) : type_(stringValue), allocated_(false) @@ -268,7 +255,7 @@ Value::Value(const Value& other) : type_(other.type_) case stringValue: if (other.value_.string_) { - value_.string_ = valueAllocator()->duplicateStringValue( + value_.string_ = valueAllocator->duplicateStringValue( other.value_.string_); allocated_ = true; } @@ -300,7 +287,7 @@ Value::~Value() case stringValue: if (allocated_) - valueAllocator()->releaseStringValue(value_.string_); + valueAllocator->releaseStringValue(value_.string_); break; diff --git a/src/ripple/json/json_value.h b/src/ripple/json/json_value.h index 1c0adbaed..3e6999e4a 100644 --- a/src/ripple/json/json_value.h +++ b/src/ripple/json/json_value.h @@ -480,20 +480,33 @@ operator>=(const Value& x, const Value& y) class ValueAllocator { public: - enum { unknown = (unsigned)-1 }; - virtual ~ValueAllocator() = default; virtual char* - makeMemberName(const char* memberName) = 0; + makeMemberName(const char* memberName) + { + return duplicateStringValue(memberName); + } + virtual void - releaseMemberName(char* memberName) = 0; + releaseMemberName(char* memberName) + { + releaseStringValue(memberName); + } + virtual char* - duplicateStringValue(const char* value, unsigned int length = unknown) = 0; + duplicateStringValue(const char* value, std::size_t length = 0); + virtual void - releaseStringValue(char* value) = 0; + releaseStringValue(char* value); }; +/** Assigns an allocator to use for string-related JSON memory requests. + + @note This can only be called once, and should be called early. + */ +void setAllocator(ValueAllocator *allocator); + /** \brief base class for Value iterators. * */