CurtHagenlocher commented on code in PR #463:
URL: https://github.com/apache/arrow-dotnet/pull/463#discussion_r4307477411
##########
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:
Good catch, thanks. Fixed in 93b7ca9: `BuildBuffer` now clears everything
from the written length to the end of the owner's memory.
`GarbageMemoryAllocator` can now return more memory than requested, and the
padding tests use that option. I confirmed the padding test fails when only the
padding up to the 64-byte boundary is cleared.
--
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]
