Skip to content

Commit

Permalink
8238669: Long.divideUnsigned is extremely slow for certain values (Ne…
Browse files Browse the repository at this point in the history
…eds to be Intrinsic)

Reviewed-by: bpb
  • Loading branch information
rgiulietti authored and Brian Burkhalter committed Oct 21, 2020
1 parent 365f19c commit 0efdde1
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 42 deletions.
57 changes: 32 additions & 25 deletions src/java.base/share/classes/java/lang/Long.java
Original file line number Diff line number Diff line change
Expand Up @@ -1668,24 +1668,13 @@ public static int compareUnsigned(long x, long y) {
* @since 1.8
*/
public static long divideUnsigned(long dividend, long divisor) {
if (divisor < 0L) { // signed comparison
// Answer must be 0 or 1 depending on relative magnitude
// of dividend and divisor.
return (compareUnsigned(dividend, divisor)) < 0 ? 0L :1L;
}

if (dividend > 0) // Both inputs non-negative
return dividend/divisor;
else {
/*
* For simple code, leveraging BigInteger. Longer and faster
* code written directly in terms of operations on longs is
* possible; see "Hacker's Delight" for divide and remainder
* algorithms.
*/
return toUnsignedBigInteger(dividend).
divide(toUnsignedBigInteger(divisor)).longValue();
/* See Hacker's Delight (2nd ed), section 9.3 */
if (divisor >= 0) {
final long q = (dividend >>> 1) / divisor << 1;
final long r = dividend - q * divisor;
return q + ((r | ~(r - divisor)) >>> (Long.SIZE - 1));
}
return (dividend & ~(dividend - divisor)) >>> (Long.SIZE - 1);
}

/**
Expand All @@ -1701,15 +1690,33 @@ public static long divideUnsigned(long dividend, long divisor) {
* @since 1.8
*/
public static long remainderUnsigned(long dividend, long divisor) {
if (dividend > 0 && divisor > 0) { // signed comparisons
return dividend % divisor;
} else {
if (compareUnsigned(dividend, divisor) < 0) // Avoid explicit check for 0 divisor
return dividend;
else
return toUnsignedBigInteger(dividend).
remainder(toUnsignedBigInteger(divisor)).longValue();
/* See Hacker's Delight (2nd ed), section 9.3 */
if (divisor >= 0) {
final long q = (dividend >>> 1) / divisor << 1;
final long r = dividend - q * divisor;
/*
* Here, 0 <= r < 2 * divisor
* (1) When 0 <= r < divisor, the remainder is simply r.
* (2) Otherwise the remainder is r - divisor.
*
* In case (1), r - divisor < 0. Applying ~ produces a long with
* sign bit 0, so >> produces 0. The returned value is thus r.
*
* In case (2), a similar reasoning shows that >> produces -1,
* so the returned value is r - divisor.
*/
return r - ((~(r - divisor) >> (Long.SIZE - 1)) & divisor);
}
/*
* (1) When dividend >= 0, the remainder is dividend.
* (2) Otherwise
* (2.1) When dividend < divisor, the remainder is dividend.
* (2.2) Otherwise the remainder is dividend - divisor
*
* A reasoning similar to the above shows that the returned value
* is as expected.
*/
return dividend - (((dividend & ~(dividend - divisor)) >> (Long.SIZE - 1)) & divisor);
}

// Bit Twiddling
Expand Down
64 changes: 47 additions & 17 deletions test/jdk/java/lang/Long/Unsigned.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2009, 2012, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2009, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -375,24 +375,54 @@ private static int testUnsignedOverflow(String s, int radix, boolean exception)

private static int testDivideAndRemainder() {
int errors = 0;
long MAX_UNSIGNED_INT = Integer.toUnsignedLong(0xffff_ffff);
long TWO_31 = 1L << Integer.SIZE - 1;
long TWO_32 = 1L << Integer.SIZE;
long TWO_33 = 1L << Integer.SIZE + 1;
BigInteger NINETEEN = BigInteger.valueOf(19L);
BigInteger TWO_63 = BigInteger.ONE.shiftLeft(Long.SIZE - 1);
BigInteger TWO_64 = BigInteger.ONE.shiftLeft(Long.SIZE);

BigInteger[] inRange = {
BigInteger.valueOf(0L),
BigInteger.valueOf(1L),
BigInteger.valueOf(10L),
BigInteger.valueOf(2147483646L), // Integer.MAX_VALUE - 1
BigInteger.valueOf(2147483647L), // Integer.MAX_VALUE
BigInteger.valueOf(2147483648L), // Integer.MAX_VALUE + 1

BigInteger.valueOf(MAX_UNSIGNED_INT - 1L),
BigInteger.valueOf(MAX_UNSIGNED_INT),

BigInteger.valueOf(Long.MAX_VALUE - 1L),
BigInteger.valueOf(Long.MAX_VALUE),
BigInteger.valueOf(Long.MAX_VALUE).add(BigInteger.ONE),

TWO.pow(64).subtract(BigInteger.ONE)
BigInteger.ZERO,
BigInteger.ONE,
BigInteger.TEN,
NINETEEN,

BigInteger.valueOf(TWO_31 - 19L),
BigInteger.valueOf(TWO_31 - 10L),
BigInteger.valueOf(TWO_31 - 1L),
BigInteger.valueOf(TWO_31),
BigInteger.valueOf(TWO_31 + 1L),
BigInteger.valueOf(TWO_31 + 10L),
BigInteger.valueOf(TWO_31 + 19L),

BigInteger.valueOf(TWO_32 - 19L),
BigInteger.valueOf(TWO_32 - 10L),
BigInteger.valueOf(TWO_32 - 1L),
BigInteger.valueOf(TWO_32),
BigInteger.valueOf(TWO_32 + 1L),
BigInteger.valueOf(TWO_32 + 10L),
BigInteger.valueOf(TWO_32 - 19L),

BigInteger.valueOf(TWO_33 - 19L),
BigInteger.valueOf(TWO_33 - 10L),
BigInteger.valueOf(TWO_33 - 1L),
BigInteger.valueOf(TWO_33),
BigInteger.valueOf(TWO_33 + 1L),
BigInteger.valueOf(TWO_33 + 10L),
BigInteger.valueOf(TWO_33 + 19L),

TWO_63.subtract(NINETEEN),
TWO_63.subtract(BigInteger.TEN),
TWO_63.subtract(BigInteger.ONE),
TWO_63,
TWO_63.add(BigInteger.ONE),
TWO_63.add(BigInteger.TEN),
TWO_63.add(NINETEEN),

TWO_64.subtract(NINETEEN),
TWO_64.subtract(BigInteger.TEN),
TWO_64.subtract(BigInteger.ONE),
};

for(BigInteger dividend : inRange) {
Expand Down

1 comment on commit 0efdde1

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented on 0efdde1 Oct 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.