Minor: try and avoid an allocation creating GenericByteViewArray from ArrayData

[alamb]

Which issue does this PR close?

• part of Reduce overhead to create an Array from ArrayData (make_array) #9061
• follow on Avoid clones in make_array for StructArray and GenericByteViewArray #9114

Rationale for this change

@scovich noted in #9114 (comment) that calling Vec::remove does an extra copy and that Vec::from doesn't actually reuse the allocation the way I thought it did

What changes are included in this PR?

Build the Arc for buffers directly

Are these changes tested?

BY existing tests

Are there any user-facing changes?

[alamb]

the idea is that remove(0) copies all the buffers and then Arc::from allocates/copies it again. Using Arc::from_iter builds the results directly

[scovich]

LGTM, one nit.

I guess it would be hard to measure a benchmark impact from the change, without going to an extreme case?

[scovich]

nit:

Suggested change

	// first buffer is views
	let views = buffers[0].clone();
	// remaining buffers are data buffers
	let buffers = Arc::from_iter(buffers.into_iter().skip(1));
	// first buffer is views, remaining buffers are data buffers
	let buffers = buffers.into_iter();
	let views = buffers.next().unwrap(); // safety: never empty
	let buffers = Arc::from_iter(buffers);

(question tho -- why do we know buffers is never empty?)

Also -- it would be fewer lines of code to fold those two values into the constructor call?

let buffers = buffers.into_iter();
Self {
    data_type: T::DATA_TYPE,
    views: buffers.next().unwrap(),
    buffers: Arc::from_iter(buffers),

[alamb]

(question tho -- why do we know buffers is never empty?)

Basically because the ArrayData is validated for each Arrow type -- and since it is validated by

arrow-rs/arrow-data/src/data.rs

Line 353 in f122d77

new_self.validate_data()?;

there must always be at least buffer

[scovich]

That's about what I figured, but I couldn't find where the validation actually happened. Thanks for the pointer!

[alamb]

I guess it would be hard to measure a benchmark impact from the change, without going to an extreme case?

Yeah, thank you. I have found benchmarking anything related to allocations quite tricky

[alamb]

Also -- it would be fewer lines of code to fold those two values into the constructor call?

Good call -- done in 707bf41

[etseidl]

Looks good now. Thanks @alamb and @scovich

[alamb]

Thanks everyone