GH-39122: [C++] [Parquet] FLBA reader reading preallocs and null count check

[Hattonuri]

We can reserve memory before running loops in reading.
Also we can put check on zero null count, because this function can get it despite called Spaced

This code is run here if column descriptor HasSpacedValues

arrow/cpp/src/parquet/column_reader.cc

Lines 1194 to 1227 in ef3797d

	const bool has_spaced_values = HasSpacedValues(this->descr_);
	int64_t null_count = 0;
	if (!has_spaced_values) {
	int values_to_read = 0;
	for (int64_t i = 0; i < num_def_levels; ++i) {
	if (def_levels[i] == this->max_def_level_) {
	++values_to_read;
	}
	}
	total_values = this->ReadValues(values_to_read, values);
	::arrow::bit_util::SetBitsTo(valid_bits, valid_bits_offset,
	/*length=*/total_values,
	/*bits_are_set=*/true);
	*values_read = total_values;
	} else {
	internal::LevelInfo info;
	info.repeated_ancestor_def_level = this->max_def_level_ - 1;
	info.def_level = this->max_def_level_;
	info.rep_level = this->max_rep_level_;
	internal::ValidityBitmapInputOutput validity_io;
	validity_io.values_read_upper_bound = num_def_levels;
	validity_io.valid_bits = valid_bits;
	validity_io.valid_bits_offset = valid_bits_offset;
	validity_io.null_count = null_count;
	validity_io.values_read = *values_read;
	internal::DefLevelsToBitmap(def_levels, num_def_levels, info, &validity_io);
	null_count = validity_io.null_count;
	*values_read = validity_io.values_read;
	total_values =
	this->ReadValuesSpaced(*values_read, values, static_cast<int>(null_count),
	valid_bits, valid_bits_offset);
	}

But this function does not look on actual values

arrow/cpp/src/parquet/column_reader.cc

Lines 77 to 93 in ef3797d

	inline bool HasSpacedValues(const ColumnDescriptor* descr) {
	if (descr->max_repetition_level() > 0) {
	// repeated+flat case
	return !descr->schema_node()->is_required();
	} else {
	// non-repeated+nested case
	// Find if a node forces nulls in the lowest level along the hierarchy
	const schema::Node* node = descr->schema_node().get();
	while (node) {
	if (node->is_optional()) {
	return true;
	}
	node = node->parent();
	}
	return false;
	}
	}

In my case i have many optional decimal fields, but they can be null only in the beginning and the ending and i think that this scenario is not rare

For now i have a flamegraph of my reading looking like this(even I don't only have decimals in schema their parsing takes most of the time). And I optimize that part of FLBA record reader appends

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

• Closes: [C++] [Parquet] FLBA reader does not pre-reserve memory #39122

[wgtmac]

Thanks for the improvement! I think this PR worths creating an issue.

[mapleFU]

The code LGTM, will approve it after an issue created

[pitrou]

This looks ok to me.

Note this could probably be improved still. The silly part in this code is that we are generating a bitmap in valid_bits, then read it again bit by bit and append bit by bit into the final bitmap... The alternative would be to build the buffers separately (one BufferBuilder for the data and one TypedBufferBuilder<bool> for the null bitmap).

[pitrou]

Also, did you run any benchmarks?

[github-actions]

⚠️ GitHub issue #39122 has been automatically assigned in GitHub to PR creator.

[Hattonuri]

Also, did you run any benchmarks?

I did not find appropriate ones in arrow. But in my reading code now this part is 5%-7% instead of 11%

[Hattonuri]

Actually you right about bitmap and now i can see that AppendToBitmap takes some percents

[mapleFU]

LGTM, waiting for Bitmap part finished

[pitrou]

I submitted #39124 for an alternative.

[pitrou]

I did a quick-and-dirty benchmark on PLAIN-encoded uncompressed FLBA columns:

• on git main: 465 MB/s
• with this PR: 600 MB/s
• with PR GH-39122: [C++][Parquet] Optimize FLBA record reader #39124: 715 MB/s

[mapleFU]

Since #39124 is merged, would you mind close this pr?