Copilot commented on code in PR #55308:
URL: https://github.com/apache/arrow/pull/55308#discussion_r4408965120
##########
cpp/src/parquet/column_writer.cc:
##########
@@ -1745,11 +1743,20 @@ class TypedColumnWriterImpl : public ColumnWriterImpl,
*out_spaced_values_to_write = batch_size;
*null_count = 0;
} else {
+ // Accumulate in locals: accumulating through the int64_t* out-params
makes
+ // every iteration a memory read-modify-write, which blocks
vectorization.
+ const int16_t def_level = level_info_.def_level;
+ const int16_t repeated_ancestor_def_level =
+ level_info_.repeated_ancestor_def_level;
+ int64_t values_to_write = 0;
+ int64_t spaced_values_to_write = 0;
for (int x = 0; x < batch_size; x++) {
- *out_values_to_write += def_levels[x] == level_info_.def_level ? 1 :
0;
- *out_spaced_values_to_write +=
+ values_to_write += def_levels[x] == level_info_.def_level ? 1 : 0;
+ spaced_values_to_write +=
def_levels[x] >= level_info_.repeated_ancestor_def_level ? 1 : 0;
Review Comment:
These comparisons still read `level_info_`, leaving both newly cached level
variables unused. Arrow's CHECKIN debug configuration enables `-Wall -Werror`,
so the unused-variable warnings break compilation. Use the cached locals in
these comparisons to keep debug builds working.
--
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]
