I would be wary of this simplification without a performance test. In the Numbers class the int methods do not use long arithmetic. The long methods do not use BigInteger. This is unlike those methods in my JDK 8 source code which do and are likely much slower. A quick check in JDK 21 finds this is still not an intrinsic method [1]. My only issue with the Numbers methods is they are based on the Hacker's Delight book which is not free, thus it is not easy to check the implementation against the source.
Alex [1] https://chriswhocodes.com/hotspot_intrinsics_openjdk21.html On Sun, 24 Dec 2023 at 13:20, <s...@apache.org> wrote: > > This is an automated email from the ASF dual-hosted git repository. > > sebb pushed a commit to branch java8-simplify > in repository https://gitbox.apache.org/repos/asf/commons-numbers.git > > commit 2f60d424725ca619abf324858593368e66dd5fc2 > Author: Sebb <s...@apache.org> > AuthorDate: Sun Dec 24 13:20:13 2023 +0000 > > Use Java 1.8 methods to simplify the ArithmeticUtils methods > remainderUnsigned and divideUnsigned (both int and long) > --- > .../commons/numbers/core/ArithmeticUtils.java | 68 > +++------------------- > src/changes/changes.xml | 3 + > 2 files changed, 11 insertions(+), 60 deletions(-) > > diff --git > a/commons-numbers-core/src/main/java/org/apache/commons/numbers/core/ArithmeticUtils.java > > b/commons-numbers-core/src/main/java/org/apache/commons/numbers/core/ArithmeticUtils.java > index d34d7e2b..7afde103 100644 > --- > a/commons-numbers-core/src/main/java/org/apache/commons/numbers/core/ArithmeticUtils.java > +++ > b/commons-numbers-core/src/main/java/org/apache/commons/numbers/core/ArithmeticUtils.java > @@ -470,7 +470,7 @@ public final class ArithmeticUtils { > * Returns the unsigned remainder from dividing the first argument > * by the second where each argument and the result is interpreted > * as an unsigned value. > - * <p>This method does not use the {@code long} datatype.</p> > + * <p>This method uses the Java 1.8+ method {@link > Integer#remainderUnsigned(int, int)}.</p> > * > * @param dividend the value to be divided > * @param divisor the value doing the dividing > @@ -478,27 +478,14 @@ public final class ArithmeticUtils { > * the second argument. > */ > public static int remainderUnsigned(int dividend, int divisor) { > - if (divisor >= 0) { > - if (dividend >= 0) { > - return dividend % divisor; > - } > - // The implementation is a Java port of algorithm described in > the book > - // "Hacker's Delight" (section "Unsigned short division from > signed division"). > - final int q = ((dividend >>> 1) / divisor) << 1; > - dividend -= q * divisor; > - if (dividend < 0 || dividend >= divisor) { > - dividend -= divisor; > - } > - return dividend; > - } > - return dividend >= 0 || dividend < divisor ? dividend : dividend - > divisor; > + return Integer.remainderUnsigned(dividend, divisor); > } > > /** > * Returns the unsigned remainder from dividing the first argument > * by the second where each argument and the result is interpreted > * as an unsigned value. > - * <p>This method does not use the {@code BigInteger} datatype.</p> > + * <p>This method uses the Java 1.8+ method {@link > Long#remainderUnsigned(int, int)}.</p> > * > * @param dividend the value to be divided > * @param divisor the value doing the dividing > @@ -506,20 +493,7 @@ public final class ArithmeticUtils { > * the second argument. > */ > public static long remainderUnsigned(long dividend, long divisor) { > - if (divisor >= 0L) { > - if (dividend >= 0L) { > - return dividend % divisor; > - } > - // The implementation is a Java port of algorithm described in > the book > - // "Hacker's Delight" (section "Unsigned short division from > signed division"). > - final long q = ((dividend >>> 1) / divisor) << 1; > - dividend -= q * divisor; > - if (dividend < 0L || dividend >= divisor) { > - dividend -= divisor; > - } > - return dividend; > - } > - return dividend >= 0L || dividend < divisor ? dividend : dividend - > divisor; > + return Long.remainderUnsigned(dividend, divisor); > } > > /** > @@ -531,7 +505,7 @@ public final class ArithmeticUtils { > * bit-wise identical if the two operands are regarded as both > * being signed or both being unsigned. Therefore separate {@code > * addUnsigned}, etc. methods are not provided.</p> > - * <p>This method does not use the {@code long} datatype.</p> > + * <p>This method uses the Java 1.8+ method {@link > Integer#divideUnsigned(int, int)}.</p> > * > * @param dividend the value to be divided > * @param divisor the value doing the dividing > @@ -539,20 +513,7 @@ public final class ArithmeticUtils { > * the second argument > */ > public static int divideUnsigned(int dividend, int divisor) { > - if (divisor >= 0) { > - if (dividend >= 0) { > - return dividend / divisor; > - } > - // The implementation is a Java port of algorithm described in > the book > - // "Hacker's Delight" (section "Unsigned short division from > signed division"). > - int q = ((dividend >>> 1) / divisor) << 1; > - dividend -= q * divisor; > - if (dividend < 0L || dividend >= divisor) { > - q++; > - } > - return q; > - } > - return dividend >= 0 || dividend < divisor ? 0 : 1; > + return Integer.divideUnsigned(dividend, divisor); > } > > /** > @@ -564,7 +525,7 @@ public final class ArithmeticUtils { > * bit-wise identical if the two operands are regarded as both > * being signed or both being unsigned. Therefore separate {@code > * addUnsigned}, etc. methods are not provided.</p> > - * <p>This method does not use the {@code BigInteger} datatype.</p> > + * <p>This method uses the Java 1.8+ method {@link > Long#divideUnsigned(int, int)}.</p> > * > * @param dividend the value to be divided > * @param divisor the value doing the dividing > @@ -572,20 +533,7 @@ public final class ArithmeticUtils { > * the second argument. > */ > public static long divideUnsigned(long dividend, long divisor) { > - if (divisor >= 0L) { > - if (dividend >= 0L) { > - return dividend / divisor; > - } > - // The implementation is a Java port of algorithm described in > the book > - // "Hacker's Delight" (section "Unsigned short division from > signed division"). > - long q = ((dividend >>> 1) / divisor) << 1; > - dividend -= q * divisor; > - if (dividend < 0L || dividend >= divisor) { > - q++; > - } > - return q; > - } > - return dividend >= 0L || dividend < divisor ? 0L : 1L; > + return Long.divideUnsigned(dividend, divisor); > } > > /** > diff --git a/src/changes/changes.xml b/src/changes/changes.xml > index d9b796b2..c9e96a1f 100644 > --- a/src/changes/changes.xml > +++ b/src/changes/changes.xml > @@ -56,6 +56,9 @@ If the output is not quite correct, check for invisible > trailing spaces! > <release version="1.2" date="TBD" description=" > New features, updates and bug fixes. > "> > + <action dev="sebb" type="update"> > + Use Java 1.8 methods to simplify the ArithmeticUtils methods > remainderUnsigned and divideUnsigned (both int and long) > + </action> > <action dev="aherbert" type="add" due-to="Harald Kirsch" > issue="NUMBERS-205"> > "Addition/Multiplication": Introduces isZero to Addition and isOne > to Multiplication > interfaces. Override the default implementation in implementing > classes to avoid > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
