Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-22 Thread John Rose
On Dec 22, 2017, at 11:57 AM, John Rose wrote: > > On Dec 21, 2017, at 5:31 PM, Stuart Marks wrote: >> >> I'd like a blanket assertion that view collections are not serializable. >> >> Now this should be considered distinct from whether view

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-22 Thread John Rose
On Dec 21, 2017, at 5:31 PM, Stuart Marks wrote: > > I'd like a blanket assertion that view collections are not serializable. > > Now this should be considered distinct from whether view collections are > value-based; I'm fine with considering view collections to be

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-22 Thread Jason Mehrens
What are your thoughts on pushing the static EMPTY_XXX declarations from ImmutableCollections down in to each respective inner class? Doing that should allow for on demand class loading instead of getting everything when any factory is used.

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-21 Thread Stuart Marks
John Rose wrote: Can anyone point out a reason why the value based lists of List.of() should serialize while the value based lists of List.of().subList() should not? Or is there some reason we should not allow subList to produce value based lists? I think one can interpret the @implSpec in

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-21 Thread Stuart Marks
Finally catching up with this thread Yes, there should be some additional test cases added. When I added the immutable collection classes in JDK 9, I did modify MOAT.java so that its test cases would apply to the new implementations. A few more cases could be added that apply not only to

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-09 Thread Peter Levart
Peter Levart je 09. 12. 2017 ob 21:01 napisal: Hi Claes, Claes Redestad je 09. 12. 2017 ob 03:19 napisal: Hi John, On 2017-12-09 02:20, John Rose wrote: On Dec 8, 2017, at 4:45 PM, John Rose > wrote: Can anyone point out a reason why

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-09 Thread Peter Levart
Hi Claes, Claes Redestad je 09. 12. 2017 ob 03:19 napisal: Hi John, On 2017-12-09 02:20, John Rose wrote: On Dec 8, 2017, at 4:45 PM, John Rose > wrote: Can anyone point out a reason why the value based lists of List.of() should

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-09 Thread Andrej Golovnin
Hi Claes, > For example, returning Spliterators.emptySpliterator() and > Collections.singletonSpliterator when appropriate *sounds* like nice little > optimizations, but Arrays.spliterator(T[]) with an empty or single-element > array > appears to be relatively cheap operations, whereas mixing

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-09 Thread Claes Redestad
Hi Andrej, forEach seems like a no-brainer, but spliterator might require some extra care. For example, returning Spliterators.emptySpliterator() and Collections.singletonSpliterator when appropriate *sounds* like nice little optimizations, but Arrays.spliterator(T[]) with an empty or

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-09 Thread Andrej Golovnin
Hi Claes, > One more thing: Could you please add specialised implementations also > for the following methods: > > List.forEach(Consumer) > > List.spliterator() > For List12 when List12.size() == 1 please use > Collections.singletonSpliterator() > (this method should be moved to the

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-09 Thread Andrej Golovnin
Hi Claes, > I think one can interpret the @implSpec in AbstracList::subList to suggest > that the implementation retrieved will subclass AbstractList, which implies > it's *not* Serializable. Not for me. java.util.ArrayList is a subclass of AbstractList and is serializable. And you can use

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-08 Thread John Rose
On Dec 8, 2017, at 6:19 PM, Claes Redestad wrote: > > I think one can interpret the @implSpec in AbstracList::subList to suggest > that the implementation retrieved will subclass AbstractList, which implies > it's *not* Serializable. As we move away from AbstractList

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-08 Thread Claes Redestad
On 2017-12-09 02:09, John Rose wrote: On Dec 8, 2017, at 7:13 AM, Claes Redestad wrote: http://cr.openjdk.java.net/~redestad/8193128/open.03/ +for (int i = elements.length - 1; i > 0; i--) { There's an OBOE in this line; should be i >= 0. I thought I

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-08 Thread Claes Redestad
Hi John, On 2017-12-09 02:20, John Rose wrote: On Dec 8, 2017, at 4:45 PM, John Rose > wrote: Can anyone point out a reason why the value based lists of List.of() should serialize while the value based lists of List.of().subList() should

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-08 Thread John Rose
On Dec 8, 2017, at 4:45 PM, John Rose wrote: > > Can anyone point out a reason why the value based > lists of List.of() should serialize while the value based > lists of List.of().subList() should not? Or is there some > reason we should not allow subList to produce

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-08 Thread John Rose
+1 (there should) On Dec 8, 2017, at 9:44 AM, Martin Buchholz wrote: > > Should there be changes to general purpose collection testing classes like > test/jdk/java/util/concurrent/tck/CollectionTest.java > test/jdk/java/util/Collection/MOAT.java > that would have caught

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-08 Thread John Rose
On Dec 8, 2017, at 7:13 AM, Claes Redestad wrote: > > http://cr.openjdk.java.net/~redestad/8193128/open.03/ > +for (int i = elements.length - 1; i > 0; i--) { There's an OBOE in this line; should be i >= 0. Errors like this are a risk of

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-08 Thread John Rose
On Dec 7, 2017, at 3:41 PM, Claes Redestad wrote: > > - the ListFactories test didn't explicitly verify that sublists retrieved > from various List.of() impls aren't Serializable. Added tests to check no > sub-list is Serializable, > and as a bonus this test now

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-08 Thread Martin Buchholz
Should there be changes to general purpose collection testing classes like test/jdk/java/util/concurrent/tck/CollectionTest.java test/jdk/java/util/Collection/MOAT.java that would have caught this bug? (although those are mostly designed for mutable collections, but I think some immutable

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-08 Thread Claes Redestad
Hi, On 2017-12-08 07:54, Andrej Golovnin wrote: Hi Claes, http://cr.openjdk.java.net/~redestad/8193128/open.02/ I think you should provide specialised implementations of the #indexOf() and #lastIndexOf() methods in the classes List12 and ListN. Using an iterator in List12 is an overkill. But

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-07 Thread Andrej Golovnin
Hi Claes, > http://cr.openjdk.java.net/~redestad/8193128/open.02/ I think you should provide specialised implementations of the #indexOf() and #lastIndexOf() methods in the classes List12 and ListN. Using an iterator in List12 is an overkill. But even in ListN using a simple for loop would be

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-07 Thread Claes Redestad
On 2017-12-07 01:12, Claes Redestad wrote: SubList is now final, inherits from AbstractImmutableList instead of AbstractList and reuses more of the shared code. I also moved 'implements Serializable' from AbstractImmutableList to List12 and ListN respectively to not change the behavior that

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-07 Thread John Rose
On Dec 7, 2017, at 12:05 PM, Remi Forax wrote: > > have a bit saying if an object is fully initialized or not, this bit will be > true at the end of the constructor and can be set for the de-serialization too Yes, that's the hard part of fixing final. Sometimes we call this

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-07 Thread Remi Forax
udi 7 Décembre 2017 20:50:01 > Objet: Re: [10?] RFR: 8193128: Reduce number of implementation classes > returned by List/Set/Map.of() >> On Dec 6, 2017, at 5:16 PM, Paul Sandoz <paul.san...@oracle.com> wrote: >> >> It can, since final fields are not treated as really fi

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-07 Thread John Rose
> On Dec 6, 2017, at 5:16 PM, Paul Sandoz wrote: > > It can, since final fields are not treated as really final (unless in > java.lang.invoke package, where it’s as if all such fields are annotated with > @Stable). C2 will treat the field value a constant value if the

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-06 Thread Paul Sandoz
> On 6 Dec 2017, at 16:12, Claes Redestad wrote: >> >> and i do not understand why the field size is not declared @Stable anymore, >> ok, it can be equals to zero, but in that case the JIT will emit a move >> so it's better than always asking for a move (or i

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-06 Thread Claes Redestad
Hi Rémi, On 2017-12-06 23:57, Remi Forax wrote: Hi Claes, - both constructors of SubList should be package private, deal! - in listIterator, i can be declared outside of the ListIterator as a local variable that would be captured by the anonymous class, so index is not used inside the

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-06 Thread Claes Redestad
Hi, On 2017-12-06 22:20, Martin Buchholz wrote: Guava struggled with this as well with their immutable collections.  You could look at their revision history (I haven't). Maybe they got rid of SingletonImmutableList for the same reason? I've not looked at guava sources, but I've seen

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-06 Thread Remi Forax
quot; <core-libs-dev@openjdk.java.net>, "Stuart Marks" > <stuart.ma...@oracle.com> > Envoyé: Mercredi 6 Décembre 2017 21:21:55 > Objet: [10?] RFR: 8193128: Reduce number of implementation classes returned > by List/Set/Map.of() > Hi, > > please help revi

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-06 Thread Claes Redestad
Hi Jonathan, On 2017-12-06 21:58, Jonathan Bluett-Duncan wrote: Hi Claes, Looking at http://cr.openjdk.java.net/~redestad/8193128/open.00/src/java.base/share/classes/java/util/ImmutableCollections.java.cdiff.html

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-06 Thread Martin Buchholz
Guava struggled with this as well with their immutable collections. You could look at their revision history (I haven't). Maybe they got rid of SingletonImmutableList for the same reason? On Wed, Dec 6, 2017 at 12:21 PM, Claes Redestad wrote: > Hi, > > please help

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-06 Thread Jonathan Bluett-Duncan
Hi Claes, Looking at http://cr.openjdk.java.net/~redestad/8193128/open.00/src/java.base/share/classes/java/util/ImmutableCollections.java.cdiff.html, there are sections labelled --- 646,657 and --- 834,845 where lines like `Objects.requireNonNull(0 /* zero */);` are written. I believe

[10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

2017-12-06 Thread Claes Redestad
Hi, please help review this patch to consolidate the number of implementation classes returned by the static collection factories: http://cr.openjdk.java.net/~redestad/8193128/open.00/ I set out to explore options for addressing small inefficiencies we've been running into, the latest after