Mercurial > octave

changeset 29979:ac5e1b64f8c9

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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];