octave-bug-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Octave-bug-tracker] [bug #61912] Slow performance of complex()


From: Rik
Subject: [Octave-bug-tracker] [bug #61912] Slow performance of complex()
Date: Tue, 25 Jan 2022 17:27:56 -0500 (EST)

Follow-up Comment #6, bug #61912 (project octave):

Adding jwe to the CC list as I'd like his opinion.

The code for Fcomplex is in data.cc.  It is unusual in that it uses the new
operator from C++ whereas all but one other function in data.cc do not.  Why
is that necessary?  It doesn't seem to be the cause of the performance issues
(although one could imagine that a lot of new/delete operations would be
slow).  For reference, I quote an m-file case of


val = rand (1e4, 1);
z = complex (val, val);


The code in data.cc begins at line 3683.


          const NDArray re_val = re.array_value ();

          if (im.numel () == 1)
            {
              double im_val = im.double_value ();

              ComplexNDArray result (re_val.dims (), Complex ());

              for (octave_idx_type i = 0; i < re_val.numel (); i++)
                result.xelem (i) = Complex (re_val(i), im_val);

              retval = octave_value (new octave_complex_matrix (result));
            }
          else
            {
              const NDArray im_val = im.array_value ();

              if (re_val.dims () != im_val.dims ())
                error ("complex: dimension mismatch");

              ComplexNDArray result (re_val.dims (), Complex ());

              for (octave_idx_type i = 0; i < re_val.numel (); i++)
                result.xelem (i) = Complex (re_val(i), im_val(i));

              retval = octave_value (new octave_complex_matrix (result));
            }


It doesn't speed things up, but why not write the simpler code below?


diff -r 20b05c5c41a1 libinterp/corefcn/data.cc
--- a/libinterp/corefcn/data.cc Mon Jan 24 16:50:27 2022 +0100
+++ b/libinterp/corefcn/data.cc Tue Jan 25 13:59:12 2022 -0800
@@ -3705,7 +3705,7 @@ complex ([1, 2], [3, 4])
               for (octave_idx_type i = 0; i < re_val.numel (); i++)
                 result.xelem (i) = Complex (re_val(i), im_val(i));
 
-              retval = octave_value (new octave_complex_matrix (result));
+              retval = octave_value (result);
             }
         }
     }


Similarly, it doesn't make a timing difference but why fill the array with a
known value when the next for loop is guaranteed to overwrite each value?  It
seems better to just initialize the array with a dimension vector and then
fill it.


-              ComplexNDArray result (re_val.dims (), Complex ());
+              ComplexNDArray result (re_val.dims ());


Lastly, since the for loop is guaranteed to run over only the existing
elements it theoretically should be faster us use xelem() rather than operator
() which calls elem.  In practice, it seems to have slowed things down by ~2%
but there is noise there.


               for (octave_idx_type i = 0; i < re_val.numel (); i++)
-                result.xelem (i) = Complex (re_val(i), im_val(i));
+                result.xelem (i) = Complex (re_val.xelem (i), im_val.xelem
(i));


In the past avoiding elem() and xelem() and using direct array [] access has
been faster by about 10%, but it wasn't the case here.  I tried


              const double *re_data = re_val.data ();
              const double *im_data = im_val.data ();
              Complex *result_data = result.fortran_vec ();
              for (octave_idx_type i = 0; i < re_val.numel (); i++)
                result_data[i] = Complex (re_data[i], im_data[i]);


Even going to great lengths didn't seem to produce a measurable improvement


              const double *re_data = re_val.data ();
              const double *im_data = im_val.data ();
              double *result_data = reinterpret_cast<double *>
(result.fortran_vec ());
              octave_idx_type n = re_val.numel ();
              for (octave_idx_type i = 0; i < n; i++)
                {
                result_data[2*i] = re_data[i];
                result_data[2*i + 1] = im_data[i];
                }


So, I don't really have a good way to speed this up but I wonder whether we
can at least get rid of the new operator since it is so different from all the
other functions in data.cc 

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?61912>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]