From 57cf0bb7e42404fc76c24bde873d9335724443eb Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Tue, 25 Aug 2020 03:56:38 +0800 Subject: [PATCH 1/6] refine performance of Fraction.pow --- .../apache/commons/lang3/math/Fraction.java | 60 +++++++++++++++---- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/apache/commons/lang3/math/Fraction.java b/src/main/java/org/apache/commons/lang3/math/Fraction.java index e90e340ac55..93da5c38486 100644 --- a/src/main/java/org/apache/commons/lang3/math/Fraction.java +++ b/src/main/java/org/apache/commons/lang3/math/Fraction.java @@ -536,21 +536,57 @@ public Fraction abs() { public Fraction pow(final int power) { if (power == 1) { return this; - } else if (power == 0) { + } + if (power == 0) { return ONE; - } else if (power < 0) { - if (power == Integer.MIN_VALUE) { // MIN_VALUE can't be negated. - return this.invert().pow(2).pow(-(power / 2)); - } - return this.invert().pow(-power); + } + if (power == -1) { + return this.invert(); + } + Fraction result = ONE; + Fraction base; + int nowPower; + if (power > 0) { + base = this; + nowPower = power; + } else if (power == Integer.MIN_VALUE) { // MIN_VALUE can't be negated. + base = this.invert().pow(2); + nowPower = -(power / 2); } else { - final Fraction f = this.multiplyBy(this); - if (power % 2 == 0) { // if even... - return f.pow(power / 2); - } - return f.pow(power / 2).multiplyBy(this); + base = this.invert(); + nowPower = -power; } - } + while (true) { + if ((nowPower & 1) != 0) { + result = result.multiplyBy(base); + } + nowPower >>>= 1; + if (nowPower == 0) { + break; + } + base = base.multiplyBy(base); + } + return result; + } + +// public Fraction pow(final int power) { +// if (power == 1) { +// return this; +// } else if (power == 0) { +// return ONE; +// } else if (power < 0) { +// if (power == Integer.MIN_VALUE) { // MIN_VALUE can't be negated. +// return this.invert().pow(2).pow(-(power / 2)); +// } +// return this.invert().pow(-power); +// } else { +// final Fraction f = this.multiplyBy(this); +// if (power % 2 == 0) { // if even... +// return f.pow(power / 2); +// } +// return f.pow(power / 2).multiplyBy(this); +// } +// } /** *

Gets the greatest common divisor of the absolute value of From 20a21955ac1512b18618f95d0839342b9c5326ce Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Tue, 25 Aug 2020 04:31:58 +0800 Subject: [PATCH 2/6] refine performance of Fraction.pow --- .../apache/commons/lang3/math/Fraction.java | 36 +++++----- .../math/FractionPowPerformanceTest.java | 70 +++++++++++++++++++ 2 files changed, 88 insertions(+), 18 deletions(-) create mode 100644 src/test/java/org/apache/commons/lang3/math/FractionPowPerformanceTest.java diff --git a/src/main/java/org/apache/commons/lang3/math/Fraction.java b/src/main/java/org/apache/commons/lang3/math/Fraction.java index 93da5c38486..ca53f791d48 100644 --- a/src/main/java/org/apache/commons/lang3/math/Fraction.java +++ b/src/main/java/org/apache/commons/lang3/math/Fraction.java @@ -569,24 +569,24 @@ public Fraction pow(final int power) { return result; } -// public Fraction pow(final int power) { -// if (power == 1) { -// return this; -// } else if (power == 0) { -// return ONE; -// } else if (power < 0) { -// if (power == Integer.MIN_VALUE) { // MIN_VALUE can't be negated. -// return this.invert().pow(2).pow(-(power / 2)); -// } -// return this.invert().pow(-power); -// } else { -// final Fraction f = this.multiplyBy(this); -// if (power % 2 == 0) { // if even... -// return f.pow(power / 2); -// } -// return f.pow(power / 2).multiplyBy(this); -// } -// } + public Fraction powOld(final int power) { + if (power == 1) { + return this; + } else if (power == 0) { + return ONE; + } else if (power < 0) { + if (power == Integer.MIN_VALUE) { // MIN_VALUE can't be negated. + return this.invert().pow(2).pow(-(power / 2)); + } + return this.invert().pow(-power); + } else { + final Fraction f = this.multiplyBy(this); + if (power % 2 == 0) { // if even... + return f.pow(power / 2); + } + return f.pow(power / 2).multiplyBy(this); + } + } /** *

Gets the greatest common divisor of the absolute value of diff --git a/src/test/java/org/apache/commons/lang3/math/FractionPowPerformanceTest.java b/src/test/java/org/apache/commons/lang3/math/FractionPowPerformanceTest.java new file mode 100644 index 00000000000..142660cf994 --- /dev/null +++ b/src/test/java/org/apache/commons/lang3/math/FractionPowPerformanceTest.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.lang3.math; + +import java.util.Random; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; + +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@State(Scope.Thread) +@Warmup(iterations = 5, time = 10, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 5, time = 10, timeUnit = TimeUnit.SECONDS) +@Fork(value = 1, jvmArgs = {"-server", "-Xms2048M", "-Xms2048M"}) +public class FractionPowPerformanceTest { + private Random random = new Random(0); + private int[] a = buildInts(1000, 8); + private int[] b = buildInts(1000, 8); + private int[] c = buildInts(1000, 8); + + private int[] buildInts(int length, int bound) { + int[] res = new int[length]; + for (int i = 0; i < length; i++) { + res[i] = random.nextInt(bound); + } + return res; + } + + @Benchmark + public Fraction[] testNew() { + final int length = a.length; + Fraction[] res = new Fraction[length]; + for (int i = 0; i < length; i++) { + res[i] = Fraction.getFraction(a[i], b[i]+1).pow(c[i]); + } + return res; + } + + @Benchmark + public Fraction[] testOld() { + final int length = a.length; + Fraction[] res = new Fraction[length]; + for (int i = 0; i < length; i++) { + res[i] = Fraction.getFraction(a[i], b[i]+1).powOld(c[i]); + } + return res; + } +} From 8057e48f856a2e1249b944cf599aa5f64e797a47 Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Tue, 25 Aug 2020 14:56:37 +0800 Subject: [PATCH 3/6] refine test --- .../lang3/math/FractionPowPerformanceTest.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/apache/commons/lang3/math/FractionPowPerformanceTest.java b/src/test/java/org/apache/commons/lang3/math/FractionPowPerformanceTest.java index 142660cf994..97b77a03129 100644 --- a/src/test/java/org/apache/commons/lang3/math/FractionPowPerformanceTest.java +++ b/src/test/java/org/apache/commons/lang3/math/FractionPowPerformanceTest.java @@ -18,6 +18,8 @@ import java.util.Random; import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; @@ -53,7 +55,7 @@ public Fraction[] testNew() { final int length = a.length; Fraction[] res = new Fraction[length]; for (int i = 0; i < length; i++) { - res[i] = Fraction.getFraction(a[i], b[i]+1).pow(c[i]); + res[i] = Fraction.getFraction(a[i], b[i] + 1).pow(c[i]); } return res; } @@ -63,8 +65,19 @@ public Fraction[] testOld() { final int length = a.length; Fraction[] res = new Fraction[length]; for (int i = 0; i < length; i++) { - res[i] = Fraction.getFraction(a[i], b[i]+1).powOld(c[i]); + res[i] = Fraction.getFraction(a[i], b[i] + 1).powOld(c[i]); } return res; } + + @Test + public void testEquals() { + final int length = a.length; + for (int i = 0; i < length; i++) { + Assertions.assertEquals( + Fraction.getFraction(a[i], b[i] + 1).powOld(c[i]), + Fraction.getFraction(a[i], b[i] + 1).pow(c[i]) + ); + } + } } From 9b9e77a0afed1b1d3109a783b1474461064ea69e Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Tue, 25 Aug 2020 15:39:01 +0800 Subject: [PATCH 4/6] refine implementation --- .../apache/commons/lang3/math/Fraction.java | 80 ++++++++++++++----- 1 file changed, 59 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/apache/commons/lang3/math/Fraction.java b/src/main/java/org/apache/commons/lang3/math/Fraction.java index ca53f791d48..ccd4e5a95af 100644 --- a/src/main/java/org/apache/commons/lang3/math/Fraction.java +++ b/src/main/java/org/apache/commons/lang3/math/Fraction.java @@ -543,30 +543,38 @@ public Fraction pow(final int power) { if (power == -1) { return this.invert(); } - Fraction result = ONE; - Fraction base; - int nowPower; - if (power > 0) { - base = this; - nowPower = power; - } else if (power == Integer.MIN_VALUE) { // MIN_VALUE can't be negated. - base = this.invert().pow(2); - nowPower = -(power / 2); - } else { - base = this.invert(); - nowPower = -power; - } - while (true) { - if ((nowPower & 1) != 0) { - result = result.multiplyBy(base); + final Fraction base = this.reduce(); + if (base.denominator == 0) { + if (power > 0) { + throw new ArithmeticException("denominator MUST not be 0!"); + } else { + return ZERO; } - nowPower >>>= 1; - if (nowPower == 0) { - break; + } + if (base.numerator == 0) { + if (power < 0) { + throw new ArithmeticException("denominator MUST not be 0!"); + } else { + return ZERO; } - base = base.multiplyBy(base); } - return result; + if (power > 0) { + return new Fraction( + pow(base.numerator, power), + pow(base.denominator, power) + ); + } + if (power == Integer.MIN_VALUE) { + final int tmp = -(power / 2); + return new Fraction( + pow(pow(this.denominator, 2), tmp), + pow(pow(this.numerator, 2), tmp) + ); + } + return new Fraction( + pow(base.denominator, -power), + pow(base.numerator, -power) + ); } public Fraction powOld(final int power) { @@ -588,6 +596,36 @@ public Fraction powOld(final int power) { } } + /** + * Raise an int to an int power. + * Notice that this function is copied and modified directly from class ArithmeticUtils in commons-numbers-core. + * + * @param k Number to raise. + * @param e Exponent (must be positive or zero). + * @return \( k^e \) + * @throws ArithmeticException if the result would overflow. + */ + private static int pow(final int k, + final int e) { + int exp = e; + int result = 1; + int k2p = k; + while (true) { + if ((exp & 0x1) != 0) { + result = Math.multiplyExact(result, k2p); + } + + exp >>= 1; + if (exp == 0) { + break; + } + + k2p = Math.multiplyExact(k2p, k2p); + } + + return result; + } + /** *

Gets the greatest common divisor of the absolute value of * two numbers, using the "binary gcd" method which avoids From a5b3153087945f1c387e5e7930849a121afc13a8 Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Tue, 25 Aug 2020 18:52:47 +0800 Subject: [PATCH 5/6] unify error message. --- src/main/java/org/apache/commons/lang3/math/Fraction.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/lang3/math/Fraction.java b/src/main/java/org/apache/commons/lang3/math/Fraction.java index ccd4e5a95af..591b820b11b 100644 --- a/src/main/java/org/apache/commons/lang3/math/Fraction.java +++ b/src/main/java/org/apache/commons/lang3/math/Fraction.java @@ -546,14 +546,14 @@ public Fraction pow(final int power) { final Fraction base = this.reduce(); if (base.denominator == 0) { if (power > 0) { - throw new ArithmeticException("denominator MUST not be 0!"); + throw new ArithmeticException("The denominator must not be zero"); } else { return ZERO; } } if (base.numerator == 0) { if (power < 0) { - throw new ArithmeticException("denominator MUST not be 0!"); + throw new ArithmeticException("The denominator must not be zero"); } else { return ZERO; } From 97388fde21a00bb7a25192bf70a059f8c46e8534 Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Wed, 26 Aug 2020 01:18:27 +0800 Subject: [PATCH 6/6] even faster refine. --- src/main/java/org/apache/commons/lang3/math/Fraction.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/commons/lang3/math/Fraction.java b/src/main/java/org/apache/commons/lang3/math/Fraction.java index 591b820b11b..97cef5f3c7c 100644 --- a/src/main/java/org/apache/commons/lang3/math/Fraction.java +++ b/src/main/java/org/apache/commons/lang3/math/Fraction.java @@ -565,10 +565,9 @@ public Fraction pow(final int power) { ); } if (power == Integer.MIN_VALUE) { - final int tmp = -(power / 2); return new Fraction( - pow(pow(this.denominator, 2), tmp), - pow(pow(this.numerator, 2), tmp) + pow(this.denominator, Integer.MAX_VALUE) * this.denominator, + pow(this.numerator, Integer.MAX_VALUE) * this.numerator ); } return new Fraction(