Mercurial > jwe > octave
changeset 29984:ac5e1b64f8c9
Resolve two FIXME notes in Array::sort routines.
FIXME note in sort routines wondered whether there was a performance impact
from testing for NaN values on integer types (which cannot have IEEE NaN
values). Performance testing reveals there is no slowdown to using the
function, presumably because library routines have a very quick shortcut
to eject integers from consideration.
* Array.cc (sort (dim, mode)): Add comment explaining special case for
"stride == 1". Add note for future programmers that no special case
is needed for integers to avoid sort_isnan() function call. Remove
FIXME notes about impact on integers.
* Array.cc (sort (sidx, dim, mode)): Add comment for "stride == 1"
pointing to further explanation in Array::sort (dim,mode). Remove
FIXME notes about impact on integers.
author | Rik <rik@octave.org> |
---|---|
date | Tue, 17 Aug 2021 09:12:20 -0700 |
parents | c7c69808356f |
children | e3e0193963ea |
files | liboctave/array/Array.cc |
diffstat | 1 files changed, 8 insertions(+), 7 deletions(-) [+] |
line wrap: on
line diff
--- a/liboctave/array/Array.cc Tue Aug 17 09:55:02 2021 +0200 +++ b/liboctave/array/Array.cc Tue Aug 17 09:12:20 2021 -0700 @@ -1793,10 +1793,14 @@ if (stride == 1) { + // Special case along first dimension avoids gather/scatter AND directly + // sorts into destination buffer for an 11% performance boost. for (octave_idx_type j = 0; j < iter; j++) { - // copy and partition out NaNs. - // FIXME: impact on integer types noticeable? + // Copy and partition out NaNs. + // No need to special case integer types <T> from floating point + // types <T> to avoid sort_isnan() test as it makes no discernible + // performance impact. octave_idx_type kl = 0; octave_idx_type ku = ns; for (octave_idx_type i = 0; i < ns; i++) @@ -1834,7 +1838,6 @@ offset += n_strides * stride * (ns - 1); // gather and partition out NaNs. - // FIXME: impact on integer types noticeable? octave_idx_type kl = 0; octave_idx_type ku = ns; for (octave_idx_type i = 0; i < ns; i++) @@ -1906,10 +1909,11 @@ if (stride == 1) { + // Special case for dim 1 avoids gather/scatter for performance boost. + // See comments in Array::sort (dim, mode). for (octave_idx_type j = 0; j < iter; j++) { // copy and partition out NaNs. - // FIXME: impact on integer types noticeable? octave_idx_type kl = 0; octave_idx_type ku = ns; for (octave_idx_type i = 0; i < ns; i++) @@ -1961,7 +1965,6 @@ offset += n_strides * stride * (ns - 1); // gather and partition out NaNs. - // FIXME: impact on integer types noticeable? octave_idx_type kl = 0; octave_idx_type ku = ns; for (octave_idx_type i = 0; i < ns; i++) @@ -2394,7 +2397,6 @@ if (stride == 1) { // copy without NaNs. - // FIXME: impact on integer types noticeable? for (octave_idx_type i = 0; i < ns; i++) { T tmp = ov[i]; @@ -2410,7 +2412,6 @@ { octave_idx_type offset = j % stride; // copy without NaNs. - // FIXME: impact on integer types noticeable? for (octave_idx_type i = 0; i < ns; i++) { T tmp = ov[offset + i*stride];