diff --git a/velox/exec/prefixsort/CMakeLists.txt b/velox/exec/prefixsort/CMakeLists.txt index 767d312e2ac3..aac7db501ecd 100644 --- a/velox/exec/prefixsort/CMakeLists.txt +++ b/velox/exec/prefixsort/CMakeLists.txt @@ -14,6 +14,8 @@ if(${VELOX_BUILD_TESTING}) add_subdirectory(tests) +elseif(${VELOX_BUILD_TEST_UTILS}) + add_subdirectory(tests/utils) endif() if(${VELOX_ENABLE_BENCHMARKS}) diff --git a/velox/exec/prefixsort/PrefixSortEncoder.h b/velox/exec/prefixsort/PrefixSortEncoder.h index 151f74c3b5b9..d8a0cd8c923f 100644 --- a/velox/exec/prefixsort/PrefixSortEncoder.h +++ b/velox/exec/prefixsort/PrefixSortEncoder.h @@ -32,8 +32,6 @@ class PrefixSortEncoder { /// 2. Encoding is compatible with sorting ascending with no nulls. template static FOLLY_ALWAYS_INLINE void encode(T value, char* row); - template - static FOLLY_ALWAYS_INLINE void decode(char* row); private: FOLLY_ALWAYS_INLINE static uint8_t flipSignBit(uint8_t byte) { @@ -53,27 +51,4 @@ FOLLY_ALWAYS_INLINE void PrefixSortEncoder::encode(int64_t value, char* row) { row[0] = flipSignBit(row[0]); } -template <> -FOLLY_ALWAYS_INLINE void PrefixSortEncoder::decode(char* row) { - row[0] = flipSignBit(row[0]); - const auto v = __builtin_bswap64(*reinterpret_cast(row)); - simd::memcpy(row, &v, sizeof(int64_t)); -} - -/// For testing only. -namespace { -template -FOLLY_ALWAYS_INLINE void testingEncodeInPlace(const std::vector& data) { - for (auto i = 0; i < data.size(); i++) { - PrefixSortEncoder::encode(data[i], (char*)data.data() + i * sizeof(T)); - } -} - -template -FOLLY_ALWAYS_INLINE void testingDecodeInPlace(const std::vector& data) { - for (auto i = 0; i < data.size(); i++) { - PrefixSortEncoder::decode((char*)data.data() + i * sizeof(T)); - } -} -} // namespace } // namespace facebook::velox::exec::prefixsort diff --git a/velox/exec/prefixsort/benchmarks/CMakeLists.txt b/velox/exec/prefixsort/benchmarks/CMakeLists.txt index 2f5119b4492a..7d29a84f520d 100644 --- a/velox/exec/prefixsort/benchmarks/CMakeLists.txt +++ b/velox/exec/prefixsort/benchmarks/CMakeLists.txt @@ -13,5 +13,6 @@ # limitations under the License. add_executable(velox_prefix_sort_algorithm_benchmark PrefixSortAlgorithmBenchmark.cpp) -target_link_libraries(velox_prefix_sort_algorithm_benchmark - velox_vector_test_lib ${FOLLY_BENCHMARK}) +target_link_libraries( + velox_prefix_sort_algorithm_benchmark velox_exec_prefixsort_test_lib + velox_vector_test_lib ${FOLLY_BENCHMARK}) diff --git a/velox/exec/prefixsort/benchmarks/PrefixSortAlgorithmBenchmark.cpp b/velox/exec/prefixsort/benchmarks/PrefixSortAlgorithmBenchmark.cpp index f65bc18c6e74..87c1c33bc21d 100644 --- a/velox/exec/prefixsort/benchmarks/PrefixSortAlgorithmBenchmark.cpp +++ b/velox/exec/prefixsort/benchmarks/PrefixSortAlgorithmBenchmark.cpp @@ -17,7 +17,7 @@ #include #include "velox/buffer/Buffer.h" #include "velox/exec/prefixsort/PrefixSortAlgorithm.h" -#include "velox/exec/prefixsort/PrefixSortEncoder.h" +#include "velox/exec/prefixsort/tests/utils/EncoderTestUtils.h" DEFINE_int32(sort_data_seed, 1, "random test data generate seed."); @@ -49,7 +49,7 @@ class PrefixSortAlgorithmBenchmark { std::generate(randomTestVec.begin(), randomTestVec.end(), [&]() { return folly::Random::rand64(rng_); }); - prefixsort::testingEncodeInPlace(randomTestVec); + prefixsort::test::encodeInPlace(randomTestVec); return randomTestVec; } diff --git a/velox/exec/prefixsort/tests/CMakeLists.txt b/velox/exec/prefixsort/tests/CMakeLists.txt index eddfa56d0502..2ce2b2e9975b 100644 --- a/velox/exec/prefixsort/tests/CMakeLists.txt +++ b/velox/exec/prefixsort/tests/CMakeLists.txt @@ -11,6 +11,8 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +add_subdirectory(utils) + add_executable(velox_exec_prefixsort_test PrefixSortAlgorithmTest.cpp) add_test( @@ -20,4 +22,5 @@ add_test( set_tests_properties(velox_exec_prefixsort_test PROPERTIES TIMEOUT 3000) -target_link_libraries(velox_exec_prefixsort_test velox_vector_test_lib) +target_link_libraries(velox_exec_prefixsort_test velox_exec_prefixsort_test_lib + velox_vector_fuzzer velox_vector_test_lib) diff --git a/velox/exec/prefixsort/tests/PrefixSortAlgorithmTest.cpp b/velox/exec/prefixsort/tests/PrefixSortAlgorithmTest.cpp index fdeccf6e4bfe..f379561b140f 100644 --- a/velox/exec/prefixsort/tests/PrefixSortAlgorithmTest.cpp +++ b/velox/exec/prefixsort/tests/PrefixSortAlgorithmTest.cpp @@ -20,6 +20,7 @@ #include "velox/exec/prefixsort/PrefixSortAlgorithm.h" #include "velox/exec/prefixsort/PrefixSortEncoder.h" +#include "velox/exec/prefixsort/tests/utils/EncoderTestUtils.h" #include "velox/vector/tests/VectorTestUtils.h" namespace facebook::velox::exec::prefixsort::test { @@ -43,14 +44,14 @@ class PrefixSortAlgorithmTest : public testing::Test, uint32_t entrySize = sizeof(int64_t); auto swapBuffer = AlignedBuffer::allocate(entrySize, pool()); PrefixSortRunner sortRunner(entrySize, swapBuffer->asMutable()); - testingEncodeInPlace(data1); + encodeInPlace(data1); sortRunner.quickSort( start, end, [&](char* a, char* b) { return memcmp(a, b, 8); }); } // Sort data2 with std::sort. std::sort(data2.begin(), data2.end()); - testingDecodeInPlace(data1); + decodeInPlace(data1); ASSERT_EQ(data1, data2); } @@ -75,7 +76,7 @@ TEST_F(PrefixSortAlgorithmTest, testingMedian3) { std::generate( data1.begin(), data1.end(), [&]() { return folly::Random::rand64(); }); std::vector data2(data1); - testingEncodeInPlace(data1); + encodeInPlace(data1); size_t entrySize = sizeof(int64_t); auto ptr1 = (char*)data1.data(); @@ -85,7 +86,7 @@ TEST_F(PrefixSortAlgorithmTest, testingMedian3) { ptr1, ptr2, ptr3, entrySize, [&](char* a, char* b) { return memcmp(a, b, entrySize); }); - testingDecodeInPlace(data1); + decodeInPlace(data1); auto median = *(reinterpret_cast(medianPtr)); // Sort the input vector data, the middle element must be equal to the // median we calculated. diff --git a/velox/exec/prefixsort/tests/utils/CMakeLists.txt b/velox/exec/prefixsort/tests/utils/CMakeLists.txt new file mode 100644 index 000000000000..c4691246a33f --- /dev/null +++ b/velox/exec/prefixsort/tests/utils/CMakeLists.txt @@ -0,0 +1,17 @@ +# Copyright (c) Facebook, Inc. and its affiliates. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +add_library(velox_exec_prefixsort_test_lib EncoderTestUtils.cpp) + +target_link_libraries(velox_exec_prefixsort_test_lib velox_vector_test_lib) diff --git a/velox/exec/prefixsort/tests/utils/EncoderTestUtils.cpp b/velox/exec/prefixsort/tests/utils/EncoderTestUtils.cpp new file mode 100644 index 000000000000..e475364d2607 --- /dev/null +++ b/velox/exec/prefixsort/tests/utils/EncoderTestUtils.cpp @@ -0,0 +1,45 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "velox/exec/prefixsort/tests/utils/EncoderTestUtils.h" + +namespace facebook::velox::exec::prefixsort::test { + +namespace { + +/// The decode method will only be used in testing scenarios. In production +/// code, we have no scenarios that require decoding a normalized key. +void decodeNoNulls(char* encoded, int64_t& value) { + value = *reinterpret_cast(encoded); + value = __builtin_bswap64(value ^ 128); +} + +} // namespace + +void encodeInPlace(std::vector& data) { + for (auto i = 0; i < data.size(); i++) { + PrefixSortEncoder::encode( + data[i], (char*)data.data() + i * sizeof(int64_t)); + } +} + +void decodeInPlace(std::vector& data) { + for (auto i = 0; i < data.size(); i++) { + decodeNoNulls((char*)data.data() + i * sizeof(int64_t), data[i]); + } +} + +} // namespace facebook::velox::exec::prefixsort::test diff --git a/velox/exec/prefixsort/tests/utils/EncoderTestUtils.h b/velox/exec/prefixsort/tests/utils/EncoderTestUtils.h new file mode 100644 index 000000000000..65da5d89a794 --- /dev/null +++ b/velox/exec/prefixsort/tests/utils/EncoderTestUtils.h @@ -0,0 +1,31 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +#include "velox/exec/prefixsort/PrefixSortEncoder.h" + +namespace facebook::velox::exec::prefixsort::test { + +/// Replace the elements in data with encoded ones(compatible with sorting in +/// ascending order), assuming that the elements in data are all non-null. +void encodeInPlace(std::vector& data); + +/// Replace the elements in data with decoded ones, assuming that the elements +/// in data are all non-null and encoded(compatible with sorting in ascending +/// order). +void decodeInPlace(std::vector& data); + +} // namespace facebook::velox::exec::prefixsort::test