From d84bccb0c7b6657ff3de6f6406c6888a80a039ba Mon Sep 17 00:00:00 2001 From: Derek Bailey Date: Fri, 21 May 2021 11:09:24 -0700 Subject: [PATCH] Removed most heap allocations in builder (#6662) * [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 * Replaced iterator with for loop * remove std::to_string in test to be compatible --- include/flatbuffers/base.h | 5 +++ include/flatbuffers/flatbuffers.h | 73 +++++++++++++++++++++++-------- tests/test.cpp | 10 +++++ 3 files changed, 69 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..1306bb33b 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,35 @@ 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 i = 1; i <= size; i++) { + // Note we re-evaluate the buf location each iteration to account for any + // underlying buffer resizing that may occur. + PushElement(*reinterpret_cast *>( + buf_.scratch_end() - i * sizeof(Offset))); + } + buf_.scratch_pop(scratch_buffer_usage); + return Offset>>(EndVector(size)); } /// @brief Serialize an array of structs into a FlatBuffer `vector`. @@ -1826,9 +1861,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 +2029,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 diff --git a/tests/test.cpp b/tests/test.cpp index 3b94e7b69..868003ebe 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ #include +#include #include "flatbuffers/flatbuffers.h" #include "flatbuffers/idl.h" @@ -156,6 +157,15 @@ flatbuffers::DetachedBuffer CreateFlatBufferTest(std::string &buffer) { names2.push_back("mary"); auto vecofstrings2 = builder.CreateVectorOfStrings(names2); + // Create many vectors of strings + std::vector manyNames; + for (auto i = 0; i < 100; i++) { manyNames.push_back("john_doe"); } + auto manyNamesVec = builder.CreateVectorOfStrings(manyNames); + TEST_EQ(false, manyNamesVec.IsNull()); + auto manyNamesVec2 = + builder.CreateVectorOfStrings(manyNames.cbegin(), manyNames.cend()); + TEST_EQ(false, manyNamesVec2.IsNull()); + // Create an array of sorted tables, can be used with binary search when read: auto vecoftables = builder.CreateVectorOfSortedTables(mlocs, 3);