[C++][Parquet] DeltaLengthByteArrayEncoder::Put may reserve too much space

[pitrou]

Describe the bug, including details regarding any error messages, version, and platform.

DeltaLengthByteArrayEncoder::Put passes the total desired capacity (encoded_size_) to BufferBuilder::Reserve. However, BufferBuilder::Reserve expects the additional desired capacity. This will therefore request much more memory than desired, and perhaps reallocate more often than expected.

arrow/cpp/src/parquet/encoding.cc

Lines 2651 to 2675 in 3e4eaa9

	template <typename DType>
	void DeltaLengthByteArrayEncoder<DType>::Put(const T* src, int num_values) {
	if (num_values == 0) {
	return;
	}
	constexpr int kBatchSize = 256;
	std::array<int32_t, kBatchSize> lengths;
	for (int idx = 0; idx < num_values; idx += kBatchSize) {
	const int batch_size = std::min(kBatchSize, num_values - idx);
	for (int j = 0; j < batch_size; ++j) {
	const int32_t len = src[idx + j].len;
	if (AddWithOverflow(encoded_size_, len, &encoded_size_)) {
	throw ParquetException("excess expansion in DELTA_LENGTH_BYTE_ARRAY");
	}
	lengths[j] = len;
	}
	length_encoder_.Put(lengths.data(), batch_size);
	}
	PARQUET_THROW_NOT_OK(sink_.Reserve(encoded_size_));
	for (int idx = 0; idx < num_values; idx++) {
	sink_.UnsafeAppend(src[idx].ptr, src[idx].len);
	}
	}

Component(s)

C++

[pitrou]

@felipecrv Do you want to take a look at this?

Also @rok FYI.

[pitrou]

It would be a good idea to look for similar mistakes in other Parquet encoders/decoders.

[mapleFU]

Maybe I can take a look and fix them.

[pitrou]

@mapleFU Feel free to.

[rok]

Thanks for catching this @pitrou! I seem to have mixed up Resize and Reserve.

[mapleFU]

(Actually it's a bit misleading. C++ std::vector<T>::reserve interface works like rok does. But our arrow code use it as incremental...)