Yes makes sense. Good catch :) Thanks :) Regards, Vivek -----Original Message----- From: Ivan Gerasimov [mailto:ivan.gerasi...@oracle.com] Sent: Wednesday, June 20, 2018 5:24 PM To: Deshpande, Vivek R <vivek.r.deshpa...@intel.com> Cc: Paul Sandoz <paul.san...@oracle.com>; core-libs-dev@openjdk.java.net Subject: Re: RFR(S): 8205194: Improve the Array Comparison when there is a mismatch at first element.
Hi Vivek! I think you don't need this if block: 464 if (a[aFromIndex] != b[bFromIndex]) { 465 i = 0; 466 } i is already 0 anyway, and the following line is just enough. 467 if (a[aFromIndex] == b[bFromIndex]) { The same applies to other float/double variants. With kind regards, Ivan On 6/20/18 5:05 PM, Deshpande, Vivek R 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.san...@oracle.com] > Sent: Wednesday, June 20, 2018 2:30 PM > To: Deshpande, Vivek R <vivek.r.deshpa...@intel.com> > Cc: roger.ri...@oracle.com; David Holmes <david.hol...@oracle.com>; > core-libs-dev@openjdk.java.net; Viswanathan, Sandhya > <sandhya.viswanat...@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.deshpa...@intel.com<mailto:vivek.r.deshpa...@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.san...@oracle.com<mailto:paul.san...@oracle.com>>; > roger.ri...@oracle.com<mailto:roger.ri...@oracle.com> > Cc: David Holmes > <david.hol...@oracle.com<mailto:david.hol...@oracle.com>>; > core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net>; > Viswanathan, Sandhya > <sandhya.viswanat...@intel.com<mailto:sandhya.viswanat...@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.san...@oracle.com<mailto:paul.san...@oracle.com>> > Cc: David Holmes > <david.hol...@oracle.com<mailto:david.hol...@oracle.com>>; > core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net>; > Viswanathan, Sandhya > <sandhya.viswanat...@intel.com<mailto:sandhya.viswanat...@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.san...@oracle.com] > Sent: Tuesday, June 19, 2018 9:55 AM > To: Deshpande, Vivek R > <vivek.r.deshpa...@intel.com<mailto:vivek.r.deshpa...@intel.com>> > Cc: David Holmes > <david.hol...@oracle.com<mailto:david.hol...@oracle.com>>; > core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net>; > Viswanathan, Sandhya > <sandhya.viswanat...@intel.com<mailto:sandhya.viswanat...@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.deshpa...@intel.com<mailto:vivek.r.deshpa...@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.hol...@oracle.com] > Sent: Monday, June 18, 2018 10:32 PM > To: Deshpande, Vivek R > <vivek.r.deshpa...@intel.com<mailto:vivek.r.deshpa...@intel.com>>; > jdk-...@openjdk.java.net<mailto:jdk-...@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-...@openjdk.java.net<mailto:jdk-...@openjdk.java.net>' > <jdk-...@openjdk.java.net<mailto:jdk-...@openjdk.java.net>> > Cc: 'Paul Sandoz' > <paul.san...@oracle.com<mailto:paul.san...@oracle.com>>; Viswanathan, > Sandhya > <sandhya.viswanat...@intel.com<mailto:sandhya.viswanat...@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 > -- With kind regards, Ivan Gerasimov