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