Skip to content

Commit 27af864

Browse files
perf(iteration): use move instead of clone for iteration (#1586)
@scho-furiosa pointed out in #1584 that there was an unnecessary `clone` in `Baseiter::next` that was degrading performance for dynamic-dimensional arrays. This change switches to a move, netting a 74% speedup in iteration for dynamic-dimensional arrays. Tests (detailed in #1584) showed that there was a trade-off in performance between dynamic- and fixed-dimensional iteration. `ArrayD` performs best when mutating its index; `ArrayN` performs best when using a move. This is an indication that, in the future, `Baseiter::next` should dispatch to the "layout" / `Dimension` underlying type. Co-authored-by: Sungkeun Cho <scho@furiosa.ai>
1 parent fd67f70 commit 27af864

File tree

2 files changed

+2
-6
lines changed

2 files changed

+2
-6
lines changed

src/dimension/dimension_trait.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,8 @@ pub trait Dimension:
197197
/// or None if there are no more.
198198
// FIXME: use &Self for index or even &mut?
199199
#[inline]
200-
fn next_for(&self, index: Self) -> Option<Self>
200+
fn next_for(&self, mut index: Self) -> Option<Self>
201201
{
202-
let mut index = index;
203202
let mut done = false;
204203
for (&dim, ix) in zip(self.slice(), index.slice_mut()).rev() {
205204
*ix += 1;

src/iterators/mod.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,7 @@ impl<A, D: Dimension> Iterator for Baseiter<A, D>
7272
#[inline]
7373
fn next(&mut self) -> Option<Self::Item>
7474
{
75-
let index = match self.index {
76-
None => return None,
77-
Some(ref ix) => ix.clone(),
78-
};
75+
let index = self.index.take()?;
7976
let offset = D::stride_offset(&index, &self.strides);
8077
self.index = self.dim.next_for(index);
8178
unsafe { Some(self.ptr.offset(offset)) }

0 commit comments

Comments
 (0)