This is an automated email from the ASF dual-hosted git repository.

uwe pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new 75c8357  ARROW-4349: [C++] Add static linking option for benchmarks, 
fix Windows benchmark build failures
75c8357 is described below

commit 75c83578ced6de80015f3b93c1d290b9e74430e5
Author: Wes McKinney <[email protected]>
AuthorDate: Thu Jan 24 14:51:51 2019 +0100

    ARROW-4349: [C++] Add static linking option for benchmarks, fix Windows 
benchmark build failures
    
    I disabled the benchmarks with preprocessor macros in arrow-file-benchmark 
per ARROW-4187 -- that will require some more work to use Windows APIs.
    
    Because the Parquet benchmarks require some Thrift internal symbols at 
present, I had to add an option to statically link things
    
    Author: Wes McKinney <[email protected]>
    
    Closes #3468 from wesm/ARROW-4349 and squashes the following commits:
    
    378ea366 <Wes McKinney> consistent macro name
    f38b4036 <Wes McKinney> Only statically link Parquet benchmarks on Windows. 
Add ADD_PARQUET_BENCHMARK function for DRY
    0c183bf8 <Wes McKinney> Add static linking option for benchmarks, fix all 
Windows benchmark builds
---
 cpp/cmake_modules/BuildUtils.cmake               | 18 +++++++----
 cpp/src/arrow/io/file-benchmark.cc               |  8 +++++
 cpp/src/parquet/CMakeLists.txt                   | 39 +++++++++++++++++-------
 cpp/src/parquet/arrow/CMakeLists.txt             |  6 ++--
 cpp/src/parquet/arrow/reader-writer-benchmark.cc |  7 +++--
 cpp/src/parquet/encoding-benchmark.cc            |  4 +--
 6 files changed, 57 insertions(+), 25 deletions(-)

diff --git a/cpp/cmake_modules/BuildUtils.cmake 
b/cpp/cmake_modules/BuildUtils.cmake
index fffd158..1275bc0 100644
--- a/cpp/cmake_modules/BuildUtils.cmake
+++ b/cpp/cmake_modules/BuildUtils.cmake
@@ -189,7 +189,7 @@ function(ADD_ARROW_LIB LIB_NAME)
       # expecting that the symbols have been loaded separately. This happens
       # with libpython* where there can be conflicts between system Python and
       # the Python from a thirdparty distribution
