From 0ba176ab1d973d3c675716e5988d47253b2e19b8 Mon Sep 17 00:00:00 2001 From: James Xu Date: Thu, 25 Jul 2024 13:45:57 +0800 Subject: [PATCH] [GLUTEN-6550] Fix build failure when BUILD_TESTS is enabled on macOS --- cpp/CMakeLists.txt | 6 ++++++ cpp/core/benchmarks/CompressionBenchmark.cc | 15 +++++++++++++-- cpp/core/jni/JniCommon.h | 2 +- cpp/core/utils/ObjectStore.h | 2 +- cpp/core/utils/macros.h | 2 +- cpp/core/utils/qpl/qpl_codec.cc | 4 ++-- cpp/velox/benchmarks/ColumnarToRowBenchmark.cc | 5 +++++ cpp/velox/benchmarks/ParquetWriteBenchmark.cc | 5 +++++ cpp/velox/benchmarks/common/BenchmarkUtils.cc | 5 +++++ cpp/velox/operators/plannodes/RowVectorStream.h | 2 +- cpp/velox/tests/CMakeLists.txt | 2 +- cpp/velox/tests/FunctionTest.cc | 2 +- cpp/velox/tests/VeloxRowToColumnarTest.cc | 6 +++--- cpp/velox/tests/VeloxShuffleWriterTest.cc | 6 +----- 14 files changed, 46 insertions(+), 18 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index c5cbab0697bf4..fc93cdc83c50a 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -141,6 +141,12 @@ if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin") # macOS. See https://github.com/boostorg/stacktrace/issues/88 and comments # therein. add_compile_definitions(_GNU_SOURCE) + + # Only make default symbol visibility as 'default' when we want to build test + # or benchmark + if(BUILD_TESTS OR BUILD_BENCHMARKS) + add_compile_options(-fvisibility=default) + endif() endif() set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${KNOWN_WARNINGS}") diff --git a/cpp/core/benchmarks/CompressionBenchmark.cc b/cpp/core/benchmarks/CompressionBenchmark.cc index 1966c8ef1c189..808ce9192a4be 100644 --- a/cpp/core/benchmarks/CompressionBenchmark.cc +++ b/cpp/core/benchmarks/CompressionBenchmark.cc @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -213,11 +214,21 @@ class BenchmarkCompression { } protected: - long setCpu(uint32_t cpuindex) { + void setCpu(uint32_t cpuindex) { +#ifndef __APPLE__ + static const auto kTotalCores = std::thread::hardware_concurrency(); + cpuindex = cpuindex % kTotalCores; cpu_set_t cs; CPU_ZERO(&cs); CPU_SET(cpuindex, &cs); - return sched_setaffinity(0, sizeof(cs), &cs); + if (sched_setaffinity(0, sizeof(cs), &cs) == -1) { + LOG(WARNING) << "Error binding CPU " << std::to_string(cpuindex); + exit(EXIT_FAILURE); + } +#else + LOG(WARNING) << "Binding CPU is currently not supported on macOS." << std::endl; + exit(EXIT_FAILURE); +#endif } virtual void doCompress( diff --git a/cpp/core/jni/JniCommon.h b/cpp/core/jni/JniCommon.h index 1d784f3a5eda8..817624c8dcd12 100644 --- a/cpp/core/jni/JniCommon.h +++ b/cpp/core/jni/JniCommon.h @@ -237,7 +237,7 @@ class SafeNativeArray { private: SafeNativeArray(JNIEnv* env, JavaArrayType javaArray, JniNativeArrayType nativeArray) - : env_(env), javaArray_(javaArray), nativeArray_(nativeArray){}; + : env_(env), javaArray_(javaArray), nativeArray_(nativeArray) {}; JNIEnv* env_; JavaArrayType javaArray_; diff --git a/cpp/core/utils/ObjectStore.h b/cpp/core/utils/ObjectStore.h index 5a38a1af33311..1580035c8e1ac 100644 --- a/cpp/core/utils/ObjectStore.h +++ b/cpp/core/utils/ObjectStore.h @@ -92,7 +92,7 @@ class ObjectStore { void release0(ResourceHandle handle); - ObjectStore(StoreHandle storeId) : storeId_(storeId){}; + ObjectStore(StoreHandle storeId) : storeId_(storeId) {}; StoreHandle storeId_; ResourceMap> store_; std::set aliveObjects_; diff --git a/cpp/core/utils/macros.h b/cpp/core/utils/macros.h index fef8ae1e75de9..55a38ef600c11 100644 --- a/cpp/core/utils/macros.h +++ b/cpp/core/utils/macros.h @@ -105,4 +105,4 @@ #define TIME_NANO_TO_STRING(time) \ (time > 1e7 ? time / 1e6 : ((time > 1e4) ? time / 1e3 : time)) << (time > 1e7 ? "ms" : (time > 1e4 ? "us" : "ns")) -#define ROUND_TO_LINE(n, round) (((n) + (round)-1) & ~((round)-1)) +#define ROUND_TO_LINE(n, round) (((n) + (round) - 1) & ~((round) - 1)) diff --git a/cpp/core/utils/qpl/qpl_codec.cc b/cpp/core/utils/qpl/qpl_codec.cc index a1d6970b265c9..39d626eb4eee3 100644 --- a/cpp/core/utils/qpl/qpl_codec.cc +++ b/cpp/core/utils/qpl/qpl_codec.cc @@ -30,7 +30,7 @@ class HardwareCodecDeflateQpl { /// RET_ERROR stands for hardware codec fail,need fallback to software codec. static constexpr int64_t RET_ERROR = -1; - explicit HardwareCodecDeflateQpl(qpl_compression_levels compressionLevel) : compressionLevel_(compressionLevel){}; + explicit HardwareCodecDeflateQpl(qpl_compression_levels compressionLevel) : compressionLevel_(compressionLevel) {}; int64_t doCompressData(const uint8_t* source, uint32_t source_size, uint8_t* dest, uint32_t dest_size) const { uint32_t job_id; @@ -99,7 +99,7 @@ class HardwareCodecDeflateQpl { class SoftwareCodecDeflateQpl final { public: - explicit SoftwareCodecDeflateQpl(qpl_compression_levels compressionLevel) : compressionLevel_(compressionLevel){}; + explicit SoftwareCodecDeflateQpl(qpl_compression_levels compressionLevel) : compressionLevel_(compressionLevel) {}; ~SoftwareCodecDeflateQpl() { if (swJob) { diff --git a/cpp/velox/benchmarks/ColumnarToRowBenchmark.cc b/cpp/velox/benchmarks/ColumnarToRowBenchmark.cc index 8a55050015d03..9c1e10f175dec 100644 --- a/cpp/velox/benchmarks/ColumnarToRowBenchmark.cc +++ b/cpp/velox/benchmarks/ColumnarToRowBenchmark.cc @@ -87,10 +87,15 @@ class GoogleBenchmarkColumnarToRow { protected: long setCpu(uint32_t cpuindex) { +#ifndef __APPLE__ cpu_set_t cs; CPU_ZERO(&cs); CPU_SET(cpuindex, &cs); return sched_setaffinity(0, sizeof(cs), &cs); +#else + LOG(WARNING) << "Binding CPU is currently not supported on macOS." << std::endl; + exit(EXIT_FAILURE); +#endif } velox::VectorPtr recordBatch2RowVector(const arrow::RecordBatch& rb) { diff --git a/cpp/velox/benchmarks/ParquetWriteBenchmark.cc b/cpp/velox/benchmarks/ParquetWriteBenchmark.cc index 45348ed4a63b1..0904e48dec67c 100644 --- a/cpp/velox/benchmarks/ParquetWriteBenchmark.cc +++ b/cpp/velox/benchmarks/ParquetWriteBenchmark.cc @@ -92,10 +92,15 @@ class GoogleBenchmarkParquetWrite { protected: long setCpu(uint32_t cpuindex) { +#ifndef __APPLE__ cpu_set_t cs; CPU_ZERO(&cs); CPU_SET(cpuindex, &cs); return sched_setaffinity(0, sizeof(cs), &cs); +#else + LOG(WARNING) << "Binding CPU is currently not supported on macOS." << std::endl; + exit(EXIT_FAILURE); +#endif } std::shared_ptr recordBatch2VeloxColumnarBatch(const arrow::RecordBatch& rb) { diff --git a/cpp/velox/benchmarks/common/BenchmarkUtils.cc b/cpp/velox/benchmarks/common/BenchmarkUtils.cc index 345f9da8e16d1..66afd7f5e9c4d 100644 --- a/cpp/velox/benchmarks/common/BenchmarkUtils.cc +++ b/cpp/velox/benchmarks/common/BenchmarkUtils.cc @@ -140,6 +140,7 @@ std::shared_ptr createReader(const std::string& path) #endif void setCpu(uint32_t cpuindex) { +#ifndef __APPLE__ static const auto kTotalCores = std::thread::hardware_concurrency(); cpuindex = cpuindex % kTotalCores; cpu_set_t cs; @@ -149,6 +150,10 @@ void setCpu(uint32_t cpuindex) { LOG(WARNING) << "Error binding CPU " << std::to_string(cpuindex); exit(EXIT_FAILURE); } +#else + LOG(WARNING) << "Binding CPU is currently not supported on macOS." << std::endl; + exit(EXIT_FAILURE); +#endif } arrow::Status diff --git a/cpp/velox/operators/plannodes/RowVectorStream.h b/cpp/velox/operators/plannodes/RowVectorStream.h index c72e9137f4a4c..91a378e9780f7 100644 --- a/cpp/velox/operators/plannodes/RowVectorStream.h +++ b/cpp/velox/operators/plannodes/RowVectorStream.h @@ -88,7 +88,7 @@ class ValueStreamNode final : public facebook::velox::core::PlanNode { } private: - void addDetails(std::stringstream& stream) const override{}; + void addDetails(std::stringstream& stream) const override {}; const facebook::velox::RowTypePtr outputType_; std::unique_ptr valueStream_; diff --git a/cpp/velox/tests/CMakeLists.txt b/cpp/velox/tests/CMakeLists.txt index b8ea12e944aae..52af155b892cd 100644 --- a/cpp/velox/tests/CMakeLists.txt +++ b/cpp/velox/tests/CMakeLists.txt @@ -27,7 +27,7 @@ function(add_velox_test TEST_EXEC) endif() add_executable(${TEST_EXEC} ${SOURCES} ${VELOX_TEST_COMMON_SRCS}) target_include_directories( - ${TEST_EXEC} PRIVATE ${CMAKE_SOURCE_DIR}/velox ${CMAKE_SOURCE_DIR}/src + ${TEST_EXEC} PRIVATE ${CMAKE_SOURCE_DIR}/src ${VELOX_BUILD_PATH}/_deps/duckdb-src/src/include) target_link_libraries(${TEST_EXEC} velox GTest::gtest GTest::gtest_main google::glog) diff --git a/cpp/velox/tests/FunctionTest.cc b/cpp/velox/tests/FunctionTest.cc index b55b64ba98117..73b3898e2f020 100644 --- a/cpp/velox/tests/FunctionTest.cc +++ b/cpp/velox/tests/FunctionTest.cc @@ -72,7 +72,7 @@ TEST_F(FunctionTest, getIdxFromNodeName) { TEST_F(FunctionTest, getNameBeforeDelimiter) { std::string functionSpec = "lte:fp64_fp64"; - std::string_view funcName = SubstraitParser::getNameBeforeDelimiter(functionSpec); + std::string funcName = SubstraitParser::getNameBeforeDelimiter(functionSpec); ASSERT_EQ(funcName, "lte"); functionSpec = "lte:"; diff --git a/cpp/velox/tests/VeloxRowToColumnarTest.cc b/cpp/velox/tests/VeloxRowToColumnarTest.cc index c784dbd59c346..8994aef8711d0 100644 --- a/cpp/velox/tests/VeloxRowToColumnarTest.cc +++ b/cpp/velox/tests/VeloxRowToColumnarTest.cc @@ -42,16 +42,16 @@ class VeloxRowToColumnarTest : public ::testing::Test, public test::VectorTestBa uint8_t* address = columnarToRowConverter->getBufferAddress(); auto lengthVec = columnarToRowConverter->getLengths(); - long lengthArr[lengthVec.size()]; + std::vector int64Lengths(lengthVec.size()); for (int i = 0; i < lengthVec.size(); i++) { - lengthArr[i] = lengthVec[i]; + int64Lengths[i] = lengthVec[i]; } ArrowSchema cSchema; toArrowSchema(vector->type(), pool(), &cSchema); auto rowToColumnarConverter = std::make_shared(&cSchema, pool_); - auto cb = rowToColumnarConverter->convert(numRows, lengthArr, address); + auto cb = rowToColumnarConverter->convert(numRows, int64Lengths.data(), address); auto vp = std::dynamic_pointer_cast(cb)->getRowVector(); velox::test::assertEqualVectors(vector, vp); } diff --git a/cpp/velox/tests/VeloxShuffleWriterTest.cc b/cpp/velox/tests/VeloxShuffleWriterTest.cc index 7cbfbcd79cc95..47b751dcea21e 100644 --- a/cpp/velox/tests/VeloxShuffleWriterTest.cc +++ b/cpp/velox/tests/VeloxShuffleWriterTest.cc @@ -165,11 +165,7 @@ TEST_P(HashPartitioningShuffleWriter, hashPart1Vector) { makeFlatVector( 4, [](vector_size_t row) { return row % 2; }, nullEvery(5), DATE()), makeFlatVector( - 4, - [](vector_size_t row) { - return Timestamp{row % 2, 0}; - }, - nullEvery(5)), + 4, [](vector_size_t row) { return Timestamp{row % 2, 0}; }, nullEvery(5)), }); auto rowType = facebook::velox::asRowType(vector->type());