diff --git a/fdbrpc/ReplicationPolicy.h b/fdbrpc/ReplicationPolicy.h index 0f4f350cae..c20a3de821 100644 --- a/fdbrpc/ReplicationPolicy.h +++ b/fdbrpc/ReplicationPolicy.h @@ -70,6 +70,13 @@ struct IReplicationPolicy : public ReferenceCounted { return keys; } virtual void attributeKeys(std::set*) const = 0; + + // For flatbuffers, IReplicationPolicy is just encoded as a string using + // |serializeReplicationPolicy|. |writer| is a member of IReplicationPolicy + // so that this string outlives all calls to + // dynamic_size_traits>::save + mutable BinaryWriter writer{ IncludeVersion() }; + mutable bool alreadyWritten = false; }; template @@ -276,12 +283,28 @@ void serializeReplicationPolicy(Ar& ar, Reference& policy) { template <> struct dynamic_size_traits> : std::true_type { - static WriteRawMemory save(const Reference& value) { - BinaryWriter writer(IncludeVersion()); - serializeReplicationPolicy(writer, const_cast&>(value)); - std::unique_ptr memory(new uint8_t[writer.getLength()]); - memcpy(memory.get(), writer.getData(), writer.getLength()); - return std::make_pair, size_t>(ownedPtr(const_cast(memory.release())), writer.getLength()); + static Block save(const Reference& 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()); + } + if (!value->alreadyWritten) { + value->alreadyWritten = true; + serializeReplicationPolicy(value->writer, const_cast&>(value)); + } + return unownedPtr(const_cast(reinterpret_cast(value->writer.getData())), + value->writer.getLength()); + } + + static void serialization_done(const Reference& value) { + if (value.getPtr() == nullptr) { + return; + } + value->alreadyWritten = false; + value->writer = BinaryWriter{ IncludeVersion() }; } // Context is an arbitrary type that is plumbed by reference throughout the @@ -294,5 +317,4 @@ struct dynamic_size_traits> : std::true_type { } }; - #endif diff --git a/flow/Arena.h b/flow/Arena.h index b5280bae99..90ff501d25 100644 --- a/flow/Arena.h +++ b/flow/Arena.h @@ -767,7 +767,7 @@ inline void save( Archive& ar, const StringRef& value ) { template<> struct dynamic_size_traits : std::true_type { - static WriteRawMemory save(const StringRef& str) { return { { unownedPtr(str.begin()), str.size() } }; } + static Block save(const StringRef& str) { return unownedPtr(str.begin(), str.size()); } 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 3301214e76..37b8b2ece3 100644 --- a/flow/ObjectSerializerTraits.h +++ b/flow/ObjectSerializerTraits.h @@ -62,42 +62,15 @@ struct index_impl<0, pack> { template using index_t = typename index_impl::type; -// A smart pointer that knows whether or not to delete itself. -template -using OwnershipErasedPtr = std::unique_ptr>; - -// Creates an OwnershipErasedPtr that will delete itself. -template > -OwnershipErasedPtr ownedPtr(T* t, Deleter&& d = Deleter{}) { - return OwnershipErasedPtr{ t, std::forward(d) }; -} - -// Creates an OwnershipErasedPtr that will not delete itself. -template -OwnershipErasedPtr unownedPtr(T* t) { - return OwnershipErasedPtr{ t, [](T*) {} }; -} - -struct WriteRawMemory { - using Block = std::pair, size_t>; - std::vector blocks; - - WriteRawMemory() {} - WriteRawMemory(Block&& b) { blocks.emplace_back(std::move(b.first), b.second); } - WriteRawMemory(std::vector&& v) : blocks(std::move(v)) {} - - WriteRawMemory(WriteRawMemory&&) = default; - WriteRawMemory& operator=(WriteRawMemory&&) = default; - - size_t size() const { - size_t result = 0; - for (const auto& b : blocks) { - result += b.second; - } - return result; - } +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 { @@ -113,7 +86,8 @@ struct scalar_traits : std::false_type { template struct dynamic_size_traits : std::false_type { - static WriteRawMemory save(const T&); + static Block save(const T&); + static void serialization_done(const T&); // Optional. Called after the last call to save. // Context is an arbitrary type that is plumbed by reference throughout the // load call tree. @@ -140,7 +114,6 @@ struct vector_like_traits : std::false_type { static insert_iterator insert(VectorLike&); static iterator begin(const VectorLike&); - static void deserialization_done(VectorLike&); // Optional }; template diff --git a/flow/flat_buffers.cpp b/flow/flat_buffers.cpp index ce8261f54f..6c6c442e52 100644 --- a/flow/flat_buffers.cpp +++ b/flow/flat_buffers.cpp @@ -329,51 +329,10 @@ TEST_CASE("flow/FlatBuffers/vectorBool") { return Void(); } -struct DynamicSizeThingy { - std::string x; - mutable int saves = 0; -}; - } // namespace unit_tests -template <> -struct dynamic_size_traits : std::true_type { -private: - using T = unit_tests::DynamicSizeThingy; - -public: - static WriteRawMemory save(const T& t) { - ++t.saves; - T* t2 = new T(t); - return { { ownedPtr(reinterpret_cast(t2->x.data()), [t2](auto*) { delete t2; }), - t2->x.size() } }; - } - - // Context is an arbitrary type that is plumbed by reference throughout the - // load call tree. - template - static void load(const uint8_t* p, size_t n, T& t, Context&) { - t.x.assign(reinterpret_cast(p), n); - } -}; - namespace unit_tests { -TEST_CASE("flow/FlatBuffers/dynamic_size_owned") { - DynamicSizeThingy x1 = { "abcdefg" }; - DynamicSizeThingy x2; - Arena arena; - DummyContext context; - const uint8_t* out; - - out = save_members(arena, FileIdentifier{}, x1); - ASSERT(x1.saves == 1); - // print_buffer(out, arena.get_size(out)); - load_members(out, context, x2); - ASSERT(x1.x == x2.x); - return Void(); -} - struct Y1 { int a; diff --git a/flow/flat_buffers.h b/flow/flat_buffers.h index 4eb454c4d9..ac9a8124c2 100644 --- a/flow/flat_buffers.h +++ b/flow/flat_buffers.h @@ -174,9 +174,7 @@ private: using T = std::string; public: - static WriteRawMemory save(const T& t) { - return { { unownedPtr(reinterpret_cast(t.data())), t.size() } }; - }; + static Block save(const T& t) { return unownedPtr(reinterpret_cast(t.data()), t.size()); }; // Context is an arbitrary type that is plumbed by reference throughout the // load call tree. @@ -233,13 +231,13 @@ template struct sfinae_true : std::true_type {}; template -auto test_deserialization_done(int) -> sfinae_true; +auto test_serialization_done(int) -> sfinae_true; template -auto test_deserialization_done(long) -> std::false_type; +auto test_serialization_done(long) -> std::false_type; template -struct has_deserialization_done : decltype(test_deserialization_done(0)) {}; +struct has_serialization_done : decltype(test_serialization_done(0)) {}; template constexpr int fb_scalar_size = is_scalar ? scalar_traits::size : sizeof(RelativeOffset); @@ -324,19 +322,6 @@ struct PrecomputeSize { // offset. void write(const void*, int offset, int len) { current_buffer_size = std::max(current_buffer_size, offset); } - template - void writeRawMemory(ToRawMemory&& to_raw_memory) { - auto w = std::forward(to_raw_memory)(); - int start = RightAlign(current_buffer_size + w.size() + 4, 4); - write(nullptr, start, 4); - start -= 4; - for (auto& block : w.blocks) { - write(nullptr, start, block.second); - start -= block.second; - } - writeRawMemories.emplace_back(std::move(w)); - } - struct Noop { void write(const void* src, int offset, int len) {} void writeTo(PrecomputeSize& writer, int offset) { @@ -355,12 +340,13 @@ 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. const int vtable_start = -1; // Dummy, the value of this should not affect anything. std::vector writeToOffsets; - std::vector writeRawMemories; }; template @@ -382,26 +368,9 @@ struct WriteToBuffer { current_buffer_size = std::max(current_buffer_size, offset); } - template - void writeRawMemory(ToRawMemory&&) { - auto& w = *write_raw_memories_iter; - uint32_t size = w.size(); - int start = RightAlign(current_buffer_size + size + 4, 4); - write(&size, start, 4); - start -= 4; - for (auto& p : w.blocks) { - if (p.second > 0) { - write(reinterpret_cast(p.first.get()), start, p.second); - } - start -= p.second; - } - ++write_raw_memories_iter; - } - - WriteToBuffer(int buffer_length, int vtable_start, uint8_t* buffer, std::vector writeToOffsets, - std::vector::iterator write_raw_memories_iter) + WriteToBuffer(int buffer_length, int vtable_start, uint8_t* buffer, std::vector writeToOffsets) : buffer_length(buffer_length), vtable_start(vtable_start), buffer(buffer), - writeToOffsets(std::move(writeToOffsets)), write_raw_memories_iter(write_raw_memories_iter) {} + writeToOffsets(std::move(writeToOffsets)) {} struct MessageWriter { template @@ -433,12 +402,13 @@ struct WriteToBuffer { 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); } std::vector writeToOffsets; - std::vector::iterator write_raw_memories_iter; int writeToIndex = 0; uint8_t* buffer; }; @@ -781,9 +751,6 @@ struct LoadMember { ++inserter; current += sizeof(RelativeOffset); } - if constexpr (has_deserialization_done::value) { - VectorTraits::deserialization_done(member); - } } else if constexpr (is_union_like) { if (!field_present()) { i += 2; @@ -909,9 +876,6 @@ struct LoadSaveHelper { ++inserter; current += fb_size; } - if constexpr (has_deserialization_done::value) { - VectorTraits::deserialization_done(member); - } } template >> @@ -942,7 +906,15 @@ struct LoadSaveHelper { template >> RelativeOffset save(const U& message, Writer& writer, const VTableSet*, std::enable_if_t, int> _ = 0) { - writer.writeRawMemory([&]() { return dynamic_size_traits::save(message); }); + 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); + } return RelativeOffset{ writer.current_buffer_size }; } @@ -1058,7 +1030,7 @@ uint8_t* save(Allocator& allocator, const Root& root, FileIdentifier file_identi uint8_t* out = allocator(precompute_size.current_buffer_size); memset(out, 0, precompute_size.current_buffer_size); WriteToBuffer writeToBuffer{ precompute_size.current_buffer_size, vtable_start, out, - std::move(precompute_size.writeToOffsets), precompute_size.writeRawMemories.begin() }; + std::move(precompute_size.writeToOffsets) }; save_with_vtables(root, vtableset, writeToBuffer, &vtable_start, file_identifier); return out; }