Copilot commented on code in PR #463:
URL: https://github.com/apache/arrow-dotnet/pull/463#discussion_r4307491777
##########
src/Apache.Arrow/Arrays/ArrayDataConcatenator.cs:
##########
@@ -1057,47 +1084,104 @@ private ArrayData
ConcatenateChildrenOrEmpty(List<ArrayData> children)
throw new InvalidOperationException("Nested array data must
contain child data.");
}
- private ArrowBuffer ConcatenateLargeOffsetBuffer()
+ private IMemoryOwner<byte> AllocateBuffer(int byteLength, out
Span<byte> destination)
{
- var builder = new ArrowBuffer.Builder<long>(_totalLength + 1);
- long baseOffset = 0;
+ // Buffers are padded to a multiple of 64 bytes, like those
built by ArrowBuffer.Builder
+ int paddedLength =
checked((int)BitUtility.RoundUpToMultipleOf64(byteLength));
+ IMemoryOwner<byte> owner = (_allocator ??
MemoryAllocator.Default.Value).Allocate(paddedLength);
+ destination = owner.Memory.Span.Slice(0, byteLength);
+ return owner;
+ }
- foreach (ArrayData arrayData in _arrayDataList)
+ private IMemoryOwner<byte> AllocateBitmap(out Span<byte>
destination)
+ {
+ IMemoryOwner<byte> owner =
AllocateBuffer(BitUtility.ByteCount(_totalLength), out destination);
+ if (destination.Length > 0)
{
- if (arrayData.Length == 0)
- {
- continue;
- }
+ // Bits are copied one range at a time, which leaves the
bits past the end untouched
+ destination[destination.Length - 1] = 0;
+ }
+
+ return owner;
+ }
- ReadOnlySpan<long> span =
arrayData.Buffers[1].Span.CastTo<long>().Slice(arrayData.Offset,
arrayData.Length + 1);
- var firstOffset = span[0];
+ private static ArrowBuffer BuildBuffer(IMemoryOwner<byte> owner,
int byteLength)
+ {
+ // Allocators don't have to return zeroed memory, and may
return more than was requested,
+ // so clear everything past the written bytes explicitly
+ owner.Memory.Span.Slice(byteLength).Clear();
+ return new ArrowBuffer(owner);
+ }
- foreach (long offset in span.Slice(0, arrayData.Length))
+ private ArrowBuffer ConcatenateLargeOffsetBuffer()
+ {
+ int byteLength = checked((_totalLength + 1) * sizeof(long));
+ IMemoryOwner<byte> owner = AllocateBuffer(byteLength, out
Span<byte> bytes);
+ try
+ {
+ Span<long> destination = bytes.CastTo<long>();
+ int position = 0;
+ long baseOffset = 0;
+
+ foreach (ArrayData arrayData in _arrayDataList)
{
- builder.Append(baseOffset + offset - firstOffset);
+ if (arrayData.Length == 0)
+ {
+ continue;
+ }
+
+ ReadOnlySpan<long> span =
arrayData.Buffers[1].Span.CastTo<long>().Slice(arrayData.Offset,
arrayData.Length + 1);
+ var firstOffset = span[0];
+
+ for (int i = 0; i < arrayData.Length; i++)
+ {
+ destination[position++] = baseOffset + span[i] -
firstOffset;
+ }
+
+ baseOffset += span[arrayData.Length] - firstOffset;
}
- baseOffset += span[arrayData.Length] - firstOffset;
+ destination[position] = baseOffset;
+ }
+ catch
+ {
+ owner.Dispose();
+ throw;
}
- builder.Append(baseOffset);
-
- return builder.Build(_allocator);
+ return BuildBuffer(owner, byteLength);
}
private ArrowBuffer ConcatenateLargeVariableBinaryValueBuffer()
{
- var builder = new ArrowBuffer.Builder<byte>();
-
+ int byteLength = 0;
foreach (ArrayData arrayData in _arrayDataList)
{
var offsets =
arrayData.Buffers[1].Span.CastTo<long>().Slice(arrayData.Offset,
arrayData.Length + 1);
- var firstOffset = checked((int)offsets[0]);
- var lastOffset = checked((int)offsets[arrayData.Length]);
-
builder.Append(arrayData.Buffers[2].Span.Slice(firstOffset, lastOffset -
firstOffset));
+ byteLength = checked(byteLength +
(int)(offsets[arrayData.Length] - offsets[0]));
Review Comment:
The `long` byte-span is cast to `int` before the surrounding `checked`
addition, so a valid LargeBinary chunk whose value buffer exceeds
`Int32.MaxValue` wraps to a negative length instead of throwing an overflow.
That can make `AllocateBuffer` fail with an unrelated argument error (or
produce an incorrect total when combined with other chunks); the checked
conversion needs to happen before the cast is used in the sum, as
`LargeBinaryArray.GetValueLength` does.
--
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]
