pitrou commented on code in PR #55449:
URL: https://github.com/apache/arrow/pull/55449#discussion_r4551443111
##########
cpp/src/parquet/encoder.cc:
##########
@@ -1013,40 +1038,56 @@ class AlpEncoder : public EncoderImpl, virtual public
TypedEncoder<DType> {
public:
using T = typename DType::c_type;
using ArrowType = typename EncodingTraits<DType>::ArrowType;
+ using Codec = ::arrow::util::alp::AlpCodec<T>;
+ using EncodingPreset = ::arrow::util::alp::AlpEncodingPreset;
+ using Sampler = ::arrow::util::alp::AlpSampler<T>;
using TypedEncoder<DType>::Put;
static_assert(std::is_same_v<T, float> || std::is_same_v<T, double>,
"ALP only supports float and double types");
- explicit AlpEncoder(const ColumnDescriptor* descr,
- ::arrow::MemoryPool* pool =
::arrow::default_memory_pool())
- : EncoderImpl(descr, Encoding::ALP, pool), sink_{pool} {}
+ explicit AlpEncoder(const ColumnDescriptor* descr, ::arrow::MemoryPool* pool,
+ const AlpOptions& alp_options)
+ : EncoderImpl(descr, Encoding::ALP, pool),
+ sink_{pool},
+ alp_options_(alp_options),
+ sampler_(alp_options.sampling_window_size) {}
// TODO(GH-48701): use a ratio estimate instead of reporting the raw buffer
size
int64_t EstimatedDataEncodedSize() override { return sink_.length(); }
std::shared_ptr<Buffer> FlushValues() override {
- // TODO(GH-48701): allow configuring the vector size.
- constexpr auto kVectorSize =
- ::arrow::util::alp::AlpFormatConstants::kDefaultVectorSize;
-
- // An all-null optional page still has to produce a header-only page.
const int64_t num_elements = sink_.length() /
static_cast<int64_t>(sizeof(T));
+ const std::span<const T> input{reinterpret_cast<const T*>(sink_.data()),
+ static_cast<size_t>(num_elements)};
+
+ if (num_elements > 0 && !preset_.has_value()) {
+ sampler_.AddSample(input);
+ sampled_values_ += num_elements;
+ if (sampled_values_ >= alp_options_.sampling_window_size) {
+ preset_ = sampler_.MakePreset();
+ }
+ }
+
+ EncodingPreset page_preset;
+ if (!preset_.has_value()) {
+ // Use a page-specific sampler to avoid affecting the global preset.
Review Comment:
> I think this is expensive
Did you validate this and your approach using benchmarks?
Also, IMHO we should be careful to not pessimize the compression efficiency
in search for encoding speed improvements.
This might not be a problem on synthetic random data, but reduce compression
efficiency on real-world data with non-random ordering.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
