RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.
Deshpande, Vivek R
vivek.r.deshpande at intel.com
Fri Jun 22 18:48:40 UTC 2018
Hi Paul
I have updated the patch in this webrev:
http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.04/
Will push that soon.
Regards,
Vivek
-----Original Message-----
From: Deshpande, Vivek R
Sent: Thursday, June 21, 2018 4:14 PM
To: Paul Sandoz <paul.sandoz at oracle.com>
Cc: Roger.Riggs at oracle.com; David Holmes <david.holmes at oracle.com>; core-libs-dev at openjdk.java.net; Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; Ivan Gerasimov <ivan.gerasimov at oracle.com>
Subject: RE: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.
Thanks Paul for taking some time and thinking about it.
I will make the change you have mentioned and send the patch again.
Regards,
Vivek
-----Original Message-----
From: Paul Sandoz [mailto:paul.sandoz at oracle.com]
Sent: Thursday, June 21, 2018 3:32 PM
To: Deshpande, Vivek R <vivek.r.deshpande at intel.com>
Cc: Roger.Riggs at oracle.com; David Holmes <david.holmes at oracle.com>; core-libs-dev at openjdk.java.net; Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; Ivan Gerasimov <ivan.gerasimov at oracle.com>
Subject: Re: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.
Hi Vivek,
This looks better. I thought a bit more about NaNs. I am concerned about edge case performance regressions if the first two values are NaNs (with the same bit pattern). Here’s a patch on top of your patch that compares the raw bits for float/double elements. If you are ok with this and apply it on top of your patch then i don’t think there is a need for another review round.
Thanks,
Paul.
diff -r ef8fd07e4440 src/java.base/share/classes/java/nio/BufferMismatch.java
--- a/src/java.base/share/classes/java/nio/BufferMismatch.java Thu Jun 21 13:43:04 2018 -0700
+++ b/src/java.base/share/classes/java/nio/BufferMismatch.java Thu Jun 21 14:02:19 2018 -0700
@@ -118,7 +118,7 @@
static int mismatch(FloatBuffer a, int aOff, FloatBuffer b, int bOff, int length) {
int i = 0;
if (length > 1 && a.order() == b.order()) {
- if (a.get(aOff) == b.get(bOff)) {
+ if (Float.floatToRawIntBits(a.get(aOff)) ==
+ Float.floatToRawIntBits(b.get(bOff))) {
i = ArraysSupport.vectorizedMismatch(
a.base(), a.address + (aOff << ArraysSupport.LOG2_ARRAY_FLOAT_INDEX_SCALE),
b.base(), b.address + (bOff << ArraysSupport.LOG2_ARRAY_FLOAT_INDEX_SCALE),
@@ -175,7 +175,7 @@
static int mismatch(DoubleBuffer a, int aOff, DoubleBuffer b, int bOff, int length) {
int i = 0;
if (length > 0 && a.order() == b.order()) {
- if (a.get(aOff) == b.get(bOff)) {
+ if (Double.doubleToRawLongBits(a.get(aOff)) ==
+ Double.doubleToRawLongBits(b.get(bOff))) {
i = ArraysSupport.vectorizedMismatch(
a.base(), a.address + (aOff << ArraysSupport.LOG2_ARRAY_DOUBLE_INDEX_SCALE),
b.base(), b.address + (bOff << ArraysSupport.LOG2_ARRAY_DOUBLE_INDEX_SCALE),
diff -r ef8fd07e4440 src/java.base/share/classes/jdk/internal/util/ArraysSupport.java
--- a/src/java.base/share/classes/jdk/internal/util/ArraysSupport.java Thu Jun 21 13:43:04 2018 -0700
+++ b/src/java.base/share/classes/jdk/internal/util/ArraysSupport.java Thu Jun 21 14:02:19 2018 -0700
@@ -461,7 +461,7 @@
int length) {
int i = 0;
if (length > 1) {
- if (a[aFromIndex] == b[bFromIndex]) {
+ if (Float.floatToRawIntBits(a[aFromIndex]) ==
+ Float.floatToRawIntBits(b[bFromIndex])) {
int aOffset = Unsafe.ARRAY_FLOAT_BASE_OFFSET + (aFromIndex << LOG2_ARRAY_FLOAT_INDEX_SCALE);
int bOffset = Unsafe.ARRAY_FLOAT_BASE_OFFSET + (bFromIndex << LOG2_ARRAY_FLOAT_INDEX_SCALE);
i = vectorizedMismatch( @@ -545,7 +545,7 @@
return -1;
}
int i = 0;
- if (a[aFromIndex] == b[bFromIndex]) {
+ if (Double.doubleToRawLongBits(a[aFromIndex]) ==
+ Double.doubleToRawLongBits(b[bFromIndex])) {
int aOffset = Unsafe.ARRAY_DOUBLE_BASE_OFFSET + (aFromIndex << LOG2_ARRAY_DOUBLE_INDEX_SCALE);
int bOffset = Unsafe.ARRAY_DOUBLE_BASE_OFFSET + (bFromIndex << LOG2_ARRAY_DOUBLE_INDEX_SCALE);
i = vectorizedMismatch(
> On Jun 21, 2018, at 11:50 AM, Deshpande, Vivek R <vivek.r.deshpande at intel.com> wrote:
>
> Hi Paul
>
> The folding of if/else is the right way in this patch which I missed in earlier webrev.
> http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.0
> 3/ I think I would keep the same earlier path if both the values are
> NaN.
>
> Thanks.
> Regards,
> Vivek
> From: Paul Sandoz [mailto:paul.sandoz at oracle.com]
> Sent: Wednesday, June 20, 2018 5:29 PM
> To: Deshpande, Vivek R <vivek.r.deshpande at intel.com>
> Cc: Roger.Riggs at oracle.com; David Holmes <david.holmes at oracle.com>;
> core-libs-dev at openjdk.java.net; Viswanathan, Sandhya
> <sandhya.viswanathan at intel.com>
> Subject: Re: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.
>
> Hi Vivek,
>
> 459 public static int mismatch(float[] a, int aFromIndex,
> 460 float[] b, int bFromIndex,
> 461 int length) {
> 462 int i = 0;
> 463 if (length > 1) {
> 464 if (a[aFromIndex] != b[bFromIndex]) {
> 465 i = 0;
> 466 }
> 467 if (a[aFromIndex] == b[bFromIndex]) {
>
> Sorry, i don’t think i was clear. You can reduce down to a single equality check, so Lines #464-466 are redundant. This does result in taking the slow path if both values are NaN, which i think was the same for your previous patch. If that is important we can mitigate that by performing the negation of the same checks in the slow loop.
>
> WDYT?
>
> Paul.
>
>
> On Jun 20, 2018, at 5:05 PM, Deshpande, Vivek R <vivek.r.deshpande at intel.com> wrote:
>
> Hi Paul
>
> I have made the change you have suggested.
> Please find the updated webrev at this location:
> http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.0
> 2/
>
> Regards,
> Vivek
> From: Paul Sandoz [mailto:paul.sandoz at oracle.com]
> Sent: Wednesday, June 20, 2018 2:30 PM
> To: Deshpande, Vivek R <vivek.r.deshpande at intel.com>
> Cc: Roger.Riggs at oracle.com; David Holmes <david.holmes at oracle.com>;
> core-libs-dev at openjdk.java.net; Viswanathan, Sandhya
> <sandhya.viswanathan at intel.com>
> Subject: Re: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.
>
> 459 public static int mismatch(float[] a, int aFromIndex,
> 460 float[] b, int bFromIndex,
> 461 int length) {
> 462 int i = 0;
> 463 if (length > 1) {
> 464 if (a[aFromIndex] != b[bFromIndex]) {
> 465 i = 0;
> 466 } else {
>
> You fold the if/else to;
>
> if (a[aFromIndex] == b[bFromIndex]) {
> …
> }
>
> same applies in other cases in both source files.
>
>
>
>
> On Jun 20, 2018, at 1:21 PM, Deshpande, Vivek R <vivek.r.deshpande at intel.com> wrote:
>
> Hi All
>
> I have updated the webrev with all the suggestions, which passes ArraysEqCmpTest.java.
> Earlier webrev failed the same test.
>
> Thanks for confirming.
>
>
>
> This webrev also modifies the BufferMismatch.java in similar way.
>
> Great!
>
> Paul.
>
>
>
> Please take a look and let me know what you think.
>
> Updated webrev is here:
> http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.0
> 1/ I have also modified the bug with updated webrev.
> Regards,
> Vivek
>
> From: Deshpande, Vivek R
> Sent: Tuesday, June 19, 2018 10:57 AM
> To: Paul Sandoz <paul.sandoz at oracle.com>; Roger.Riggs at oracle.com
> Cc: David Holmes <david.holmes at oracle.com>;
> core-libs-dev at openjdk.java.net; Viswanathan, Sandhya
> <sandhya.viswanathan at intel.com>
> Subject: RE: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.
>
> Hi Roger
>
> I will also test with zero length arrays and let you know.
> Thanks for the input.
>
> Regards,
> Vivek
>
> From: Deshpande, Vivek R
> Sent: Tuesday, June 19, 2018 10:17 AM
> To: 'Paul Sandoz' <paul.sandoz at oracle.com>
> Cc: David Holmes <david.holmes at oracle.com>;
> core-libs-dev at openjdk.java.net; Viswanathan, Sandhya
> <sandhya.viswanathan at intel.com>
> Subject: RE: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.
>
> Thanks Paul for quick review. I will work on the things you have mentioned and get back soon.
> I will also test this with test/jdk/java/util/Arrays/ArraysEqCmpTest.java.
>
> Regards,
> Vivek
>
> From: Paul Sandoz [mailto:paul.sandoz at oracle.com]
> Sent: Tuesday, June 19, 2018 9:55 AM
> To: Deshpande, Vivek R <vivek.r.deshpande at intel.com>
> Cc: David Holmes <david.holmes at oracle.com>;
> core-libs-dev at openjdk.java.net; Viswanathan, Sandhya
> <sandhya.viswanathan at intel.com>
> Subject: Re: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.
>
> Hi Vivek,
>
> Thanks for investigating this.
>
> 164 public static int mismatch(boolean[] a,
> 165 boolean[] b,
> 166 int length) {
> 167 int i = 0;
> 168 if (a[i] != b[i])
> 169 return i;
> You might as well replace the use if i with 0 and i think you can move this to be performed when the length is greater than the threshold, that way you don’t impact small arrays below the threshold.
>
> 186 public static int mismatch(boolean[] a, int aFromIndex,
> 187 boolean[] b, int bFromIndex,
> 188 int length) {
> 189 int i = 0;
> 190 if (a[i] != b[i])
> 191 return i;
> This is incorrect. You need to use aFromIndex and bFromIndex.
>
> Do you run the test test/jdk/java/util/Arrays/ArraysEqCmpTest.java? If that passed then we need to strengthen for the case of a mismatch on the first relative element in each array.
>
> Paul.
>
>
> On Jun 19, 2018, at 9:36 AM, Deshpande, Vivek R <vivek.r.deshpande at intel.com> wrote:
>
> Thanks David.
> Sending it to core-libs-dev.
>
> I would like to contribute a patch which improves the array comparison when there is a mismatch for the first element.
> This avoids call to vectorizedMismatch method and gives ~80x speed up.
> Could you please review and sponsor the patch.
> Link to bug:
> https://bugs.openjdk.java.net/browse/JDK-8205194
> webrev:
> http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.0
> 0/
>
> Regards,
> Vivek
>
> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Monday, June 18, 2018 10:32 PM
> To: Deshpande, Vivek R <vivek.r.deshpande at intel.com>;
> jdk-dev at openjdk.java.net
> Subject: Re: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.
>
> Hi Vivek,
>
> Reviews should take place on the appropriate mailing list for the code being changed, not on the jdk-dev list. Please takes this to core-libs-dev.
>
> Thanks,
> David
>
> On 19/06/2018 9:52 AM, Deshpande, Vivek R wrote:
>
> Hi All
>
> Forgot to add the links:
> https://bugs.openjdk.java.net/browse/JDK-8205194
> webrev:
> http://cr.openjdk.java.net/~vdeshpande/vectorizedMismatch_jdk/webrev.0
> 0/
>
> Regards.
> Vivek
>
> From: Deshpande, Vivek R
> Sent: Monday, June 18, 2018 4:50 PM
> To: 'jdk-dev at openjdk.java.net' <jdk-dev at openjdk.java.net>
> Cc: 'Paul Sandoz' <paul.sandoz at oracle.com>; Viswanathan, Sandhya
> <sandhya.viswanathan at intel.com>
> Subject: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.
>
> Hi All
>
> I would like to contribute a patch which improves the array comparison when there is a mismatch for the first element.
> This avoids call to vectorizedMismatch method and gives ~80x speed up.
> Please review and sponsor the patch.
>
> Regards,
> Vivek
>
More information about the core-libs-dev
mailing list