From f8584d0df86bab50132fa28f0b37d33137b756b4 Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Mon, 1 Jul 2019 16:32:25 -0700 Subject: [PATCH] Implement new dynamic_size_traits --- fdbrpc/ReplicationPolicy.h | 30 +++++++++++---------- flow/Arena.h | 5 ++-- flow/ObjectSerializerTraits.h | 18 ++++--------- flow/flat_buffers.h | 51 +++++++++++++++-------------------- 4 files changed, 45 insertions(+), 59 deletions(-) diff --git a/fdbrpc/ReplicationPolicy.h b/fdbrpc/ReplicationPolicy.h index 27752271d1..cb7b3697b5 100644 --- a/fdbrpc/ReplicationPolicy.h +++ b/fdbrpc/ReplicationPolicy.h @@ -283,28 +283,32 @@ void serializeReplicationPolicy(Ar& ar, Reference& policy) { template <> struct dynamic_size_traits> : std::true_type { - static Block save(const Reference& value) { +private: + using T = Reference; + static BinaryWriter writer{ IncludeVersion() }; + +public: + static size_t size(const T& value) { if (value.getPtr() == nullptr) { - static BinaryWriter writer{ IncludeVersion() }; - writer = BinaryWriter{ IncludeVersion() }; - serializeReplicationPolicy(writer, const_cast&>(value)); - return unownedPtr(const_cast(reinterpret_cast(writer.getData())), - writer.getLength()); + return writer.getLength(); } if (!value->alreadyWritten) { + value->writer = BinaryWriter{ IncludeVersion() }; serializeReplicationPolicy(value->writer, const_cast&>(value)); value->alreadyWritten = true; } - return unownedPtr(const_cast(reinterpret_cast(value->writer.getData())), - value->writer.getLength()); + return value->writer.getLength(); } - static void serialization_done(const Reference& value) { + // Guaranteed to be called only once during serialization + static void save(uint8_t* out, const T& value) { if (value.getPtr() == nullptr) { - return; + memcpy(out, writer.getData(), writer.getLength()); + } else { + ASSERT(value->alreadyWritten) + memcpy(out, value->writer.getData(), value->writer.getLength()); + value->alreadyWritten = false; } - value->alreadyWritten = false; - value->writer = BinaryWriter{ IncludeVersion() }; } // Context is an arbitrary type that is plumbed by reference throughout the @@ -317,6 +321,4 @@ struct dynamic_size_traits> : std::true_type { } }; -static_assert(detail::has_serialization_done>>::value); - #endif diff --git a/flow/Arena.h b/flow/Arena.h index 90ff501d25..936a65a04a 100644 --- a/flow/Arena.h +++ b/flow/Arena.h @@ -765,9 +765,10 @@ inline void save( Archive& ar, const StringRef& value ) { ar.serializeBytes( value.begin(), value.size() ); } -template<> +template <> struct dynamic_size_traits : std::true_type { - static Block save(const StringRef& str) { return unownedPtr(str.begin(), str.size()); } + static size_t size(const StringRef& t) { return t.size(); } + static void save(uint8_t* out, const StringRef& t) { std::copy(t.begin(), t.end(), out); } template static void load(const uint8_t* ptr, size_t sz, StringRef& str, Context& context) { diff --git a/flow/ObjectSerializerTraits.h b/flow/ObjectSerializerTraits.h index 37b8b2ece3..ef32bfb3c2 100644 --- a/flow/ObjectSerializerTraits.h +++ b/flow/ObjectSerializerTraits.h @@ -62,16 +62,6 @@ struct index_impl<0, pack> { template using index_t = typename index_impl::type; -struct Block { - const uint8_t* data; - size_t size; -}; - -template -Block unownedPtr(T* t, size_t s) { - return Block{ t, s }; -} - template struct scalar_traits : std::false_type { constexpr static size_t size = 0; @@ -83,11 +73,13 @@ struct scalar_traits : std::false_type { static void load(const uint8_t*, T&, Context&); }; - template struct dynamic_size_traits : std::false_type { - static Block save(const T&); - static void serialization_done(const T&); // Optional. Called after the last call to save. + // May be called multiple times during one serialization. Guaranteed not to be called after save. + static size_t size(const T&); + + // Guaranteed to be called only once during serialization + static void save(uint8_t*, const T&); // Context is an arbitrary type that is plumbed by reference throughout the // load call tree. diff --git a/flow/flat_buffers.h b/flow/flat_buffers.h index 420fa9f83a..f14545ab93 100644 --- a/flow/flat_buffers.h +++ b/flow/flat_buffers.h @@ -174,7 +174,8 @@ private: using T = std::string; public: - static Block save(const T& t) { return unownedPtr(reinterpret_cast(t.data()), t.size()); }; + static size_t size(const T& t) { return t.size(); } + static void save(uint8_t* out, const T& t) { std::copy(t.begin(), t.end(), out); } // Context is an arbitrary type that is plumbed by reference throughout the // load call tree. @@ -227,20 +228,6 @@ constexpr bool use_indirection = !(is_scalar || is_struct_like); using VTable = std::vector; -template -struct sfinae_true : std::true_type {}; - -template -auto test_serialization_done(int) -> sfinae_true; - -template -auto test_serialization_done(long) -> std::false_type; - -// int is a better match for 0 than long. If substituting T::serialization_done succeeds the true_type overload is -// selected. -template -struct has_serialization_done : decltype(test_serialization_done(0)) {}; - template constexpr int fb_scalar_size = is_scalar ? scalar_traits::size : sizeof(RelativeOffset); @@ -322,10 +309,17 @@ struct _SizeOf { struct PrecomputeSize { // |offset| is measured from the end of the buffer. Precondition: len <= // offset. - void write(const void*, int offset, int len) { current_buffer_size = std::max(current_buffer_size, offset); } + void write(const void*, int offset, int /*len*/) { current_buffer_size = std::max(current_buffer_size, offset); } + + template + std::enable_if_t> visitDynamicSize(const T& t) { + uint32_t size = dynamic_size_traits::size(t); + int start = RightAlign(current_buffer_size + size + 4, 4); + current_buffer_size = std::max(current_buffer_size, start); + } struct Noop { - void write(const void* src, int offset, int len) {} + void write(const void* src, int offset, int /*len*/) {} void writeTo(PrecomputeSize& writer, int offset) { writer.write(nullptr, offset, size); @@ -342,8 +336,6 @@ struct PrecomputeSize { return Noop{ size, writeToIndex }; } - static constexpr bool finalPass = false; - int current_buffer_size = 0; const int buffer_length = -1; // Dummy, the value of this should not affect anything. @@ -400,12 +392,19 @@ struct WriteToBuffer { return m; } + template + std::enable_if_t> visitDynamicSize(const T& t) { + uint32_t size = dynamic_size_traits::size(t); + int start = RightAlign(current_buffer_size + size + 4, 4); + write(&size, start, 4); + start -= 4; + dynamic_size_traits::save(&buffer[buffer_length - start], t); + } + const int buffer_length; const int vtable_start; int current_buffer_size = 0; - static constexpr bool finalPass = true; - private: void copy_memory(const void* src, int offset, int len) { memcpy(static_cast(&buffer[buffer_length - offset]), src, len); @@ -908,15 +907,7 @@ struct LoadSaveHelper { template >> RelativeOffset save(const U& message, Writer& writer, const VTableSet*, std::enable_if_t, int> _ = 0) { - auto block = dynamic_size_traits::save(message); - uint32_t size = block.size; - int start = RightAlign(writer.current_buffer_size + size + 4, 4); - writer.write(&size, start, 4); - start -= 4; - writer.write(block.data, start, block.size); - if constexpr (has_serialization_done>::value && Writer::finalPass) { - dynamic_size_traits::serialization_done(message); - } + writer.visitDynamicSize(message); return RelativeOffset{ writer.current_buffer_size }; }