Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherry pick bug fixes for PODArray (#1357) #1358

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions dbms/src/Common/PODArray.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,14 +228,20 @@ class PODArrayBase : private boost::noncopyable, private TAllocator /// empty
return c_start;
}

template <typename ... TAllocatorParams>
template <typename... TAllocatorParams>
void push_back_raw(const char * ptr, TAllocatorParams &&... allocator_params)
{
if (unlikely(c_end >= c_end_of_storage))
reserveForNextSize(std::forward<TAllocatorParams>(allocator_params)...);
push_back_raw_many(1, ptr, std::forward<TAllocatorParams>(allocator_params)...);
}

memcpy(c_end, ptr, ELEMENT_SIZE);
c_end += byte_size(1);
template <typename... TAllocatorParams>
void push_back_raw_many(size_t number_of_items, const void * ptr, TAllocatorParams &&... allocator_params)
{
size_t items_byte_size = byte_size(number_of_items);
if (unlikely(c_end + items_byte_size > c_end_of_storage))
reserve(size() + number_of_items, std::forward<TAllocatorParams>(allocator_params)...);
memcpy(c_end, ptr, items_byte_size);
c_end += items_byte_size;
}

void protect()
Expand Down Expand Up @@ -434,11 +440,12 @@ class PODArray : public PODArrayBase<sizeof(T), INITIAL_SIZE, TAllocator, pad_ri
template <typename It1, typename It2>
void insert(iterator it, It1 from_begin, It2 from_end)
{
insertPrepare(from_begin, from_end);

size_t bytes_to_copy = this->byte_size(from_end - from_begin);
size_t bytes_to_move = (end() - it) * sizeof(T);

// This may make `it` invalid, do it after calculating `bytes_to_move`
insertPrepare(from_begin, from_end);

if (unlikely(bytes_to_move))
memcpy(this->c_end + bytes_to_copy - bytes_to_move, this->c_end - bytes_to_move, bytes_to_move);

Expand Down
67 changes: 67 additions & 0 deletions dbms/src/Common/tests/gtest_pod_array.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
#include <Common/PODArray.h>
#include <gtest/gtest.h>

using namespace DB;

TEST(Common, PODArrayInsert)
{
std::string str = "test_string_abacaba";
PODArray<char> chars;
chars.insert(chars.end(), str.begin(), str.end());
EXPECT_EQ(str, std::string(chars.data(), chars.size()));

std::string insert_in_the_middle = "insert_in_the_middle";
auto pos = str.size() / 2;
str.insert(str.begin() + pos, insert_in_the_middle.begin(), insert_in_the_middle.end());
chars.insert(chars.begin() + pos, insert_in_the_middle.begin(), insert_in_the_middle.end());
EXPECT_EQ(str, std::string(chars.data(), chars.size()));

std::string insert_with_resize;
insert_with_resize.reserve(chars.capacity() * 2);
char cur_char = 'a';
while (insert_with_resize.size() < insert_with_resize.capacity())
{
insert_with_resize += cur_char;
if (cur_char == 'z')
cur_char = 'a';
else
++cur_char;
}
str.insert(str.begin(), insert_with_resize.begin(), insert_with_resize.end());
chars.insert(chars.begin(), insert_with_resize.begin(), insert_with_resize.end());
EXPECT_EQ(str, std::string(chars.data(), chars.size()));
}

TEST(Common, PODPushBackRawMany)
{
PODArray<char> chars;
chars.push_back_raw_many(5, "first");
EXPECT_EQ(std::string("first"), std::string(chars.data(), chars.size()));
EXPECT_EQ(5UL, chars.size());
EXPECT_LE(chars.capacity() - chars.size(), 10UL);
chars.push_back_raw_many(10, "0123456789");
EXPECT_EQ(15UL, chars.size());
EXPECT_EQ(std::string("first0123456789"), std::string(chars.data(), chars.size()));
}

TEST(Common, PODNoOverallocation)
{
/// Check that PaddedPODArray allocates for smaller number of elements than the power of two due to padding.
/// NOTE: It's Ok to change these numbers if you will modify initial size or padding.

PaddedPODArray<char> chars;
std::vector<size_t> capacities;

size_t prev_capacity = 0;
for (size_t i = 0; i < 1000000; ++i)
{
chars.emplace_back();
if (chars.capacity() != prev_capacity)
{
prev_capacity = chars.capacity();
capacities.emplace_back(prev_capacity);
}
}

EXPECT_EQ(capacities, (std::vector<size_t>{4065, 8161, 16353, 32737, 65505, 131041, 262113, 524257, 1048545}));
}