felipecrv commented on code in PR #39817:
URL: https://github.com/apache/arrow/pull/39817#discussion_r1468892601
##########
cpp/src/arrow/compute/kernels/vector_sort.cc:
##########
@@ -748,11 +749,15 @@ class TableSorter {
auto& comparator = comparator_;
const auto& first_sort_key = sort_keys_[0];
+ ChunkLocation left_loc{0, 0};
+ ChunkLocation right_loc{0, 0};
std::merge(nulls_begin, nulls_middle, nulls_middle, nulls_end,
temp_indices,
[&](uint64_t left, uint64_t right) {
// First column is either null or nan
- const auto left_loc = left_resolver_.Resolve(left);
- const auto right_loc = right_resolver_.Resolve(right);
+ left_loc =
+ left_resolver_.ResolveWithChunkIndexHint(left,
left_loc.chunk_index);
Review Comment:
Memory access really: keeping the `ChunkLocation::chunk_index` in registers
throughout the loop -- of course it depends on the register allocation and
inlining of `ResolveWithChunkIndexHint`. It improved the sort benchmarks
slightly.
--
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]