[jira] Created: (MATH-316) Perf improvement: ArrayRealVector makes superfluous copies and often doesn't optimally operate on array values

Perf improvement: ArrayRealVector makes superfluous copies and often doesn't optimally operate on array values
--------------------------------------------------------------------------------------------------------------

                 Key: MATH-316
                 URL: https://issues.apache.org/jira/browse/MATH-316
             Project: Commons Math
          Issue Type: Improvement
    Affects Versions: 2.0
         Environment: all
            Reporter: Jake Mannix
            Priority: Minor
             Fix For: 2.1


As discussed in the mailing list, things like ArrayRealVector#add:
{code}
            double[] out = new double[data.length];
            for (int i = 0; i < data.length; i++) {
                out[i] = data[i] + v.getEntry(i);
            }
            return new ArrayRealVector(out);
{code}
can be improved in the inner loop by simply
{code}
            double[] out = out.clone();
            for (int i = 0; i < data.length; i++) {
                out[i] += v.getEntry(i);
            }
            return new ArrayRealVector(out);
{code}

Which cuts down on array accesses.

Even more importantly, the last return line should pass in the boolean false, for "shallow copy", or else this whole temporary array is being copied again and then the original discarded.

{code}
  return new ArrayRealVector(out, false);
{code}

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/MATH-316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12772572#action_12772572 ]

Jake Mannix commented on MATH-316:
----------------------------------

or course, out.clone() should be data.clone();

> Perf improvement: ArrayRealVector makes superfluous copies and often doesn't optimally operate on array values
> --------------------------------------------------------------------------------------------------------------
>
>                 Key: MATH-316
>                 URL: https://issues.apache.org/jira/browse/MATH-316
>             Project: Commons Math
>          Issue Type: Improvement
>    Affects Versions: 2.0
>         Environment: all
>            Reporter: Jake Mannix
>            Priority: Minor
>             Fix For: 2.1
>
>
> As discussed in the mailing list, things like ArrayRealVector#add:
> {code}
>             double[] out = new double[data.length];
>             for (int i = 0; i < data.length; i++) {
>                 out[i] = data[i] + v.getEntry(i);
>             }
>             return new ArrayRealVector(out);
> {code}
> can be improved in the inner loop by simply
> {code}
>             double[] out = out.clone();
>             for (int i = 0; i < data.length; i++) {
>                 out[i] += v.getEntry(i);
>             }
>             return new ArrayRealVector(out);
> {code}
> Which cuts down on array accesses.
> Even more importantly, the last return line should pass in the boolean false, for "shallow copy", or else this whole temporary array is being copied again and then the original discarded.
> {code}
>   return new ArrayRealVector(out, false);
> {code}

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/MATH-316?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jake Mannix updated MATH-316:
-----------------------------

    Attachment: MATH-316.patch

All unit tests pass after patch is applied, and since it has no new features, only perf improvements, no new unit tests are attached.  Only ArrayRealVector.java is modified.

In addition to what is described in the bug, the idiom of implementing mapXXX as return copy().mapXXXtoSelf() which is followed in OpenMapRealVector is copied here, as these methods all had the "return new ArrayRealVector(v);" extra copy bug and had to be modified anyways.

> Perf improvement: ArrayRealVector makes superfluous copies and often doesn't optimally operate on array values
> --------------------------------------------------------------------------------------------------------------
>
>                 Key: MATH-316
>                 URL: https://issues.apache.org/jira/browse/MATH-316
>             Project: Commons Math
>          Issue Type: Improvement
>    Affects Versions: 2.0
>         Environment: all
>            Reporter: Jake Mannix
>            Priority: Minor
>             Fix For: 2.1
>
>         Attachments: MATH-316.patch
>
>
> As discussed in the mailing list, things like ArrayRealVector#add:
> {code}
>             double[] out = new double[data.length];
>             for (int i = 0; i < data.length; i++) {
>                 out[i] = data[i] + v.getEntry(i);
>             }
>             return new ArrayRealVector(out);
> {code}
> can be improved in the inner loop by simply
> {code}
>             double[] out = out.clone();
>             for (int i = 0; i < data.length; i++) {
>                 out[i] += v.getEntry(i);
>             }
>             return new ArrayRealVector(out);
> {code}
> Which cuts down on array accesses.
> Even more importantly, the last return line should pass in the boolean false, for "shallow copy", or else this whole temporary array is being copied again and then the original discarded.
> {code}
>   return new ArrayRealVector(out, false);
> {code}

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.