Hi Brent, I made a manual comparison of your webrev.4 and my previous patch. - I noticed plenty of javadoc / comment changes that seem totally appropriate in Arrays class: ok for me. - You also fixed the problem with the ForkJoinPool field: good. - No change in the DualPivotQuickSort class except copyright header: OK (you reverted reordering I suppose)
My former question was more about the process: how to produce incremental web revs easily ? it is easier for me to have a quick look, than inspecting the global patch where DPQS class was almost rewritten. Thanks for your help, Laurent Le mer. 7 août 2019 à 01:33, Brent Christian <brent.christ...@oracle.com> a écrit : > On 8/6/19 12:47 PM, Vladimir Yaroslavskiy wrote: > > > > I moved Object sorting related stuff after primitives sorting methods > > to separate functionality logically. > > Sure, fine to keep that all together. I can move that back: > http://cr.openjdk.java.net/~bchristi/8226297/webrev04/ > > > The order of methods in my version is: > > > > 1. primitives (sequential sorting) > > - int > > - long > > - byte > > - char > > - short > > - float > > - double > > The order for sequential sorting of primitives in Arrays.java checked > into the JDK is: > > - int > - long > * short > * char > * byte > - float > - double > > It simplifies the webrev for reviewing to keep that ordering, so that's > what I've done. > > Thanks, > -Brent > > > Вторник, 6 августа 2019, 21:35 +03:00 от Brent Christian > > <brent.christ...@oracle.com>: > > > > Hi, Laurent > > > > I'm not sure what exactly is causing the problem, but here's my > hunch: > > > > In Vladimir's version of Arrays.java: > > MIN_ARRAY_SORT_GRAN > > NaturalOrder > > rangeCheck > > got moved around, but were unchanged. > > > > In the interest of keeping the change as simple as possible, I > restored > > these to their original location, so they don't show up in my > changes. > > That could confuse things when comparing diffs. > > > > One idea would be to restore those elements back in their original > > locations in your version, and re-generate your patch. I don't know > if > > that would be less work than just comparing raw files. > > > > Alternatively, if it would be easiest for those familiar with the > > evolution of this fix to leave things where Vladimir had them, I can > do > > that. > > > > Thanks, > > -Brent > > > > On 8/6/19 6:32 AM, Laurent Bourgès wrote: > > > Hi Brent, > > > > > > Thank you for sponsoring this patch. > > > > > > I tried to compare your webrev against my latest (diff patch > > files) but > > > it gives me too many changes lines. > > > > > > Do you have another idea to see incremental changes only ? > > > (anyway I can compare raw files) > > > > > > Thanks, > > > Laurent > > > > > > Le lun. 5 août 2019 à 23:43, Brent Christian > > <brent.christ...@oracle.com <mailto:brent.christ...@oracle.com> > > > <mailto:brent.christ...@oracle.com>> a écrit : > > > > > > Hi, > > > > > > Please review Vladimir Yaroslavskiy's changes to > DualPivotQuickSort > > > (seen earlier[1] on this alias). I will be sponsoring this > change. > > > > > > I have a webrev against jdk-jdk here: > > > http://cr.openjdk.java.net/~bchristi/8226297/webrev03-rfr/ > > > > > > (Note that I did a little re-ordering, and removed some > superfluous > > > spacing changes, in order to simplify the webrev. I've also > included > > > Vladimir's FailedFloat[2] test case.) > > > > > > Information about benchmarking the changes was posted[3] recently. > > > An automated test run passes cleanly. > > > > > > Thanks! > > > -Brent > > > -- > > > 1. > > > > > > http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-July/061363.html > > > > > > 2. > > > > > > https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-July/061513.html > > > > > > 3. > > > > > > http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-July/061553.html > > > > > > > > > > > -- > > Vladimir Yaroslavskiy > -- -- Laurent Bourgès
