zeroshade commented on code in PR #289:
URL: https://github.com/apache/arrow-go/pull/289#discussion_r1960112822
##########
arrow/memory/allocator.go:
##########
@@ -25,3 +25,58 @@ type Allocator interface {
Reallocate(size int, b []byte) []byte
Free(b []byte)
}
+
+// AlignedAllocator is an Allocator that returns buffers aligned to 64-byte
boundaries.
+type AlignedAllocator interface {
+ Allocator
+ // AllocateAligned returns a buffer of the requested size aligned to
64-byte boundaries. buf is the aligned buffer, and alloc is the actual
allocation that should be passed to Free. If alloc is nil, then it is the same
as buf.
+ AllocateAligned(size int) (buf []byte, alloc []byte)
+ ReallocateAligned(size int, buf []byte, alloc []byte) (newBuf []byte,
newAlloc []byte)
+}
+
+// MakeAlignedAllocator makes an AlignedAllocator for the given Allocator. If
the Allocator already implements AlignedAllocator, it is returned as-is. There
is usually no need to call this directly as Buffer will call this for you.
Review Comment:
same here, can we make this a multi-line comment instead?
##########
arrow/memory/allocator.go:
##########
@@ -25,3 +25,58 @@ type Allocator interface {
Reallocate(size int, b []byte) []byte
Free(b []byte)
}
+
+// AlignedAllocator is an Allocator that returns buffers aligned to 64-byte
boundaries.
+type AlignedAllocator interface {
+ Allocator
+ // AllocateAligned returns a buffer of the requested size aligned to
64-byte boundaries. buf is the aligned buffer, and alloc is the actual
allocation that should be passed to Free. If alloc is nil, then it is the same
as buf.
Review Comment:
can we make this a multi-line comment just to make things cleaner?
##########
arrow/memory/allocator.go:
##########
@@ -25,3 +25,58 @@ type Allocator interface {
Reallocate(size int, b []byte) []byte
Free(b []byte)
}
+
+// AlignedAllocator is an Allocator that returns buffers aligned to 64-byte
boundaries.
+type AlignedAllocator interface {
+ Allocator
+ // AllocateAligned returns a buffer of the requested size aligned to
64-byte boundaries. buf is the aligned buffer, and alloc is the actual
allocation that should be passed to Free. If alloc is nil, then it is the same
as buf.
+ AllocateAligned(size int) (buf []byte, alloc []byte)
+ ReallocateAligned(size int, buf []byte, alloc []byte) (newBuf []byte,
newAlloc []byte)
+}
+
+// MakeAlignedAllocator makes an AlignedAllocator for the given Allocator. If
the Allocator already implements AlignedAllocator, it is returned as-is. There
is usually no need to call this directly as Buffer will call this for you.
+func MakeAlignedAllocator(mem Allocator) AlignedAllocator {
+ if aligned, ok := mem.(AlignedAllocator); ok {
+ return aligned
+ }
+ return &alignedAllocator{mem}
+}
+
+type alignedAllocator struct {
+ mem Allocator
+}
+
+func (a *alignedAllocator) Allocate(size int) []byte {
+ return a.mem.Allocate(size)
+}
+
+func (a *alignedAllocator) Reallocate(size int, b []byte) []byte {
+ return a.mem.Reallocate(size, b)
+}
+
+func (a *alignedAllocator) Free(b []byte) {
+ a.mem.Free(b)
+}
+
+func (a *alignedAllocator) AllocateAligned(size int) ([]byte, []byte) {
+ buf := a.mem.Allocate(size + alignment)
+ addr := int(addressOf(buf))
+ next := roundUpToMultipleOf64(addr)
+ if addr != next {
+ shift := next - addr
+ return buf[shift : size+shift : size+shift], buf
Review Comment:
shouldn't the capacity be just `size`?
##########
arrow/memory/allocator.go:
##########
@@ -25,3 +25,58 @@ type Allocator interface {
Reallocate(size int, b []byte) []byte
Free(b []byte)
}
+
+// AlignedAllocator is an Allocator that returns buffers aligned to 64-byte
boundaries.
+type AlignedAllocator interface {
+ Allocator
+ // AllocateAligned returns a buffer of the requested size aligned to
64-byte boundaries. buf is the aligned buffer, and alloc is the actual
allocation that should be passed to Free. If alloc is nil, then it is the same
as buf.
+ AllocateAligned(size int) (buf []byte, alloc []byte)
+ ReallocateAligned(size int, buf []byte, alloc []byte) (newBuf []byte,
newAlloc []byte)
+}
+
+// MakeAlignedAllocator makes an AlignedAllocator for the given Allocator. If
the Allocator already implements AlignedAllocator, it is returned as-is. There
is usually no need to call this directly as Buffer will call this for you.
+func MakeAlignedAllocator(mem Allocator) AlignedAllocator {
+ if aligned, ok := mem.(AlignedAllocator); ok {
+ return aligned
+ }
+ return &alignedAllocator{mem}
+}
+
+type alignedAllocator struct {
+ mem Allocator
+}
+
+func (a *alignedAllocator) Allocate(size int) []byte {
+ return a.mem.Allocate(size)
+}
+
+func (a *alignedAllocator) Reallocate(size int, b []byte) []byte {
+ return a.mem.Reallocate(size, b)
+}
+
+func (a *alignedAllocator) Free(b []byte) {
+ a.mem.Free(b)
+}
Review Comment:
probably simpler to just embed the allocator instead of having to recreate
these:
```go
type alignedAllocator struct {
Allocator
}
```
and creating it would be:
```go
return &alignedAllocator{Allocator: mem}
```
that way it just picks up the embedded methods instead of having to write
them ourselves. If we ever change anything about the Allocator interface we
won't have to also make a change here.
##########
arrow/memory/allocator.go:
##########
@@ -25,3 +25,58 @@ type Allocator interface {
Reallocate(size int, b []byte) []byte
Free(b []byte)
}
+
+// AlignedAllocator is an Allocator that returns buffers aligned to 64-byte
boundaries.
Review Comment:
any particular reason to pick 64-byte boundary as opposed to letting it be
configurable?
--
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]
