You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by er...@apache.org on 2015/07/11 14:58:34 UTC
[math] MATH-1248
Repository: commons-math
Updated Branches:
refs/heads/master 32d33210a -> 1fe7a4350
MATH-1248
Unnecessary allocations in "BigFraction".
Thanks to Chris Popp.
Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-math/commit/1fe7a435
Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/1fe7a435
Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/1fe7a435
Branch: refs/heads/master
Commit: 1fe7a43505aa9d00bd9dde45aee77935a5147561
Parents: 32d3321
Author: Gilles <er...@apache.org>
Authored: Sat Jul 11 14:54:43 2015 +0200
Committer: Gilles <er...@apache.org>
Committed: Sat Jul 11 14:54:43 2015 +0200
----------------------------------------------------------------------
pom.xml | 3 +
src/changes/changes.xml | 3 +
.../commons/math4/fraction/BigFraction.java | 103 ++++++++++++++++---
3 files changed, 97 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-math/blob/1fe7a435/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 320c3fd..1675d08 100644
--- a/pom.xml
+++ b/pom.xml
@@ -353,6 +353,9 @@
<contributor>
<name>Xiaogang Zhang</name>
</contributor>
+ <contributor>
+ <name>Chris Popp</name>
+ </contributor>
</contributors>
<dependencies>
http://git-wip-us.apache.org/repos/asf/commons-math/blob/1fe7a435/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index b6baf47..bbb67e7 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -54,6 +54,9 @@ If the output is not quite correct, check for invisible trailing spaces!
</release>
<release version="4.0" date="XXXX-XX-XX" description="">
+ <action dev="erans" type="fix" issue="MATH-1248" due-to="Chris Popp">
+ Removed unnecessary allocations in "BigFraction" (package "o.a.c.m.fraction").
+ </action>
<action dev="psteitz" type="fix" issue="MATH-1245">
Fixed error in computing discrete distribution of D statistics for small-sample
2-sample Kolmogorov-Smirnov tests. Error was causing incorrect p-values returned
http://git-wip-us.apache.org/repos/asf/commons-math/blob/1fe7a435/src/main/java/org/apache/commons/math4/fraction/BigFraction.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/math4/fraction/BigFraction.java b/src/main/java/org/apache/commons/math4/fraction/BigFraction.java
index 078682d..1be1eea 100644
--- a/src/main/java/org/apache/commons/math4/fraction/BigFraction.java
+++ b/src/main/java/org/apache/commons/math4/fraction/BigFraction.java
@@ -119,10 +119,10 @@ public class BigFraction
public BigFraction(BigInteger num, BigInteger den) {
MathUtils.checkNotNull(num, LocalizedFormats.NUMERATOR);
MathUtils.checkNotNull(den, LocalizedFormats.DENOMINATOR);
- if (BigInteger.ZERO.equals(den)) {
+ if (den.signum() == 0) {
throw new ZeroException(LocalizedFormats.ZERO_DENOMINATOR);
}
- if (BigInteger.ZERO.equals(num)) {
+ if (num.signum() == 0) {
numerator = BigInteger.ZERO;
denominator = BigInteger.ONE;
} else {
@@ -135,7 +135,7 @@ public class BigFraction
}
// move sign to numerator
- if (BigInteger.ZERO.compareTo(den) > 0) {
+ if (den.signum() == -1) {
num = num.negate();
den = den.negate();
}
@@ -450,7 +450,7 @@ public class BigFraction
* @return the absolute value as a {@link BigFraction}.
*/
public BigFraction abs() {
- return (BigInteger.ZERO.compareTo(numerator) <= 0) ? this : negate();
+ return (numerator.signum() == 1) ? this : negate();
}
/**
@@ -467,6 +467,14 @@ public class BigFraction
*/
public BigFraction add(final BigInteger bg) throws NullArgumentException {
MathUtils.checkNotNull(bg);
+
+ if (numerator.signum() == 0) {
+ return new BigFraction(bg);
+ }
+ if (bg.signum() == 0) {
+ return this;
+ }
+
return new BigFraction(numerator.add(denominator.multiply(bg)), denominator);
}
@@ -514,9 +522,12 @@ public class BigFraction
if (fraction == null) {
throw new NullArgumentException(LocalizedFormats.FRACTION);
}
- if (ZERO.equals(fraction)) {
+ if (fraction.numerator.signum() == 0) {
return this;
}
+ if (numerator.signum() == 0) {
+ return fraction;
+ }
BigInteger num = null;
BigInteger den = null;
@@ -528,6 +539,11 @@ public class BigFraction
num = (numerator.multiply(fraction.denominator)).add((fraction.numerator).multiply(denominator));
den = denominator.multiply(fraction.denominator);
}
+
+ if (num.signum() == 0) {
+ return ZERO;
+ }
+
return new BigFraction(num, den);
}
@@ -599,6 +615,16 @@ public class BigFraction
*/
@Override
public int compareTo(final BigFraction object) {
+ int lhsSigNum = numerator.signum();
+ int rhsSigNum = object.numerator.signum();
+
+ if (lhsSigNum != rhsSigNum) {
+ return (lhsSigNum > rhsSigNum) ? 1 : -1;
+ }
+ if (lhsSigNum == 0) {
+ return 0;
+ }
+
BigInteger nOd = numerator.multiply(object.denominator);
BigInteger dOn = denominator.multiply(object.numerator);
return nOd.compareTo(dOn);
@@ -619,9 +645,12 @@ public class BigFraction
if (bg == null) {
throw new NullArgumentException(LocalizedFormats.FRACTION);
}
- if (BigInteger.ZERO.equals(bg)) {
+ if (bg.signum() == 0) {
throw new MathArithmeticException(LocalizedFormats.ZERO_DENOMINATOR);
}
+ if (numerator.signum() == 0) {
+ return ZERO;
+ }
return new BigFraction(numerator, denominator.multiply(bg));
}
@@ -669,9 +698,12 @@ public class BigFraction
if (fraction == null) {
throw new NullArgumentException(LocalizedFormats.FRACTION);
}
- if (BigInteger.ZERO.equals(fraction.numerator)) {
+ if (fraction.numerator.signum() == 0) {
throw new MathArithmeticException(LocalizedFormats.ZERO_DENOMINATOR);
}
+ if (numerator.signum() == 0) {
+ return ZERO;
+ }
return multiply(fraction.reciprocal());
}
@@ -873,6 +905,9 @@ public class BigFraction
if (bg == null) {
throw new NullArgumentException();
}
+ if (numerator.signum() == 0 || bg.signum() == 0) {
+ return ZERO;
+ }
return new BigFraction(bg.multiply(numerator), denominator);
}
@@ -888,6 +923,10 @@ public class BigFraction
*/
@Override
public BigFraction multiply(final int i) {
+ if (i == 0 || numerator.signum() == 0) {
+ return ZERO;
+ }
+
return multiply(BigInteger.valueOf(i));
}
@@ -902,6 +941,10 @@ public class BigFraction
* @return a {@link BigFraction} instance with the resulting values.
*/
public BigFraction multiply(final long l) {
+ if (l == 0 || numerator.signum() == 0) {
+ return ZERO;
+ }
+
return multiply(BigInteger.valueOf(l));
}
@@ -920,8 +963,8 @@ public class BigFraction
if (fraction == null) {
throw new NullArgumentException(LocalizedFormats.FRACTION);
}
- if (numerator.equals(BigInteger.ZERO) ||
- fraction.numerator.equals(BigInteger.ZERO)) {
+ if (numerator.signum() == 0 ||
+ fraction.numerator.signum() == 0) {
return ZERO;
}
return new BigFraction(numerator.multiply(fraction.numerator),
@@ -965,6 +1008,13 @@ public class BigFraction
* @return <tt>this<sup>exponent</sup></tt>.
*/
public BigFraction pow(final int exponent) {
+ if (exponent == 0) {
+ return ONE;
+ }
+ if (numerator.signum() == 0) {
+ return this;
+ }
+
if (exponent < 0) {
return new BigFraction(denominator.pow(-exponent), numerator.pow(-exponent));
}
@@ -982,6 +1032,13 @@ public class BigFraction
* @return <tt>this<sup>exponent</sup></tt> as a <code>BigFraction</code>.
*/
public BigFraction pow(final long exponent) {
+ if (exponent == 0) {
+ return ONE;
+ }
+ if (numerator.signum() == 0) {
+ return this;
+ }
+
if (exponent < 0) {
return new BigFraction(ArithmeticUtils.pow(denominator, -exponent),
ArithmeticUtils.pow(numerator, -exponent));
@@ -1001,7 +1058,14 @@ public class BigFraction
* @return <tt>this<sup>exponent</sup></tt> as a <code>BigFraction</code>.
*/
public BigFraction pow(final BigInteger exponent) {
- if (exponent.compareTo(BigInteger.ZERO) < 0) {
+ if (exponent.signum() == 0) {
+ return ONE;
+ }
+ if (numerator.signum() == 0) {
+ return this;
+ }
+
+ if (exponent.signum() == -1) {
final BigInteger eNeg = exponent.negate();
return new BigFraction(ArithmeticUtils.pow(denominator, eNeg),
ArithmeticUtils.pow(numerator, eNeg));
@@ -1047,7 +1111,12 @@ public class BigFraction
*/
public BigFraction reduce() {
final BigInteger gcd = numerator.gcd(denominator);
- return new BigFraction(numerator.divide(gcd), denominator.divide(gcd));
+
+ if (BigInteger.ONE.compareTo(gcd) < 0) {
+ return new BigFraction(numerator.divide(gcd), denominator.divide(gcd));
+ } else {
+ return this;
+ }
}
/**
@@ -1064,6 +1133,13 @@ public class BigFraction
if (bg == null) {
throw new NullArgumentException();
}
+ if (bg.signum() == 0) {
+ return this;
+ }
+ if (numerator.signum() == 0) {
+ return new BigFraction(bg.negate());
+ }
+
return new BigFraction(numerator.subtract(denominator.multiply(bg)), denominator);
}
@@ -1108,9 +1184,12 @@ public class BigFraction
if (fraction == null) {
throw new NullArgumentException(LocalizedFormats.FRACTION);
}
- if (ZERO.equals(fraction)) {
+ if (fraction.numerator.signum() == 0) {
return this;
}
+ if (numerator.signum() == 0) {
+ return fraction.negate();
+ }
BigInteger num = null;
BigInteger den = null;