Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v8]

2021-03-04 Thread Joe Darcy
On Thu, 4 Mar 2021 22:35:09 GMT, Ian Graves wrote: >> Modify the `unmodifiable*` methods in `java.util.Collections` to be >> idempotent. That is, when given an immutable collection from >> `java.util.ImmutableCollections` or `java.util.Collections`, these methods >> will return the reference

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v7]

2021-03-04 Thread Ian Graves
On Thu, 4 Mar 2021 21:37:45 GMT, Joe Darcy wrote: >> Marked as reviewed by smarks (Reviewer). > > If the checks for Navigable set and the like are omitted, I'd prefer to a > comment in the sources noting this is intention as a Navigable set is-a > Sorted set. Added comments to the relevant

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v8]

2021-03-04 Thread Ian Graves
> Modify the `unmodifiable*` methods in `java.util.Collections` to be > idempotent. That is, when given an immutable collection from > `java.util.ImmutableCollections` or `java.util.Collections`, these methods > will return the reference instead of creating a new immutable collection that >

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v7]

2021-03-04 Thread Joe Darcy
On Thu, 4 Mar 2021 20:28:58 GMT, Stuart Marks wrote: >> Ian Graves has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Revert "Avoid wrapping subclasses where relevant. Updated tests." >> >> This reverts commit

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v7]

2021-03-04 Thread Stuart Marks
On Thu, 4 Mar 2021 04:13:12 GMT, Ian Graves wrote: >> Modify the `unmodifiable*` methods in `java.util.Collections` to be >> idempotent. That is, when given an immutable collection from >> `java.util.ImmutableCollections` or `java.util.Collections`, these methods >> will return the reference

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v7]

2021-03-03 Thread Ian Graves
> Modify the `unmodifiable*` methods in `java.util.Collections` to be > idempotent. That is, when given an immutable collection from > `java.util.ImmutableCollections` or `java.util.Collections`, these methods > will return the reference instead of creating a new immutable collection that >

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v5]

2021-03-03 Thread Ian Graves
On Thu, 4 Mar 2021 03:57:35 GMT, Stuart Marks wrote: >> The `@implNote` additions are good, and the test rewrite looks good too. > > Hm. I had thought of this previously but I was a bit suspicious, and it > didn't seem like it would make much difference, so I didn't say anything. But >

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v5]

2021-03-03 Thread Stuart Marks
On Fri, 26 Feb 2021 21:37:14 GMT, Stuart Marks wrote: >> Ian Graves has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Test refactoring. Adding implNote to modified methods > > The `@implNote` additions are good, and the test rewrite looks

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v6]

2021-03-03 Thread Ian Graves
> Modify the `unmodifiable*` methods in `java.util.Collections` to be > idempotent. That is, when given an immutable collection from > `java.util.ImmutableCollections` or `java.util.Collections`, these methods > will return the reference instead of creating a new immutable collection that >

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v5]

2021-03-03 Thread Ian Graves
On Thu, 4 Mar 2021 02:01:02 GMT, Ian Graves wrote: >> src/java.base/share/classes/java/util/Collections.java line 1168: >> >>> 1166: */ >>> 1167: public static SortedSet unmodifiableSortedSet(SortedSet >>> s) { >>> 1168: if (s.getClass() == UnmodifiableSortedSet.class) { >>

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v5]

2021-03-03 Thread Ian Graves
On Wed, 3 Mar 2021 23:29:33 GMT, Joe Darcy wrote: >> Ian Graves has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Test refactoring. Adding implNote to modified methods > > src/java.base/share/classes/java/util/Collections.java line 1168:

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v5]

2021-03-03 Thread Joe Darcy
On Fri, 26 Feb 2021 20:15:19 GMT, Ian Graves wrote: >> Modify the `unmodifiable*` methods in `java.util.Collections` to be >> idempotent. That is, when given an immutable collection from >> `java.util.ImmutableCollections` or `java.util.Collections`, these methods >> will return the reference

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v5]

2021-02-26 Thread Stuart Marks
On Fri, 26 Feb 2021 20:15:19 GMT, Ian Graves wrote: >> Modify the `unmodifiable*` methods in `java.util.Collections` to be >> idempotent. That is, when given an immutable collection from >> `java.util.ImmutableCollections` or `java.util.Collections`, these methods >> will return the reference

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v4]