-      # 
+      #
       # When running with the Emscripten Compiler, we need not worry about
       # python, and the Emscripten Compiler does not support this option.
       set(ARG_SHARED_LINK_FLAGS
@@ -213,7 +213,7 @@ function(ADD_ARROW_LIB LIB_NAME)
     endif()
 
     target_link_libraries(${LIB_NAME}_shared
-      LINK_PUBLIC 
+      LINK_PUBLIC
       "$<BUILD_INTERFACE:${ARG_SHARED_LINK_LIBS}>"
       "$<INSTALL_INTERFACE:${INTERFACE_LIBS}>"
       LINK_PRIVATE ${ARG_SHARED_PRIVATE_LINK_LIBS})
@@ -292,7 +292,7 @@ function(ADD_ARROW_LIB LIB_NAME)
     endif()
 
     target_link_libraries(${LIB_NAME}_static
-      LINK_PUBLIC 
+      LINK_PUBLIC
       "$<BUILD_INTERFACE:${ARG_STATIC_LINK_LIBS}>"
       "$<INSTALL_INTERFACE:${INTERFACE_LIBS}>")
 
@@ -339,7 +339,7 @@ endfunction()
 function(ADD_BENCHMARK REL_BENCHMARK_NAME)
   set(options)
   set(one_value_args)
-  set(multi_value_args EXTRA_LINK_LIBS DEPENDENCIES PREFIX LABELS)
+  set(multi_value_args EXTRA_LINK_LIBS STATIC_LINK_LIBS DEPENDENCIES PREFIX 
LABELS)
   cmake_parse_arguments(ARG "${options}" "${one_value_args}" 
"${multi_value_args}" ${ARGN})
   if(ARG_UNPARSED_ARGUMENTS)
     message(SEND_ERROR "Error: unrecognized arguments: 
${ARG_UNPARSED_ARGUMENTS}")
@@ -358,12 +358,18 @@ function(ADD_BENCHMARK REL_BENCHMARK_NAME)
     # This benchmark has a corresponding .cc file, set it up as an executable.
     set(BENCHMARK_PATH "${EXECUTABLE_OUTPUT_PATH}/${BENCHMARK_NAME}")
     add_executable(${BENCHMARK_NAME} "${REL_BENCHMARK_NAME}.cc")
-    target_link_libraries(${BENCHMARK_NAME} ${ARROW_BENCHMARK_LINK_LIBS})
+
+    if (ARG_STATIC_LINK_LIBS)
+      # Customize link libraries
+      target_link_libraries(${BENCHMARK_NAME} PRIVATE ${ARG_STATIC_LINK_LIBS})
+    else()
+      target_link_libraries(${BENCHMARK_NAME} PRIVATE 
${ARROW_BENCHMARK_LINK_LIBS})
+    endif()
     add_dependencies(benchmark ${BENCHMARK_NAME})
     set(NO_COLOR "--color_print=false")
 
     if (ARG_EXTRA_LINK_LIBS)
-      target_link_libraries(${BENCHMARK_NAME} ${ARG_EXTRA_LINK_LIBS})
+      target_link_libraries(${BENCHMARK_NAME} PRIVATE ${ARG_EXTRA_LINK_LIBS})
     endif()
   else()
     # No executable, just invoke the benchmark (probably a script) directly.
diff --git a/cpp/src/arrow/io/file-benchmark.cc 
b/cpp/src/arrow/io/file-benchmark.cc
index 4439a18..3e99ba0 100644
--- a/cpp/src/arrow/io/file-benchmark.cc
+++ b/cpp/src/arrow/io/file-benchmark.cc
@@ -30,12 +30,18 @@
 #include <thread>
 #include <valarray>
 
+#ifndef _WIN32
+
 #include <fcntl.h>
 #include <poll.h>
 #include <unistd.h>
 
+#endif
+
 namespace arrow {
 
+#ifndef _WIN32
+
 std::string GetNullFile() { return "/dev/null"; }
 
 const std::valarray<int64_t> small_sizes = {8, 24, 33, 1, 32, 192, 16, 40};
@@ -244,4 +250,6 @@ BENCHMARK(BM_BufferedOutputStreamLargeWritesToPipe)
     ->MinTime(1.0)
     ->UseRealTime();
 
+#endif  // ifndef _WIN32
+
 }  // namespace arrow
diff --git a/cpp/src/parquet/CMakeLists.txt b/cpp/src/parquet/CMakeLists.txt
index f679672..54007df 100644
--- a/cpp/src/parquet/CMakeLists.txt
+++ b/cpp/src/parquet/CMakeLists.txt
@@ -55,6 +55,23 @@ function(ADD_PARQUET_TEST REL_TEST_NAME)
   endif()
 endfunction()
 
+function(ADD_PARQUET_BENCHMARK REL_TEST_NAME)
+  set(options)
+  set(one_value_args PREFIX)
+  set(multi_value_args)
+  cmake_parse_arguments(ARG "${options}" "${one_value_args}" 
"${multi_value_args}" ${ARGN})
+  if (ARG_PREFIX)
+    set(PREFIX ${ARG_PREFIX})
+  else()
+    set(PREFIX "parquet")
+  endif()
+  ADD_BENCHMARK(${REL_TEST_NAME}
+    PREFIX ${PREFIX}
+    LABELS "parquet-benchmarks"
+    ${PARQUET_BENCHMARK_LINK_OPTION}
+    ${ARG_UNPARSED_ARGUMENTS})
+endfunction()
+
 # ----------------------------------------------------------------------
 # Link libraries setup
 
@@ -106,9 +123,15 @@ set(PARQUET_STATIC_TEST_LINK_LIBS
   ${ARROW_LIBRARIES_FOR_STATIC_TESTS}
   parquet_static)
 
-set(PARQUET_BENCHMARK_LINK_LIBRARIES
-  arrow_benchmark_main
-  parquet_shared)
+if (WIN32)
+  # The benchmarks depend on some static Thrift symbols
+  set(PARQUET_BENCHMARK_LINK_OPTION
+    STATIC_LINK_LIBS arrow_benchmark_main
+    parquet_static)
+else()
+  set(PARQUET_BENCHMARK_LINK_OPTION
+    EXTRA_LINK_LIBS parquet_shared)
+endif()
 
 ############################################################
 # Generated Thrift sources
@@ -274,14 +297,8 @@ ADD_PARQUET_TEST(reader-test)
 ADD_PARQUET_TEST(file-deserialize-test USE_STATIC_LINKING)
 ADD_PARQUET_TEST(schema-test USE_STATIC_LINKING)
 
-ADD_ARROW_BENCHMARK(column-io-benchmark
-  PREFIX "parquet"
-  LABELS "parquet-benchmarks"
-  EXTRA_LINK_LIBS ${PARQUET_BENCHMARK_LINK_LIBRARIES})
-ADD_ARROW_BENCHMARK(encoding-benchmark
-  PREFIX "parquet"
-  LABELS "parquet-benchmarks"
-  EXTRA_LINK_LIBS ${PARQUET_BENCHMARK_LINK_LIBRARIES})
+ADD_PARQUET_BENCHMARK(column-io-benchmark)
+ADD_PARQUET_BENCHMARK(encoding-benchmark)
 
 # Required for tests, the ExternalProject for zstd does not build on CMake < 
3.7
 if (ARROW_WITH_ZSTD)
diff --git a/cpp/src/parquet/arrow/CMakeLists.txt 
b/cpp/src/parquet/arrow/CMakeLists.txt
index f4e4f7e..ba9e93d 100644
--- a/cpp/src/parquet/arrow/CMakeLists.txt
+++ b/cpp/src/parquet/arrow/CMakeLists.txt
@@ -18,9 +18,7 @@
 ADD_PARQUET_TEST(arrow-schema-test)
 ADD_PARQUET_TEST(arrow-reader-writer-test)
 
-ADD_BENCHMARK(reader-writer-benchmark
-  PREFIX "parquet-arrow"
-  LABELS "parquet-benchmarks"
-  EXTRA_LINK_LIBS ${PARQUET_BENCHMARK_LINK_LIBRARIES})
+ADD_PARQUET_BENCHMARK(reader-writer-benchmark
+  PREFIX "parquet-arrow")
 
 ARROW_INSTALL_ALL_HEADERS("parquet/arrow")
diff --git a/cpp/src/parquet/arrow/reader-writer-benchmark.cc 
b/cpp/src/parquet/arrow/reader-writer-benchmark.cc
index 775c102..1889006 100644
--- a/cpp/src/parquet/arrow/reader-writer-benchmark.cc
+++ b/cpp/src/parquet/arrow/reader-writer-benchmark.cc
@@ -142,7 +142,8 @@ std::shared_ptr<::arrow::Table> 
TableFromVector<BooleanType>(const std::vector<b
 
 template <bool nullable, typename ParquetType>
 static void BM_WriteColumn(::benchmark::State& state) {
-  std::vector<typename ParquetType::c_type> values(BENCHMARK_SIZE, 128);
+  using T = typename ParquetType::c_type;
+  std::vector<T> values(BENCHMARK_SIZE, static_cast<T>(128));
   std::shared_ptr<::arrow::Table> table = TableFromVector<ParquetType>(values, 
nullable);
 
   while (state.KeepRunning()) {
@@ -167,7 +168,9 @@ BENCHMARK_TEMPLATE2(BM_WriteColumn, true, BooleanType);
 
 template <bool nullable, typename ParquetType>
 static void BM_ReadColumn(::benchmark::State& state) {
-  std::vector<typename ParquetType::c_type> values(BENCHMARK_SIZE, 128);
+  using T = typename ParquetType::c_type;
+
+  std::vector<T> values(BENCHMARK_SIZE, static_cast<T>(128));
   std::shared_ptr<::arrow::Table> table = TableFromVector<ParquetType>(values, 
nullable);
   auto output = std::make_shared<InMemoryOutputStream>();
   EXIT_NOT_OK(WriteTable(*table, ::arrow::default_memory_pool(), output, 
BENCHMARK_SIZE));
diff --git a/cpp/src/parquet/encoding-benchmark.cc 
b/cpp/src/parquet/encoding-benchmark.cc
index f8d2839..48183b4 100644
--- a/cpp/src/parquet/encoding-benchmark.cc
+++ b/cpp/src/parquet/encoding-benchmark.cc
@@ -36,7 +36,7 @@ std::shared_ptr<ColumnDescriptor> 
Int64Schema(Repetition::type repetition) {
 }
 
 static void BM_PlainEncodingBoolean(::benchmark::State& state) {
-  std::vector<bool> values(state.range(0), 64);
+  std::vector<bool> values(state.range(0), true);
   PlainEncoder<BooleanType> encoder(nullptr);
 
   while (state.KeepRunning()) {
@@ -49,7 +49,7 @@ static void BM_PlainEncodingBoolean(::benchmark::State& 
state) {
 BENCHMARK(BM_PlainEncodingBoolean)->Range(1024, 65536);
 
 static void BM_PlainDecodingBoolean(::benchmark::State& state) {
-  std::vector<bool> values(state.range(0), 64);
+  std::vector<bool> values(state.range(0), true);
   bool* output = new bool[state.range(0)];
   PlainEncoder<BooleanType> encoder(nullptr);
   encoder.Put(values, static_cast<int>(values.size()));

Reply via email to