Repository navigation
ARROW-17479: [Go] Add ArraySpan and utilities - #13929
Conversation
|
@zeroshade Keep in mind that ArraySpan in C++ is merely a performance optimization over ArrayData. That may not make sense or be important for Go. |
|
@pitrou yup I'm aware. I haven't done the testing yet to prove it (I'll do so when I have more of the compute stuff fleshed out) but in theory it should provide similar optimization in Go by reducing the number of pointers and heap objects due to the statically sized arrays. In addition the semantics are different. ArrayData has an atomic ref count which is managed with a retain and release function. When constructed it automatically calls retain on any buffers and children it was constructed with, etc. ArraySpan doesn't attempt to make any handling of that ref count and isn't considered to own the buffers, and the ArraySpan objects can typically be passed around and copied when needed, adjusting the buffer count for parent buffers only when converted back to an array data at the end. Thus saving all of the retain and release management during kernel execution. |
|
Just wondering - would ArraySpan be an idiomatic name in Go? I think the C++ name derives from |
|
@lidavidm that's fair point, maybe |
lidavidm
left a comment
lidavidm
left a comment
There was a problem hiding this comment.
LGTM.
On second thought, naming it ArraySlice might be confusing if it doesn't really have the same semantics as a Golang slice, so keeping ArraySpan is reasonable
| Offset int64 | ||
| Buffers [3]BufferSpan | ||
|
|
||
| Scratch [2]uint64 |
There was a problem hiding this comment.
(IIRC, the scratch space is for dealing with offsets right? Might be good to document what this field is for since it's not immediately obvious)
There was a problem hiding this comment.
yea, it's used for offsets and the union type code
|
Benchmark runs are scheduled for baseline = 8c17925 and contender = 258173d. 258173d is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Relating to the building of the functionality for Compute in Go with Arrow, this is the implementation of ArraySpan / ExecValue / ExecResult etc. It was able to be separated out from the function interface definitions, so I was able to make this PR while apache#13924 is still being reviewed Authored-by: Matt Topol <[email protected]> Signed-off-by: Matt Topol <[email protected]>
Relating to the building of the functionality for Compute in Go with Arrow, this is the implementation of ArraySpan / ExecValue / ExecResult etc.
It was able to be separated out from the function interface definitions, so I was able to make this PR while #13924 is still being reviewed