Skip to content

Commit

Permalink
Revert "Optimize agg with collation & Optimize mem utils (pingcap#5834)"
Browse files Browse the repository at this point in the history
This reverts commit dfac6a5.
  • Loading branch information
solotzg committed Sep 19, 2022
1 parent a0f9865 commit 5b78864
Show file tree
Hide file tree
Showing 33 changed files with 248 additions and 1,515 deletions.
5 changes: 0 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -511,11 +511,6 @@ if (ARCH_AMD64)
else()
add_definitions(-DTIFLASH_COMPILER_VPCLMULQDQ_SUPPORT=0)
endif()

check_cxx_compiler_flag("-mmovbe" TIFLASH_COMPILER_MOVBE_SUPPORT)
if (TIFLASH_COMPILER_MOVBE_SUPPORT)
set(COMPILER_MOVBE_FLAG "-mmovbe")
endif()
else()
add_definitions(-DTIFLASH_COMPILER_VPCLMULQDQ_SUPPORT=0)
endif()
Expand Down
18 changes: 9 additions & 9 deletions dbms/src/AggregateFunctions/AggregateFunctionMinMaxAny.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,36 +193,36 @@ struct SingleValueDataString

Int32 size = -1; /// -1 indicates that there is no value.
Int32 capacity = 0; /// power of two or zero
char * large_data{};
TiDB::TiDBCollatorPtr collator{};
char * large_data;
TiDB::TiDBCollatorPtr collator = nullptr;

bool less(const StringRef & a, const StringRef & b) const
{
if (unlikely(collator == nullptr))
if (collator == nullptr)
return a < b;
return collator->compareFastPath(a.data, a.size, b.data, b.size) < 0;
return collator->compare(a.data, a.size, b.data, b.size) < 0;
}

bool greater(const StringRef & a, const StringRef & b) const
{
if (unlikely(collator == nullptr))
if (collator == nullptr)
return a > b;
return collator->compareFastPath(a.data, a.size, b.data, b.size) > 0;
return collator->compare(a.data, a.size, b.data, b.size) > 0;
}

bool equalTo(const StringRef & a, const StringRef & b) const
{
if (unlikely(collator == nullptr))
if (collator == nullptr)
return a == b;
return collator->compareFastPath(a.data, a.size, b.data, b.size) == 0;
return collator->compare(a.data, a.size, b.data, b.size) == 0;
}

public:
static constexpr Int32 AUTOMATIC_STORAGE_SIZE = 64;
static constexpr Int32 MAX_SMALL_STRING_SIZE = AUTOMATIC_STORAGE_SIZE - sizeof(size) - sizeof(capacity) - sizeof(large_data) - sizeof(collator);

private:
char small_data[MAX_SMALL_STRING_SIZE]{}; /// Including the terminating zero.
char small_data[MAX_SMALL_STRING_SIZE]; /// Including the terminating zero.

public:
bool has() const
Expand Down
2 changes: 1 addition & 1 deletion dbms/src/AggregateFunctions/IAggregateFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ struct AggregationCollatorsWrapper<true>
if (likely(collators.size() > column_index))
{
if (collators[column_index] != nullptr)
return collators[column_index]->sortKeyFastPath(in.data, in.size, sort_key_containers[column_index]);
return collators[column_index]->sortKey(in.data, in.size, sort_key_containers[column_index]);
return in;
}
else if (collators.empty())
Expand Down
13 changes: 7 additions & 6 deletions dbms/src/Columns/ColumnString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include <Common/HashTable/Hash.h>
#include <DataStreams/ColumnGathererStream.h>
#include <Storages/Transaction/CollatorUtils.h>
#include <common/memcpy.h>
#include <fmt/core.h>


