You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/05/14 04:34:56 UTC
[GitHub] [arrow] emkornfield opened a new pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
emkornfield opened a new pull request #7175:
URL: https://github.com/apache/arrow/pull/7175
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#discussion_r426591555
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,37 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
+template <typename T>
+std::vector<T> RandomVector(int64_t true_percentage, int64_t vector_size,
+ const std::array<T, 2>& sample_values) {
+ std::vector<T> values(BENCHMARK_SIZE, {});
+ if (true_percentage == kAlternatingOrNa) {
+ int n = {0};
+ std::generate(values.begin(), values.end(), [&n] { return n++ % 2; });
+ } else {
+ std::default_random_engine rng(500);
+ double true_probability = static_cast<double>(true_percentage) / 100.0;
+ std::bernoulli_distribution dist(true_probability);
+ std::generate(values.begin(), values.end(), [&] { return sample_values[dist(rng)]; });
+ }
+ return values;
+}
+
template <typename ParquetType>
std::shared_ptr<::arrow::Table> TableFromVector(
- const std::vector<typename ParquetType::c_type>& vec, bool nullable) {
+ const std::vector<typename ParquetType::c_type>& vec, bool nullable,
+ int64_t null_percentage = kAlternatingOrNa) {
+ if (!nullable) {
+ DCHECK(null_percentage = kAlternatingOrNa);
+ }
std::shared_ptr<::arrow::DataType> type = std::make_shared<ArrowType<ParquetType>>();
NumericBuilder<ArrowType<ParquetType>> builder;
if (nullable) {
- std::vector<uint8_t> valid_bytes(BENCHMARK_SIZE, 0);
- int n = {0};
- std::generate(valid_bytes.begin(), valid_bytes.end(), [&n] { return n++ % 2; });
+ // Note true values select index 1 of sample_values
+ auto valid_bytes = RandomVector<uint8_t>(/*true_percengate=*/null_percentage,
+ BENCHMARK_SIZE, /*sample_values=*/{1, 0});
Review comment:
Does this mean the valid bitmap only contains bytes `0x00` and `0x01`?
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,37 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
+template <typename T>
+std::vector<T> RandomVector(int64_t true_percentage, int64_t vector_size,
Review comment:
Can't this be factored out in `testing/random.h`?
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,37 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
+template <typename T>
+std::vector<T> RandomVector(int64_t true_percentage, int64_t vector_size,
+ const std::array<T, 2>& sample_values) {
+ std::vector<T> values(BENCHMARK_SIZE, {});
+ if (true_percentage == kAlternatingOrNa) {
+ int n = {0};
+ std::generate(values.begin(), values.end(), [&n] { return n++ % 2; });
+ } else {
+ std::default_random_engine rng(500);
+ double true_probability = static_cast<double>(true_percentage) / 100.0;
+ std::bernoulli_distribution dist(true_probability);
+ std::generate(values.begin(), values.end(), [&] { return sample_values[dist(rng)]; });
+ }
+ return values;
+}
+
template <typename ParquetType>
std::shared_ptr<::arrow::Table> TableFromVector(
- const std::vector<typename ParquetType::c_type>& vec, bool nullable) {
+ const std::vector<typename ParquetType::c_type>& vec, bool nullable,
+ int64_t null_percentage = kAlternatingOrNa) {
+ if (!nullable) {
+ DCHECK(null_percentage = kAlternatingOrNa);
Review comment:
`ARROW_CHECK_EQ`. There's an error in your statement. Also, `DCHECK`s are compiled out in non-debug mode, which is usually the case for benchmarks...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] fsaintjacques commented on pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
fsaintjacques commented on pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#issuecomment-630510263
I just validated and the reader opening doesn't show up in profile much. On a more interesting note, `BM_ReadColumn<true,Int32Type>` reflects a lot the profile I get with real-life dataset (nyc taxi dataset). If this can guide you in further performance validation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#issuecomment-628383810
https://issues.apache.org/jira/browse/ARROW-8794
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#discussion_r426996139
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,37 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
+template <typename T>
+std::vector<T> RandomVector(int64_t true_percentage, int64_t vector_size,
+ const std::array<T, 2>& sample_values) {
+ std::vector<T> values(BENCHMARK_SIZE, {});
+ if (true_percentage == kAlternatingOrNa) {
+ int n = {0};
+ std::generate(values.begin(), values.end(), [&n] { return n++ % 2; });
+ } else {
+ std::default_random_engine rng(500);
+ double true_probability = static_cast<double>(true_percentage) / 100.0;
+ std::bernoulli_distribution dist(true_probability);
+ std::generate(values.begin(), values.end(), [&] { return sample_values[dist(rng)]; });
+ }
+ return values;
+}
+
template <typename ParquetType>
std::shared_ptr<::arrow::Table> TableFromVector(
- const std::vector<typename ParquetType::c_type>& vec, bool nullable) {
+ const std::vector<typename ParquetType::c_type>& vec, bool nullable,
+ int64_t null_percentage = kAlternatingOrNa) {
+ if (!nullable) {
+ DCHECK(null_percentage = kAlternatingOrNa);
+ }
std::shared_ptr<::arrow::DataType> type = std::make_shared<ArrowType<ParquetType>>();
NumericBuilder<ArrowType<ParquetType>> builder;
if (nullable) {
- std::vector<uint8_t> valid_bytes(BENCHMARK_SIZE, 0);
- int n = {0};
- std::generate(valid_bytes.begin(), valid_bytes.end(), [&n] { return n++ % 2; });
+ // Note true values select index 1 of sample_values
+ auto valid_bytes = RandomVector<uint8_t>(/*true_percengate=*/null_percentage,
+ BENCHMARK_SIZE, /*sample_values=*/{1, 0});
Review comment:
I do not think that is [true](https://arrow.apache.org/docs/cpp/api/builder.html#_CPPv4N5arrow14NumericBuilder12AppendValuesEPK10value_type7int64_tPK7uint8_t) it is a confusing contract (maybe taking bool* would be better?) but I read this as converting 1 and 0 to corresponding bits (Under the covers if I traced correctly this calls [`ArrayBuilder::UnsafeAppendToBitmap`](https://github.com/apache/arrow/blob/2849b643883793347ab87e0770aba4ccbda34c90/cpp/src/arrow/array/builder_primitive.cc#L96) which ultimately calls [GenerateBitsUnrolled](https://github.com/apache/arrow/blob/b4bd0d869dfa398dcaa65a1f4f6f13015d7fe8c4/cpp/src/arrow/buffer_builder.h#L305) which [coverts bytes to bits](https://github.com/apache/arrow/blob/bf722a01eebb42e5d49450dd6695469bac99ffcd/cpp/src/arrow/util/bit_util.h#L687) )
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
emkornfield commented on pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#issuecomment-630568952
> BM_ReadColumn<true,Int32Type> reflects a lot the profile I get with real-life dataset (nyc taxi dataset). If this can guide you in further performance validation.
I don't think I'm going to be doing much more performance related work past https://github.com/apache/arrow/pull/7143 (which if you don't mind trying out it would be good to see if that improves performance on real world data). The last potential easy performance win is pushing the all null/no nulls remaining checks directly into the loops (for small batch sizes I wouldn't expect a huge difference there). My main goal is to get full nested functionality working, and I got a little distracted
Other changes will probably require a bigger refactoring then I want to take on right now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#discussion_r426119989
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,29 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
template <typename ParquetType>
std::shared_ptr<::arrow::Table> TableFromVector(
- const std::vector<typename ParquetType::c_type>& vec, bool nullable) {
+ const std::vector<typename ParquetType::c_type>& vec, bool nullable,
+ int null_percentage = kAlternatingOrNa) {
Review comment:
done.
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -187,17 +219,56 @@ static void BM_ReadColumn(::benchmark::State& state) {
SetBytesProcessed<nullable, ParquetType>(state);
}
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int64Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int64Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, DoubleType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, DoubleType);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, BooleanType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, BooleanType);
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 1})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 10})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 50});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, /*first_value_percentage=*/0})
Review comment:
yep that sit added a comment at the very top.
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,29 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
template <typename ParquetType>
std::shared_ptr<::arrow::Table> TableFromVector(
- const std::vector<typename ParquetType::c_type>& vec, bool nullable) {
+ const std::vector<typename ParquetType::c_type>& vec, bool nullable,
+ int null_percentage = kAlternatingOrNa) {
+ if (!nullable) {
+ DCHECK(null_percentage = kAlternatingOrNa);
+ }
std::shared_ptr<::arrow::DataType> type = std::make_shared<ArrowType<ParquetType>>();
NumericBuilder<ArrowType<ParquetType>> builder;
if (nullable) {
std::vector<uint8_t> valid_bytes(BENCHMARK_SIZE, 0);
- int n = {0};
- std::generate(valid_bytes.begin(), valid_bytes.end(), [&n] { return n++ % 2; });
+ if (null_percentage == -1) {
Review comment:
yes
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -118,13 +134,22 @@ std::shared_ptr<::arrow::Table> TableFromVector(
template <>
std::shared_ptr<::arrow::Table> TableFromVector<BooleanType>(const std::vector<bool>& vec,
- bool nullable) {
+ bool nullable,
+ int null_percentage) {
BooleanBuilder builder;
if (nullable) {
std::vector<bool> valid_bytes(BENCHMARK_SIZE, 0);
- int n = {0};
- std::generate(valid_bytes.begin(), valid_bytes.end(),
- [&n] { return (n++ % 2) != 0; });
+ if (null_percentage == -1) {
Review comment:
done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#discussion_r427457149
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,37 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
+template <typename T>
+std::vector<T> RandomVector(int64_t true_percentage, int64_t vector_size,
+ const std::array<T, 2>& sample_values) {
+ std::vector<T> values(BENCHMARK_SIZE, {});
+ if (true_percentage == kAlternatingOrNa) {
+ int n = {0};
+ std::generate(values.begin(), values.end(), [&n] { return n++ % 2; });
+ } else {
+ std::default_random_engine rng(500);
+ double true_probability = static_cast<double>(true_percentage) / 100.0;
+ std::bernoulli_distribution dist(true_probability);
+ std::generate(values.begin(), values.end(), [&] { return sample_values[dist(rng)]; });
+ }
+ return values;
+}
+
template <typename ParquetType>
std::shared_ptr<::arrow::Table> TableFromVector(
- const std::vector<typename ParquetType::c_type>& vec, bool nullable) {
+ const std::vector<typename ParquetType::c_type>& vec, bool nullable,
+ int64_t null_percentage = kAlternatingOrNa) {
+ if (!nullable) {
+ ARROW_CHECK_EQ(null_percentage, kAlternatingOrNa);
+ }
std::shared_ptr<::arrow::DataType> type = std::make_shared<ArrowType<ParquetType>>();
NumericBuilder<ArrowType<ParquetType>> builder;
if (nullable) {
- std::vector<uint8_t> valid_bytes(BENCHMARK_SIZE, 0);
- int n = {0};
- std::generate(valid_bytes.begin(), valid_bytes.end(), [&n] { return n++ % 2; });
+ // Note true values select index 1 of sample_values
+ auto valid_bytes = RandomVector<uint8_t>(/*true_percengate=*/null_percentage,
Review comment:
fixed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#discussion_r426997534
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,37 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
+template <typename T>
+std::vector<T> RandomVector(int64_t true_percentage, int64_t vector_size,
+ const std::array<T, 2>& sample_values) {
+ std::vector<T> values(BENCHMARK_SIZE, {});
+ if (true_percentage == kAlternatingOrNa) {
+ int n = {0};
+ std::generate(values.begin(), values.end(), [&n] { return n++ % 2; });
+ } else {
+ std::default_random_engine rng(500);
+ double true_probability = static_cast<double>(true_percentage) / 100.0;
+ std::bernoulli_distribution dist(true_probability);
+ std::generate(values.begin(), values.end(), [&] { return sample_values[dist(rng)]; });
+ }
+ return values;
+}
+
template <typename ParquetType>
std::shared_ptr<::arrow::Table> TableFromVector(
- const std::vector<typename ParquetType::c_type>& vec, bool nullable) {
+ const std::vector<typename ParquetType::c_type>& vec, bool nullable,
+ int64_t null_percentage = kAlternatingOrNa) {
+ if (!nullable) {
+ DCHECK(null_percentage = kAlternatingOrNa);
Review comment:
thanks, good point.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#issuecomment-631450695
Rebased.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#discussion_r427100999
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,37 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
+template <typename T>
+std::vector<T> RandomVector(int64_t true_percentage, int64_t vector_size,
+ const std::array<T, 2>& sample_values) {
+ std::vector<T> values(BENCHMARK_SIZE, {});
+ if (true_percentage == kAlternatingOrNa) {
+ int n = {0};
+ std::generate(values.begin(), values.end(), [&n] { return n++ % 2; });
+ } else {
+ std::default_random_engine rng(500);
+ double true_probability = static_cast<double>(true_percentage) / 100.0;
+ std::bernoulli_distribution dist(true_probability);
+ std::generate(values.begin(), values.end(), [&] { return sample_values[dist(rng)]; });
+ }
+ return values;
+}
+
template <typename ParquetType>
std::shared_ptr<::arrow::Table> TableFromVector(
- const std::vector<typename ParquetType::c_type>& vec, bool nullable) {
+ const std::vector<typename ParquetType::c_type>& vec, bool nullable,
+ int64_t null_percentage = kAlternatingOrNa) {
+ if (!nullable) {
+ ARROW_CHECK_EQ(null_percentage, kAlternatingOrNa);
+ }
std::shared_ptr<::arrow::DataType> type = std::make_shared<ArrowType<ParquetType>>();
NumericBuilder<ArrowType<ParquetType>> builder;
if (nullable) {
- std::vector<uint8_t> valid_bytes(BENCHMARK_SIZE, 0);
- int n = {0};
- std::generate(valid_bytes.begin(), valid_bytes.end(), [&n] { return n++ % 2; });
+ // Note true values select index 1 of sample_values
+ auto valid_bytes = RandomVector<uint8_t>(/*true_percengate=*/null_percentage,
Review comment:
"true_percentage"
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#discussion_r426124767
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -187,17 +219,56 @@ static void BM_ReadColumn(::benchmark::State& state) {
SetBytesProcessed<nullable, ParquetType>(state);
}
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int64Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int64Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, DoubleType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, DoubleType);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, BooleanType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, BooleanType);
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 1})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 10})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 50});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, /*first_value_percentage=*/0})
+ ->Args({/*null_percentage=*/1, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/10, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/25, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/100})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/100});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int64Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 1})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 10})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 50});
+BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int64Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, /*first_value_percentage=*/0})
+ ->Args({/*null_percentage=*/1, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/5, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/10, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/25, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/30, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/35, /*first_value_percentage=*/25})
Review comment:
I adjusted these a little bit to have a little bit more consistency and bias towards runs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#discussion_r427100722
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,37 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
+template <typename T>
+std::vector<T> RandomVector(int64_t true_percentage, int64_t vector_size,
+ const std::array<T, 2>& sample_values) {
+ std::vector<T> values(BENCHMARK_SIZE, {});
+ if (true_percentage == kAlternatingOrNa) {
+ int n = {0};
+ std::generate(values.begin(), values.end(), [&n] { return n++ % 2; });
+ } else {
+ std::default_random_engine rng(500);
+ double true_probability = static_cast<double>(true_percentage) / 100.0;
+ std::bernoulli_distribution dist(true_probability);
+ std::generate(values.begin(), values.end(), [&] { return sample_values[dist(rng)]; });
+ }
+ return values;
+}
+
template <typename ParquetType>
std::shared_ptr<::arrow::Table> TableFromVector(
- const std::vector<typename ParquetType::c_type>& vec, bool nullable) {
+ const std::vector<typename ParquetType::c_type>& vec, bool nullable,
+ int64_t null_percentage = kAlternatingOrNa) {
+ if (!nullable) {
+ DCHECK(null_percentage = kAlternatingOrNa);
+ }
std::shared_ptr<::arrow::DataType> type = std::make_shared<ArrowType<ParquetType>>();
NumericBuilder<ArrowType<ParquetType>> builder;
if (nullable) {
- std::vector<uint8_t> valid_bytes(BENCHMARK_SIZE, 0);
- int n = {0};
- std::generate(valid_bytes.begin(), valid_bytes.end(), [&n] { return n++ % 2; });
+ // Note true values select index 1 of sample_values
+ auto valid_bytes = RandomVector<uint8_t>(/*true_percengate=*/null_percentage,
+ BENCHMARK_SIZE, /*sample_values=*/{1, 0});
Review comment:
Ah, I see. Pity.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#discussion_r425110880
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -118,13 +134,22 @@ std::shared_ptr<::arrow::Table> TableFromVector(
template <>
std::shared_ptr<::arrow::Table> TableFromVector<BooleanType>(const std::vector<bool>& vec,
- bool nullable) {
+ bool nullable,
+ int null_percentage) {
BooleanBuilder builder;
if (nullable) {
std::vector<bool> valid_bytes(BENCHMARK_SIZE, 0);
- int n = {0};
- std::generate(valid_bytes.begin(), valid_bytes.end(),
- [&n] { return (n++ % 2) != 0; });
+ if (null_percentage == -1) {
Review comment:
Perhaps nulls generation can be factored out?
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,29 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
template <typename ParquetType>
std::shared_ptr<::arrow::Table> TableFromVector(
- const std::vector<typename ParquetType::c_type>& vec, bool nullable) {
+ const std::vector<typename ParquetType::c_type>& vec, bool nullable,
+ int null_percentage = kAlternatingOrNa) {
+ if (!nullable) {
+ DCHECK(null_percentage = kAlternatingOrNa);
+ }
std::shared_ptr<::arrow::DataType> type = std::make_shared<ArrowType<ParquetType>>();
NumericBuilder<ArrowType<ParquetType>> builder;
if (nullable) {
std::vector<uint8_t> valid_bytes(BENCHMARK_SIZE, 0);
- int n = {0};
- std::generate(valid_bytes.begin(), valid_bytes.end(), [&n] { return n++ % 2; });
+ if (null_percentage == -1) {
Review comment:
Should this be `kAlternatingOrNa`?
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -187,17 +219,56 @@ static void BM_ReadColumn(::benchmark::State& state) {
SetBytesProcessed<nullable, ParquetType>(state);
}
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int64Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int64Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, DoubleType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, DoubleType);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, BooleanType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, BooleanType);
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 1})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 10})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 50});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, /*first_value_percentage=*/0})
Review comment:
Can you add a comment why we're varying `first_value_percentage`? (trigger RLE perhaps?)
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -187,17 +219,56 @@ static void BM_ReadColumn(::benchmark::State& state) {
SetBytesProcessed<nullable, ParquetType>(state);
}
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int64Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int64Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, DoubleType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, DoubleType);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, BooleanType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, BooleanType);
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 1})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 10})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 50});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, /*first_value_percentage=*/0})
+ ->Args({/*null_percentage=*/1, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/10, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/25, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/100})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/100});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int64Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 1})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 10})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 50});
+BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int64Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, /*first_value_percentage=*/0})
+ ->Args({/*null_percentage=*/1, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/5, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/10, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/25, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/30, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/35, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/45, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/1})
+ ->Args({/*null_percentage=*/75, /*first_value_percentage=*/1})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/100});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, DoubleType)
+ ->Args({kAlternatingOrNa, 0})
+ ->Args({1, 20});
Review comment:
Why `1`?
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,29 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
template <typename ParquetType>
std::shared_ptr<::arrow::Table> TableFromVector(
- const std::vector<typename ParquetType::c_type>& vec, bool nullable) {
+ const std::vector<typename ParquetType::c_type>& vec, bool nullable,
+ int null_percentage = kAlternatingOrNa) {
Review comment:
`int64_t` above
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -187,17 +219,56 @@ static void BM_ReadColumn(::benchmark::State& state) {
SetBytesProcessed<nullable, ParquetType>(state);
}
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int64Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int64Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, DoubleType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, DoubleType);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, BooleanType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, BooleanType);
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 1})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 10})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 50});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, /*first_value_percentage=*/0})
+ ->Args({/*null_percentage=*/1, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/10, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/25, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/100})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/100});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int64Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 1})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 10})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 50});
+BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int64Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, /*first_value_percentage=*/0})
+ ->Args({/*null_percentage=*/1, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/5, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/10, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/25, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/30, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/35, /*first_value_percentage=*/25})
Review comment:
Do we need such a granularity in `null_percentage` values?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#discussion_r426124722
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -187,17 +219,56 @@ static void BM_ReadColumn(::benchmark::State& state) {
SetBytesProcessed<nullable, ParquetType>(state);
}
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int64Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int64Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, DoubleType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, DoubleType);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, BooleanType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, BooleanType);
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 1})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 10})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 50});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, /*first_value_percentage=*/0})
+ ->Args({/*null_percentage=*/1, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/10, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/25, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/100})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/100});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int64Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 1})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 10})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 50});
+BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int64Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, /*first_value_percentage=*/0})
+ ->Args({/*null_percentage=*/1, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/5, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/10, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/25, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/30, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/35, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/45, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/1})
+ ->Args({/*null_percentage=*/75, /*first_value_percentage=*/1})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/100});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, DoubleType)
+ ->Args({kAlternatingOrNa, 0})
+ ->Args({1, 20});
Review comment:
mistake.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#discussion_r425268221
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -187,17 +219,56 @@ static void BM_ReadColumn(::benchmark::State& state) {
SetBytesProcessed<nullable, ParquetType>(state);
}
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int64Type);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int64Type);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, DoubleType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, DoubleType);
-
-BENCHMARK_TEMPLATE2(BM_ReadColumn, false, BooleanType);
-BENCHMARK_TEMPLATE2(BM_ReadColumn, true, BooleanType);
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 1})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 10})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 50});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int32Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, /*first_value_percentage=*/0})
+ ->Args({/*null_percentage=*/1, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/10, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/25, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/50, /*first_value_percentage=*/100})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/99, /*first_value_percentage=*/100});
+
+BENCHMARK_TEMPLATE2(BM_ReadColumn, false, Int64Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 1})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 10})
+ ->Args({/*null_percentage=*/kAlternatingOrNa, 50});
+BENCHMARK_TEMPLATE2(BM_ReadColumn, true, Int64Type)
+ ->Args({/*null_percentage=*/kAlternatingOrNa, /*first_value_percentage=*/0})
+ ->Args({/*null_percentage=*/1, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/5, /*first_value_percentage=*/10})
+ ->Args({/*null_percentage=*/10, /*first_value_percentage=*/50})
+ ->Args({/*null_percentage=*/25, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/30, /*first_value_percentage=*/25})
+ ->Args({/*null_percentage=*/35, /*first_value_percentage=*/25})
Review comment:
Maybe not permanently, but this uncovered an interesting pattern for https://github.com/apache/arrow/pull/7143
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] fsaintjacques commented on a change in pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
fsaintjacques commented on a change in pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#discussion_r426958606
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,37 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
+template <typename T>
+std::vector<T> RandomVector(int64_t true_percentage, int64_t vector_size,
Review comment:
It'll need to depend on libarrow_testing.so, not sure if this is a problem.
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,37 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
+template <typename T>
+std::vector<T> RandomVector(int64_t true_percentage, int64_t vector_size,
+ const std::array<T, 2>& sample_values) {
+ std::vector<T> values(BENCHMARK_SIZE, {});
+ if (true_percentage == kAlternatingOrNa) {
+ int n = {0};
+ std::generate(values.begin(), values.end(), [&n] { return n++ % 2; });
+ } else {
+ std::default_random_engine rng(500);
+ double true_probability = static_cast<double>(true_percentage) / 100.0;
+ std::bernoulli_distribution dist(true_probability);
+ std::generate(values.begin(), values.end(), [&] { return sample_values[dist(rng)]; });
+ }
+ return values;
+}
+
template <typename ParquetType>
std::shared_ptr<::arrow::Table> TableFromVector(
- const std::vector<typename ParquetType::c_type>& vec, bool nullable) {
+ const std::vector<typename ParquetType::c_type>& vec, bool nullable,
+ int64_t null_percentage = kAlternatingOrNa) {
+ if (!nullable) {
+ DCHECK(null_percentage = kAlternatingOrNa);
+ }
std::shared_ptr<::arrow::DataType> type = std::make_shared<ArrowType<ParquetType>>();
NumericBuilder<ArrowType<ParquetType>> builder;
if (nullable) {
- std::vector<uint8_t> valid_bytes(BENCHMARK_SIZE, 0);
- int n = {0};
- std::generate(valid_bytes.begin(), valid_bytes.end(), [&n] { return n++ % 2; });
+ // Note true values select index 1 of sample_values
+ auto valid_bytes = RandomVector<uint8_t>(/*true_percengate=*/null_percentage,
+ BENCHMARK_SIZE, /*sample_values=*/{1, 0});
Review comment:
Good catch, the bitmap only has `0b00000001` and `0b00000000` as possible words, or more-or-less one bit every 8th position.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou closed pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
pitrou closed pull request #7175:
URL: https://github.com/apache/arrow/pull/7175
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #7175: ARROW-8794: [C++] Expand performance coverage of parquet to arrow reading
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #7175:
URL: https://github.com/apache/arrow/pull/7175#discussion_r426996411
##########
File path: cpp/src/parquet/arrow/reader_writer_benchmark.cc
##########
@@ -95,15 +97,37 @@ void SetBytesProcessed(::benchmark::State& state) {
state.SetBytesProcessed(bytes_processed);
}
+constexpr int64_t kAlternatingOrNa = -1;
+
+template <typename T>
+std::vector<T> RandomVector(int64_t true_percentage, int64_t vector_size,
Review comment:
If it is OK, I'd prefer to save this refactoring for a later point in time, in case more changes are need to this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org