2021-02-26 Thread Ian Graves
On Wed, 24 Feb 2021 01:58:48 GMT, Stuart Marks wrote: >>> Is there any behavior change here that merits a CSR review? >> >> Yes. See my comments in the bug report: >> >>

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v5]

2021-02-26 Thread Ian Graves
> Modify the `unmodifiable*` methods in `java.util.Collections` to be > idempotent. That is, when given an immutable collection from > `java.util.ImmutableCollections` or `java.util.Collections`, these methods > will return the reference instead of creating a new immutable collection that >

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v4]

2021-02-23 Thread Stuart Marks
On Tue, 23 Feb 2021 23:32:01 GMT, Stuart Marks wrote: >>> > Is there any behavior change here that merits a CSR review? >>> >>> Maybe. The one observable change is that calling `Collections.bar(foo)` >>> with a `foo` that is already a `bar` will return the instance rather than >>>

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v4]

2021-02-23 Thread Stuart Marks
On Tue, 23 Feb 2021 16:27:06 GMT, Ian Graves wrote: > Is there any behavior change here that merits a CSR review? Yes. See my comments in the bug report:

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v4]

2021-02-23 Thread Stuart Marks
On Fri, 19 Feb 2021 01:52:51 GMT, liach wrote: >> Maybe it is not correct for UnmodifiableEntrySet::contains to short circuit? >> What if the implementation was changed to: >> >> `public boolean contains(Object o) { >> if (!(o instanceof Map.Entry)) >> return

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v4]

2021-02-23 Thread Ian Graves
On Tue, 23 Feb 2021 16:18:29 GMT, Claes Redestad wrote: > > Is there any behavior change here that merits a CSR review? > > Maybe. The one observable change is that calling `Collections.bar(foo)` with > a `foo` that is already a `bar` will return the instance rather than > unnecessarily wrap

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v4]

2021-02-23 Thread Claes Redestad
On Tue, 23 Feb 2021 06:12:54 GMT, Joe Darcy wrote: > Is there any behavior change here that merits a CSR review? Maybe. The one observable change is that calling `Collections.bar(foo)` with a `foo` that is already a `bar` will return the instance rather than unnecessarily wrap it. This could

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v4]

2021-02-22 Thread Joe Darcy
On Mon, 22 Feb 2021 23:39:15 GMT, Claes Redestad wrote: >> Ian Graves has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Updating Collections.java copyright > > This looks good to me. Is there any behavior change here that merits a CSR

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v4]

2021-02-22 Thread Claes Redestad
On Mon, 22 Feb 2021 22:08:56 GMT, Ian Graves wrote: >> Modify the `unmodifiable*` methods in `java.util.Collections` to be >> idempotent. That is, when given an immutable collection from >> `java.util.ImmutableCollections` or `java.util.Collections`, these methods >> will return the reference

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v4]

2021-02-22 Thread Ian Graves
> Modify the `unmodifiable*` methods in `java.util.Collections` to be > idempotent. That is, when given an immutable collection from > `java.util.ImmutableCollections` or `java.util.Collections`, these methods > will return the reference instead of creating a new immutable collection that >

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v3]

2021-02-20 Thread liach
On Wed, 17 Feb 2021 19:56:01 GMT, Ian Graves wrote: >> Modify the `unmodifiable*` methods in `java.util.Collections` to be >> idempotent. That is, when given an immutable collection from >> `java.util.ImmutableCollections` or `java.util.Collections`, these methods >> will return the reference

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v3]

2021-02-18 Thread liach
On Thu, 18 Feb 2021 16:18:42 GMT, jmehrens wrote: >> Yes -- I think in response to this it makes more sense to pull the >> `ImmutableCollections` classes out for now and only focus on the wrapping of >> the classes within `Collections` so we aren't blocked by studying and >> rectifying these

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v3]

2021-02-18 Thread jmehrens
On Wed, 17 Feb 2021 19:12:19 GMT, Ian Graves wrote: >> This raises some interesting issues and makes me wonder if we should allow a >> single-wrap of the `ImmutableCollections` classes for now to make this less >> onerous. > > Yes -- I think in response to this it makes more sense to pull the

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v3]

2021-02-17 Thread Ian Graves
On Wed, 17 Feb 2021 14:14:57 GMT, Claes Redestad wrote: >> No? This unmodifiable set here just delegates call to the backing field `c`, >> so all exceptions from `c`'s calls are just delegated, aren't they? The NPE >> will still be thrown; it's just that the stack trace will be different (i.e.

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v3]

2021-02-17 Thread Ian Graves
> Modify the `unmodifiable*` methods in `java.util.Collections` to be > idempotent. That is, when given an immutable collection from > `java.util.ImmutableCollections` or `java.util.Collections`, these methods > will return the reference instead of creating a new immutable collection that >

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v2]

2021-02-17 Thread Ian Graves
On Wed, 17 Feb 2021 18:24:39 GMT, Ian Graves wrote: >> 2 remarks: >> 1. MapN's entry set extends abstract set, whose `contains` is null-friendly >> like >>

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v2]

2021-02-17 Thread Ian Graves
On Wed, 17 Feb 2021 14:37:52 GMT, liach wrote: >> This sounds like an inconsistency between `Map1` and `MapN` that should >> perhaps be considered a bug that needs fixing. /ping @stuart-marks > > 2 remarks: > 1. MapN's entry set extends abstract set, whose `contains` is null-friendly > like

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v2]

2021-02-17 Thread Ian Graves
> Modify the `unmodifiable*` methods in `java.util.Collections` to be > idempotent. That is, when given an immutable collection from > `java.util.ImmutableCollections` or `java.util.Collections`, these methods > will return the reference instead of creating a new immutable collection that >

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized*

2021-02-17 Thread liach
On Wed, 17 Feb 2021 14:21:37 GMT, Claes Redestad wrote: >> src/java.base/share/classes/java/util/Collections.java line 1473: >> >>> 1471: public static Map unmodifiableMap(Map>> extends V> m) { >>> 1472: if(m.getClass() == UnmodifiableMap.class || >>> 1473:m.getClass()

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized*

2021-02-17 Thread Claes Redestad
On Wed, 17 Feb 2021 00:30:09 GMT, Michael Hixson wrote: >> Modify the `unmodifiable*` methods in `java.util.Collections` to be >> idempotent. That is, when given an immutable collection from >> `java.util.ImmutableCollections` or `java.util.Collections`, these methods >> will return the

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized*

2021-02-17 Thread Claes Redestad
On Wed, 17 Feb 2021 02:27:57 GMT, liach wrote: >> src/java.base/share/classes/java/util/Collections.java line 1130: >> >>> 1128: public static Set unmodifiableSet(Set s) { >>> 1129: if(s.getClass() == UnmodifiableSet.class || >>> 1130:s.getClass() ==

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized*

2021-02-16 Thread liach
On Tue, 16 Feb 2021 23:18:55 GMT, Claes Redestad wrote: >> Modify the `unmodifiable*` methods in `java.util.Collections` to be >> idempotent. That is, when given an immutable collection from >> `java.util.ImmutableCollections` or `java.util.Collections`, these methods >> will return the

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized*

2021-02-16 Thread Michael Hixson
On Tue, 16 Feb 2021 21:57:43 GMT, Ian Graves wrote: > Modify the `unmodifiable*` methods in `java.util.Collections` to be > idempotent. That is, when given an immutable collection from > `java.util.ImmutableCollections` or `java.util.Collections`, these methods > will return the reference

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized*

2021-02-16 Thread Claes Redestad
On Tue, 16 Feb 2021 21:57:43 GMT, Ian Graves wrote: > Modify the `unmodifiable*` methods in `java.util.Collections` to be > idempotent. That is, when given an immutable collection from > `java.util.ImmutableCollections` or `java.util.Collections`, these methods > will return the reference

Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized*

2021-02-16 Thread Roger Riggs
On Tue, 16 Feb 2021 21:57:43 GMT, Ian Graves wrote: > Modify the `unmodifiable*` methods in `java.util.Collections` to be > idempotent. That is, when given an immutable collection from > `java.util.ImmutableCollections` or `java.util.Collections`, these methods > will return the reference

RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized*

2021-02-16 Thread Ian Graves
Modify the `unmodifiable*` methods in `java.util.Collections` to be idempotent. That is, when given an immutable collection from `java.util.ImmutableCollections` or `java.util.Collections`, these methods will return the reference instead of creating a new immutable collection that wraps the