Expand Down Expand Up @@ -100,7 +99,7 @@ void ColumnString::insertRangeFrom(const IColumn & src, size_t start, size_t len

size_t old_chars_size = chars.size();
chars.resize(old_chars_size + nested_length);
inline_memcpy(&chars[old_chars_size], &src_concrete.chars[nested_offset], nested_length);
memcpy(&chars[old_chars_size], &src_concrete.chars[nested_offset], nested_length);

if (start == 0 && offsets.empty())
{
Expand Down Expand Up @@ -321,10 +320,12 @@ int ColumnString::compareAtWithCollationImpl(size_t n, size_t m, const IColumn &
{
const auto & rhs = static_cast<const ColumnString &>(rhs_);

auto a = getDataAt(n);
auto b = rhs.getDataAt(m);

return collator.compare(a.data, a.size, b.data, b.size);
return collator.compare(
reinterpret_cast<const char *>(&chars[offsetAt(n)]),
sizeAt(n) - 1, // Skip last zero byte.
reinterpret_cast<const char *>(&rhs.chars[rhs.offsetAt(m)]),
rhs.sizeAt(m) - 1 // Skip last zero byte.
);
}

// Derived must implement function `int compare(const char *, size_t, const char *, size_t)`.
Expand Down
19 changes: 14 additions & 5 deletions dbms/src/Columns/ColumnString.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,10 @@ class ColumnString final : public COWPtrHelper<IColumn, ColumnString>

StringRef res;

if (likely(collator != nullptr))
if (collator != nullptr)
{
// Skip last zero byte.
auto sort_key = collator->sortKeyFastPath(reinterpret_cast<const char *>(src), string_size - 1, sort_key_container);
auto sort_key = collator->sortKey(reinterpret_cast<const char *>(src), string_size - 1, sort_key_container);
string_size = sort_key.size;
src = sort_key.data;
}
Expand Down Expand Up @@ -244,10 +244,10 @@ class ColumnString final : public COWPtrHelper<IColumn, ColumnString>
{
size_t string_size = sizeAt(n);
size_t offset = offsetAt(n);
if (likely(collator != nullptr))
if (collator != nullptr)
{
// Skip last zero byte.
auto sort_key = collator->sortKeyFastPath(reinterpret_cast<const char *>(&chars[offset]), string_size - 1, sort_key_container);
auto sort_key = collator->sortKey(reinterpret_cast<const char *>(&chars[offset]), string_size - 1, sort_key_container);
string_size = sort_key.size;
hash.update(reinterpret_cast<const char *>(&string_size), sizeof(string_size));
hash.update(sort_key.data, sort_key.size);
Expand Down Expand Up @@ -278,7 +278,16 @@ class ColumnString final : public COWPtrHelper<IColumn, ColumnString>
int compareAt(size_t n, size_t m, const IColumn & rhs_, int /*nan_direction_hint*/) const override
{
const auto & rhs = static_cast<const ColumnString &>(rhs_);
return getDataAtWithTerminatingZero(n).compare(rhs.getDataAtWithTerminatingZero(m));

const size_t size = sizeAt(n);
const size_t rhs_size = rhs.sizeAt(m);

int cmp = memcmp(&chars[offsetAt(n)], &rhs.chars[rhs.offsetAt(m)], std::min(size, rhs_size));

if (cmp != 0)
return cmp;
else
return size > rhs_size ? 1 : (size < rhs_size ? -1 : 0);
}

int compareAt(size_t n, size_t m, const IColumn & rhs_, int, const ICollator & collator) const override
Expand Down
25 changes: 13 additions & 12 deletions dbms/src/Columns/ColumnsCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#if __SSE2__
#include <emmintrin.h>
#endif

#include <Columns/ColumnsCommon.h>
#include <Columns/IColumn.h>
#include <common/memcpy.h>


namespace DB
{
Expand Down Expand Up @@ -142,7 +146,7 @@ struct ResultOffsetsBuilder
{
const auto offsets_size_old = res_offsets.size();
res_offsets.resize(offsets_size_old + SIMD_BYTES);
inline_memcpy(&res_offsets[offsets_size_old], src_offsets_pos, SIMD_BYTES * sizeof(IColumn::Offset));
memcpy(&res_offsets[offsets_size_old], src_offsets_pos, SIMD_BYTES * sizeof(IColumn::Offset));

if (!first)
{
Expand Down Expand Up @@ -190,7 +194,7 @@ void filterArraysImplGeneric(
{
const size_t size = src_offsets.size();
if (size != filt.size())
throw Exception(fmt::format("size of filter {} doesn't match size of column {}", filt.size(), size), ErrorCodes::SIZES_OF_COLUMNS_DOESNT_MATCH);
throw Exception("Size of filter doesn't match size of column.", ErrorCodes::SIZES_OF_COLUMNS_DOESNT_MATCH);

ResultOffsetsBuilder result_offsets_builder(res_offsets);

Expand Down Expand Up @@ -219,7 +223,7 @@ void filterArraysImplGeneric(

const auto elems_size_old = res_elems.size();
res_elems.resize(elems_size_old + size);
inline_memcpy(&res_elems[elems_size_old], &src_elems[offset], size * sizeof(T));
memcpy(&res_elems[elems_size_old], &src_elems[offset], size * sizeof(T));
};

#if __SSE2__
Expand All @@ -229,7 +233,7 @@ void filterArraysImplGeneric(

while (filt_pos < filt_end_aligned)
{
uint32_t mask = _mm_movemask_epi8(_mm_cmpgt_epi8(
const auto mask = _mm_movemask_epi8(_mm_cmpgt_epi8(
_mm_loadu_si128(reinterpret_cast<const __m128i *>(filt_pos)),
zero_vec));

Expand All @@ -250,16 +254,13 @@ void filterArraysImplGeneric(
/// copy elements for SIMD_BYTES arrays at once
const auto elems_size_old = res_elems.size();
res_elems.resize(elems_size_old + chunk_size);
inline_memcpy(&res_elems[elems_size_old], &src_elems[chunk_offset], chunk_size * sizeof(T));
memcpy(&res_elems[elems_size_old], &src_elems[chunk_offset], chunk_size * sizeof(T));
}
else
{
while (mask)
{
size_t index = __builtin_ctz(mask);
copy_array(offsets_pos + index);
mask = mask & (mask - 1);
}
for (size_t i = 0; i < SIMD_BYTES; ++i)
if (filt_pos[i])
copy_array(offsets_pos + i);
}

filt_pos += SIMD_BYTES;
Expand Down
Loading

0 comments on commit 5b78864

Please sign in to comment.