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

apache · Jul 25, 2024 · bb81d54 · bb81d54
1 parent 751fa83
commit bb81d54
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 8 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
@@ -213,11 +213,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/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/shuffle/VeloxSortShuffleWriter.cc b/cpp/velox/shuffle/VeloxSortShuffleWriter.cc
@@ -278,7 +278,7 @@ uint32_t VeloxSortShuffleWriter::maxRowsToInsert(uint32_t offset, uint32_t rows)
 }
 
 void VeloxSortShuffleWriter::acquireNewBuffer(int64_t memLimit, uint64_t minSizeRequired) {
-  auto size = std::max(std::min((uint64_t)memLimit >> 2, 64UL * 1024 * 1024), minSizeRequired);
+  auto size = std::max(std::min((uint64_t)memLimit >> 2, (uint64_t) 64UL * 1024 * 1024), minSizeRequired);
   // Allocating new buffer can trigger spill.
   auto newBuffer = facebook::velox::AlignedBuffer::allocate<char>(size, veloxPool_.get(), 0);
   pages_.emplace_back(std::move(newBuffer));

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);
   }