[GLUTEN-6550] Fix build failure when BUILD_TESTS is enabled on macOS

apache · Aug 17, 2024 · 0ba176a · 0ba176a
1 parent a038e93
commit 0ba176a
Show file tree

Hide file tree

Showing 14 changed files with 46 additions and 18 deletions.
diff --git 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
@@ -29,6 +29,7 @@
 #include <parquet/arrow/reader.h>
 #include <parquet/file_reader.h>
 #include <sched.h>
+#include <thread>
 
 #include <chrono>
 #include <iostream>
@@ -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
@@ -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
@@ -92,7 +92,7 @@ class ObjectStore {
 
   void release0(ResourceHandle handle);
 
-  ObjectStore(StoreHandle storeId) : storeId_(storeId){};
+  ObjectStore(StoreHandle storeId) : storeId_(storeId) {};
   StoreHandle storeId_;
   ResourceMap<std::shared_ptr<void>> store_;
   std::set<ResourceHandle> aliveObjects_;

diff --git 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
@@ -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
@@ -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
@@ -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<ColumnarBatch> recordBatch2VeloxColumnarBatch(const arrow::RecordBatch& rb) {

diff --git a/cpp/velox/benchmarks/common/BenchmarkUtils.cc b/cpp/velox/benchmarks/common/BenchmarkUtils.cc
@@ -140,6 +140,7 @@ std::shared_ptr<arrow::RecordBatchReader> 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
@@ -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<RowVectorStream> valueStream_;

diff --git 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
@@ -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
@@ -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<int64_t> 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<VeloxRowToColumnarConverter>(&cSchema, pool_);
 
-    auto cb = rowToColumnarConverter->convert(numRows, lengthArr, address);
+    auto cb = rowToColumnarConverter->convert(numRows, int64Lengths.data(), address);
     auto vp = std::dynamic_pointer_cast<VeloxColumnarBatch>(cb)->getRowVector();
     velox::test::assertEqualVectors(vector, vp);
   }

diff --git a/cpp/velox/tests/VeloxShuffleWriterTest.cc b/cpp/velox/tests/VeloxShuffleWriterTest.cc
@@ -165,11 +165,7 @@ TEST_P(HashPartitioningShuffleWriter, hashPart1Vector) {
       makeFlatVector<int32_t>(
           4, [](vector_size_t row) { return row % 2; }, nullEvery(5), DATE()),
       makeFlatVector<Timestamp>(
-          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());