RFR: 8226297: Dual-pivot quicksort improvements

[Laurent Bourgès]

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.christian at 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.christian at 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.christian at oracle.com <mailto:brent.christian at oracle.com>
> >      > <mailto:brent.christian at 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