From 72730ecd8ae62a67fe3d4fd83522537424924b74 Mon Sep 17 00:00:00 2001 From: Derek Bailey Date: Mon, 17 May 2021 10:53:32 -0700 Subject: [PATCH] [C++] Removed most heap allocations in builder (#6620) * [C++] Removed most heap allocations in builder * Updated API docs to indicate heap usage * Override assertion for heap allocation in parser * Cleaned up implemenations, enable heap alloc for tests * Generalized CreateVectorOfStrings * remove cmake option for heap alloc. reverted two heap removals * Only use scratch space for vector of strings * Override Windows SCL warning * Changed std::transform to for loop to avoid MSCV warnings * switched to const iterators --- include/flatbuffers/base.h | 5 +++ include/flatbuffers/flatbuffers.h | 72 +++++++++++++++++++++++-------- 2 files changed, 58 insertions(+), 19 deletions(-) diff --git a/include/flatbuffers/base.h b/include/flatbuffers/base.h index cc9e22dcc..4efa42ada 100644 --- a/include/flatbuffers/base.h +++ b/include/flatbuffers/base.h @@ -247,6 +247,11 @@ namespace flatbuffers { #endif // __has_include #endif // !FLATBUFFERS_HAS_STRING_VIEW +#ifndef FLATBUFFERS_GENERAL_HEAP_ALLOC_OK + // Allow heap allocations to be used + #define FLATBUFFERS_GENERAL_HEAP_ALLOC_OK 1 +#endif // !FLATBUFFERS_GENERAL_HEAP_ALLOC_OK + #ifndef FLATBUFFERS_HAS_NEW_STRTOD // Modern (C++11) strtod and strtof functions are available for use. // 1) nan/inf strings as argument of strtod; diff --git a/include/flatbuffers/flatbuffers.h b/include/flatbuffers/flatbuffers.h index ee34d54ed..180bf740b 100644 --- a/include/flatbuffers/flatbuffers.h +++ b/include/flatbuffers/flatbuffers.h @@ -1356,9 +1356,8 @@ class FlatBufferBuilder { // Write a single aligned scalar to the buffer template uoffset_t PushElement(T element) { AssertScalarT(); - T litle_endian_element = EndianScalar(element); Align(sizeof(T)); - buf_.push_small(litle_endian_element); + buf_.push_small(EndianScalar(element)); return GetSize(); } @@ -1601,11 +1600,13 @@ class FlatBufferBuilder { /// @brief Store a string in the buffer, which can contain any binary data. /// If a string with this exact contents has already been serialized before, - /// instead simply returns the offset of the existing string. + /// instead simply returns the offset of the existing string. This uses a map + /// stored on the heap, but only stores the numerical offsets. /// @param[in] str A const char pointer to the data to be stored as a string. /// @param[in] len The number of bytes that should be stored from `str`. /// @return Returns the offset in the buffer where the string starts. Offset CreateSharedString(const char *str, size_t len) { + FLATBUFFERS_ASSERT(FLATBUFFERS_GENERAL_HEAP_ALLOC_OK); if (!string_pool) string_pool = new StringOffsetMap(StringOffsetCompare(buf_)); auto size_before_string = buf_.size(); @@ -1627,7 +1628,8 @@ class FlatBufferBuilder { #ifdef FLATBUFFERS_HAS_STRING_VIEW /// @brief Store a string in the buffer, which can contain any binary data. /// If a string with this exact contents has already been serialized before, - /// instead simply returns the offset of the existing string. + /// instead simply returns the offset of the existing string. This uses a map + /// stored on the heap, but only stores the numerical offsets. /// @param[in] str A const std::string_view to store in the buffer. /// @return Returns the offset in the buffer where the string starts Offset CreateSharedString(const flatbuffers::string_view str) { @@ -1636,7 +1638,8 @@ class FlatBufferBuilder { #else /// @brief Store a string in the buffer, which null-terminated. /// If a string with this exact contents has already been serialized before, - /// instead simply returns the offset of the existing string. + /// instead simply returns the offset of the existing string. This uses a map + /// stored on the heap, but only stores the numerical offsets. /// @param[in] str A const char pointer to a C-string to add to the buffer. /// @return Returns the offset in the buffer where the string starts. Offset CreateSharedString(const char *str) { @@ -1645,7 +1648,8 @@ class FlatBufferBuilder { /// @brief Store a string in the buffer, which can contain any binary data. /// If a string with this exact contents has already been serialized before, - /// instead simply returns the offset of the existing string. + /// instead simply returns the offset of the existing string. This uses a map + /// stored on the heap, but only stores the numerical offsets. /// @param[in] str A const reference to a std::string to store in the buffer. /// @return Returns the offset in the buffer where the string starts. Offset CreateSharedString(const std::string &str) { @@ -1655,7 +1659,8 @@ class FlatBufferBuilder { /// @brief Store a string in the buffer, which can contain any binary data. /// If a string with this exact contents has already been serialized before, - /// instead simply returns the offset of the existing string. + /// instead simply returns the offset of the existing string. This uses a map + /// stored on the heap, but only stores the numerical offsets. /// @param[in] str A const pointer to a `String` struct to add to the buffer. /// @return Returns the offset in the buffer where the string starts Offset CreateSharedString(const String *str) { @@ -1762,15 +1767,18 @@ class FlatBufferBuilder { /// where the vector is stored. template Offset> CreateVector(size_t vector_size, const std::function &f) { + FLATBUFFERS_ASSERT(FLATBUFFERS_GENERAL_HEAP_ALLOC_OK); std::vector elems(vector_size); for (size_t i = 0; i < vector_size; i++) elems[i] = f(i); return CreateVector(elems); } - #endif + #endif // FLATBUFFERS_CPP98_STL // clang-format on /// @brief Serialize values returned by a function into a FlatBuffer `vector`. - /// This is a convenience function that takes care of iteration for you. + /// This is a convenience function that takes care of iteration for you. This + /// uses a vector stored on the heap to store the intermediate results of the + /// iteration. /// @tparam T The data type of the `std::vector` elements. /// @param f A function that takes the current iteration 0..vector_size-1, /// and the state parameter returning any type that you can construct a @@ -1780,6 +1788,7 @@ class FlatBufferBuilder { /// where the vector is stored. template Offset> CreateVector(size_t vector_size, F f, S *state) { + FLATBUFFERS_ASSERT(FLATBUFFERS_GENERAL_HEAP_ALLOC_OK); std::vector elems(vector_size); for (size_t i = 0; i < vector_size; i++) elems[i] = f(i, state); return CreateVector(elems); @@ -1793,9 +1802,34 @@ class FlatBufferBuilder { /// where the vector is stored. Offset>> CreateVectorOfStrings( const std::vector &v) { - std::vector> offsets(v.size()); - for (size_t i = 0; i < v.size(); i++) offsets[i] = CreateString(v[i]); - return CreateVector(offsets); + return CreateVectorOfStrings(v.cbegin(), v.cend()); + } + + /// @brief Serialize a collection of Strings into a FlatBuffer `vector`. + /// This is a convenience function for a common case. + /// @param begin The begining iterator of the collection + /// @param end The ending iterator of the collection + /// @return Returns a typed `Offset` into the serialized data indicating + /// where the vector is stored. + template + Offset>> CreateVectorOfStrings(It begin, It end) { + auto size = std::distance(begin, end); + auto scratch_buffer_usage = size * sizeof(Offset); + // If there is not enough space to store the offsets, there definitely won't + // be enough space to store all the strings. So ensuring space for the + // scratch region is OK, for it it fails, it would have failed later. + buf_.ensure_space(scratch_buffer_usage); + for (auto it = begin; it != end; ++it) { + buf_.scratch_push_small(CreateString(*it)); + } + StartVector(size, sizeof(Offset)); + for (auto it = buf_.scratch_end(); + it > buf_.scratch_end() - scratch_buffer_usage;) { + it -= sizeof(Offset); + PushElement(*reinterpret_cast *>(it)); + } + buf_.scratch_pop(scratch_buffer_usage); + return Offset>>(EndVector(size)); } /// @brief Serialize an array of structs into a FlatBuffer `vector`. @@ -1826,9 +1860,9 @@ class FlatBufferBuilder { Offset> CreateVectorOfNativeStructs( const S *v, size_t len, T((*const pack_func)(const S &))) { FLATBUFFERS_ASSERT(pack_func); - std::vector vv(len); - std::transform(v, v + len, vv.begin(), pack_func); - return CreateVectorOfStructs(data(vv), vv.size()); + auto structs = StartVectorOfStructs(len); + for (size_t i = 0; i < len; i++) { structs[i] = pack_func(v[i]); } + return EndVectorOfStructs(len); } /// @brief Serialize an array of native structs into a FlatBuffer `vector`. @@ -1994,10 +2028,10 @@ class FlatBufferBuilder { Offset> CreateVectorOfSortedNativeStructs(S *v, size_t len) { extern T Pack(const S &); - typedef T (*Pack_t)(const S &); - std::vector vv(len); - std::transform(v, v + len, vv.begin(), static_cast(Pack)); - return CreateVectorOfSortedStructs(vv, len); + auto structs = StartVectorOfStructs(len); + for (size_t i = 0; i < len; i++) { structs[i] = Pack(v[i]); } + std::sort(structs, structs + len, StructKeyComparator()); + return EndVectorOfStructs(len); } /// @cond FLATBUFFERS_INTERNAL