Copilot commented on code in PR #463:
URL: https://github.com/apache/arrow-dotnet/pull/463#discussion_r4307362178
##########
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, so clear the
padding explicitly
+ int paddedLength =
checked((int)BitUtility.RoundUpToMultipleOf64(byteLength));
+ owner.Memory.Span.Slice(byteLength, paddedLength -
byteLength).Clear();
Review Comment:
An allocator can return more memory than requested, and `ArrowBuffer(owner)`
exposes all of `owner.Memory`. For example, a 132-byte payload requests 192
padded bytes; if the owner exposes 256 bytes, bytes 192–255 remain uncleared
and expose stale allocator contents. Clear from `byteLength` through the actual
end of the owner's memory. Add a test with an oversized, nonzero-filled owner:
`GarbageMemoryAllocator` currently truncates its exposed memory to the
requested length, so it misses this case.
--
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]
