Repository navigation
GH-54885: [C++][Compute] Use 64-bit offsets for basic row encoding #55170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
11f0000
bd679a0
1bc6ba1
d8b2bb1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,13 +21,15 @@ | |
| #include <gtest/gtest.h> | ||
|
|
||
| #include "arrow/array.h" | ||
| #include "arrow/array/builder_primitive.h" | ||
| #include "arrow/array/util.h" | ||
| #include "arrow/compute/api_vector.h" | ||
| #include "arrow/compute/cast.h" | ||
| #include "arrow/compute/exec.h" | ||
| #include "arrow/compute/row/grouper.h" | ||
| #include "arrow/compute/row/grouper_internal.h" | ||
| #include "arrow/compute/test_util_internal.h" | ||
| #include "arrow/scalar.h" | ||
| #include "arrow/testing/gtest_util.h" | ||
| #include "arrow/testing/matchers.h" | ||
| #include "arrow/testing/random.h" | ||
|
|
@@ -927,6 +929,104 @@ TEST(Grouper, StringKey) { | |
| } | ||
| } | ||
|
|
||
| TEST(Grouper, LARGE_MEMORY_TEST(BatchOver2GB)) { | ||
| constexpr int64_t kValueLength = 64 * 1024; | ||
| constexpr int64_t kBatchLength = 32768; | ||
| // large_utf8 selects GrouperImpl. Repeated scalar keys keep the input and | ||
| // unique-key storage small. 32,768 * 64 KiB is exactly 2 GiB of value bytes; | ||
| // adding the null byte and int64_t length per row takes the encoded batch | ||
| // beyond 2 GiB, even though there is only one distinct key. | ||
| const auto value = std::make_shared<LargeStringScalar>(std::string(kValueLength, 'x')); | ||
| const ExecBatch batch({value}, kBatchLength); | ||
| ASSERT_OK_AND_ASSIGN(auto grouper, Grouper::Make({large_utf8()})); | ||
| ASSERT_OK_AND_ASSIGN(auto expected, MakeArrayFromScalar(UInt32Scalar{0}, kBatchLength)); | ||
|
|
||
| // All rows must receive group 0, despite the large temporary batch encoding. | ||
| ASSERT_OK_AND_ASSIGN(auto groups, grouper->Consume(ExecSpan(batch))); | ||
| ASSERT_OK(groups.make_array()->ValidateFull()); | ||
| AssertArraysEqual(*expected, *groups.make_array()); | ||
| ASSERT_EQ(grouper->num_groups(), 1); | ||
|
|
||
| // Re-inserting the same key must not create more groups; Lookup must find it | ||
| // for every row, and GetUniques must decode just the one original value. | ||
| ASSERT_OK(grouper->Populate(ExecSpan(batch))); | ||
| ASSERT_OK_AND_ASSIGN(auto found, grouper->Lookup(ExecSpan(batch))); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here: validate
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
| ASSERT_OK(found.make_array()->ValidateFull()); | ||
| AssertArraysEqual(*expected, *found.make_array()); | ||
| ASSERT_EQ(grouper->num_groups(), 1); | ||
|
|
||
| ASSERT_OK_AND_ASSIGN(auto uniques, grouper->GetUniques()); | ||
| ASSERT_OK_AND_ASSIGN(auto expected_unique, MakeArrayFromScalar(*value, /*length=*/1)); | ||
| ASSERT_OK(uniques[0].make_array()->ValidateFull()); | ||
| AssertArraysEqual(*expected_unique, *uniques[0].make_array()); | ||
| } | ||
|
|
||
| TEST(Grouper, LARGE_MEMORY_TEST(UniqueKeysOver2GB)) { | ||
| constexpr int32_t kBatchLength = 1024; | ||
| constexpr int32_t kNumBatches = 32; | ||
| constexpr int32_t kNumGroups = kBatchLength * kNumBatches; | ||
| constexpr int64_t kValueLength = 64 * 1024; | ||
| // Each batch encodes about 64 MiB, but 32 * 1024 * 64 KiB is already 2 GiB | ||
| // of value bytes. The int32 key, null bytes and length prefixes take the | ||
| // persistent unique-key buffer beyond 2 GiB. Distinct integer ids prevent | ||
| // deduplication even though every row reuses the same large binary scalar. | ||
| const auto value = std::make_shared<LargeBinaryScalar>(std::string(kValueLength, 'x')); | ||
| ASSERT_OK_AND_ASSIGN(auto grouper, Grouper::Make({int32(), large_binary()})); | ||
|
|
||
| for (int32_t batch_index = 0; batch_index < kNumBatches; ++batch_index) { | ||
| SCOPED_TRACE(batch_index); | ||
| Int32Builder builder; | ||
| const int32_t first_id = batch_index * kBatchLength; | ||
| for (int32_t i = 0; i < kBatchLength; ++i) { | ||
| ASSERT_OK(builder.Append(first_id + i)); | ||
| } | ||
| ASSERT_OK_AND_ASSIGN(auto ids, builder.Finish()); | ||
| const ExecBatch batch({ids, value}, kBatchLength); | ||
| // Exercise both insertion APIs on the same persistent buffer: Populate | ||
| // first, then Consume, whose returned group ids must follow insertion order. | ||
| if (batch_index < kNumBatches / 2) { | ||
| ASSERT_OK(grouper->Populate(ExecSpan(batch))); | ||
| } else { | ||
| ASSERT_OK_AND_ASSIGN(auto groups, grouper->Consume(ExecSpan(batch))); | ||
| auto group_array = groups.make_array(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same: validate
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
| ASSERT_OK(group_array->ValidateFull()); | ||
| ASSERT_EQ(group_array->length(), kBatchLength); | ||
| ASSERT_EQ(group_array->null_count(), 0); | ||
| const auto& group_ids = checked_cast<const UInt32Array&>(*group_array); | ||
| for (int32_t i = 0; i < kBatchLength; ++i) { | ||
| ASSERT_EQ(group_ids.Value(i), first_id + i); | ||
| } | ||
| } | ||
| } | ||
| ASSERT_EQ(grouper->num_groups(), kNumGroups); | ||
|
|
||
| // Look up the last key (32767), the first key and a missing key. Lookup must | ||
| // return null for the miss without inserting a new group. | ||
| const ExecBatch query({ArrayFromJSON(int32(), "[32767, 0, -1]"), value}, /*length=*/3); | ||
| ASSERT_OK_AND_ASSIGN(auto found, grouper->Lookup(ExecSpan(query))); | ||
| ASSERT_OK(found.make_array()->ValidateFull()); | ||
| AssertArraysEqual(*ArrayFromJSON(uint32(), "[32767, 0, null]"), *found.make_array()); | ||
| ASSERT_EQ(grouper->num_groups(), kNumGroups); | ||
|
|
||
| // Decode all unique keys to validate the full offsets and value buffer, then | ||
| // check insertion order and sample large values at the beginning, middle and end. | ||
| ASSERT_OK_AND_ASSIGN(auto uniques, grouper->GetUniques()); | ||
| ASSERT_EQ(uniques.length, kNumGroups); | ||
| auto unique_ids = uniques[0].make_array(); | ||
| auto unique_values = uniques[1].make_array(); | ||
| ASSERT_OK(unique_ids->ValidateFull()); | ||
| ASSERT_OK(unique_values->ValidateFull()); | ||
| const auto& ids = checked_cast<const Int32Array&>(*unique_ids); | ||
| const auto& values = checked_cast<const LargeBinaryArray&>(*unique_values); | ||
| ASSERT_EQ(values.value_data()->size(), kValueLength * kNumGroups); | ||
| for (int32_t i = 0; i < kNumGroups; ++i) { | ||
| ASSERT_EQ(ids.Value(i), i); | ||
| } | ||
| for (int32_t i : {0, kNumGroups / 2, kNumGroups - 1}) { | ||
| ASSERT_EQ(values.GetView(i), std::string_view(*value->value)); | ||
| } | ||
| } | ||
|
|
||
| TEST(Grouper, StringViewKey) { | ||
| // Mix inline (<=12 byte) and out-of-line view keys. | ||
| for (auto ty : {utf8_view(), binary_view()}) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,13 +64,13 @@ Status KeyEncoder::DecodeNulls(MemoryPool* pool, int32_t length, uint8_t** encod | |
| } | ||
|
|
||
| void BooleanKeyEncoder::AddLength(const ExecValue&, int64_t batch_length, | ||
| int32_t* lengths) { | ||
| int64_t* lengths) { | ||
| for (int64_t i = 0; i < batch_length; ++i) { | ||
| lengths[i] += kByteWidth + kExtraByteForNull; | ||
| } | ||
| } | ||
|
|
||
| void BooleanKeyEncoder::AddLengthNull(int32_t* length) { | ||
| void BooleanKeyEncoder::AddLengthNull(int64_t* length) { | ||
| *length += kByteWidth + kExtraByteForNull; | ||
| } | ||
|
|
||
|
|
@@ -134,13 +134,13 @@ Result<std::shared_ptr<ArrayData>> BooleanKeyEncoder::Decode(uint8_t** encoded_b | |
| } | ||
|
|
||
| void FixedWidthKeyEncoder::AddLength(const ExecValue&, int64_t batch_length, | ||
| int32_t* lengths) { | ||
| int64_t* lengths) { | ||
| for (int64_t i = 0; i < batch_length; ++i) { | ||
| lengths[i] += byte_width_ + kExtraByteForNull; | ||
| } | ||
| } | ||
|
|
||
| void FixedWidthKeyEncoder::AddLengthNull(int32_t* length) { | ||
| void FixedWidthKeyEncoder::AddLengthNull(int64_t* length) { | ||
| *length += byte_width_ + kExtraByteForNull; | ||
| } | ||
|
|
||
|
|
@@ -197,7 +197,8 @@ Result<std::shared_ptr<ArrayData>> FixedWidthKeyEncoder::Decode(uint8_t** encode | |
| int32_t null_count; | ||
| RETURN_NOT_OK(DecodeNulls(pool, length, encoded_bytes, &null_buf, &null_count)); | ||
|
|
||
| ARROW_ASSIGN_OR_RAISE(auto key_buf, AllocateBuffer(length * byte_width_, pool)); | ||
| ARROW_ASSIGN_OR_RAISE(auto key_buf, | ||
| AllocateBuffer(int64_t{length} * byte_width_, pool)); | ||
|
|
||
| uint8_t* raw_output = key_buf->mutable_data(); | ||
| for (int32_t i = 0; i < length; ++i) { | ||
|
|
@@ -339,7 +340,7 @@ void RowEncoder::Init(const std::vector<TypeHolder>& column_types, ExecContext* | |
| ARROW_DCHECK(false); | ||
| } | ||
|
|
||
| int32_t total_length = 0; | ||
| int64_t total_length = 0; | ||
| for (size_t i = 0; i < column_types.size(); ++i) { | ||
| encoders_[i]->AddLengthNull(&total_length); | ||
| } | ||
|
|
@@ -356,34 +357,33 @@ void RowEncoder::Clear() { | |
| } | ||
|
|
||
| Status RowEncoder::EncodeAndAppend(const ExecSpan& batch) { | ||
| const int64_t batch_length = batch.length; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a point in this? Casting to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| if (offsets_.empty()) { | ||
| offsets_.resize(1); | ||
| offsets_[0] = 0; | ||
| } | ||
| size_t length_before = offsets_.size() - 1; | ||
| offsets_.resize(length_before + batch.length + 1); | ||
| for (int64_t i = 0; i < batch.length; ++i) { | ||
| offsets_[length_before + 1 + i] = 0; | ||
| } | ||
| // resize value-initializes the new offsets to zero. | ||
| offsets_.resize(length_before + batch_length + 1); | ||
|
|
||
| for (int i = 0; i < batch.num_values(); ++i) { | ||
| encoders_[i]->AddLength(batch[i], batch.length, offsets_.data() + length_before + 1); | ||
| encoders_[i]->AddLength(batch[i], batch_length, offsets_.data() + length_before + 1); | ||
| } | ||
|
|
||
| int32_t total_length = offsets_[length_before]; | ||
| for (int64_t i = 0; i < batch.length; ++i) { | ||
| int64_t total_length = offsets_[length_before]; | ||
| for (int64_t i = 0; i < batch_length; ++i) { | ||
| total_length += offsets_[length_before + 1 + i]; | ||
| offsets_[length_before + 1 + i] = total_length; | ||
| } | ||
|
|
||
| bytes_.resize(total_length); | ||
| std::vector<uint8_t*> buf_ptrs(batch.length); | ||
| for (int64_t i = 0; i < batch.length; ++i) { | ||
| std::vector<uint8_t*> buf_ptrs(batch_length); | ||
| for (int64_t i = 0; i < batch_length; ++i) { | ||
| buf_ptrs[i] = bytes_.data() + offsets_[length_before + i]; | ||
| } | ||
|
|
||
| for (int i = 0; i < batch.num_values(); ++i) { | ||
| RETURN_NOT_OK(encoders_[i]->Encode(batch[i], batch.length, buf_ptrs.data())); | ||
| RETURN_NOT_OK(encoders_[i]->Encode(batch[i], batch_length, buf_ptrs.data())); | ||
| } | ||
|
|
||
| return Status::OK(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also validate
groups?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
ValidateFull()before checking the group IDs.