From 8f306e8b4593ee3910319e4814312c6996fd7cef Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 25 Dec 2018 22:31:18 +0300 Subject: [PATCH 1/3] Small improvements #3920 --- dbms/src/Common/PODArray.h | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/dbms/src/Common/PODArray.h b/dbms/src/Common/PODArray.h index fe2084c85c7..280e0f9811b 100644 --- a/dbms/src/Common/PODArray.h +++ b/dbms/src/Common/PODArray.h @@ -20,6 +20,11 @@ namespace DB { +inline constexpr size_t integerRoundUp(size_t value, size_t dividend) +{ + return ((value + dividend - 1) / dividend) * dividend; +} + /** A dynamic array for POD types. * Designed for a small number of large arrays (rather than a lot of small ones). * To be more precise - for use in ColumnVector. @@ -37,6 +42,10 @@ namespace DB * The template parameter `pad_right` - always allocate at the end of the array as many unused bytes. * Can be used to make optimistic reading, writing, copying with unaligned SIMD instructions. * + * The template parameter `pad_left` - always allocate memory before 0th element of the array (rounded up to the whole number of elements) + * and zero initialize -1th element. It allows to use -1th element that will have value 0. + * This gives performance benefits when converting an array of offsets to array of sizes. + * * Some methods using allocator have TAllocatorParams variadic arguments. * These arguments will be passed to corresponding methods of TAllocator. * Example: pointer to Arena, that is used for allocations. @@ -57,14 +66,13 @@ class PODArray : private boost::noncopyable, private TAllocator /// empty bas { protected: /// Round padding up to an whole number of elements to simplify arithmetic. - static constexpr size_t pad_right = (pad_right_ + sizeof(T) - 1) / sizeof(T) * sizeof(T); - - static constexpr size_t pad_left_unaligned = (pad_left_ + sizeof(T) - 1) / sizeof(T) * sizeof(T); - static constexpr size_t pad_left = pad_left_unaligned ? (pad_left_unaligned + 15) / 16 * 16 : 0; - + static constexpr size_t pad_right = integerRoundUp(pad_right_, sizeof(T)); + /// pad_left is also rounded up to 16 bytes to maintain alignment of allocated memory. + static constexpr size_t pad_left = integerRoundUp(integerRoundUp(pad_left_, sizeof(T)), 16); + /// Empty array will point to this static memory as padding. static constexpr char * null = pad_left ? const_cast(EmptyPODArray) + EmptyPODArraySize : nullptr; - static_assert(pad_left <= EmptyPODArraySize && "Left Padding exceeds EmptyPODArraySize. Element size too large?"); + static_assert(pad_left <= EmptyPODArraySize && "Left Padding exceeds EmptyPODArraySize. Is the element size too large?"); char * c_start = null; /// Does not include pad_left. char * c_end = null; @@ -521,13 +529,7 @@ void swap(PODArray & lhs, PODArray> using PaddedPODArray = PODArray; - -inline constexpr size_t integerRound(size_t value, size_t dividend) -{ - return ((value + dividend - 1) / dividend) * dividend; -} - template -using PODArrayWithStackMemory = PODArray, integerRound(stack_size_in_bytes, sizeof(T))>>; +using PODArrayWithStackMemory = PODArray, integerRoundUp(stack_size_in_bytes, sizeof(T))>>; } From 444eb21d280182bf85e2818ed7747f686385c4d1 Mon Sep 17 00:00:00 2001 From: alesapin Date: Wed, 26 Dec 2018 12:56:52 +0300 Subject: [PATCH 2/3] Fix exponential backoff for replication queue --- dbms/src/Storages/StorageReplicatedMergeTree.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/dbms/src/Storages/StorageReplicatedMergeTree.cpp b/dbms/src/Storages/StorageReplicatedMergeTree.cpp index afe8cbc02ab..5e7dd32b8e6 100644 --- a/dbms/src/Storages/StorageReplicatedMergeTree.cpp +++ b/dbms/src/Storages/StorageReplicatedMergeTree.cpp @@ -2077,7 +2077,16 @@ bool StorageReplicatedMergeTree::queueTask() LogEntryPtr & entry = selected.first; if (!entry) - return false; + { + /// Nothing to do, we can sleep for some time, just not to + /// abuse background pool scheduling policy + std::this_thread::sleep_for(std::chrono::seconds(5)); + + /// If we return false, than background pool for this task + /// will accumulate exponential backoff and after empty replication queue + /// we will sleep for a long time + return true; + } time_t prev_attempt_time = entry->last_attempt_time; From 98b372415d176dc37e125d4f1f90bc66d1f0a5f6 Mon Sep 17 00:00:00 2001 From: alesapin Date: Wed, 26 Dec 2018 14:23:15 +0300 Subject: [PATCH 3/3] Decrease sleep time --- dbms/src/Storages/StorageReplicatedMergeTree.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/StorageReplicatedMergeTree.cpp b/dbms/src/Storages/StorageReplicatedMergeTree.cpp index 5e7dd32b8e6..ae1e563d33f 100644 --- a/dbms/src/Storages/StorageReplicatedMergeTree.cpp +++ b/dbms/src/Storages/StorageReplicatedMergeTree.cpp @@ -2080,7 +2080,7 @@ bool StorageReplicatedMergeTree::queueTask() { /// Nothing to do, we can sleep for some time, just not to /// abuse background pool scheduling policy - std::this_thread::sleep_for(std::chrono::seconds(5)); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); /// If we return false, than background pool for this task /// will accumulate exponential backoff and after empty replication queue