From 1155ed2048a24f4dfcfa65d63243b77973f820d6 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Thu, 2 May 2024 17:40:30 +0200 Subject: fix zero size attachment replies (#69) - Bugfix: Don't try to respond with zero size partial cache value when partial size is zero - Improvement: Added more validation of data read from cache / cas --- src/zencore/compactbinarypackage.cpp | 85 +++++++++++++++--------------------- 1 file changed, 34 insertions(+), 51 deletions(-) (limited to 'src/zencore/compactbinarypackage.cpp') diff --git a/src/zencore/compactbinarypackage.cpp b/src/zencore/compactbinarypackage.cpp index a4fa38a1d..4d4e0951e 100644 --- a/src/zencore/compactbinarypackage.cpp +++ b/src/zencore/compactbinarypackage.cpp @@ -11,32 +11,45 @@ namespace zen { /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -CbAttachment::CbAttachment(const CompressedBuffer& InValue, const IoHash& Hash) : CbAttachment(InValue.MakeOwned(), Hash) +CbAttachment::CbAttachment(const CbObject& InValue, const IoHash* const InHash) { -} + auto SetValue = [&](const CbObject& ValueToSet) { + if (InHash) + { + Value.emplace(ValueToSet); + Hash = *InHash; + } + else + { + Value.emplace(ValueToSet); + Hash = ValueToSet.GetHash(); + } + }; -CbAttachment::CbAttachment(const SharedBuffer& InValue) : CbAttachment(CompositeBuffer(InValue)) -{ + MemoryView View; + if (!InValue.IsOwned() || !InValue.TryGetSerializedView(View)) + { + SetValue(CbObject::Clone(InValue)); + } + else + { + SetValue(InValue); + } } -CbAttachment::CbAttachment(const SharedBuffer& InValue, const IoHash& InHash) : CbAttachment(CompositeBuffer(InValue), InHash) +CbAttachment::CbAttachment(const CompressedBuffer& InValue, const IoHash& Hash) : Hash(Hash), Value(InValue) { + ZEN_ASSERT(!std::get(Value).IsNull()); } -CbAttachment::CbAttachment(const CompositeBuffer& InValue) -: Hash(InValue.IsNull() ? IoHash::Zero : IoHash::HashBuffer(InValue)) -, Value(InValue) +CbAttachment::CbAttachment(CompressedBuffer&& InValue, const IoHash& InHash) : Hash(InHash), Value(std::move(InValue)) { - if (std::get(Value).IsNull()) - { - Value.emplace(); - } + ZEN_ASSERT(!std::get(Value).IsNull()); } CbAttachment::CbAttachment(CompositeBuffer&& InValue) : Hash(InValue.IsNull() ? IoHash::Zero : IoHash::HashBuffer(InValue)) , Value(std::move(InValue)) - { if (std::get(Value).IsNull()) { @@ -44,7 +57,7 @@ CbAttachment::CbAttachment(CompositeBuffer&& InValue) } } -CbAttachment::CbAttachment(CompositeBuffer&& InValue, const IoHash& InHash) : Hash(InHash), Value(InValue) +CbAttachment::CbAttachment(CompositeBuffer&& InValue, const IoHash& InHash) : Hash(InHash), Value(std::move(InValue)) { if (std::get(Value).IsNull()) { @@ -52,40 +65,6 @@ CbAttachment::CbAttachment(CompositeBuffer&& InValue, const IoHash& InHash) : Ha } } -CbAttachment::CbAttachment(CompressedBuffer&& InValue, const IoHash& InHash) : Hash(InHash), Value(InValue) -{ - if (std::get(Value).IsNull()) - { - Value.emplace(); - } -} - -CbAttachment::CbAttachment(const CbObject& InValue, const IoHash* const InHash) -{ - auto SetValue = [&](const CbObject& ValueToSet) { - if (InHash) - { - Value.emplace(ValueToSet); - Hash = *InHash; - } - else - { - Value.emplace(ValueToSet); - Hash = ValueToSet.GetHash(); - } - }; - - MemoryView View; - if (!InValue.IsOwned() || !InValue.TryGetSerializedView(View)) - { - SetValue(CbObject::Clone(InValue)); - } - else - { - SetValue(InValue); - } -} - bool CbAttachment::TryLoad(IoBuffer& InBuffer, BufferAllocator Allocator) { @@ -186,7 +165,7 @@ TryLoad_ArchiveFieldIntoAttachment(CbAttachment& TargetAttachment, CbField&& Fie { return false; } - TargetAttachment = CbAttachment(CompositeBuffer(Buffer), BinaryAttachmentHash); + TargetAttachment = CbAttachment(std::move(Buffer), BinaryAttachmentHash); } else if (SharedBuffer Buffer = Field.AsBinary(); !Field.HasError()) { @@ -201,7 +180,7 @@ TryLoad_ArchiveFieldIntoAttachment(CbAttachment& TargetAttachment, CbField&& Fie else { // Is an uncompressed empty binary blob - TargetAttachment = CbAttachment(CompositeBuffer(Buffer), IoHash::HashBuffer(nullptr, 0)); + TargetAttachment = CbAttachment(std::move(Buffer), IoHash::HashBuffer(nullptr, 0)); } } else @@ -386,6 +365,10 @@ CbPackage::AddAttachments(std::span InAttachments) { return; } + for (const CbAttachment& Attachment : InAttachments) + { + ZEN_ASSERT(!Attachment.IsNull()); + } // Assume we have no duplicates! Attachments.insert(Attachments.end(), InAttachments.begin(), InAttachments.end()); std::sort(Attachments.begin(), Attachments.end()); @@ -741,7 +724,7 @@ namespace legacy { } else { - Package.AddAttachment(CbAttachment(CompositeBuffer(std::move(Buffer)), Hash)); + Package.AddAttachment(CbAttachment(std::move(Buffer), Hash)); } } } -- cgit v1.2.3 From 8ce1dc72cce381b2adae256504331f2e8893f262 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Thu, 30 May 2024 14:44:34 +0200 Subject: cache optimizations (#88) * message formatting optimizations * bump iostorecompression small value threshold to 1MB --- src/zencore/compactbinarypackage.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/zencore/compactbinarypackage.cpp') diff --git a/src/zencore/compactbinarypackage.cpp b/src/zencore/compactbinarypackage.cpp index 4d4e0951e..0117b326e 100644 --- a/src/zencore/compactbinarypackage.cpp +++ b/src/zencore/compactbinarypackage.cpp @@ -261,7 +261,7 @@ CbAttachment::GetHash() const return Hash; } -CompositeBuffer +const CompositeBuffer& CbAttachment::AsCompositeBinary() const { if (const CompositeBuffer* BinValue = std::get_if(&Value)) @@ -283,7 +283,7 @@ CbAttachment::AsBinary() const return {}; } -CompressedBuffer +const CompressedBuffer& CbAttachment::AsCompressedBinary() const { if (const CompressedBuffer* CompValue = std::get_if(&Value)) -- cgit v1.2.3 From 9b2fb8069869ec5cea32816a3589d9842595ffea Mon Sep 17 00:00:00 2001 From: Stefan Boberg Date: Mon, 2 Dec 2024 14:26:11 +0100 Subject: reduce memory churn (#248) * eliminated allocation in SetCurrentThreadName * reduced memory allocator activity in cache RPC response building * reduced allocations in compact binary building --- src/zencore/compactbinarypackage.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/zencore/compactbinarypackage.cpp') diff --git a/src/zencore/compactbinarypackage.cpp b/src/zencore/compactbinarypackage.cpp index 0117b326e..7de161845 100644 --- a/src/zencore/compactbinarypackage.cpp +++ b/src/zencore/compactbinarypackage.cpp @@ -308,6 +308,11 @@ CbAttachment::AsObject() const /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +CbPackage::CbPackage() +{ + Attachments.reserve(16); +} + void CbPackage::SetObject(CbObject InObject, const IoHash* InObjectHash, AttachmentResolver* InResolver) { -- cgit v1.2.3 From 920120bbcec9f91df3336f62970b3e010a4fa6c2 Mon Sep 17 00:00:00 2001 From: Stefan Boberg Date: Thu, 6 Mar 2025 16:18:32 +0100 Subject: reduced memory churn using fixed_xxx containers (#236) * Added EASTL to help with eliminating memory allocations * Applied EASTL to eliminate memory allocations, primarily by using `fixed_vector` et al to use stack allocations / inline struct allocations Reduces memory events in traces by close to a factor of 10 in test scenario (starting editor for project F) --- src/zencore/compactbinarypackage.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'src/zencore/compactbinarypackage.cpp') diff --git a/src/zencore/compactbinarypackage.cpp b/src/zencore/compactbinarypackage.cpp index 7de161845..ffe64f2e9 100644 --- a/src/zencore/compactbinarypackage.cpp +++ b/src/zencore/compactbinarypackage.cpp @@ -3,10 +3,13 @@ #include "zencore/compactbinarypackage.h" #include #include +#include #include #include #include +#include + namespace zen { /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -340,6 +343,12 @@ CbPackage::SetObject(CbObject InObject, const IoHash* InObjectHash, AttachmentRe } } +void +CbPackage::ReserveAttachments(size_t Count) +{ + Attachments.reserve(Count); +} + void CbPackage::AddAttachment(const CbAttachment& Attachment, AttachmentResolver* Resolver) { @@ -374,17 +383,18 @@ CbPackage::AddAttachments(std::span InAttachments) { ZEN_ASSERT(!Attachment.IsNull()); } + // Assume we have no duplicates! Attachments.insert(Attachments.end(), InAttachments.begin(), InAttachments.end()); std::sort(Attachments.begin(), Attachments.end()); - ZEN_ASSERT_SLOW(std::unique(Attachments.begin(), Attachments.end()) == Attachments.end()); + ZEN_ASSERT_SLOW(eastl::unique(Attachments.begin(), Attachments.end()) == Attachments.end()); } int32_t CbPackage::RemoveAttachment(const IoHash& Hash) { return gsl::narrow_cast( - std::erase_if(Attachments, [&Hash](const CbAttachment& Attachment) -> bool { return Attachment.GetHash() == Hash; })); + erase_if(Attachments, [&Hash](const CbAttachment& Attachment) -> bool { return Attachment.GetHash() == Hash; })); } bool -- cgit v1.2.3