From a719933c586c25b4d34907b980821f8e40607f98 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Fri, 28 Jun 2019 15:51:01 +0300 Subject: [PATCH] Fix initial size of some inline PODArray's. A template parameter of PODArray named INITIAL_SIZE didn't make its units clear, which made some callers to erroneously assume that it specifies the number of elements and not the number of bytes. Rename it, fix the wrong usages and, where possible, use the PODArrayWithStackMemory typedef for arrays with inline memory. --- .../AggregateFunctionSequenceMatch.h | 6 ++-- .../AggregateFunctionTimeSeriesGroupSum.h | 3 +- .../AggregateFunctionWindowFunnel.h | 5 +-- dbms/src/AggregateFunctions/QuantileExact.h | 3 +- dbms/src/AggregateFunctions/QuantileTDigest.h | 3 +- .../src/AggregateFunctions/ReservoirSampler.h | 3 +- .../ReservoirSamplerDeterministic.h | 3 +- dbms/src/Common/PODArray.h | 31 ++++++++++++------- dbms/src/Functions/FunctionsVisitParam.h | 3 +- 9 files changed, 28 insertions(+), 32 deletions(-) diff --git a/dbms/src/AggregateFunctions/AggregateFunctionSequenceMatch.h b/dbms/src/AggregateFunctions/AggregateFunctionSequenceMatch.h index 017b6d113dc..80860fdb62a 100644 --- a/dbms/src/AggregateFunctions/AggregateFunctionSequenceMatch.h +++ b/dbms/src/AggregateFunctions/AggregateFunctionSequenceMatch.h @@ -47,8 +47,7 @@ struct AggregateFunctionSequenceMatchData final using Comparator = ComparePairFirst; bool sorted = true; - static constexpr size_t bytes_in_arena = 64; - PODArray, bytes_in_arena>> events_list; + PODArrayWithStackMemory events_list; void add(const Timestamp timestamp, const Events & events) { @@ -203,8 +202,7 @@ private: PatternAction(const PatternActionType type, const std::uint64_t extra = 0) : type{type}, extra{extra} {} }; - static constexpr size_t bytes_on_stack = 64; - using PatternActions = PODArray, bytes_on_stack>>; + using PatternActions = PODArrayWithStackMemory; Derived & derived() { return static_cast(*this); } diff --git a/dbms/src/AggregateFunctions/AggregateFunctionTimeSeriesGroupSum.h b/dbms/src/AggregateFunctions/AggregateFunctionTimeSeriesGroupSum.h index c74ad8c0bdb..5e2a9b15f4e 100644 --- a/dbms/src/AggregateFunctions/AggregateFunctionTimeSeriesGroupSum.h +++ b/dbms/src/AggregateFunctions/AggregateFunctionTimeSeriesGroupSum.h @@ -68,9 +68,8 @@ struct AggregateFunctionTimeSeriesGroupSumData } }; - static constexpr size_t bytes_on_stack = 128; typedef std::map Series; - typedef PODArray, bytes_on_stack>> AggSeries; + typedef PODArrayWithStackMemory AggSeries; Series ss; AggSeries result; diff --git a/dbms/src/AggregateFunctions/AggregateFunctionWindowFunnel.h b/dbms/src/AggregateFunctions/AggregateFunctionWindowFunnel.h index 9a738d3fefb..1e3c005f73f 100644 --- a/dbms/src/AggregateFunctions/AggregateFunctionWindowFunnel.h +++ b/dbms/src/AggregateFunctions/AggregateFunctionWindowFunnel.h @@ -35,10 +35,7 @@ template struct AggregateFunctionWindowFunnelData { using TimestampEvent = std::pair; - - static constexpr size_t bytes_on_stack = 64; - using TimestampEvents = PODArray, bytes_on_stack>>; - + using TimestampEvents = PODArray; using Comparator = ComparePairFirst; bool sorted = true; diff --git a/dbms/src/AggregateFunctions/QuantileExact.h b/dbms/src/AggregateFunctions/QuantileExact.h index b4398e8bb7f..a5b616669b9 100644 --- a/dbms/src/AggregateFunctions/QuantileExact.h +++ b/dbms/src/AggregateFunctions/QuantileExact.h @@ -27,8 +27,7 @@ struct QuantileExact { /// The memory will be allocated to several elements at once, so that the state occupies 64 bytes. static constexpr size_t bytes_in_arena = 64 - sizeof(PODArray); - - using Array = PODArray, bytes_in_arena>>; + using Array = PODArrayWithStackMemory; Array array; void add(const Value & x) diff --git a/dbms/src/AggregateFunctions/QuantileTDigest.h b/dbms/src/AggregateFunctions/QuantileTDigest.h index e9f261d4c21..f7201ef3b0d 100644 --- a/dbms/src/AggregateFunctions/QuantileTDigest.h +++ b/dbms/src/AggregateFunctions/QuantileTDigest.h @@ -86,8 +86,7 @@ class QuantileTDigest /// The memory will be allocated to several elements at once, so that the state occupies 64 bytes. static constexpr size_t bytes_in_arena = 128 - sizeof(PODArray) - sizeof(Count) - sizeof(UInt32); - - using Summary = PODArray, bytes_in_arena>>; + using Summary = PODArrayWithStackMemory; Summary summary; Count count = 0; diff --git a/dbms/src/AggregateFunctions/ReservoirSampler.h b/dbms/src/AggregateFunctions/ReservoirSampler.h index ad5bf10f48f..30d72709ac2 100644 --- a/dbms/src/AggregateFunctions/ReservoirSampler.h +++ b/dbms/src/AggregateFunctions/ReservoirSampler.h @@ -194,8 +194,7 @@ private: friend void rs_perf_test(); /// We allocate a little memory on the stack - to avoid allocations when there are many objects with a small number of elements. - static constexpr size_t bytes_on_stack = 64; - using Array = DB::PODArray, bytes_on_stack>>; + using Array = DB::PODArrayWithStackMemory; size_t sample_count; size_t total_values = 0; diff --git a/dbms/src/AggregateFunctions/ReservoirSamplerDeterministic.h b/dbms/src/AggregateFunctions/ReservoirSamplerDeterministic.h index c543e662b2a..4beeecd93bc 100644 --- a/dbms/src/AggregateFunctions/ReservoirSamplerDeterministic.h +++ b/dbms/src/AggregateFunctions/ReservoirSamplerDeterministic.h @@ -164,9 +164,8 @@ public: private: /// We allocate some memory on the stack to avoid allocations when there are many objects with a small number of elements. - static constexpr size_t bytes_on_stack = 64; using Element = std::pair; - using Array = DB::PODArray, bytes_on_stack>>; + using Array = DB::PODArray; size_t sample_count; size_t total_values{}; diff --git a/dbms/src/Common/PODArray.h b/dbms/src/Common/PODArray.h index 0e7d547a7d0..01085a2c5a7 100644 --- a/dbms/src/Common/PODArray.h +++ b/dbms/src/Common/PODArray.h @@ -45,7 +45,7 @@ inline constexpr size_t integerRoundUp(size_t value, size_t dividend) * Only part of the std::vector interface is supported. * * The default constructor creates an empty object that does not allocate memory. - * Then the memory is allocated at least INITIAL_SIZE bytes. + * Then the memory is allocated at least initial_bytes bytes. * * If you insert elements with push_back, without making a `reserve`, then PODArray is about 2.5 times faster than std::vector. * @@ -74,7 +74,7 @@ extern const char EmptyPODArray[EmptyPODArraySize]; /** Base class that depend only on size of element, not on element itself. * You can static_cast to this class if you want to insert some data regardless to the actual type T. */ -template +template class PODArrayBase : private boost::noncopyable, private TAllocator /// empty base optimization { protected: @@ -161,7 +161,8 @@ protected: { // The allocated memory should be multiplication of ELEMENT_SIZE to hold the element, otherwise, // memory issue such as corruption could appear in edge case. - realloc(std::max(((INITIAL_SIZE - 1) / ELEMENT_SIZE + 1) * ELEMENT_SIZE, minimum_memory_for_elements(1)), + realloc(std::max(integerRoundUp(initial_bytes, ELEMENT_SIZE), + minimum_memory_for_elements(1)), std::forward(allocator_params)...); } else @@ -257,11 +258,11 @@ public: } }; -template , size_t pad_right_ = 0, size_t pad_left_ = 0> -class PODArray : public PODArrayBase +template , size_t pad_right_ = 0, size_t pad_left_ = 0> +class PODArray : public PODArrayBase { protected: - using Base = PODArrayBase; + using Base = PODArrayBase; T * t_start() { return reinterpret_cast(this->c_start); } T * t_end() { return reinterpret_cast(this->c_end); } @@ -618,17 +619,23 @@ public: } }; -template -void swap(PODArray & lhs, PODArray & rhs) +template +void swap(PODArray & lhs, PODArray & rhs) { lhs.swap(rhs); } /** For columns. Padding is enough to read and write xmm-register at the address of the last element. */ -template > -using PaddedPODArray = PODArray; +template > +using PaddedPODArray = PODArray; -template -using PODArrayWithStackMemory = PODArray, integerRoundUp(stack_size_in_bytes, sizeof(T))>>; +/** A helper for declaring PODArray that uses inline memory. + * The initial size is set to use all the inline bytes, since using less would + * only add some extra allocation calls. + */ +template +using PODArrayWithStackMemory = PODArray, rounded_bytes>>; } diff --git a/dbms/src/Functions/FunctionsVisitParam.h b/dbms/src/Functions/FunctionsVisitParam.h index 09cc3106719..41a49dfd908 100644 --- a/dbms/src/Functions/FunctionsVisitParam.h +++ b/dbms/src/Functions/FunctionsVisitParam.h @@ -91,8 +91,7 @@ struct ExtractBool struct ExtractRaw { - static constexpr size_t bytes_on_stack = 64; - using ExpectChars = PODArray, bytes_on_stack>>; + using ExpectChars = PODArrayWithStackMemory; static void extract(const UInt8 * pos, const UInt8 * end, ColumnString::Chars & res_data) {