pitrou commented on code in PR #35565:
URL: https://github.com/apache/arrow/pull/35565#discussion_r1202361461
##########
cpp/src/arrow/util/align_util_test.cc:
##########
@@ -278,4 +279,225 @@ TEST(EnsureAlignment, Table) {
ASSERT_EQ(util::CheckAlignment(*aligned_table, 2048, &needs_alignment),
true);
}
+using TypesRequiringSomeKindOfAlignment =
+ testing::Types<Int16Type, Int32Type, Int64Type, UInt16Type, UInt32Type,
UInt64Type,
+ FloatType, DoubleType, Date32Type, Date64Type, Time32Type,
Time64Type,
+ Decimal128Type, Decimal256Type, TimestampType,
DurationType, MapType,
+ DenseUnionType, LargeBinaryType, LargeListType,
LargeStringType,
+ MonthIntervalType, DayTimeIntervalType,
MonthDayNanoIntervalType>;
+
+using TypesNotRequiringAlignment =
+ testing::Types<NullType, Int8Type, UInt8Type, FixedSizeListType,
FixedSizeBinaryType,
+ BooleanType, SparseUnionType>;
+
+TEST(EnsureAlignment, Malloc) {}
+
+template <typename ArrowType>
+std::shared_ptr<DataType> sample_type() {
+ return TypeTraits<ArrowType>::type_singleton();
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeBinaryType>() {
+ return fixed_size_binary(16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeListType>() {
+ return fixed_size_list(uint8(), 16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal128Type>() {
+ return decimal128(32, 6);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal256Type>() {
+ return decimal256(60, 10);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<LargeListType>() {
+ return large_list(int8());
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DenseUnionType>() {
+ return dense_union({field("x", int8()), field("y", uint8())});
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<MapType>() {
+ return map(utf8(), field("item", utf8()));
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DurationType>() {
+ return duration(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<TimestampType>() {
+ return timestamp(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time32Type>() {
+ return time32(TimeUnit::SECOND);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time64Type>() {
+ return time64(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<SparseUnionType>() {
+ return sparse_union({field("x", uint8()), field("y", int8())}, {1, 2});
+}
+
+template <typename ArrowType>
+std::shared_ptr<ArrayData> SampleArray() {
+ random::RandomArrayGenerator gen(42);
+ return gen.ArrayOf(sample_type<ArrowType>(), 100)->data();
+}
+
+template <>
+std::shared_ptr<ArrayData> SampleArray<SparseUnionType>() {
+ auto ty = sparse_union({field("ints", int64()), field("strs", utf8())}, {2,
7});
+ auto ints = ArrayFromJSON(int64(), "[0, 1, 2, 3]");
+ auto strs = ArrayFromJSON(utf8(), R"(["a", null, "c", "d"])");
+ auto ids = ArrayFromJSON(int8(), "[2, 7, 2, 7]")->data()->buffers[1];
+ const int length = 4;
+ SparseUnionArray arr(ty, length, {ints, strs}, ids);
+ return arr.data();
+}
+
+class MallocAlignment : public ::testing::Test {
+ public:
+ void CheckModified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ if (src.buffers[i]->address() != dst.buffers[i]->address()) {
+ return;
+ }
+ }
+ FAIL() << "Expected at least one buffer to have been modified by
EnsureAlignment";
+ }
+
+ void CheckUnmodified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ ASSERT_EQ(src.buffers[i]->address(), dst.buffers[i]->address());
+ }
+ }
+};
+
+template <typename T>
+class MallocAlignmentRequired : public MallocAlignment {};
+template <typename T>
+class MallocAlignmentNotRequired : public MallocAlignment {};
+
+TYPED_TEST_SUITE(MallocAlignmentRequired, TypesRequiringSomeKindOfAlignment);
+TYPED_TEST_SUITE(MallocAlignmentNotRequired, TypesNotRequiringAlignment);
+
+TYPED_TEST(MallocAlignmentRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
+ this->CheckModified(*unaligned, *aligned);
+}
+
+TYPED_TEST(MallocAlignmentNotRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
+ this->CheckUnmodified(*unaligned, *aligned);
+}
+
+TEST_F(MallocAlignment, RunEndEncoded) {
+ // Run end requires alignment, value type does not
+ std::shared_ptr<Array> run_ends = ArrayFromJSON(int32(), "[3, 5]");
+ std::shared_ptr<Array> values = ArrayFromJSON(int8(), "[50, 100]");
+ ASSERT_OK_AND_ASSIGN(std::shared_ptr<Array> array,
+ RunEndEncodedArray::Make(/*logical_length=*/5,
std::move(run_ends),
+ std::move(values), 0));
+
+ std::shared_ptr<ArrayData> unaligned_ree =
std::make_shared<ArrayData>(*array->data());
+ unaligned_ree->child_data[0] = UnalignValues(*unaligned_ree->child_data[0]);
+ unaligned_ree->child_data[1] = UnalignValues(*unaligned_ree->child_data[1]);
+
+ std::shared_ptr<ArrayData> aligned_ree =
std::make_shared<ArrayData>(*unaligned_ree);
+
+ ASSERT_OK_AND_ASSIGN(
+ aligned_ree,
+ util::EnsureAlignment(aligned_ree, util::kValueAlignment,
default_memory_pool()));
+
+ this->CheckModified(*unaligned_ree->child_data[0],
*aligned_ree->child_data[0]);
+ this->CheckUnmodified(*unaligned_ree->child_data[1],
*aligned_ree->child_data[1]);
+}
+
+TEST_F(MallocAlignment, Dictionary) {
+ // Dictionary values require alignment, dictionary keys do not
+ std::shared_ptr<DataType> int8_utf8 = dictionary(int8(), utf8());
+ std::shared_ptr<Array> array = ArrayFromJSON(int8_utf8, R"(["x", "x",
"y"])");
+
+ std::shared_ptr<ArrayData> unaligned_dict =
std::make_shared<ArrayData>(*array->data());
+ unaligned_dict->dictionary = UnalignValues(*unaligned_dict->dictionary);
+ unaligned_dict = UnalignValues(*unaligned_dict);
+
+ std::shared_ptr<ArrayData> aligned_dict =
std::make_shared<ArrayData>(*unaligned_dict);
+
+ ASSERT_OK_AND_ASSIGN(
+ aligned_dict,
+ util::EnsureAlignment(aligned_dict, util::kValueAlignment,
default_memory_pool()));
+
+ this->CheckUnmodified(*unaligned_dict, *aligned_dict);
+ this->CheckModified(*unaligned_dict->dictionary, *aligned_dict->dictionary);
+
+ // Dictionary values do not require alignment, dictionary keys do
Review Comment:
```suggestion
// Dictionary values do not require alignment, dictionary indices do
```
##########
cpp/src/arrow/util/align_util_test.cc:
##########
@@ -278,4 +279,225 @@ TEST(EnsureAlignment, Table) {
ASSERT_EQ(util::CheckAlignment(*aligned_table, 2048, &needs_alignment),
true);
}
+using TypesRequiringSomeKindOfAlignment =
+ testing::Types<Int16Type, Int32Type, Int64Type, UInt16Type, UInt32Type,
UInt64Type,
+ FloatType, DoubleType, Date32Type, Date64Type, Time32Type,
Time64Type,
+ Decimal128Type, Decimal256Type, TimestampType,
DurationType, MapType,
+ DenseUnionType, LargeBinaryType, LargeListType,
LargeStringType,
+ MonthIntervalType, DayTimeIntervalType,
MonthDayNanoIntervalType>;
+
+using TypesNotRequiringAlignment =
+ testing::Types<NullType, Int8Type, UInt8Type, FixedSizeListType,
FixedSizeBinaryType,
+ BooleanType, SparseUnionType>;
+
+TEST(EnsureAlignment, Malloc) {}
+
+template <typename ArrowType>
+std::shared_ptr<DataType> sample_type() {
+ return TypeTraits<ArrowType>::type_singleton();
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeBinaryType>() {
+ return fixed_size_binary(16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeListType>() {
+ return fixed_size_list(uint8(), 16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal128Type>() {
+ return decimal128(32, 6);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal256Type>() {
+ return decimal256(60, 10);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<LargeListType>() {
+ return large_list(int8());
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DenseUnionType>() {
+ return dense_union({field("x", int8()), field("y", uint8())});
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<MapType>() {
+ return map(utf8(), field("item", utf8()));
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DurationType>() {
+ return duration(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<TimestampType>() {
+ return timestamp(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time32Type>() {
+ return time32(TimeUnit::SECOND);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time64Type>() {
+ return time64(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<SparseUnionType>() {
+ return sparse_union({field("x", uint8()), field("y", int8())}, {1, 2});
+}
+
+template <typename ArrowType>
+std::shared_ptr<ArrayData> SampleArray() {
+ random::RandomArrayGenerator gen(42);
+ return gen.ArrayOf(sample_type<ArrowType>(), 100)->data();
+}
+
+template <>
+std::shared_ptr<ArrayData> SampleArray<SparseUnionType>() {
+ auto ty = sparse_union({field("ints", int64()), field("strs", utf8())}, {2,
7});
+ auto ints = ArrayFromJSON(int64(), "[0, 1, 2, 3]");
+ auto strs = ArrayFromJSON(utf8(), R"(["a", null, "c", "d"])");
+ auto ids = ArrayFromJSON(int8(), "[2, 7, 2, 7]")->data()->buffers[1];
+ const int length = 4;
+ SparseUnionArray arr(ty, length, {ints, strs}, ids);
+ return arr.data();
+}
+
+class MallocAlignment : public ::testing::Test {
+ public:
+ void CheckModified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ if (src.buffers[i]->address() != dst.buffers[i]->address()) {
+ return;
+ }
+ }
+ FAIL() << "Expected at least one buffer to have been modified by
EnsureAlignment";
+ }
+
+ void CheckUnmodified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ ASSERT_EQ(src.buffers[i]->address(), dst.buffers[i]->address());
+ }
+ }
+};
+
+template <typename T>
+class MallocAlignmentRequired : public MallocAlignment {};
+template <typename T>
+class MallocAlignmentNotRequired : public MallocAlignment {};
+
+TYPED_TEST_SUITE(MallocAlignmentRequired, TypesRequiringSomeKindOfAlignment);
+TYPED_TEST_SUITE(MallocAlignmentNotRequired, TypesNotRequiringAlignment);
+
+TYPED_TEST(MallocAlignmentRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
Review Comment:
Let's also validate `aligned` for correctness?
##########
cpp/src/arrow/util/align_util.h:
##########
@@ -70,8 +70,27 @@ inline BitmapWordAlignParams BitmapWordAlign(const uint8_t*
data, int64_t bit_of
namespace util {
// Functions to check if the provided Arrow object is aligned by the specified
alignment
+
+/// \brief if this is specified in one of the CheckAlignment or
EnsureAlignment functions
+///
+/// then the function will ensure each buffer is suitably aligned for the data
type of the
+/// array. For example, given an int32 buffer the validity buffer must be a
multiple of 8
+/// and the values buffer must be a multiple of 32. Given a large_string
buffer the
+/// validity buffer and values buffers must be multiples of 8 and the offsets
buffer must
+/// be a multiple of 64.
+constexpr int64_t kValueAlignment = -3;
+
+/// \brief calculate if the buffer's address is a multiple of `alignment`
Review Comment:
Nit: capitalize
```suggestion
/// \brief Calculate if the buffer's address is a multiple of `alignment`
```
##########
cpp/src/arrow/util/align_util.h:
##########
@@ -82,29 +101,109 @@ ARROW_EXPORT bool CheckAlignment(const Array& array,
int64_t alignment);
// of the constituent objects during the EnsureAlignment function where certain
// objects can be ignored for further checking if we already know that they are
// completely aligned.
+
+/// \brief calculate which (if any) chunks in a chunked array are unaligned
+/// \param array the array to check
+/// \param alignment the alignment to check for
+/// \param needs_alignment an output vector that will store the results of the
check
+/// it must be set to a valid vector. Extra elements will be added to
the end
+/// of the vector for each chunk that is checked. `true` will be
stored if
+/// the chunk is unaligned.
+/// \param offset an optional offset to specify which chunk to start checking
at
Review Comment:
Is it a logical offset into the chunked array, or a chunk number?
##########
cpp/src/arrow/util/align_util.h:
##########
@@ -70,8 +70,27 @@ inline BitmapWordAlignParams BitmapWordAlign(const uint8_t*
data, int64_t bit_of
namespace util {
// Functions to check if the provided Arrow object is aligned by the specified
alignment
+
+/// \brief if this is specified in one of the CheckAlignment or
EnsureAlignment functions
+///
+/// then the function will ensure each buffer is suitably aligned for the data
type of the
+/// array. For example, given an int32 buffer the validity buffer must be a
multiple of 8
+/// and the values buffer must be a multiple of 32. Given a large_string
buffer the
+/// validity buffer and values buffers must be multiples of 8 and the offsets
buffer must
+/// be a multiple of 64.
+constexpr int64_t kValueAlignment = -3;
+
+/// \brief calculate if the buffer's address is a multiple of `alignment`
+/// \param buffer the buffer to check
+/// \param alignment the alignment to check for
ARROW_EXPORT bool CheckAlignment(const Buffer& buffer, int64_t alignment);
+/// \brief calculate if all buffer's in the array data are aligned
Review Comment:
```suggestion
/// \brief Calculate if all buffers in the array data are aligned
```
##########
cpp/src/arrow/util/align_util.cc:
##########
@@ -94,12 +135,17 @@ bool CheckAlignment(const Table& table, int64_t alignment,
return all_aligned;
}
+// Most allocators require a minimum of 8-byte alignment.
+constexpr int64_t kMinimumAlignment = 8;
+
Result<std::shared_ptr<Buffer>> EnsureAlignment(std::shared_ptr<Buffer> buffer,
int64_t alignment,
MemoryPool* memory_pool) {
if (!CheckAlignment(*buffer, alignment)) {
Review Comment:
Should we check somewhere for non-CPU buffers? Perhaps return
`NotImplemented` then?
##########
cpp/src/arrow/util/align_util.cc:
##########
@@ -94,12 +135,17 @@ bool CheckAlignment(const Table& table, int64_t alignment,
return all_aligned;
}
+// Most allocators require a minimum of 8-byte alignment.
+constexpr int64_t kMinimumAlignment = 8;
+
Result<std::shared_ptr<Buffer>> EnsureAlignment(std::shared_ptr<Buffer> buffer,
int64_t alignment,
MemoryPool* memory_pool) {
Review Comment:
Should we check that `alignment` is positive? People could misunderstand the
API and try to pass `kValueAlignment`.
##########
cpp/src/arrow/util/align_util.cc:
##########
@@ -94,12 +135,17 @@ bool CheckAlignment(const Table& table, int64_t alignment,
return all_aligned;
}
+// Most allocators require a minimum of 8-byte alignment.
+constexpr int64_t kMinimumAlignment = 8;
Review Comment:
I don't think this is these routines' job to ensure this. If the caller
wants to ensure a lesser alignment, then let them do.
##########
cpp/src/arrow/util/align_util_test.cc:
##########
@@ -278,4 +279,225 @@ TEST(EnsureAlignment, Table) {
ASSERT_EQ(util::CheckAlignment(*aligned_table, 2048, &needs_alignment),
true);
}
+using TypesRequiringSomeKindOfAlignment =
+ testing::Types<Int16Type, Int32Type, Int64Type, UInt16Type, UInt32Type,
UInt64Type,
+ FloatType, DoubleType, Date32Type, Date64Type, Time32Type,
Time64Type,
+ Decimal128Type, Decimal256Type, TimestampType,
DurationType, MapType,
+ DenseUnionType, LargeBinaryType, LargeListType,
LargeStringType,
+ MonthIntervalType, DayTimeIntervalType,
MonthDayNanoIntervalType>;
+
+using TypesNotRequiringAlignment =
+ testing::Types<NullType, Int8Type, UInt8Type, FixedSizeListType,
FixedSizeBinaryType,
+ BooleanType, SparseUnionType>;
+
+TEST(EnsureAlignment, Malloc) {}
+
+template <typename ArrowType>
+std::shared_ptr<DataType> sample_type() {
+ return TypeTraits<ArrowType>::type_singleton();
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeBinaryType>() {
+ return fixed_size_binary(16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeListType>() {
+ return fixed_size_list(uint8(), 16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal128Type>() {
+ return decimal128(32, 6);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal256Type>() {
+ return decimal256(60, 10);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<LargeListType>() {
+ return large_list(int8());
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DenseUnionType>() {
+ return dense_union({field("x", int8()), field("y", uint8())});
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<MapType>() {
+ return map(utf8(), field("item", utf8()));
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DurationType>() {
+ return duration(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<TimestampType>() {
+ return timestamp(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time32Type>() {
+ return time32(TimeUnit::SECOND);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time64Type>() {
+ return time64(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<SparseUnionType>() {
+ return sparse_union({field("x", uint8()), field("y", int8())}, {1, 2});
+}
+
+template <typename ArrowType>
+std::shared_ptr<ArrayData> SampleArray() {
+ random::RandomArrayGenerator gen(42);
+ return gen.ArrayOf(sample_type<ArrowType>(), 100)->data();
+}
+
+template <>
+std::shared_ptr<ArrayData> SampleArray<SparseUnionType>() {
+ auto ty = sparse_union({field("ints", int64()), field("strs", utf8())}, {2,
7});
+ auto ints = ArrayFromJSON(int64(), "[0, 1, 2, 3]");
+ auto strs = ArrayFromJSON(utf8(), R"(["a", null, "c", "d"])");
+ auto ids = ArrayFromJSON(int8(), "[2, 7, 2, 7]")->data()->buffers[1];
+ const int length = 4;
+ SparseUnionArray arr(ty, length, {ints, strs}, ids);
+ return arr.data();
+}
+
+class MallocAlignment : public ::testing::Test {
+ public:
+ void CheckModified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ if (src.buffers[i]->address() != dst.buffers[i]->address()) {
+ return;
+ }
+ }
+ FAIL() << "Expected at least one buffer to have been modified by
EnsureAlignment";
+ }
+
+ void CheckUnmodified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ ASSERT_EQ(src.buffers[i]->address(), dst.buffers[i]->address());
+ }
+ }
+};
+
+template <typename T>
+class MallocAlignmentRequired : public MallocAlignment {};
+template <typename T>
+class MallocAlignmentNotRequired : public MallocAlignment {};
+
+TYPED_TEST_SUITE(MallocAlignmentRequired, TypesRequiringSomeKindOfAlignment);
+TYPED_TEST_SUITE(MallocAlignmentNotRequired, TypesNotRequiringAlignment);
+
Review Comment:
Why not also a test that aligned values do not need any realignment, or did
I miss an existing test for this?
##########
cpp/src/arrow/util/align_util_test.cc:
##########
@@ -278,4 +279,225 @@ TEST(EnsureAlignment, Table) {
ASSERT_EQ(util::CheckAlignment(*aligned_table, 2048, &needs_alignment),
true);
}
+using TypesRequiringSomeKindOfAlignment =
+ testing::Types<Int16Type, Int32Type, Int64Type, UInt16Type, UInt32Type,
UInt64Type,
+ FloatType, DoubleType, Date32Type, Date64Type, Time32Type,
Time64Type,
+ Decimal128Type, Decimal256Type, TimestampType,
DurationType, MapType,
+ DenseUnionType, LargeBinaryType, LargeListType,
LargeStringType,
+ MonthIntervalType, DayTimeIntervalType,
MonthDayNanoIntervalType>;
+
+using TypesNotRequiringAlignment =
+ testing::Types<NullType, Int8Type, UInt8Type, FixedSizeListType,
FixedSizeBinaryType,
+ BooleanType, SparseUnionType>;
+
+TEST(EnsureAlignment, Malloc) {}
+
+template <typename ArrowType>
+std::shared_ptr<DataType> sample_type() {
+ return TypeTraits<ArrowType>::type_singleton();
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeBinaryType>() {
+ return fixed_size_binary(16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeListType>() {
+ return fixed_size_list(uint8(), 16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal128Type>() {
+ return decimal128(32, 6);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal256Type>() {
+ return decimal256(60, 10);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<LargeListType>() {
+ return large_list(int8());
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DenseUnionType>() {
+ return dense_union({field("x", int8()), field("y", uint8())});
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<MapType>() {
+ return map(utf8(), field("item", utf8()));
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DurationType>() {
+ return duration(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<TimestampType>() {
+ return timestamp(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time32Type>() {
+ return time32(TimeUnit::SECOND);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time64Type>() {
+ return time64(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<SparseUnionType>() {
+ return sparse_union({field("x", uint8()), field("y", int8())}, {1, 2});
+}
+
+template <typename ArrowType>
+std::shared_ptr<ArrayData> SampleArray() {
+ random::RandomArrayGenerator gen(42);
+ return gen.ArrayOf(sample_type<ArrowType>(), 100)->data();
+}
+
+template <>
+std::shared_ptr<ArrayData> SampleArray<SparseUnionType>() {
+ auto ty = sparse_union({field("ints", int64()), field("strs", utf8())}, {2,
7});
+ auto ints = ArrayFromJSON(int64(), "[0, 1, 2, 3]");
+ auto strs = ArrayFromJSON(utf8(), R"(["a", null, "c", "d"])");
+ auto ids = ArrayFromJSON(int8(), "[2, 7, 2, 7]")->data()->buffers[1];
+ const int length = 4;
+ SparseUnionArray arr(ty, length, {ints, strs}, ids);
+ return arr.data();
+}
+
+class MallocAlignment : public ::testing::Test {
+ public:
+ void CheckModified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ if (src.buffers[i]->address() != dst.buffers[i]->address()) {
+ return;
+ }
+ }
+ FAIL() << "Expected at least one buffer to have been modified by
EnsureAlignment";
+ }
+
+ void CheckUnmodified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ ASSERT_EQ(src.buffers[i]->address(), dst.buffers[i]->address());
+ }
+ }
+};
+
+template <typename T>
+class MallocAlignmentRequired : public MallocAlignment {};
+template <typename T>
+class MallocAlignmentNotRequired : public MallocAlignment {};
+
+TYPED_TEST_SUITE(MallocAlignmentRequired, TypesRequiringSomeKindOfAlignment);
+TYPED_TEST_SUITE(MallocAlignmentNotRequired, TypesNotRequiringAlignment);
+
+TYPED_TEST(MallocAlignmentRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
+ this->CheckModified(*unaligned, *aligned);
+}
+
+TYPED_TEST(MallocAlignmentNotRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
+ this->CheckUnmodified(*unaligned, *aligned);
+}
+
+TEST_F(MallocAlignment, RunEndEncoded) {
+ // Run end requires alignment, value type does not
+ std::shared_ptr<Array> run_ends = ArrayFromJSON(int32(), "[3, 5]");
+ std::shared_ptr<Array> values = ArrayFromJSON(int8(), "[50, 100]");
+ ASSERT_OK_AND_ASSIGN(std::shared_ptr<Array> array,
+ RunEndEncodedArray::Make(/*logical_length=*/5,
std::move(run_ends),
+ std::move(values), 0));
+
+ std::shared_ptr<ArrayData> unaligned_ree =
std::make_shared<ArrayData>(*array->data());
+ unaligned_ree->child_data[0] = UnalignValues(*unaligned_ree->child_data[0]);
+ unaligned_ree->child_data[1] = UnalignValues(*unaligned_ree->child_data[1]);
+
+ std::shared_ptr<ArrayData> aligned_ree =
std::make_shared<ArrayData>(*unaligned_ree);
+
+ ASSERT_OK_AND_ASSIGN(
+ aligned_ree,
+ util::EnsureAlignment(aligned_ree, util::kValueAlignment,
default_memory_pool()));
+
+ this->CheckModified(*unaligned_ree->child_data[0],
*aligned_ree->child_data[0]);
+ this->CheckUnmodified(*unaligned_ree->child_data[1],
*aligned_ree->child_data[1]);
+}
+
+TEST_F(MallocAlignment, Dictionary) {
+ // Dictionary values require alignment, dictionary keys do not
+ std::shared_ptr<DataType> int8_utf8 = dictionary(int8(), utf8());
+ std::shared_ptr<Array> array = ArrayFromJSON(int8_utf8, R"(["x", "x",
"y"])");
+
+ std::shared_ptr<ArrayData> unaligned_dict =
std::make_shared<ArrayData>(*array->data());
+ unaligned_dict->dictionary = UnalignValues(*unaligned_dict->dictionary);
+ unaligned_dict = UnalignValues(*unaligned_dict);
+
+ std::shared_ptr<ArrayData> aligned_dict =
std::make_shared<ArrayData>(*unaligned_dict);
+
+ ASSERT_OK_AND_ASSIGN(
+ aligned_dict,
+ util::EnsureAlignment(aligned_dict, util::kValueAlignment,
default_memory_pool()));
+
+ this->CheckUnmodified(*unaligned_dict, *aligned_dict);
+ this->CheckModified(*unaligned_dict->dictionary, *aligned_dict->dictionary);
+
+ // Dictionary values do not require alignment, dictionary keys do
+ std::shared_ptr<DataType> int16_int8 = dictionary(int16(), int8());
+ array = ArrayFromJSON(int16_int8, R"([7, 11])");
+
+ unaligned_dict = std::make_shared<ArrayData>(*array->data());
+ unaligned_dict->dictionary = UnalignValues(*unaligned_dict->dictionary);
+ unaligned_dict = UnalignValues(*unaligned_dict);
+
+ aligned_dict = std::make_shared<ArrayData>(*unaligned_dict);
+
+ ASSERT_OK_AND_ASSIGN(
+ aligned_dict,
+ util::EnsureAlignment(aligned_dict, util::kValueAlignment,
default_memory_pool()));
+
+ this->CheckModified(*unaligned_dict, *aligned_dict);
+ this->CheckUnmodified(*unaligned_dict->dictionary,
*aligned_dict->dictionary);
+}
+
+TEST_F(MallocAlignment, Extension) {
+ std::shared_ptr<Array> array = ExampleSmallint();
+
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*array->data());
+
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
Review Comment:
Same here (validate).
##########
cpp/src/arrow/util/align_util_test.cc:
##########
@@ -278,4 +279,225 @@ TEST(EnsureAlignment, Table) {
ASSERT_EQ(util::CheckAlignment(*aligned_table, 2048, &needs_alignment),
true);
}
+using TypesRequiringSomeKindOfAlignment =
+ testing::Types<Int16Type, Int32Type, Int64Type, UInt16Type, UInt32Type,
UInt64Type,
+ FloatType, DoubleType, Date32Type, Date64Type, Time32Type,
Time64Type,
+ Decimal128Type, Decimal256Type, TimestampType,
DurationType, MapType,
+ DenseUnionType, LargeBinaryType, LargeListType,
LargeStringType,
+ MonthIntervalType, DayTimeIntervalType,
MonthDayNanoIntervalType>;
+
+using TypesNotRequiringAlignment =
+ testing::Types<NullType, Int8Type, UInt8Type, FixedSizeListType,
FixedSizeBinaryType,
+ BooleanType, SparseUnionType>;
+
+TEST(EnsureAlignment, Malloc) {}
+
+template <typename ArrowType>
+std::shared_ptr<DataType> sample_type() {
+ return TypeTraits<ArrowType>::type_singleton();
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeBinaryType>() {
+ return fixed_size_binary(16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeListType>() {
+ return fixed_size_list(uint8(), 16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal128Type>() {
+ return decimal128(32, 6);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal256Type>() {
+ return decimal256(60, 10);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<LargeListType>() {
+ return large_list(int8());
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DenseUnionType>() {
+ return dense_union({field("x", int8()), field("y", uint8())});
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<MapType>() {
+ return map(utf8(), field("item", utf8()));
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DurationType>() {
+ return duration(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<TimestampType>() {
+ return timestamp(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time32Type>() {
+ return time32(TimeUnit::SECOND);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time64Type>() {
+ return time64(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<SparseUnionType>() {
+ return sparse_union({field("x", uint8()), field("y", int8())}, {1, 2});
+}
+
+template <typename ArrowType>
+std::shared_ptr<ArrayData> SampleArray() {
+ random::RandomArrayGenerator gen(42);
+ return gen.ArrayOf(sample_type<ArrowType>(), 100)->data();
+}
+
+template <>
+std::shared_ptr<ArrayData> SampleArray<SparseUnionType>() {
+ auto ty = sparse_union({field("ints", int64()), field("strs", utf8())}, {2,
7});
+ auto ints = ArrayFromJSON(int64(), "[0, 1, 2, 3]");
+ auto strs = ArrayFromJSON(utf8(), R"(["a", null, "c", "d"])");
+ auto ids = ArrayFromJSON(int8(), "[2, 7, 2, 7]")->data()->buffers[1];
+ const int length = 4;
+ SparseUnionArray arr(ty, length, {ints, strs}, ids);
+ return arr.data();
+}
+
+class MallocAlignment : public ::testing::Test {
+ public:
+ void CheckModified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ if (src.buffers[i]->address() != dst.buffers[i]->address()) {
+ return;
+ }
+ }
+ FAIL() << "Expected at least one buffer to have been modified by
EnsureAlignment";
+ }
+
+ void CheckUnmodified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ ASSERT_EQ(src.buffers[i]->address(), dst.buffers[i]->address());
+ }
+ }
+};
+
+template <typename T>
+class MallocAlignmentRequired : public MallocAlignment {};
+template <typename T>
+class MallocAlignmentNotRequired : public MallocAlignment {};
+
+TYPED_TEST_SUITE(MallocAlignmentRequired, TypesRequiringSomeKindOfAlignment);
+TYPED_TEST_SUITE(MallocAlignmentNotRequired, TypesNotRequiringAlignment);
+
+TYPED_TEST(MallocAlignmentRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
+ this->CheckModified(*unaligned, *aligned);
+}
+
+TYPED_TEST(MallocAlignmentNotRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
+ this->CheckUnmodified(*unaligned, *aligned);
+}
+
+TEST_F(MallocAlignment, RunEndEncoded) {
+ // Run end requires alignment, value type does not
+ std::shared_ptr<Array> run_ends = ArrayFromJSON(int32(), "[3, 5]");
+ std::shared_ptr<Array> values = ArrayFromJSON(int8(), "[50, 100]");
+ ASSERT_OK_AND_ASSIGN(std::shared_ptr<Array> array,
+ RunEndEncodedArray::Make(/*logical_length=*/5,
std::move(run_ends),
+ std::move(values), 0));
+
+ std::shared_ptr<ArrayData> unaligned_ree =
std::make_shared<ArrayData>(*array->data());
+ unaligned_ree->child_data[0] = UnalignValues(*unaligned_ree->child_data[0]);
+ unaligned_ree->child_data[1] = UnalignValues(*unaligned_ree->child_data[1]);
+
+ std::shared_ptr<ArrayData> aligned_ree =
std::make_shared<ArrayData>(*unaligned_ree);
+
+ ASSERT_OK_AND_ASSIGN(
+ aligned_ree,
+ util::EnsureAlignment(aligned_ree, util::kValueAlignment,
default_memory_pool()));
Review Comment:
Same here.
##########
cpp/src/arrow/acero/exec_plan.h:
##########
@@ -496,6 +496,16 @@ struct ARROW_ACERO_EXPORT Declaration {
std::string label;
};
+/// \brief describes how to handle unaligned buffers
Review Comment:
```suggestion
/// \brief How to handle unaligned buffers
///
/// Please see the QueryOptions docstring for details.
```
##########
cpp/src/arrow/util/align_util_test.cc:
##########
@@ -278,4 +279,225 @@ TEST(EnsureAlignment, Table) {
ASSERT_EQ(util::CheckAlignment(*aligned_table, 2048, &needs_alignment),
true);
}
+using TypesRequiringSomeKindOfAlignment =
+ testing::Types<Int16Type, Int32Type, Int64Type, UInt16Type, UInt32Type,
UInt64Type,
+ FloatType, DoubleType, Date32Type, Date64Type, Time32Type,
Time64Type,
+ Decimal128Type, Decimal256Type, TimestampType,
DurationType, MapType,
+ DenseUnionType, LargeBinaryType, LargeListType,
LargeStringType,
+ MonthIntervalType, DayTimeIntervalType,
MonthDayNanoIntervalType>;
+
+using TypesNotRequiringAlignment =
+ testing::Types<NullType, Int8Type, UInt8Type, FixedSizeListType,
FixedSizeBinaryType,
+ BooleanType, SparseUnionType>;
+
+TEST(EnsureAlignment, Malloc) {}
+
+template <typename ArrowType>
+std::shared_ptr<DataType> sample_type() {
+ return TypeTraits<ArrowType>::type_singleton();
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeBinaryType>() {
+ return fixed_size_binary(16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeListType>() {
+ return fixed_size_list(uint8(), 16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal128Type>() {
+ return decimal128(32, 6);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal256Type>() {
+ return decimal256(60, 10);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<LargeListType>() {
+ return large_list(int8());
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DenseUnionType>() {
+ return dense_union({field("x", int8()), field("y", uint8())});
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<MapType>() {
+ return map(utf8(), field("item", utf8()));
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DurationType>() {
+ return duration(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<TimestampType>() {
+ return timestamp(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time32Type>() {
+ return time32(TimeUnit::SECOND);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time64Type>() {
+ return time64(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<SparseUnionType>() {
+ return sparse_union({field("x", uint8()), field("y", int8())}, {1, 2});
+}
+
+template <typename ArrowType>
+std::shared_ptr<ArrayData> SampleArray() {
+ random::RandomArrayGenerator gen(42);
+ return gen.ArrayOf(sample_type<ArrowType>(), 100)->data();
+}
+
+template <>
+std::shared_ptr<ArrayData> SampleArray<SparseUnionType>() {
+ auto ty = sparse_union({field("ints", int64()), field("strs", utf8())}, {2,
7});
+ auto ints = ArrayFromJSON(int64(), "[0, 1, 2, 3]");
+ auto strs = ArrayFromJSON(utf8(), R"(["a", null, "c", "d"])");
+ auto ids = ArrayFromJSON(int8(), "[2, 7, 2, 7]")->data()->buffers[1];
+ const int length = 4;
+ SparseUnionArray arr(ty, length, {ints, strs}, ids);
+ return arr.data();
+}
+
+class MallocAlignment : public ::testing::Test {
+ public:
+ void CheckModified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ if (src.buffers[i]->address() != dst.buffers[i]->address()) {
+ return;
+ }
+ }
+ FAIL() << "Expected at least one buffer to have been modified by
EnsureAlignment";
+ }
+
+ void CheckUnmodified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ ASSERT_EQ(src.buffers[i]->address(), dst.buffers[i]->address());
+ }
+ }
+};
+
+template <typename T>
+class MallocAlignmentRequired : public MallocAlignment {};
+template <typename T>
+class MallocAlignmentNotRequired : public MallocAlignment {};
+
+TYPED_TEST_SUITE(MallocAlignmentRequired, TypesRequiringSomeKindOfAlignment);
+TYPED_TEST_SUITE(MallocAlignmentNotRequired, TypesNotRequiringAlignment);
+
+TYPED_TEST(MallocAlignmentRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
+ this->CheckModified(*unaligned, *aligned);
+}
+
+TYPED_TEST(MallocAlignmentNotRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
Review Comment:
Same here.
##########
cpp/src/arrow/util/align_util_test.cc:
##########
@@ -278,4 +279,225 @@ TEST(EnsureAlignment, Table) {
ASSERT_EQ(util::CheckAlignment(*aligned_table, 2048, &needs_alignment),
true);
}
+using TypesRequiringSomeKindOfAlignment =
+ testing::Types<Int16Type, Int32Type, Int64Type, UInt16Type, UInt32Type,
UInt64Type,
+ FloatType, DoubleType, Date32Type, Date64Type, Time32Type,
Time64Type,
+ Decimal128Type, Decimal256Type, TimestampType,
DurationType, MapType,
+ DenseUnionType, LargeBinaryType, LargeListType,
LargeStringType,
+ MonthIntervalType, DayTimeIntervalType,
MonthDayNanoIntervalType>;
+
+using TypesNotRequiringAlignment =
+ testing::Types<NullType, Int8Type, UInt8Type, FixedSizeListType,
FixedSizeBinaryType,
+ BooleanType, SparseUnionType>;
+
+TEST(EnsureAlignment, Malloc) {}
+
+template <typename ArrowType>
+std::shared_ptr<DataType> sample_type() {
+ return TypeTraits<ArrowType>::type_singleton();
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeBinaryType>() {
+ return fixed_size_binary(16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeListType>() {
+ return fixed_size_list(uint8(), 16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal128Type>() {
+ return decimal128(32, 6);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal256Type>() {
+ return decimal256(60, 10);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<LargeListType>() {
+ return large_list(int8());
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DenseUnionType>() {
+ return dense_union({field("x", int8()), field("y", uint8())});
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<MapType>() {
+ return map(utf8(), field("item", utf8()));
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DurationType>() {
+ return duration(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<TimestampType>() {
+ return timestamp(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time32Type>() {
+ return time32(TimeUnit::SECOND);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time64Type>() {
+ return time64(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<SparseUnionType>() {
+ return sparse_union({field("x", uint8()), field("y", int8())}, {1, 2});
+}
+
+template <typename ArrowType>
+std::shared_ptr<ArrayData> SampleArray() {
+ random::RandomArrayGenerator gen(42);
+ return gen.ArrayOf(sample_type<ArrowType>(), 100)->data();
+}
+
+template <>
+std::shared_ptr<ArrayData> SampleArray<SparseUnionType>() {
+ auto ty = sparse_union({field("ints", int64()), field("strs", utf8())}, {2,
7});
+ auto ints = ArrayFromJSON(int64(), "[0, 1, 2, 3]");
+ auto strs = ArrayFromJSON(utf8(), R"(["a", null, "c", "d"])");
+ auto ids = ArrayFromJSON(int8(), "[2, 7, 2, 7]")->data()->buffers[1];
+ const int length = 4;
+ SparseUnionArray arr(ty, length, {ints, strs}, ids);
+ return arr.data();
+}
+
+class MallocAlignment : public ::testing::Test {
+ public:
+ void CheckModified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ if (src.buffers[i]->address() != dst.buffers[i]->address()) {
+ return;
+ }
+ }
+ FAIL() << "Expected at least one buffer to have been modified by
EnsureAlignment";
+ }
+
+ void CheckUnmodified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ ASSERT_EQ(src.buffers[i]->address(), dst.buffers[i]->address());
+ }
+ }
+};
+
+template <typename T>
+class MallocAlignmentRequired : public MallocAlignment {};
+template <typename T>
+class MallocAlignmentNotRequired : public MallocAlignment {};
+
+TYPED_TEST_SUITE(MallocAlignmentRequired, TypesRequiringSomeKindOfAlignment);
+TYPED_TEST_SUITE(MallocAlignmentNotRequired, TypesNotRequiringAlignment);
+
+TYPED_TEST(MallocAlignmentRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
+ this->CheckModified(*unaligned, *aligned);
+}
+
+TYPED_TEST(MallocAlignmentNotRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
+ this->CheckUnmodified(*unaligned, *aligned);
+}
+
+TEST_F(MallocAlignment, RunEndEncoded) {
+ // Run end requires alignment, value type does not
+ std::shared_ptr<Array> run_ends = ArrayFromJSON(int32(), "[3, 5]");
+ std::shared_ptr<Array> values = ArrayFromJSON(int8(), "[50, 100]");
+ ASSERT_OK_AND_ASSIGN(std::shared_ptr<Array> array,
+ RunEndEncodedArray::Make(/*logical_length=*/5,
std::move(run_ends),
+ std::move(values), 0));
+
+ std::shared_ptr<ArrayData> unaligned_ree =
std::make_shared<ArrayData>(*array->data());
+ unaligned_ree->child_data[0] = UnalignValues(*unaligned_ree->child_data[0]);
+ unaligned_ree->child_data[1] = UnalignValues(*unaligned_ree->child_data[1]);
+
+ std::shared_ptr<ArrayData> aligned_ree =
std::make_shared<ArrayData>(*unaligned_ree);
+
+ ASSERT_OK_AND_ASSIGN(
+ aligned_ree,
+ util::EnsureAlignment(aligned_ree, util::kValueAlignment,
default_memory_pool()));
+
+ this->CheckModified(*unaligned_ree->child_data[0],
*aligned_ree->child_data[0]);
+ this->CheckUnmodified(*unaligned_ree->child_data[1],
*aligned_ree->child_data[1]);
+}
+
+TEST_F(MallocAlignment, Dictionary) {
+ // Dictionary values require alignment, dictionary keys do not
+ std::shared_ptr<DataType> int8_utf8 = dictionary(int8(), utf8());
+ std::shared_ptr<Array> array = ArrayFromJSON(int8_utf8, R"(["x", "x",
"y"])");
+
+ std::shared_ptr<ArrayData> unaligned_dict =
std::make_shared<ArrayData>(*array->data());
+ unaligned_dict->dictionary = UnalignValues(*unaligned_dict->dictionary);
+ unaligned_dict = UnalignValues(*unaligned_dict);
+
+ std::shared_ptr<ArrayData> aligned_dict =
std::make_shared<ArrayData>(*unaligned_dict);
+
+ ASSERT_OK_AND_ASSIGN(
+ aligned_dict,
+ util::EnsureAlignment(aligned_dict, util::kValueAlignment,
default_memory_pool()));
+
+ this->CheckUnmodified(*unaligned_dict, *aligned_dict);
+ this->CheckModified(*unaligned_dict->dictionary, *aligned_dict->dictionary);
+
+ // Dictionary values do not require alignment, dictionary keys do
+ std::shared_ptr<DataType> int16_int8 = dictionary(int16(), int8());
+ array = ArrayFromJSON(int16_int8, R"([7, 11])");
+
+ unaligned_dict = std::make_shared<ArrayData>(*array->data());
+ unaligned_dict->dictionary = UnalignValues(*unaligned_dict->dictionary);
+ unaligned_dict = UnalignValues(*unaligned_dict);
+
+ aligned_dict = std::make_shared<ArrayData>(*unaligned_dict);
+
+ ASSERT_OK_AND_ASSIGN(
+ aligned_dict,
+ util::EnsureAlignment(aligned_dict, util::kValueAlignment,
default_memory_pool()));
Review Comment:
Same here (validate)
##########
cpp/src/arrow/util/align_util_test.cc:
##########
@@ -278,4 +279,225 @@ TEST(EnsureAlignment, Table) {
ASSERT_EQ(util::CheckAlignment(*aligned_table, 2048, &needs_alignment),
true);
}
+using TypesRequiringSomeKindOfAlignment =
+ testing::Types<Int16Type, Int32Type, Int64Type, UInt16Type, UInt32Type,
UInt64Type,
+ FloatType, DoubleType, Date32Type, Date64Type, Time32Type,
Time64Type,
+ Decimal128Type, Decimal256Type, TimestampType,
DurationType, MapType,
+ DenseUnionType, LargeBinaryType, LargeListType,
LargeStringType,
+ MonthIntervalType, DayTimeIntervalType,
MonthDayNanoIntervalType>;
+
+using TypesNotRequiringAlignment =
+ testing::Types<NullType, Int8Type, UInt8Type, FixedSizeListType,
FixedSizeBinaryType,
+ BooleanType, SparseUnionType>;
+
+TEST(EnsureAlignment, Malloc) {}
+
+template <typename ArrowType>
+std::shared_ptr<DataType> sample_type() {
+ return TypeTraits<ArrowType>::type_singleton();
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeBinaryType>() {
+ return fixed_size_binary(16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeListType>() {
+ return fixed_size_list(uint8(), 16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal128Type>() {
+ return decimal128(32, 6);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal256Type>() {
+ return decimal256(60, 10);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<LargeListType>() {
+ return large_list(int8());
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DenseUnionType>() {
+ return dense_union({field("x", int8()), field("y", uint8())});
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<MapType>() {
+ return map(utf8(), field("item", utf8()));
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DurationType>() {
+ return duration(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<TimestampType>() {
+ return timestamp(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time32Type>() {
+ return time32(TimeUnit::SECOND);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time64Type>() {
+ return time64(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<SparseUnionType>() {
+ return sparse_union({field("x", uint8()), field("y", int8())}, {1, 2});
+}
+
+template <typename ArrowType>
+std::shared_ptr<ArrayData> SampleArray() {
+ random::RandomArrayGenerator gen(42);
+ return gen.ArrayOf(sample_type<ArrowType>(), 100)->data();
+}
+
+template <>
+std::shared_ptr<ArrayData> SampleArray<SparseUnionType>() {
+ auto ty = sparse_union({field("ints", int64()), field("strs", utf8())}, {2,
7});
+ auto ints = ArrayFromJSON(int64(), "[0, 1, 2, 3]");
+ auto strs = ArrayFromJSON(utf8(), R"(["a", null, "c", "d"])");
+ auto ids = ArrayFromJSON(int8(), "[2, 7, 2, 7]")->data()->buffers[1];
+ const int length = 4;
+ SparseUnionArray arr(ty, length, {ints, strs}, ids);
+ return arr.data();
+}
+
+class MallocAlignment : public ::testing::Test {
+ public:
+ void CheckModified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ if (src.buffers[i]->address() != dst.buffers[i]->address()) {
+ return;
+ }
+ }
+ FAIL() << "Expected at least one buffer to have been modified by
EnsureAlignment";
+ }
+
+ void CheckUnmodified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ ASSERT_EQ(src.buffers[i]->address(), dst.buffers[i]->address());
+ }
+ }
+};
+
+template <typename T>
+class MallocAlignmentRequired : public MallocAlignment {};
+template <typename T>
+class MallocAlignmentNotRequired : public MallocAlignment {};
+
+TYPED_TEST_SUITE(MallocAlignmentRequired, TypesRequiringSomeKindOfAlignment);
+TYPED_TEST_SUITE(MallocAlignmentNotRequired, TypesNotRequiringAlignment);
+
+TYPED_TEST(MallocAlignmentRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
+ this->CheckModified(*unaligned, *aligned);
+}
+
+TYPED_TEST(MallocAlignmentNotRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
+ this->CheckUnmodified(*unaligned, *aligned);
+}
+
+TEST_F(MallocAlignment, RunEndEncoded) {
+ // Run end requires alignment, value type does not
+ std::shared_ptr<Array> run_ends = ArrayFromJSON(int32(), "[3, 5]");
+ std::shared_ptr<Array> values = ArrayFromJSON(int8(), "[50, 100]");
+ ASSERT_OK_AND_ASSIGN(std::shared_ptr<Array> array,
+ RunEndEncodedArray::Make(/*logical_length=*/5,
std::move(run_ends),
+ std::move(values), 0));
+
+ std::shared_ptr<ArrayData> unaligned_ree =
std::make_shared<ArrayData>(*array->data());
+ unaligned_ree->child_data[0] = UnalignValues(*unaligned_ree->child_data[0]);
+ unaligned_ree->child_data[1] = UnalignValues(*unaligned_ree->child_data[1]);
+
+ std::shared_ptr<ArrayData> aligned_ree =
std::make_shared<ArrayData>(*unaligned_ree);
+
+ ASSERT_OK_AND_ASSIGN(
+ aligned_ree,
+ util::EnsureAlignment(aligned_ree, util::kValueAlignment,
default_memory_pool()));
+
+ this->CheckModified(*unaligned_ree->child_data[0],
*aligned_ree->child_data[0]);
+ this->CheckUnmodified(*unaligned_ree->child_data[1],
*aligned_ree->child_data[1]);
+}
+
+TEST_F(MallocAlignment, Dictionary) {
+ // Dictionary values require alignment, dictionary keys do not
+ std::shared_ptr<DataType> int8_utf8 = dictionary(int8(), utf8());
+ std::shared_ptr<Array> array = ArrayFromJSON(int8_utf8, R"(["x", "x",
"y"])");
+
+ std::shared_ptr<ArrayData> unaligned_dict =
std::make_shared<ArrayData>(*array->data());
+ unaligned_dict->dictionary = UnalignValues(*unaligned_dict->dictionary);
+ unaligned_dict = UnalignValues(*unaligned_dict);
+
+ std::shared_ptr<ArrayData> aligned_dict =
std::make_shared<ArrayData>(*unaligned_dict);
+
+ ASSERT_OK_AND_ASSIGN(
+ aligned_dict,
+ util::EnsureAlignment(aligned_dict, util::kValueAlignment,
default_memory_pool()));
Review Comment:
Same here (validate result).
##########
cpp/src/arrow/util/align_util.h:
##########
@@ -70,8 +70,27 @@ inline BitmapWordAlignParams BitmapWordAlign(const uint8_t*
data, int64_t bit_of
namespace util {
// Functions to check if the provided Arrow object is aligned by the specified
alignment
+
+/// \brief if this is specified in one of the CheckAlignment or
EnsureAlignment functions
+///
+/// then the function will ensure each buffer is suitably aligned for the data
type of the
+/// array. For example, given an int32 buffer the validity buffer must be a
multiple of 8
+/// and the values buffer must be a multiple of 32. Given a large_string
buffer the
+/// validity buffer and values buffers must be multiples of 8 and the offsets
buffer must
+/// be a multiple of 64.
+constexpr int64_t kValueAlignment = -3;
+
+/// \brief calculate if the buffer's address is a multiple of `alignment`
+/// \param buffer the buffer to check
+/// \param alignment the alignment to check for
Review Comment:
In bytes or in bits? I would expect bytes, as bits alignment doesn't seem to
make sense.
##########
cpp/src/arrow/util/align_util.h:
##########
@@ -82,29 +101,109 @@ ARROW_EXPORT bool CheckAlignment(const Array& array,
int64_t alignment);
// of the constituent objects during the EnsureAlignment function where certain
// objects can be ignored for further checking if we already know that they are
// completely aligned.
+
+/// \brief calculate which (if any) chunks in a chunked array are unaligned
+/// \param array the array to check
+/// \param alignment the alignment to check for
+/// \param needs_alignment an output vector that will store the results of the
check
+/// it must be set to a valid vector. Extra elements will be added to
the end
+/// of the vector for each chunk that is checked. `true` will be
stored if
Review Comment:
Does this mean that the vector isn't cleared if it already has elements?
This seems like a strange API contract, is there a motivation for it?
##########
cpp/src/arrow/util/align_util.h:
##########
@@ -70,8 +70,27 @@ inline BitmapWordAlignParams BitmapWordAlign(const uint8_t*
data, int64_t bit_of
namespace util {
// Functions to check if the provided Arrow object is aligned by the specified
alignment
+
+/// \brief if this is specified in one of the CheckAlignment or
EnsureAlignment functions
+///
+/// then the function will ensure each buffer is suitably aligned for the data
type of the
+/// array. For example, given an int32 buffer the validity buffer must be a
multiple of 8
+/// and the values buffer must be a multiple of 32. Given a large_string
buffer the
+/// validity buffer and values buffers must be multiples of 8 and the offsets
buffer must
+/// be a multiple of 64.
+constexpr int64_t kValueAlignment = -3;
+
+/// \brief calculate if the buffer's address is a multiple of `alignment`
+/// \param buffer the buffer to check
+/// \param alignment the alignment to check for
ARROW_EXPORT bool CheckAlignment(const Buffer& buffer, int64_t alignment);
+/// \brief calculate if all buffer's in the array data are aligned
+/// \param array the array data to check
+/// \param alignment the alignment to check for
ARROW_EXPORT bool CheckAlignment(const ArrayData& array, int64_t alignment);
+/// \brief calculate if all buffer's in the array are aligned
Review Comment:
```suggestion
/// \brief Calculate if all buffers in the array are aligned
```
##########
cpp/src/arrow/util/align_util.h:
##########
@@ -70,8 +70,27 @@ inline BitmapWordAlignParams BitmapWordAlign(const uint8_t*
data, int64_t bit_of
namespace util {
// Functions to check if the provided Arrow object is aligned by the specified
alignment
+
+/// \brief if this is specified in one of the CheckAlignment or
EnsureAlignment functions
+///
+/// then the function will ensure each buffer is suitably aligned for the data
type of the
Review Comment:
Let's make the "brief" line a standalone description.
```suggestion
/// \brief Special alignment value to use data type-specific alignment
///
/// If this is passed as the `alignment` in one of the CheckAlignment or
EnsureAlignment
/// functions, then the function will ensure ensure each buffer is suitably
aligned
/// for the data type of the
```
##########
cpp/src/arrow/util/align_util_test.cc:
##########
@@ -278,4 +279,225 @@ TEST(EnsureAlignment, Table) {
ASSERT_EQ(util::CheckAlignment(*aligned_table, 2048, &needs_alignment),
true);
}
+using TypesRequiringSomeKindOfAlignment =
+ testing::Types<Int16Type, Int32Type, Int64Type, UInt16Type, UInt32Type,
UInt64Type,
+ FloatType, DoubleType, Date32Type, Date64Type, Time32Type,
Time64Type,
+ Decimal128Type, Decimal256Type, TimestampType,
DurationType, MapType,
+ DenseUnionType, LargeBinaryType, LargeListType,
LargeStringType,
+ MonthIntervalType, DayTimeIntervalType,
MonthDayNanoIntervalType>;
+
+using TypesNotRequiringAlignment =
+ testing::Types<NullType, Int8Type, UInt8Type, FixedSizeListType,
FixedSizeBinaryType,
+ BooleanType, SparseUnionType>;
+
+TEST(EnsureAlignment, Malloc) {}
+
+template <typename ArrowType>
+std::shared_ptr<DataType> sample_type() {
+ return TypeTraits<ArrowType>::type_singleton();
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeBinaryType>() {
+ return fixed_size_binary(16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeListType>() {
+ return fixed_size_list(uint8(), 16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal128Type>() {
+ return decimal128(32, 6);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal256Type>() {
+ return decimal256(60, 10);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<LargeListType>() {
+ return large_list(int8());
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DenseUnionType>() {
+ return dense_union({field("x", int8()), field("y", uint8())});
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<MapType>() {
+ return map(utf8(), field("item", utf8()));
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DurationType>() {
+ return duration(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<TimestampType>() {
+ return timestamp(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time32Type>() {
+ return time32(TimeUnit::SECOND);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time64Type>() {
+ return time64(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<SparseUnionType>() {
+ return sparse_union({field("x", uint8()), field("y", int8())}, {1, 2});
+}
+
+template <typename ArrowType>
+std::shared_ptr<ArrayData> SampleArray() {
+ random::RandomArrayGenerator gen(42);
+ return gen.ArrayOf(sample_type<ArrowType>(), 100)->data();
+}
+
+template <>
+std::shared_ptr<ArrayData> SampleArray<SparseUnionType>() {
+ auto ty = sparse_union({field("ints", int64()), field("strs", utf8())}, {2,
7});
+ auto ints = ArrayFromJSON(int64(), "[0, 1, 2, 3]");
+ auto strs = ArrayFromJSON(utf8(), R"(["a", null, "c", "d"])");
+ auto ids = ArrayFromJSON(int8(), "[2, 7, 2, 7]")->data()->buffers[1];
+ const int length = 4;
+ SparseUnionArray arr(ty, length, {ints, strs}, ids);
+ return arr.data();
+}
+
+class MallocAlignment : public ::testing::Test {
+ public:
+ void CheckModified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ if (src.buffers[i]->address() != dst.buffers[i]->address()) {
+ return;
+ }
+ }
+ FAIL() << "Expected at least one buffer to have been modified by
EnsureAlignment";
+ }
+
+ void CheckUnmodified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ ASSERT_EQ(src.buffers[i]->address(), dst.buffers[i]->address());
+ }
+ }
+};
+
+template <typename T>
+class MallocAlignmentRequired : public MallocAlignment {};
+template <typename T>
+class MallocAlignmentNotRequired : public MallocAlignment {};
+
+TYPED_TEST_SUITE(MallocAlignmentRequired, TypesRequiringSomeKindOfAlignment);
+TYPED_TEST_SUITE(MallocAlignmentNotRequired, TypesNotRequiringAlignment);
+
+TYPED_TEST(MallocAlignmentRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
+ this->CheckModified(*unaligned, *aligned);
+}
+
+TYPED_TEST(MallocAlignmentNotRequired, RoundTrip) {
+ std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
+ std::shared_ptr<ArrayData> unaligned = UnalignValues(*data);
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<ArrayData> aligned,
+ util::EnsureAlignment(unaligned, util::kValueAlignment,
default_memory_pool()));
+
+ AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
+ this->CheckUnmodified(*unaligned, *aligned);
+}
+
+TEST_F(MallocAlignment, RunEndEncoded) {
+ // Run end requires alignment, value type does not
+ std::shared_ptr<Array> run_ends = ArrayFromJSON(int32(), "[3, 5]");
+ std::shared_ptr<Array> values = ArrayFromJSON(int8(), "[50, 100]");
+ ASSERT_OK_AND_ASSIGN(std::shared_ptr<Array> array,
+ RunEndEncodedArray::Make(/*logical_length=*/5,
std::move(run_ends),
+ std::move(values), 0));
+
+ std::shared_ptr<ArrayData> unaligned_ree =
std::make_shared<ArrayData>(*array->data());
+ unaligned_ree->child_data[0] = UnalignValues(*unaligned_ree->child_data[0]);
+ unaligned_ree->child_data[1] = UnalignValues(*unaligned_ree->child_data[1]);
+
+ std::shared_ptr<ArrayData> aligned_ree =
std::make_shared<ArrayData>(*unaligned_ree);
+
+ ASSERT_OK_AND_ASSIGN(
+ aligned_ree,
+ util::EnsureAlignment(aligned_ree, util::kValueAlignment,
default_memory_pool()));
+
+ this->CheckModified(*unaligned_ree->child_data[0],
*aligned_ree->child_data[0]);
+ this->CheckUnmodified(*unaligned_ree->child_data[1],
*aligned_ree->child_data[1]);
+}
+
+TEST_F(MallocAlignment, Dictionary) {
+ // Dictionary values require alignment, dictionary keys do not
Review Comment:
```suggestion
// Dictionary values require alignment, dictionary indices do not
```
##########
cpp/src/arrow/acero/exec_plan.h:
##########
@@ -555,6 +565,34 @@ struct ARROW_ACERO_EXPORT QueryOptions {
///
/// If set then the number of names must equal the number of output columns
std::vector<std::string> field_names;
+
+ /// Various compute functions and acero internals will type pun array
+ /// buffers from uint8_t* to some kind of value type (e.g. we might
+ /// cast to int32_t* to add two int32 arrays)
+ ///
+ /// If the buffer is poorly algined (e.g. an int32 array is not aligned
+ /// on a 4-byte boundary) then this is technically undefined behavior.
Review Comment:
```suggestion
/// If the buffer is poorly aligned (e.g. an int32 array is not aligned
/// on a 4-byte boundary) then this is technically undefined behavior in
C++.
```
##########
cpp/src/arrow/util/align_util_test.cc:
##########
@@ -278,4 +279,225 @@ TEST(EnsureAlignment, Table) {
ASSERT_EQ(util::CheckAlignment(*aligned_table, 2048, &needs_alignment),
true);
}
+using TypesRequiringSomeKindOfAlignment =
+ testing::Types<Int16Type, Int32Type, Int64Type, UInt16Type, UInt32Type,
UInt64Type,
+ FloatType, DoubleType, Date32Type, Date64Type, Time32Type,
Time64Type,
+ Decimal128Type, Decimal256Type, TimestampType,
DurationType, MapType,
+ DenseUnionType, LargeBinaryType, LargeListType,
LargeStringType,
+ MonthIntervalType, DayTimeIntervalType,
MonthDayNanoIntervalType>;
+
+using TypesNotRequiringAlignment =
+ testing::Types<NullType, Int8Type, UInt8Type, FixedSizeListType,
FixedSizeBinaryType,
+ BooleanType, SparseUnionType>;
+
+TEST(EnsureAlignment, Malloc) {}
+
+template <typename ArrowType>
+std::shared_ptr<DataType> sample_type() {
+ return TypeTraits<ArrowType>::type_singleton();
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeBinaryType>() {
+ return fixed_size_binary(16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<FixedSizeListType>() {
+ return fixed_size_list(uint8(), 16);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal128Type>() {
+ return decimal128(32, 6);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Decimal256Type>() {
+ return decimal256(60, 10);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<LargeListType>() {
+ return large_list(int8());
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DenseUnionType>() {
+ return dense_union({field("x", int8()), field("y", uint8())});
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<MapType>() {
+ return map(utf8(), field("item", utf8()));
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<DurationType>() {
+ return duration(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<TimestampType>() {
+ return timestamp(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time32Type>() {
+ return time32(TimeUnit::SECOND);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<Time64Type>() {
+ return time64(TimeUnit::NANO);
+}
+
+template <>
+std::shared_ptr<DataType> sample_type<SparseUnionType>() {
+ return sparse_union({field("x", uint8()), field("y", int8())}, {1, 2});
+}
+
+template <typename ArrowType>
+std::shared_ptr<ArrayData> SampleArray() {
+ random::RandomArrayGenerator gen(42);
+ return gen.ArrayOf(sample_type<ArrowType>(), 100)->data();
+}
+
+template <>
+std::shared_ptr<ArrayData> SampleArray<SparseUnionType>() {
+ auto ty = sparse_union({field("ints", int64()), field("strs", utf8())}, {2,
7});
+ auto ints = ArrayFromJSON(int64(), "[0, 1, 2, 3]");
+ auto strs = ArrayFromJSON(utf8(), R"(["a", null, "c", "d"])");
+ auto ids = ArrayFromJSON(int8(), "[2, 7, 2, 7]")->data()->buffers[1];
+ const int length = 4;
+ SparseUnionArray arr(ty, length, {ints, strs}, ids);
+ return arr.data();
+}
+
+class MallocAlignment : public ::testing::Test {
+ public:
+ void CheckModified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ if (src.buffers[i]->address() != dst.buffers[i]->address()) {
+ return;
+ }
+ }
+ FAIL() << "Expected at least one buffer to have been modified by
EnsureAlignment";
+ }
+
+ void CheckUnmodified(const ArrayData& src, const ArrayData& dst) {
+ ASSERT_EQ(src.buffers.size(), dst.buffers.size());
+ for (std::size_t i = 0; i < src.buffers.size(); i++) {
+ if (!src.buffers[i] || !dst.buffers[i]) {
+ continue;
+ }
+ ASSERT_EQ(src.buffers[i]->address(), dst.buffers[i]->address());
+ }
+ }
+};
+
+template <typename T>
+class MallocAlignmentRequired : public MallocAlignment {};
+template <typename T>
+class MallocAlignmentNotRequired : public MallocAlignment {};
+
+TYPED_TEST_SUITE(MallocAlignmentRequired, TypesRequiringSomeKindOfAlignment);
+TYPED_TEST_SUITE(MallocAlignmentNotRequired, TypesNotRequiringAlignment);
+
Review Comment:
Something like:
```c++
TYPED_TEST(MallocAlignmentRequired, AlreadyAligned) {
std::shared_ptr<ArrayData> data = SampleArray<TypeParam>();
ASSERT_OK_AND_ASSIGN(
std::shared_ptr<ArrayData> aligned,
util::EnsureAlignment(data, util::kValueAlignment,
default_memory_pool()));
ASSERT_OK(ValidateArrayFull(*aligned));
AssertArraysEqual(*MakeArray(data), *MakeArray(aligned));
this->CheckUnmodified(*data, *aligned);
}
```
##########
cpp/src/arrow/util/align_util.h:
##########
@@ -82,29 +101,109 @@ ARROW_EXPORT bool CheckAlignment(const Array& array,
int64_t alignment);
// of the constituent objects during the EnsureAlignment function where certain
// objects can be ignored for further checking if we already know that they are
// completely aligned.
+
+/// \brief calculate which (if any) chunks in a chunked array are unaligned
Review Comment:
Nit: capitalize
##########
cpp/src/arrow/util/align_util.h:
##########
@@ -70,8 +70,27 @@ inline BitmapWordAlignParams BitmapWordAlign(const uint8_t*
data, int64_t bit_of
namespace util {
// Functions to check if the provided Arrow object is aligned by the specified
alignment
+
+/// \brief if this is specified in one of the CheckAlignment or
EnsureAlignment functions
+///
+/// then the function will ensure each buffer is suitably aligned for the data
type of the
+/// array. For example, given an int32 buffer the validity buffer must be a
multiple of 8
+/// and the values buffer must be a multiple of 32. Given a large_string
buffer the
+/// validity buffer and values buffers must be multiples of 8 and the offsets
buffer must
+/// be a multiple of 64.
Review Comment:
I would expect alignment values to be given in bytes?
```suggestion
/// array. For example, given an int32 buffer the validity buffer must be a
multiple of 1
/// and the values buffer must be a multiple of 4. Given a large_string
buffer the
/// validity buffer and values buffer must be multiples of 1 and the offsets
buffer must
/// be a multiple of 8.
```
##########
cpp/src/arrow/util/align_util.cc:
##########
@@ -30,12 +33,50 @@ bool CheckAlignment(const Buffer& buffer, int64_t
alignment) {
return buffer.address() % alignment == 0;
}
-bool CheckAlignment(const ArrayData& array, int64_t alignment) {
- for (const auto& buffer : array.buffers) {
- if (buffer) {
- if (!CheckAlignment(*buffer, alignment)) return false;
+namespace {
+
+// Returns the type that controls how the buffers of this ArrayData (not its
children)
+// should behave
+Type::type GetTypeForBuffers(const ArrayData& array) {
+ Type::type type_id = array.type->storage_id();
+ if (type_id == Type::DICTIONARY) {
+ return ::arrow::internal::checked_pointer_cast<DictionaryType>(array.type)
+ ->index_type()
+ ->id();
+ }
+ return type_id;
+}
+
+// Checks to see if an array's own buffers are aligned but doesn't check
+// children
+bool CheckSelfAlignment(const ArrayData& array, int64_t alignment) {
+ if (alignment == kValueAlignment) {
+ Type::type type_id = GetTypeForBuffers(array);
+ for (std::size_t i = 0; i < array.buffers.size(); i++) {
+ if (array.buffers[i]) {
+ int expected_alignment =
+ RequiredValueAlignmentForBuffer(type_id, static_cast<int>(i));
+ if (!CheckAlignment(*array.buffers[i], expected_alignment)) {
+ return false;
+ }
+ }
+ }
+ } else {
+ for (const auto& buffer : array.buffers) {
+ if (buffer) {
+ if (!CheckAlignment(*buffer, alignment)) return false;
+ }
}
}
+ return true;
+}
+
+} // namespace
+
+bool CheckAlignment(const ArrayData& array, int64_t alignment) {
+ if (!CheckSelfAlignment(array, alignment)) {
+ return false;
+ }
if (array.type->id() == Type::DICTIONARY) {
Review Comment:
This won't work for an extension array with a dictionary storage type, right?
##########
cpp/src/arrow/util/align_util.cc:
##########
@@ -112,10 +158,17 @@ Result<std::shared_ptr<ArrayData>>
EnsureAlignment(std::shared_ptr<ArrayData> ar
MemoryPool* memory_pool) {
if (!CheckAlignment(*array_data, alignment)) {
std::vector<std::shared_ptr<Buffer>> buffers_ = array_data->buffers;
Review Comment:
Can we fix the style here? Names with a trailing underscore should be for
member variables.
```suggestion
std::vector<std::shared_ptr<Buffer>> buffers = array_data->buffers;
```
##########
cpp/src/arrow/type_traits.cc:
##########
@@ -0,0 +1,96 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "arrow/type_traits.h"
+
+#include "arrow/util/logging.h"
+
+namespace arrow {
+
+int RequiredValueAlignmentForBuffer(Type::type type_id, int buffer_index) {
+ if (buffer_index != 1) {
+ // If the buffer index is 0 then either:
+ // * The array type has no buffers, thus this shouldn't be called anyways
+ // * The array has a validity buffer at 0, no alignment needed
+ // * The array is a union array and has a types buffer at 0, no alignment
needed
+ // If the buffer index is > 1 then, in all current cases, it represents
binary
+ // data and no alignment is needed
Review Comment:
Hmm, I took a look again and it's more complicated. Unions don't have a null
bitmap, but in Arrow C++ we still reserve an empty slot in buffer 0 for the
non-existent union null bitmap. Therefore, the dense union offsets buffer is
number 2, not 1, and needs to be word-aligned...
##########
cpp/src/arrow/type_traits.h:
##########
@@ -1309,6 +1309,28 @@ static inline int offset_bit_width(Type::type type_id) {
return 0;
}
+/// \brief get the alignment a buffer should have to be considered "value
aligned"
+///
+/// Some buffers are frequently type-punned. For example, in an int32 array
the
+/// values buffer is frequently cast to int32_t*
+///
+/// This sort of punning is technically only valid if the pointer is aligned
to a
+/// proper width (e.g. 4 bytes in the case of int32). However, most modern
compilers
+/// are quite permissive if we get this wrong. Note that this alignment is
something
+/// that is guaranteed by malloc (e.g. new int32_t[] will return a buffer that
is 4
+/// byte aligned) or common libraries (e.g. numpy) but it is not currently
guaranteed
+/// by flight (GH-32276).
+///
+/// We call this "value aligned" and this method will calculate that required
alignment.
+///
+/// \param type_id the type of the array containing the buffer
+/// Note: this should be the indices type for a dictionary
array since
+/// A dictionary array's buffers are indices. It should be the
storage
+/// type for an extension array.
+/// \param buffer_index the index of the buffer to check, for example 0 will
typically
+/// give you the alignment expected of the validity buffer
Review Comment:
```suggestion
/// give you the alignment expected of the validity
buffer
/// \return the required value alignment in bytes (1 if no alignment
required)
```
##########
cpp/src/arrow/type_traits.h:
##########
@@ -1309,6 +1309,28 @@ static inline int offset_bit_width(Type::type type_id) {
return 0;
}
+/// \brief get the alignment a buffer should have to be considered "value
aligned"
Review Comment:
```suggestion
/// \brief Get the alignment a buffer should have to be considered "value
aligned"
```
##########
cpp/src/arrow/testing/gtest_util.h:
##########
@@ -532,4 +532,9 @@ class ARROW_TESTING_EXPORT GatingTask {
std::shared_ptr<Impl> impl_;
};
+/// \brief modify an array so that the buffer at index 1 (if it has one) is
unaligned
+ARROW_TESTING_EXPORT std::shared_ptr<ArrayData> UnalignValues(const ArrayData&
array);
+/// \brief modify an array so that the buffer at index 1 (if it has one) is
unaligned
+ARROW_TESTING_EXPORT std::shared_ptr<Array> UnalignValues(const Array& array);
Review Comment:
Why not instead "unalign" all buffers, even if no particular alignment is
required? This would probably make things more interesting?
##########
cpp/src/arrow/acero/exec_plan.h:
##########
@@ -555,6 +565,34 @@ struct ARROW_ACERO_EXPORT QueryOptions {
///
/// If set then the number of names must equal the number of output columns
std::vector<std::string> field_names;
+
+ /// Various compute functions and acero internals will type pun array
Review Comment:
```suggestion
/// \brief Policy for unaligned buffers in source data
///
/// Various compute functions and acero internals will type pun array
```
##########
cpp/src/arrow/testing/gtest_util.h:
##########
@@ -532,4 +532,9 @@ class ARROW_TESTING_EXPORT GatingTask {
std::shared_ptr<Impl> impl_;
};
+/// \brief modify an array so that the buffer at index 1 (if it has one) is
unaligned
Review Comment:
Also mention that this doesn't recurse into children and dictionary?
##########
cpp/src/arrow/acero/source_node.cc:
##########
@@ -52,6 +52,45 @@ using arrow::internal::MapVector;
namespace acero {
namespace {
+Status HandleUnalignedBuffers(ExecBatch* batch, UnalignedBufferHandling
handling) {
+ if (handling == UnalignedBufferHandling::kIgnore) {
+ return Status::OK();
+ }
+ for (auto& value : batch->values) {
+ if (value.is_array()) {
+ switch (handling) {
+ case UnalignedBufferHandling::kIgnore:
+ // Should be impossible to get here
+ return Status::OK();
+ case UnalignedBufferHandling::kAbort:
+ if (!arrow::util::CheckAlignment(*value.array(),
+ arrow::util::kValueAlignment)) {
+ return Status::Invalid(
+ "An input buffer was poorly aligned and
UnalignedBufferHandling is set "
+ "to kAbort");
+ }
+ break;
+ case UnalignedBufferHandling::kWarn:
+ if (!arrow::util::CheckAlignment(*value.array(),
+ arrow::util::kValueAlignment)) {
+ ARROW_LOG(WARNING)
+ << "An input buffer was poorly aligned. This could lead to
crashes or "
+ "poor performance on some hardware. Please ensure that all
Acero "
+ "sources generate aligned buffers.";
Review Comment:
```suggestion
"sources generate aligned buffers, or change the
unaligned buffer "
"handling configuration.";
```
##########
cpp/src/arrow/acero/exec_plan.h:
##########
@@ -555,6 +565,34 @@ struct ARROW_ACERO_EXPORT QueryOptions {
///
/// If set then the number of names must equal the number of output columns
std::vector<std::string> field_names;
+
+ /// Various compute functions and acero internals will type pun array
+ /// buffers from uint8_t* to some kind of value type (e.g. we might
+ /// cast to int32_t* to add two int32 arrays)
+ ///
+ /// If the buffer is poorly algined (e.g. an int32 array is not aligned
+ /// on a 4-byte boundary) then this is technically undefined behavior.
+ /// However, most modern compilers and CPUs are fairly tolerant of this
+ /// behavior and nothing bad (beyond a small hit to performance) is likely
+ /// to happen.
+ ///
+ /// Note that this only applies to source buffers. All buffers allocated
interally
Review Comment:
```suggestion
/// Note that this only applies to source buffers. All buffers allocated
internally
```
##########
cpp/src/arrow/acero/exec_plan.h:
##########
@@ -496,6 +496,16 @@ struct ARROW_ACERO_EXPORT Declaration {
std::string label;
};
+/// \brief describes how to handle unaligned buffers
+enum class UnalignedBufferHandling { kWarn, kIgnore, kReallocate, kAbort };
Review Comment:
For me, "kAbort" implies the process will be aborted. Is this the case?
Otherwise, kError sounds better.
(besides, aborting the process is rather user-hostile...)
##########
cpp/src/arrow/util/align_util.h:
##########
@@ -70,8 +70,27 @@ inline BitmapWordAlignParams BitmapWordAlign(const uint8_t*
data, int64_t bit_of
namespace util {
// Functions to check if the provided Arrow object is aligned by the specified
alignment
+
+/// \brief if this is specified in one of the CheckAlignment or
EnsureAlignment functions
+///
+/// then the function will ensure each buffer is suitably aligned for the data
type of the
+/// array. For example, given an int32 buffer the validity buffer must be a
multiple of 8
+/// and the values buffer must be a multiple of 32. Given a large_string
buffer the
+/// validity buffer and values buffers must be multiples of 8 and the offsets
buffer must
+/// be a multiple of 64.
+constexpr int64_t kValueAlignment = -3;
+
+/// \brief calculate if the buffer's address is a multiple of `alignment`
+/// \param buffer the buffer to check
+/// \param alignment the alignment to check for
ARROW_EXPORT bool CheckAlignment(const Buffer& buffer, int64_t alignment);
+/// \brief calculate if all buffer's in the array data are aligned
Review Comment:
Also, does it include children and dictionary? I would expect so, but
probably want to be explicit in the docstring.
--
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]
