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

Reply via email to