You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Jake Mannix (JIRA)" <ji...@apache.org> on 2009/11/02 18:48:59 UTC
[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.
[jira] Updated: (MATH-316) Perf improvement: ArrayRealVector makes
superfluous copies and often doesn't optimally operate on array values
Posted by "Jake Mannix (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Commented: (MATH-316) Perf improvement: ArrayRealVector
makes superfluous copies and often doesn't optimally operate on array
values
Posted by "Jake Mannix (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12794777#action_12794777 ]
Jake Mannix commented on MATH-316:
----------------------------------
Is the proper copy part of this fixed?
I still see in trunk places where we
{code}
double[] out = // stuff
return new ArrayRealVector(out);
{code}
instead of
{code}
double[] out = // stuff
return new ArrayRealVector(out, false);
{code}
The default behavior for ArrayRealVector is to deep copy arrays passed in, which needs to be overridden here, no?
See ArrayRealVector.add, ArrayRealVector.subtract, in particular. I didn't pick through to see if this caught all of them.
It's entirely possible this was my fault and I missed them in my patches for the other issues which covered some of this.
> 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.
[jira] Closed: (MATH-316) Perf improvement: ArrayRealVector makes
superfluous copies and often doesn't optimally operate on array values
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-316?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phil Steitz closed MATH-316.
----------------------------
> 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.
[jira] Commented: (MATH-316) Perf improvement: ArrayRealVector
makes superfluous copies and often doesn't optimally operate on array
values
Posted by "Luc Maisonobe (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12794827#action_12794827 ]
Luc Maisonobe commented on MATH-316:
------------------------------------
The last few forgotten copies should have been fixed now I hope.
I have also added a few more constructors using RealVectors and improved test coverage which lies now somewhere between 99% and 100% for this class.
> 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.
[jira] Commented: (MATH-316) Perf improvement: ArrayRealVector
makes superfluous copies and often doesn't optimally operate on array
values
Posted by "Jake Mannix (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Resolved: (MATH-316) Perf improvement: ArrayRealVector makes
superfluous copies and often doesn't optimally operate on array values
Posted by "Luc Maisonobe (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-316?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Luc Maisonobe resolved MATH-316.
--------------------------------
Resolution: Fixed
This was solved as part of MATH-312 since the provided patch included both fixes.
> 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.