feat: faster take for BitPackedArray and SparseArray
#1133
Conversation
This reverts commit f09f578.
|
Generally the same or much faster across the board
|
|
Is the screenshot meant to have 1.0's and question marks? Or do we have some float rounding errors? |
the 1.0's are normal for |
| // if we have a large number of relatively small batches, the overhead isn't worth it, and we're better off with a bulk patch | ||
| // roughly, if we have an average of less than 64 elements per batch, we prefer bulk patching | ||
| let prefer_bulk_patch = relative_indices.len() * BULK_PATCH_THRESHOLD > indices.len(); | ||
| // Group indices into 1024-element chunks and relativise them to the beginning of each chunk |
There was a problem hiding this comment.
Comment shouldn't say "relativize them" any more as it does not.
| }); | ||
| Ok(taken.reinterpret_cast(ptype).into_array()) | ||
| } | ||
| } | ||
|
|
||
| fn take_primitive<T: NativePType + BitPacking>( | ||
| // array_chunks must use while let so that we can get the remainder | ||
| #[allow(clippy::while_let_on_iterator)] |
There was a problem hiding this comment.
I think this is correct, you can still use the for loop and then access iterator afterwards
| /// | ||
| /// If this sparse array has no indices (i.e. all elements are equal to fill_value) | ||
| /// then it returns None. | ||
| pub fn max_index(&self) -> Option<usize> { |
There was a problem hiding this comment.
I know I added min_index but we shouldn't pile more here. The methods you want here is first_non_null and last_non_null and that can be implemented for any array
| @@ -22,7 +22,7 @@ impl<'a> TreeDisplayWrapper<'a> { | |||
| } | |||
| } | |||
|
|
|||
| impl<'a> fmt::Display for TreeDisplayWrapper<'a> { | |||
| impl fmt::Display for TreeDisplayWrapper<'_> { | |||
There was a problem hiding this comment.
I think if you're going to run nightly clippy we should do it in general in ci?
fixes #1039