Re: RFR: jsr166 jdk9 integration wave 12

This finally got committed, after a lot of performance and testing rework, and discovering plenty of opportunity for future performance, scalability and testing improvements. Thanks to reviewers for performance push back, leading especially to a better ArrayDeque.

Re: RFR: jsr166 jdk9 integration wave 12

On Mon, Nov 21, 2016 at 8:47 AM, Paul Sandoz wrote: > > On 21 Nov 2016, at 07:29, Martin Buchholz wrote: > > Un-OK. Sometimes when microbenchmarking, we stumble upon a code sequence > that hotspot likes. I observed 20% improvement with the below,

Re: RFR: jsr166 jdk9 integration wave 12

> On 21 Nov 2016, at 07:29, Martin Buchholz wrote: > > Un-OK. Sometimes when microbenchmarking, we stumble upon a code sequence that > hotspot likes. I observed 20% improvement with the below, so switching to > that: > > public boolean tryAdvance(Consumer

Re: RFR: jsr166 jdk9 integration wave 12

Un-OK. Sometimes when microbenchmarking, we stumble upon a code sequence that hotspot likes. I observed 20% improvement with the below, so switching to that: public boolean tryAdvance(Consumer action) { Objects.requireNonNull(action); final Object[] es =

Re: RFR: jsr166 jdk9 integration wave 12

No more rework is planned!

Re: RFR: jsr166 jdk9 integration wave 12

Regarding the testing mess: I think there should be a project to use Junit 5 features to create interface based tests. Then a BlockingDeque implementation could easily pull in all the tests already written for BlockingDeque and its superinterfaces. Tests for interfaces would be found in a

Re: RFR: jsr166 jdk9 integration wave 12

I encourage you to look at the experiments in the valhalla repository. Such as: http://hg.openjdk.java.net/valhalla/valhalla/jdk/file/780c8eba356a/src/java.base/share/classes/java/anyutil/ArrayDeque.java

Re: RFR: jsr166 jdk9 integration wave 12

Here's a few more places to apply Remi's rule: --- src/test/tck/Collection8Test.java 15 Nov 2016 23:08:30 - 1.27 +++ src/test/tck/Collection8Test.java 18 Nov 2016 17:21:19 - @@ -304,7 +304,7 @@ switch (rnd.nextInt(4)) { case 0: survivors.addAll(c); break;

Re: RFR: jsr166 jdk9 integration wave 12

lt;marti...@google.com <mailto:marti...@google.com>> > > À: "Paul Sandoz" <paul.san...@oracle.com <mailto:paul.san...@oracle.com>> > > Cc: "core-libs-dev" <core-libs-dev@openjdk.java.net > > <mailto:core-libs-dev@openjdk.java.net>

Re: RFR: jsr166 jdk9 integration wave 12

On Fri, Nov 18, 2016 at 5:02 AM, Doug Lea wrote: > On 11/18/2016 02:28 AM, Remi Forax wrote: > >> Martin, >> for ArrayDeque, when you start to have methods like >> @SuppressWarnings("unchecked") >>static final E elementAt(Object[] es, int i) { >>return (E)

Re: RFR: jsr166 jdk9 integration wave 12

> On 18 Nov 2016, at 04:52, Doug Lea wrote: > > >> On Thu, Nov 17, 2016 at 12:03 PM, Paul Sandoz > > wrote: >> >> >>Semaphore >>— >> >> 633 /** >> 634 * Acquires and returns all permits that

Re: RFR: jsr166 jdk9 integration wave 12

ibs-dev@openjdk.java.net> > > Envoyé: Vendredi 18 Novembre 2016 05:29:12 > > Objet: Re: RFR: jsr166 jdk9 integration wave 12 > > [..] > > > >> 317 c.forEach(e -> addLast(e)); > >> > >> this::addLast, up to you which you prefer > &

Re: RFR: jsr166 jdk9 integration wave 12

On 11/18/2016 02:28 AM, Remi Forax wrote: Martin, for ArrayDeque, when you start to have methods like @SuppressWarnings("unchecked") static final E elementAt(Object[] es, int i) { return (E) es[i]; } I think it's a better idea to type the field elements as a E[] instead of

Re: RFR: jsr166 jdk9 integration wave 12

On Thu, Nov 17, 2016 at 12:03 PM, Paul Sandoz > wrote: Semaphore — 633 /** 634 * Acquires and returns all permits that are immediately available. 635 * Upon return, zero permits are available.

Re: RFR: jsr166 jdk9 integration wave 12

Martin, for ArrayDeque, when you start to have methods like @SuppressWarnings("unchecked") static final E elementAt(Object[] es, int i) { return (E) es[i]; } I think it's a better idea to type the field elements as a E[] instead of Object[]. Historically, using Object(] instead

Re: RFR: jsr166 jdk9 integration wave 12

- Mail original - > De: "Martin Buchholz" <marti...@google.com> > À: "Paul Sandoz" <paul.san...@oracle.com> > Cc: "core-libs-dev" <core-libs-dev@openjdk.java.net> > Envoyé: Vendredi 18 Novembre 2016 05:29:12 >

Re: RFR: jsr166 jdk9 integration wave 12

On Thu, Nov 17, 2016 at 12:03 PM, Paul Sandoz wrote: > > Semaphore > — > > 633 /** > 634 * Acquires and returns all permits that are immediately > available. > 635 * Upon return, zero permits are available. > 636 * > 637 * @return the number

Re: RFR: jsr166 jdk9 integration wave 12

On Thu, Nov 17, 2016 at 12:03 PM, Paul Sandoz wrote: > > Perhaps consolidate the repeated mini bit set methods as package private > static methods on AbstractCollection? Unclear where the j.u.concurrent ones > should go though... > Deferred. > It’s purely a subjective

Re: RFR: jsr166 jdk9 integration wave 12

On Thu, Nov 17, 2016 at 12:03 PM, Paul Sandoz wrote: > > 843 public boolean tryAdvance(Consumer action) { > 844 if (action == null) > 845 throw new NullPointerException(); > 846 int t, i; > 847 if ((t =

Re: RFR: jsr166 jdk9 integration wave 12

Here's an update that adds some explanations, addresses some of Paul's comments, and improves handling of capacities near 2**31. Coincidentally, today I got access to a machine with enough memory to run testHugeCapacity without dying, so that test gets fixed. Previous iterations tried to get rid

Re: RFR: jsr166 jdk9 integration wave 12

> On 17 Nov 2016, at 13:58, Martin Buchholz wrote: > > > > On Thu, Nov 17, 2016 at 12:03 PM, Paul Sandoz > wrote: > > ForkJoin > — > > 69 * tasks that are never joined. All worker threads are initialized >

Re: RFR: jsr166 jdk9 integration wave 12

On Thu, Nov 17, 2016 at 12:03 PM, Paul Sandoz wrote: > > > SplittableRandom > — > > 533 /** > 534 * Generates a pseudorandom number with the indicated number of > 535 * low-order bits. Because this class has no subclasses, this > 536 * method

Re: RFR: jsr166 jdk9 integration wave 12

On Thu, Nov 17, 2016 at 12:03 PM, Paul Sandoz wrote: > > ForkJoin > — > > 69 * tasks that are never joined. All worker threads are initialized > 70 * with {@link Thread#isDaemon} set {@code true}. > > CCC? > JDK 7 had """Because ForkJoinPool uses threads in daemon

Re: RFR: jsr166 jdk9 integration wave 12

> On 16 Nov 2016, at 16:03, Martin Buchholz wrote: > > Apologies for having this wave turn into a tsunami. > > CopyOnWriteArrayList was rewritten due to finding > https://bugs.openjdk.java.net/browse/JDK-8169738 > ArrayDeque and ArrayBlockingQueue now both use efficient

Re: RFR: jsr166 jdk9 integration wave 12

Apologies for having this wave turn into a tsunami. CopyOnWriteArrayList was rewritten due to finding https://bugs.openjdk.java.net/browse/JDK-8169738 ArrayDeque and ArrayBlockingQueue now both use efficient nested loop idiom for all traversals. Most bulk remove methods in all collection classes

Re: RFR: jsr166 jdk9 integration wave 12

Hi Martin, Glad you looked more closely at the perf aspects of ArrayDeque. I am struggling to determine what has changed since the last revision. Apart from ArrayDeque what files should i be looking at? Thanks, Paul. > On 16 Nov 2016, at 12:14, Martin Buchholz wrote: >

Re: RFR: jsr166 jdk9 integration wave 12

Thanks to Vitaly and Doug for being more concerned about offer/poll performance than I was initially. ArrayDeque is now back to using the head + tail + spare slot strategy, which keeps optimal performance for non-bulk operations, while everything else gets faster. We have new tests and new

Re: RFR: jsr166 jdk9 integration wave 12

On Tue, Oct 18, 2016 at 4:18 PM, Martin Buchholz wrote: > > > On Tue, Oct 18, 2016 at 4:00 PM, Vitaly Davidovich > wrote: > >> >>> > * Change in allocation/capacity policy. >>> > >>> > The removal of the power-of-two restriction, and applying a 1.5x

Re: RFR: jsr166 jdk9 integration wave 12

> On 19 Oct 2016, at 19:33, Martin Buchholz wrote: > > > > On Wed, Oct 19, 2016 at 3:44 PM, Paul Sandoz > wrote: > >> On 18 Oct 2016, at 13:58, Martin Buchholz >

Re: RFR: jsr166 jdk9 integration wave 12

Ah, I see. Thanks for answering my questions Martin. Sounds like there's no real benefit then to using guava-testlib in this changeset instead of CollectionTest. And even if the benefit of using it was considered to be high enough in the future, it would probably need its own JSR, since I presume

Re: RFR: jsr166 jdk9 integration wave 12

On Wed, Oct 19, 2016 at 4:40 PM, Jonathan Bluett-Duncan < jbluettdun...@gmail.com> wrote: > Hi Martin, > > By collections infrastructure, do you mean something like the collection > testers in guava-testlib? > > If so, I agree that JUnit 5's dynamic tests look promising for > implementing such an

Re: RFR: jsr166 jdk9 integration wave 12

On Wed, Oct 19, 2016 at 3:44 PM, Paul Sandoz wrote: > > On 18 Oct 2016, at 13:58, Martin Buchholz wrote: > > Latest webrev defers potential new methods: > >/* TODO: public */ private void trimToSize() > > Sorry to push back, i know it’s a hassle,

Re: RFR: jsr166 jdk9 integration wave 12

Hi Martin, By collections infrastructure, do you mean something like the collection testers in guava-testlib? If so, I agree that JUnit 5's dynamic tests look promising for implementing such an infrastructure. I admit I don't have all the context here, but would using guava-testlib in the

Re: RFR: jsr166 jdk9 integration wave 12

> On 18 Oct 2016, at 13:58, Martin Buchholz wrote: > > Latest webrev defers potential new methods: >/* TODO: public */ private void trimToSize() Sorry to push back, i know it’s a hassle, but i think such features should be retained in a separate proposed patch. Paul.

Re: RFR: jsr166 jdk9 integration wave 12

These tests are maintained outside of openjdk and have only recently been made available within openjdk. There may be a future where all the consumers of these tests are willing to accept a good test framework. Junit 5 dynamic tests look promising

Re: RFR: jsr166 jdk9 integration wave 12

On 10/18/16 11:00 AM, Martin Buchholz wrote: There's already a certain amount of mixing, e.g. stream tests sometimes test j.u.c. and Queue tests sometimes test all the Queue implementations. Unlike non-test code, some redundancy and divergence of testing frameworks and styles is fine. I still

Re: RFR: jsr166 jdk9 integration wave 12

On Tue, Oct 18, 2016 at 6:33 PM, Vitaly Davidovich wrote: > > Indeed - the whole inlining based on simple bytecode size is a big problem > (I've brought this up in various contexts several times on the compiler > list, but this is a known issue). But, some of the methods

Re: RFR: jsr166 jdk9 integration wave 12

On Tuesday, October 18, 2016, Martin Buchholz wrote: > > > On Tue, Oct 18, 2016 at 4:26 PM, Vitaly Davidovich > wrote: > >> "jsr166 style" - makes it easy on javac and the JIT, if not for humans. >>> >>

Re: RFR: jsr166 jdk9 integration wave 12

On Tue, Oct 18, 2016 at 4:26 PM, Vitaly Davidovich wrote: > "jsr166 style" - makes it easy on javac and the JIT, if not for humans. >> > In some of the cases here, I'd consider it a significant JIT failure if > the "jsr166 style" makes a difference (I'm not sure how this makes

Re: RFR: jsr166 jdk9 integration wave 12

On Tuesday, October 18, 2016, Martin Buchholz wrote: > > > On Tue, Oct 18, 2016 at 4:00 PM, Vitaly Davidovich > wrote: > >> >>> > * Change in allocation/capacity policy. >>> > >>> > The removal of the

Re: RFR: jsr166 jdk9 integration wave 12

On Tuesday, October 18, 2016, Martin Buchholz wrote: > On Tue, Oct 18, 2016 at 10:15 AM, Stuart Marks > > wrote: > > > > > > > On 10/17/16 6:34 PM, Martin Buchholz wrote: > > > >> Most of this is for Stuart - very collection-y. > >> >

Re: RFR: jsr166 jdk9 integration wave 12

Latest webrev defers potential new methods: /* TODO: public */ private void trimToSize()

Re: RFR: jsr166 jdk9 integration wave 12

> On 18 Oct 2016, at 12:07, Martin Buchholz wrote: > > > > On Tue, Oct 18, 2016 at 10:55 AM, Paul Sandoz wrote: > > Testing-wise there is always gonna be some overlap. It would be nice to > consolidate, although arguably in principle TCK has a

Re: RFR: jsr166 jdk9 integration wave 12

On Tue, Oct 18, 2016 at 10:55 AM, Paul Sandoz wrote: > > Testing-wise there is always gonna be some overlap. It would be nice to > consolidate, although arguably in principle TCK has a slightly different > focus. Now that the tck is in the OpenJDK repo perhaps we should

Re: RFR: jsr166 jdk9 integration wave 12

On Tue, Oct 18, 2016 at 10:15 AM, Stuart Marks wrote: > > > On 10/17/16 6:34 PM, Martin Buchholz wrote: > >> Most of this is for Stuart - very collection-y. >> >> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166- >> jdk9-integration/ >> >> This includes a

Re: RFR: jsr166 jdk9 integration wave 12

Hi, The j.u.c. changes look ok to me as do the misc changes. The ArrayDeque changes may also change the performance characteristics, a space/time trade-off e.g. in current version array bounds checks are strength reduced to a zero-based test of the array length. Unsure in practice if this

Re: RFR: jsr166 jdk9 integration wave 12

On 10/17/16 6:34 PM, Martin Buchholz wrote: Most of this is for Stuart - very collection-y. http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration/ This includes a rewrite of ArrayDeque to bring it to parity with ArrayList (except for List features). The patch includes