Return Buffers from ArrayData::buffers instead of slice (#1799) #3783
Conversation
| type Output = &'a Buffer; | ||
|
|
||
| #[inline] | ||
| fn index(&self, index: usize) -> &Self::Output { |
There was a problem hiding this comment.
This is somewhat unfortunate, having to return &&Buffer but we need to return a borrow with a 'a lifetime otherwise the following code doesn't compile.
let buffer = array.data().buffers()[0];
There was a problem hiding this comment.
I tried changing it to type Output = Buffer and got tons of errors. The one you mentioned can be fixed by introducing a temp variable for array.data().buffers() (very verbose...). But I don't know how to fix it if something else references buffer...
So I'm ok with &&Buffer here. Not sure if it's the only way but it does make other things easier (to me).
| type Output = &'a Buffer; | ||
|
|
||
| #[inline] | ||
| fn index(&self, index: usize) -> &Self::Output { |
There was a problem hiding this comment.
I tried changing it to type Output = Buffer and got tons of errors. The one you mentioned can be fixed by introducing a temp variable for array.data().buffers() (very verbose...). But I don't know how to fix it if something else references buffer...
So I'm ok with &&Buffer here. Not sure if it's the only way but it does make other things easier (to me).
| pub struct Buffers<'a>([Option<&'a Buffer>; 2]); | ||
|
|
||
| impl<'a> Buffers<'a> { | ||
| /// Temporary will be removed once ArrayData does not store `Vec<Buffer>` directly (#3769) |
There was a problem hiding this comment.
Will this whole struct be temporary or just this method?
|
Benchmark runs are scheduled for baseline = eff058f and contender = 231ae9b. 231ae9b is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Part of #1799
Relates to #1176
Rationale for this change
Currently ArrayData stores a
Vec<Buffer>and this allows it to return&[Buffer]. As we move to structured array data (#1799) implementations will no longer store a contiguous array ofBufferinstead storing a selection ofScalarBuffer. This PR adds a newBuffersstruct to smooth over this transition, and allow code to be slowly migrated.What changes are included in this PR?
Are there any user-facing changes?