Re: RFR: 8291660: Grapheme support in BreakIterator

[Stuart Marks]

On Tue, 23 Aug 2022 22:44:13 GMT, Naoto Sato  wrote:

> This is to enhance the character break analysis in `java.text.BreakIterator` 
> to conform to the extended grapheme cluster boundaries defined in 
> https://www.unicode.org/reports/tr29/#Grapheme_Cluster_Boundaries. A 
> corresponding CSR has also been drafted, as there will be behavioral changes 
> with this modification.

I took a quick look. Overall looks reasonable. How much scrutiny did you need?

Also, nice use of method handles.

Might be helpful to provide an example of the behavior change in the CSR.

This is probably also worth a release note documenting the behavior change and 
showing an example.

-

PR: https://git.openjdk.org/jdk/pull/9991

Re: RFR: 8291660: Grapheme support in BreakIterator

[Stuart Marks]

On Tue, 23 Aug 2022 22:44:13 GMT, Naoto Sato  wrote:

> This is to enhance the character break analysis in `java.text.BreakIterator` 
> to conform to the extended grapheme cluster boundaries defined in 
> https://www.unicode.org/reports/tr29/#Grapheme_Cluster_Boundaries. A 
> corresponding CSR has also been drafted, as there will be behavioral changes 
> with this modification.

src/java.base/share/classes/sun/util/locale/provider/BreakIteratorProviderImpl.java
 line 258:

> 256: .filter(i -> boundaries.get(i) > offset)
> 257: .findFirst()
> 258: .orElse(boundaries.size() - 1);

Is it worth trying to use Collections.binarySearch() here? I think the 
boundaries list is in ascending sorted order, so you might be able to drop in a 
binarySearch() call directly. (Need to be a bit careful with negative return 
values.)

-

PR: https://git.openjdk.org/jdk/pull/9991

Re: RFR: 8291660: Grapheme support in BreakIterator

[Stuart Marks]

On Tue, 23 Aug 2022 22:44:13 GMT, Naoto Sato  wrote:

> This is to enhance the character break analysis in `java.text.BreakIterator` 
> to conform to the extended grapheme cluster boundaries defined in 
> https://www.unicode.org/reports/tr29/#Grapheme_Cluster_Boundaries. A 
> corresponding CSR has also been drafted, as there will be behavioral changes 
> with this modification.

test/jdk/java/util/regex/whitebox/GraphemeTest.java line 30:

> 28:  * @library /lib/testlibrary/java/lang
> 29:  * @compile --add-exports java.base/jdk.internal.util.regex=ALL-UNNAMED 
> GraphemeTest.java
> 30:  * @run testng/othervm --add-exports 
> java.base/jdk.internal.util.regex=ALL-UNNAMED --add-opens 
> java.base/jdk.internal.util.regex=ALL-UNNAMED GraphemeTest

Can you use the `@modules` directive to export+open the internal module to the 
test?

-

PR: https://git.openjdk.org/jdk/pull/9991

Re: RFR: 8289552: Make intrinsic conversions between bit representations of half precision values and floats [v5]

[Sandhya Viswanathan]

On Wed, 24 Aug 2022 23:48:36 GMT, Smita Kamath  wrote:

>> 8289552: Make intrinsic conversions between bit representations of half 
>> precision values and floats
>
> Smita Kamath has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Updated copyright comment

src/hotspot/cpu/x86/x86.ad line 1686:

> 1684: case Op_ConvHF2F:
> 1685:   if (!VM_Version::supports_f16c() && !VM_Version::supports_evex() 
> &&
> 1686:   !VM_Version::supports_avx512vl()) {

The condition should be:
if (!VM_Version::supports_f16c() && 
!(VM_Version::supports_evex() && VM_Version::supports_avx512vl())) {
   return false;
}


For AVX-512 both evex and avx512vl are needed.

src/hotspot/cpu/x86/x86_64.ad line 11320:

> 11318:   ins_pipe( pipe_slow );
> 11319: %}
> 11320: 

For F2HF, good to also add optimized rule with StoreC to benefit from vcvtps2ph 
memory destination form of instruction.
match(Set mem (StoreC mem (ConvF2HF src)));

src/hotspot/cpu/x86/x86_64.ad line 11330:

> 11328:   ins_pipe( pipe_slow );
> 11329: %}
> 11330: 

For HF2F, good to also add optimized rule with LoadS to benefit from vcvtph2ps 
memory src form of instruction.
match(Set dst (ConvHF2F ( LoadS mem)));

-

PR: https://git.openjdk.org/jdk/pull/9781

Re: CopyOnWriteArrayList Collection.shuffle

[Jason Mehrens]

COWAL::replaceAll is the method that allows you to compose read operations 
under the internal lock.  For example, swap can be built with a stateful 
UnaryOperator.

private static void swap(List l, int i, int j) {
l.replaceAll(new UnaryOperator() {
int cursor;
Object tmp;

//This is run while lock is held so size is frozen.
public Object apply(Object t) { 
if (cursor == 0) {
if (i >= j)
throw new IllegalArgumentException();

tmp = l.get(i);
l.get(j); //Bounds check
}

Object r;
if (cursor == i) {
r = l.get(j);
} else if (cursor == j) {
r = tmp;
} else {
r = t;
}

++cursor;
return r;
}
});
}

This eliminates one extra internal array copy versus the Collections::swap for 
COWAL.  The method signature for my example swap doesn't make much sense if 
COWAL is mutating (add/remove) since I'm supplying indexes as arguments.  
Implementing a public swap method on COWAL would have the same problem. 

Jason


From: Zelva Lia 
Sent: Wednesday, August 24, 2022 5:46 PM
To: Jason Mehrens
Cc: core-libs-dev@openjdk.org
Subject: Re: CopyOnWriteArrayList Collection.shuffle

Well, yes, this is a solvable problem, albeit with additional copying, but 
solvable, which cannot be said about other operations (for example, swap), I 
lead to the fact that there are few specialized methods in COW that can work 
inside synchronization and cannot be synchronized with by the COWArrayList 
object itself, because it is synchronized with its private lock

Re: RFR: 8289552: Make intrinsic conversions between bit representations of half precision values and floats [v5]

[Paul Sandoz]

On Wed, 24 Aug 2022 23:48:36 GMT, Smita Kamath  wrote:

>> 8289552: Make intrinsic conversions between bit representations of half 
>> precision values and floats
>
> Smita Kamath has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Updated copyright comment

A general comment: since there is native runtime code to perform conversions 
for constants (IIRC) I think we need to some special tests to verify that works 
correctly (shame there is no way to leverage the Java code in such cases). It 
might require some advice from other HotSpot engineers, but i think we do need 
to follow up with another PR on that aspect.

-

PR: https://git.openjdk.org/jdk/pull/9781

Re: RFR: 8289552: Make intrinsic conversions between bit representations of half precision values and floats [v5]

[Paul Sandoz]

On Wed, 24 Aug 2022 23:48:36 GMT, Smita Kamath  wrote:

>> 8289552: Make intrinsic conversions between bit representations of half 
>> precision values and floats
>
> Smita Kamath has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Updated copyright comment

Java code looks good (HotSpot reviewers also need to approve).

-

Marked as reviewed by psandoz (Reviewer).

PR: https://git.openjdk.org/jdk/pull/9781

Re: RFR: 8289552: Make intrinsic conversions between bit representations of half precision values and floats [v5]

[Smita Kamath]

> 8289552: Make intrinsic conversions between bit representations of half 
> precision values and floats

Smita Kamath has updated the pull request incrementally with one additional 
commit since the last revision:

  Updated copyright comment

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9781/files
  - new: https://git.openjdk.org/jdk/pull/9781/files/7cfed790..1570e244

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=9781=04
 - incr: https://webrevs.openjdk.org/?repo=jdk=9781=03-04

  Stats: 23 lines in 1 file changed: 0 ins; 0 del; 23 mod
  Patch: https://git.openjdk.org/jdk/pull/9781.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9781/head:pull/9781

PR: https://git.openjdk.org/jdk/pull/9781

Re: RFR: 8289552: Make intrinsic conversions between bit representations of half precision values and floats [v4]

[Smita Kamath]

> 8289552: Make intrinsic conversions between bit representations of half 
> precision values and floats

Smita Kamath has updated the pull request incrementally with one additional 
commit since the last revision:

  Updated test cases as per review comments

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9781/files
  - new: https://git.openjdk.org/jdk/pull/9781/files/0f51243a..7cfed790

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=9781=03
 - incr: https://webrevs.openjdk.org/?repo=jdk=9781=02-03

  Stats: 7 lines in 3 files changed: 0 ins; 1 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/9781.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9781/head:pull/9781

PR: https://git.openjdk.org/jdk/pull/9781

Re: RFR: 8289552: Make intrinsic conversions between bit representations of half precision values and floats [v4]

[Paul Sandoz]

On Wed, 24 Aug 2022 23:05:34 GMT, Smita Kamath  wrote:

>> 8289552: Make intrinsic conversions between bit representations of half 
>> precision values and floats
>
> Smita Kamath has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Updated test cases as per review comments

test/micro/org/openjdk/bench/java/math/Fp16ConversionBenchmark.java line 1:

> 1: //

Can you update the copyright comment to be consistent with other source, and 
also correct the year to be only 2022? e.g. copy that from 
`Binary16Conversion.java`.

-

PR: https://git.openjdk.org/jdk/pull/9781

Re: RFR: 8289552: Make intrinsic conversions between bit representations of half precision values and floats [v3]

[Paul Sandoz]

On Wed, 24 Aug 2022 21:03:53 GMT, Smita Kamath  wrote:

>> 8289552: Make intrinsic conversions between bit representations of half 
>> precision values and floats
>
> Smita Kamath has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Addressed review comments, updated test cases and microbenchmark

test/jdk/java/lang/Float/Binary16ConversionNaN.java line 29:

> 27:  * @summary Verify NaN sign and significand bits are preserved across 
> conversions
> 28:  * @run main Binary16ConversionNaN
> 29:  * @run main/othervm/timeout=600 -XX:+UnlockDiagnosticVMOptions

Do we need to specify a timeout? The tests have been running fine without 
intrinsics, no timeouts reports.

test/micro/org/openjdk/bench/java/math/Fp16ConversionBenchmark.java line 73:

> 71:   f16out[i] = Float.floatToFloat16(fin[i]);
> 72:   }
> 73:   bh.consume(f16out);

You can simplify to this:

Suggestion:

  public float floatToFloat16() {
  for (int i = 0; i < fin.length; i++) {
  f16out[i] = Float.floatToFloat16(fin[i]);
  }
  return f16out;

-

PR: https://git.openjdk.org/jdk/pull/9781

Re: RFR: 8289552: Make intrinsic conversions between bit representations of half precision values and floats [v3]

[Smita Kamath]

> 8289552: Make intrinsic conversions between bit representations of half 
> precision values and floats

Smita Kamath has updated the pull request incrementally with one additional 
commit since the last revision:

  Addressed review comments, updated test cases and microbenchmark

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9781/files
  - new: https://git.openjdk.org/jdk/pull/9781/files/bcbc2f56..0f51243a

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=9781=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=9781=01-02

  Stats: 75 lines in 11 files changed: 14 ins; 28 del; 33 mod
  Patch: https://git.openjdk.org/jdk/pull/9781.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9781/head:pull/9781

PR: https://git.openjdk.org/jdk/pull/9781

Re: CopyOnWriteArrayList Collection.shuffle

[Jason Mehrens]

CopyOnWriteArrayList implements an efficient List::replaceAll so you could do 
something like:


public static void main(String[] args) {
List cowl = new CopyOnWriteArrayList<>(new 
String[]{"1","2","3"});
List copy = Arrays.asList(cowl.toArray(new String[0]));
Collections.shuffle(copy);
Iterator it = copy.iterator();  
try {
cowl.replaceAll(e -> it.next());
} catch (NoSuchElementException nsee) {
throw new ConcurrentModificationException();
}

if (it.hasNext()) {
throw new ConcurrentModificationException();
}

System.out.println(cowl);
}


Some of the non-random access branches in Collections.java could be updated to 
use List::replaceAll over ListIterator next/set.  Once Collections.java was 
using List::replaceAll you could then just wrap CopyOnWriteArrayList in a 
non-random access list.

Jason


From: core-libs-dev  on behalf of Zelva Lia 

Sent: Friday, August 19, 2022 5:49 AM
To: core-libs-dev@openjdk.org
Subject: CopyOnWriteArrayList Collection.shuffle

Hello, when shuffling the CopyOnWrite list with the standard 
Collections.shuffle method, performance anomalies occur, due to the fact that 
it calls the set method, which copies the array each time, a possible solution 
(crutch) is a random comparator for sorting, so sorting in COW is redefined to 
its own sub - blocking implementation

Another problem with Collections.shuffle is that it's not exactly thread safe 
because it calls the size() method; and then iterates from it, also COW does 
not support modification inside the iterator (works on snapshots)


COWCollectionsShuffle  0,008 ops/ms
COWListRandomSort 1,089 ops/ms
syncListCollectionsShuffle  0,950 ops/ms

Re: RFR: 8292877: java/util/concurrent/atomic/Serial.java uses {Double,Long}Accumulator incorrectly

[Martin Buchholz]

On Wed, 24 Aug 2022 18:15:17 GMT, Aleksey Shipilev  wrote:

> [JDK-8026344](https://bugs.openjdk.org/browse/JDK-8026344) added tests that 
> subtly contradict the contract for `{Double,Long}Accumulator`-s, which breaks 
> the tests on some platforms even in the single-threaded case.
> 
> They use accumulators with binary plus as update function and using non-zero 
> values as identity, which breaks once accumulators create many cells, reset 
> their values to identity, and then apply the function over them, producing 
> unexpected values.
> 
> See the investigation on RISC-V here:
>   https://mail.openjdk.org/pipermail/riscv-port-dev/2022-August/000594.html
> 
> We can do what `DoubleAccumulator` javadocs do as the sample, namely: "For 
> example, to maintain a running maximum value, you could supply Double::max 
> along with Double.NEGATIVE_INFINITY as the identity."
> 
> Additional testing:
>  - [x] Linux x86_64, `java/util/concurrent` tests

IIRC I worked on 
test/jdk/java/util/concurrent/tck/LongAdderTest.java:testSerialization
test/jdk/java/util/concurrent/tck/DoubleAdderTest.java:testSerialization
I should have added similar tests back then, to 
DoubleAccumulatorTest.java
LongAccumulatorTest.java
and that would allow deletion of Serial.java as a redundant test.
I would have done that now as well, but there's nothing wrong with this fix.

-

Marked as reviewed by martin (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10002

Re: RFR: 8292877: java/util/concurrent/atomic/Serial.java uses {Double,Long}Accumulator incorrectly

[Paul Sandoz]

On Wed, 24 Aug 2022 18:15:17 GMT, Aleksey Shipilev  wrote:

> [JDK-8026344](https://bugs.openjdk.org/browse/JDK-8026344) added tests that 
> subtly contradict the contract for `{Double,Long}Accumulator`-s, which breaks 
> the tests on some platforms even in the single-threaded case.
> 
> They use accumulators with binary plus as update function and using non-zero 
> values as identity, which breaks once accumulators create many cells, reset 
> their values to identity, and then apply the function over them, producing 
> unexpected values.
> 
> See the investigation on RISC-V here:
>   https://mail.openjdk.org/pipermail/riscv-port-dev/2022-August/000594.html
> 
> We can do what `DoubleAccumulator` javadocs do as the sample, namely: "For 
> example, to maintain a running maximum value, you could supply Double::max 
> along with Double.NEGATIVE_INFINITY as the identity."
> 
> Additional testing:
>  - [x] Linux x86_64, `java/util/concurrent` tests

An interesting failure!

-

Marked as reviewed by psandoz (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10002

Re: RFR: 8065554: MatchResult should provide values of named-capturing groups

[Raffaello Giulietti]

On Wed, 24 Aug 2022 17:34:17 GMT, Daniel Fuchs  wrote:

>> Not sure.
>> If the convention is to document every single `RuntimeException` that 
>> methods invoked by this one could throw, then yes.
>> In other words, should `RuntimeExcpetion`s thrown deep in an invocation 
>> stack be documented in every caller method?
>
> In principle, yes. In practice, I see that `namedGroups` doesn't have an 
> `@throws UnsupportedOperationException` but has an `@implSpec` that says that 
> the default implementation throws `UnsupportedOperationException`. This seems 
> strange to me - maybe @stuart-marks or @jddarcy can comment.
> 
> What I was hinting at here however is that we might want to extend the 
> `@implSpec` of the new methods to note that these method will throw 
> `UnsuportedOperationException` if `namedGroups` is not implemented (like the 
> `@implSpec` of `namedGroups` does).

Ah, I see.
So what you mean is not adding another @throws clause but to either improve 
@implNote or, better, to add @implSpec.

-

PR: https://git.openjdk.org/jdk/pull/1

RFR: 8292877: java/util/concurrent/atomic/Serial.java uses {Double,Long}Accumulator incorrectly

[Aleksey Shipilev]

[JDK-8026344](https://bugs.openjdk.org/browse/JDK-8026344) added tests that 
subtly contradict the contract for `{Double,Long}Accumulator`-s, which breaks 
the tests on some platforms even in the single-threaded case.

They use accumulators with binary plus as update function and using non-zero 
values as identity, which breaks once accumulators create many cells, reset 
their values to identity, and then apply the function over them, producing 
unexpected values.

See the investigation on RISC-V here:
  https://mail.openjdk.org/pipermail/riscv-port-dev/2022-August/000594.html

We can do what `DoubleAccumulator` javadocs do as the sample, namely: "For 
example, to maintain a running maximum value, you could supply Double::max 
along with Double.NEGATIVE_INFINITY as the identity."

Additional testing:
 - [x] Linux x86_64, `java/util/concurrent` tests

-

Commit messages:
 - Fix

Changes: https://git.openjdk.org/jdk/pull/10002/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=10002=00
  Issue: https://bugs.openjdk.org/browse/JDK-8292877
  Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/10002.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10002/head:pull/10002

PR: https://git.openjdk.org/jdk/pull/10002

Re: RFR: 8065554: MatchResult should provide values of named-capturing groups

[Daniel Fuchs]

On Wed, 24 Aug 2022 16:12:52 GMT, Raffaello Giulietti  wrote:

>> src/java.base/share/classes/java/util/regex/MatchResult.java line 274:
>> 
>>> 272:  *  might be preferable for other reasons.
>>> 273:  *
>>> 274:  * @since 20
>> 
>> Should the method declare that it throws `UnsupportedOperationsExceptions`?
>> Because that is what will happen if `namedGroups` is not 
>> overridden/implemented.
>> 
>> Same comment for the other new methods.
>
> Not sure.
> If the convention is to document every single `RuntimeException` that methods 
> invoked by this one could throw, then yes.
> In other words, should `RuntimeExcpetion`s thrown deep in an invocation stack 
> be documented in every caller method?

In principle, yes. In practice, I see that `namedGroups` doesn't have an 
`@throws UnsupportedOperationException` but has an `@implSpec` that says that 
the default implementation throws `UnsupportedOperationException`. This seems 
strange to me - maybe @stuart-marks or @jddarcy can comment.

What I was hinting at here however is that we might want to extend the 
`@implSpec` of the new methods to note that these method will throw 
`UnsuportedOperationException` if `namedGroups` is not implemented (like the 
`@implSpec` of `namedGroups` does).

-

PR: https://git.openjdk.org/jdk/pull/1

Re: RFR: 8065554: MatchResult should provide values of named-capturing groups

[Raffaello Giulietti]

On Wed, 24 Aug 2022 16:02:25 GMT, Daniel Fuchs  wrote:

>> Add support for named groups to java.util.regex.MatchResult
>
> src/java.base/share/classes/java/util/regex/MatchResult.java line 274:
> 
>> 272:  *  might be preferable for other reasons.
>> 273:  *
>> 274:  * @since 20
> 
> Should the method declare that it throws `UnsupportedOperationsExceptions`?
> Because that is what will happen if `namedGroups` is not 
> overridden/implemented.
> 
> Same comment for the other new methods.

Not sure.
If the convention is to document every single `RuntimeException` that methods 
invoked by this one could throw, then yes.
In other words, should `RuntimeExcpetion`s thrown deep in an invocation stack 
be documented in every caller method?

-

PR: https://git.openjdk.org/jdk/pull/1

Re: RFR: 8065554: MatchResult should provide values of named-capturing groups

[Daniel Fuchs]

On Wed, 24 Aug 2022 15:48:38 GMT, Raffaello Giulietti  wrote:

> Add support for named groups to java.util.regex.MatchResult

src/java.base/share/classes/java/util/regex/MatchResult.java line 274:

> 272:  *  might be preferable for other reasons.
> 273:  *
> 274:  * @since 20

Should the method declare that it throws `UnsupportedOperationsExceptions`?
Because that is what will happen if `namedGroups` is not overridden/implemented.

Same comment for the other new methods.

-

PR: https://git.openjdk.org/jdk/pull/1

RFR: 8065554: MatchResult should provide values of named-capturing groups

[Raffaello Giulietti]

Add support for named groups to java.util.regex.MatchResult

-

Commit messages:
 - 8065554: MatchResult should provide values of named-capturing groups

Changes: https://git.openjdk.org/jdk/pull/1/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=1=00
  Issue: https://bugs.openjdk.org/browse/JDK-8065554
  Stats: 554 lines in 4 files changed: 502 ins; 15 del; 37 mod
  Patch: https://git.openjdk.org/jdk/pull/1.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/1/head:pull/1

PR: https://git.openjdk.org/jdk/pull/1

Re: Proposal: Collection mutability marker interfaces

[forax]

> From: "Ethan McCue" 
> To: "Remi Forax" 
> Cc: "John Hendrikx" , "core-libs-dev"
> 
> Sent: Wednesday, August 24, 2022 4:27:01 PM
> Subject: Re: Proposal: Collection mutability marker interfaces

>> so it's perhaps better to call add() inside a try/catch on
> > UnsupportedOperationException.

> As much as sin is wrong, sketching out what that might look like... forgive 
> the
> toyness of the example

> VS

> final class Ex {
> private Ex() {}

> /*
> * Adds the odd numbers from 1 to 10 to the List then makes all the odds even.
> *
> * Takes ownership of the list, avoids making copies if it doesn't need to
> */
> static List addOdds(List l) {
> for (int i = 1; i <= 10; i++) {
> try {
> l.add(i);
> } catch (UnsupportedOperationException e) {
> l = new ArrayList<>(l);

i -= 1; // restart with an ArrayList 

> }
> }

> for (int i = 0; i < l.size(); i++) {
> if (l.get(i) % 2 == 1) {
> try {
> l.set(i, l.get(i) + 1);
> } catch (UnsupportedOperationException e) {
> l = new ArrayList<>(l);
> }
> }
> }
> }
> }

as Roger said, there is no way in Java to know if the caller has not kept a 
reference (unlike Rust), 
so having trouble to write this kind of code is more a feature than an issue :) 

This kind of examples scream the Stream API, which has the correct semantics 
IntStream.rangeClosed(1, 10).map(i -> i % 2 == 0? i + 1: i).boxed().toList() 

Rémi 

> On Wed, Aug 24, 2022 at 10:03 AM Remi Forax < [ mailto:fo...@univ-mlv.fr |
> fo...@univ-mlv.fr ] > wrote:

>>> From: "Ethan McCue" < [ mailto:et...@mccue.dev | et...@mccue.dev ] >
>>> To: "John Hendrikx" < [ mailto:john.hendr...@gmail.com | 
>>> john.hendr...@gmail.com
>>> ] >
>>> Cc: "core-libs-dev" < [ mailto:core-libs-dev@openjdk.org |
>>> core-libs-dev@openjdk.org ] >
>>> Sent: Wednesday, August 24, 2022 3:38:26 PM
>>> Subject: Re: Proposal: Collection mutability marker interfaces

>>> A use case that doesn't cover is adding to a collection.

>>> Say as part of a method's contract you state that you take ownership of a 
>>> List.
>>> You aren't going to copy even if the list is mutable.

>>> Later on, you may want to add to the list. Add is supported on ArrayList so 
>>> you
>>> don't need to copy and replace your reference, but you would if the list you
>>> were given was made with List.of or Arrays.asList

>> You can ask if the spliterator considers the collection as immutable or not,
>> list.spliterator().hasCharacteristics(Spliterator.IMMUTABLE)

>> sadly, List.of()/Map.of() does not report the spliterator characteristics
>> correctly (the spliterator implementations are inherited from
>> AbstracList/AbstractMap).

>> so it's perhaps better to call add() inside a try/catch on
>> UnsupportedOperationException.

>> Rémi

>>> On Wed, Aug 24, 2022, 8:13 AM John Hendrikx < [ 
>>> mailto:john.hendr...@gmail.com |
>>> john.hendr...@gmail.com ] > wrote:

 Would it be an option to not make the receiver responsible for the decision
 whether to make a copy or not? Instead put this burden (using default 
 methods)
 on the various collections?

 If List/Set/Map had a method like this:

 List immutableCopy(); // returns a (shallow) immutable copy if list is
 mutable (basically always copies, unless proven otherwise)

 Paired with methods on Collections to prevent collections from being 
 modified:

 Collections.immutableList(List)

 This wrapper is similar to `unmodifiableList` except it implements
 `immutableCopy` as `return this`.

 Then for the various scenario's, where `x` is an untrusted source of List 
 with
 unknown status:

 // Create a defensive copy; result is a private list that cannot be 
 modified:

 List y = x.immutableCopy();

 // Create a defensive copy for sharing, promising it won't ever change:

 List y = Collections.immutableList(x.immutableCopy());

 // Create a defensive copy for mutating:

 List y = new ArrayList<>(x); // same as always

 // Create a mutable copy, modify it, then expose as immutable:

 List y = new ArrayList<>(x); // same as always

 y.add(  );

 List z = Collections.immutableList(y);

 y = null; // we promise `z` won't change again by clearing the only path to
 mutating it!

 The advantage would be that this information isn't part of the type system 
 where
 it can easily get lost. The actual implementation knows best whether a copy
 must be made or not.

 Of course, the immutableList wrapper can be used incorrectly and the 
 promise
 here can be broken by keeping a reference to the original (mutable) list, 
 but I
 think that's an acceptable trade-off.

 --John

 PS. Chosen names are just for illustration; there is some discussion as 
 what
 "unmodifiable" vs "immutable" means in the context of collections that may
 contain elements that are mutable. In this post, immutable refers to 
 shallow
 immutability .
 On 24/08/2022 03:24, Ethan McCue 

Re: Proposal: Collection mutability marker interfaces

[Ethan McCue]

> Safety-wise, taking transferring ownership is fraught

True, or at the very least a true-ism we generally accept.

But that's still a deliberate choice to make that makes a performance
tradeoff. The risk of misuse is proportional always to the exposure of and
audience of the api.

On Wed, Aug 24, 2022 at 10:28 AM Roger Riggs  wrote:

> Hi,
>
> Safety-wise, taking transferring ownership is fraught, the new owner can't
> be certain that the original owner hasn't kept a reference to it or to its
> implementation and might be mucking around with it behind the new owners
> back.
>
> Its cleaner to design the APIs to be defensive, either the API should
> handle the List creation itself (and only expose immutable Lists) or make
> defensive copies before using or saving it.
>
> $0.02, Roger
>
>
> On 8/24/22 9:38 AM, Ethan McCue wrote:
>
> A use case that doesn't cover is adding to a collection.
>
> Say as part of a method's contract you state that you take ownership of a
> List. You aren't going to copy even if the list is mutable.
>
> Later on, you may want to add to the list. Add is supported on ArrayList
> so you don't need to copy and replace your reference, but you would if the
> list you were given was made with List.of or Arrays.asList
>
> On Wed, Aug 24, 2022, 8:13 AM John Hendrikx 
> wrote:
>
>> Would it be an option to not make the receiver responsible for the
>> decision whether to make a copy or not?  Instead put this burden (using
>> default methods) on the various collections?
>>
>> If List/Set/Map had a method like this:
>>
>>  List immutableCopy();  // returns a (shallow) immutable copy if
>> list is mutable (basically always copies, unless proven otherwise)
>>
>> Paired with methods on Collections to prevent collections from being
>> modified:
>>
>>  Collections.immutableList(List)
>>
>> This wrapper is similar to `unmodifiableList` except it implements
>> `immutableCopy` as `return this`.
>>
>> Then for the various scenario's, where `x` is an untrusted source of List
>> with unknown status:
>>
>>  // Create a defensive copy; result is a private list that cannot be
>> modified:
>>
>>  List y = x.immutableCopy();
>>
>>  // Create a defensive copy for sharing, promising it won't ever
>> change:
>>
>>  List y = Collections.immutableList(x.immutableCopy());
>>
>>  // Create a defensive copy for mutating:
>>
>>  List y = new ArrayList<>(x);  // same as always
>>
>>  // Create a mutable copy, modify it, then expose as immutable:
>>
>>  List y = new ArrayList<>(x);  // same as always
>>
>>  y.add(  );
>>
>>  List z = Collections.immutableList(y);
>>
>>  y = null;  // we promise `z` won't change again by clearing the only
>> path to mutating it!
>>
>> The advantage would be that this information isn't part of the type
>> system where it can easily get lost. The actual implementation knows best
>> whether a copy must be made or not.
>>
>> Of course, the immutableList wrapper can be used incorrectly and the
>> promise here can be broken by keeping a reference to the original (mutable)
>> list, but I think that's an acceptable trade-off.
>>
>> --John
>>
>> PS. Chosen names are just for illustration; there is some discussion as
>> what "unmodifiable" vs "immutable" means in the context of collections that
>> may contain elements that are mutable. In this post, immutable refers to
>> shallow immutability .
>> On 24/08/2022 03:24, Ethan McCue wrote:
>>
>> Ah, I'm an idiot.
>>
>> There is still a proposal here somewhere...maybe. right now non jdk lists
>> can't participate in the special casing?
>>
>> On Tue, Aug 23, 2022, 9:00 PM Paul Sandoz  wrote:
>>
>>> List.copyOf already does what you want.
>>>
>>>
>>> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/List.java#L1068
>>>
>>> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/ImmutableCollections.java#L168
>>>
>>> Paul.
>>>
>>> > On Aug 23, 2022, at 4:49 PM, Ethan McCue  wrote:
>>> >
>>> > Hi all,
>>> >
>>> > I am running into an issue with the collections framework where I have
>>> to choose between good semantics for users and performance.
>>> >
>>> > Specifically I am taking a java.util.List from my users and I need to
>>> choose to either
>>> > * Not defensively copy and expose a potential footgun when I pass that
>>> List to another thread
>>> > * Defensively copy and make my users pay an unnecessary runtime cost.
>>> >
>>> > What I would really want, in a nutshell, is for List.copyOf to be a
>>> no-op when used on lists made with List.of().
>>> >
>>> > Below the line is a pitch I wrote up on reddit 7 months ago for a
>>> mechanism I think could accomplish that. My goal is to share the idea a bit
>>> more widely and to this specific audience to get feedback.
>>> >
>>> >
>>> https://www.reddit.com/r/java/comments/sf8qrv/comment/hv8or92/?utm_source=share_medium=web2x=3
>>> >
>>> > Important also for context is Ron Pressler's comment above.

Re: Proposal: Collection mutability marker interfaces

[Ethan McCue]

> so it's perhaps better to call add() inside a try/catch on
UnsupportedOperationException.

As much as sin is wrong, sketching out what that might look like... forgive
the toyness of the example

final class Ex {
private Ex() {}

/*
 * Adds the odd numbers from 1 to 10 to the List then makes all the
odds even.
 *
 * Takes ownership of the list, avoids making copies if it doesn't need
to
 */
static List addOdds(List l) {
for (int i = 1; i <= 10; i++) {
l.add(i);
}

for (int i = 0; i < l.size(); i++) {
if (l.get(i) % 2 == 1) {
l.set(i, l.get(i) + 1);
}
}
}
}


VS



final class Ex {
private Ex() {}

/*
 * Adds the odd numbers from 1 to 10 to the List then makes all the
odds even.
 *
 * Takes ownership of the list, avoids making copies if it doesn't need
to
 */
static List addOdds(List l) {
for (int i = 1; i <= 10; i++) {
try {
l.add(i);
} catch (UnsupportedOperationException e) {
l = new ArrayList<>(l);
}
}

for (int i = 0; i < l.size(); i++) {
if (l.get(i) % 2 == 1) {
try {
l.set(i, l.get(i) + 1);
} catch (UnsupportedOperationException e) {
l = new ArrayList<>(l);
}
}
}
}
}

VS

final class Ex {
private Ex() {}

/*
 * Adds the odd numbers from 1 to 10 to the List then makes all the
odds even.
 *
 * Takes ownership of the list, avoids making copies if it doesn't need
to
 */
static List addOdds(List l) {
if (!(l instanceof Settable && l instanceof Addable)) {
l = new ArrayList<>(l);
}
for (int i = 1; i <= 10; i++) {
l.add(i);
}

for (int i = 0; i < l.size(); i++) {
if (l.get(i) % 2 == 1) {
l.set(i, l.get(i) + 1);
}
}
}
}



On Wed, Aug 24, 2022 at 10:03 AM Remi Forax  wrote:

>
>
> --
>
> *From: *"Ethan McCue" 
> *To: *"John Hendrikx" 
> *Cc: *"core-libs-dev" 
> *Sent: *Wednesday, August 24, 2022 3:38:26 PM
> *Subject: *Re: Proposal: Collection mutability marker interfaces
>
> A use case that doesn't cover is adding to a collection.
>
> Say as part of a method's contract you state that you take ownership of a
> List. You aren't going to copy even if the list is mutable.
>
> Later on, you may want to add to the list. Add is supported on ArrayList
> so you don't need to copy and replace your reference, but you would if the
> list you were given was made with List.of or Arrays.asList
>
>
> You can ask if the spliterator considers the collection as immutable or
> not,
>   list.spliterator().hasCharacteristics(Spliterator.IMMUTABLE)
>
> sadly, List.of()/Map.of() does not report the spliterator characteristics
> correctly (the spliterator implementations are inherited from
> AbstracList/AbstractMap).
>
> so it's perhaps better to call add() inside a try/catch on
> UnsupportedOperationException.
>
> Rémi
>
>
> On Wed, Aug 24, 2022, 8:13 AM John Hendrikx 
> wrote:
>
>> Would it be an option to not make the receiver responsible for the
>> decision whether to make a copy or not?  Instead put this burden (using
>> default methods) on the various collections?
>>
>> If List/Set/Map had a method like this:
>>
>>  List immutableCopy();  // returns a (shallow) immutable copy if
>> list is mutable (basically always copies, unless proven otherwise)
>>
>> Paired with methods on Collections to prevent collections from being
>> modified:
>>
>>  Collections.immutableList(List)
>>
>> This wrapper is similar to `unmodifiableList` except it implements
>> `immutableCopy` as `return this`.
>>
>> Then for the various scenario's, where `x` is an untrusted source of List
>> with unknown status:
>>
>>  // Create a defensive copy; result is a private list that cannot be
>> modified:
>>
>>  List y = x.immutableCopy();
>>
>>  // Create a defensive copy for sharing, promising it won't ever
>> change:
>>
>>  List y = Collections.immutableList(x.immutableCopy());
>>
>>  // Create a defensive copy for mutating:
>>
>>  List y = new ArrayList<>(x);  // same as always
>>
>>  // Create a mutable copy, modify it, then expose as immutable:
>>
>>  List y = new ArrayList<>(x);  // same as always
>>
>>  y.add(  );
>>
>>  List z = Collections.immutableList(y);
>>
>>  y = null;  // we promise `z` won't change again by clearing the only
>> path to mutating it!
>>
>> The advantage would be that this information isn't part of the type
>> system where it can easily get lost. The actual implementation knows best
>> whether a copy must be made or not.
>>
>> Of course, the immutableList wrapper can be used incorrectly and the
>> promise here can be broken by keeping a reference to the 

Re: Proposal: Collection mutability marker interfaces

[Roger Riggs]

Hi,

Safety-wise, taking transferring ownership is fraught, the new owner 
can't be certain that the original owner hasn't kept a reference to it 
or to its implementation and might be mucking around with it behind the 
new owners back.


Its cleaner to design the APIs to be defensive, either the API should 
handle the List creation itself (and only expose immutable Lists) or 
make defensive copies before using or saving it.


$0.02, Roger


On 8/24/22 9:38 AM, Ethan McCue wrote:

A use case that doesn't cover is adding to a collection.

Say as part of a method's contract you state that you take ownership 
of a List. You aren't going to copy even if the list is mutable.


Later on, you may want to add to the list. Add is supported on 
ArrayList so you don't need to copy and replace your reference, but 
you would if the list you were given was made with List.of or 
Arrays.asList


On Wed, Aug 24, 2022, 8:13 AM John Hendrikx  
wrote:


Would it be an option to not make the receiver responsible for the
decision whether to make a copy or not?  Instead put this burden
(using default methods) on the various collections?

If List/Set/Map had a method like this:

 List immutableCopy();  // returns a (shallow) immutable
copy if list is mutable (basically always copies, unless proven
otherwise)

Paired with methods on Collections to prevent collections from
being modified:

 Collections.immutableList(List)

This wrapper is similar to `unmodifiableList` except it implements
`immutableCopy` as `return this`.

Then for the various scenario's, where `x` is an untrusted source
of List with unknown status:

 // Create a defensive copy; result is a private list that
cannot be modified:

 List y = x.immutableCopy();

 // Create a defensive copy for sharing, promising it won't
ever change:

 List y = Collections.immutableList(x.immutableCopy());

 // Create a defensive copy for mutating:

 List y = new ArrayList<>(x);  // same as always

 // Create a mutable copy, modify it, then expose as immutable:

 List y = new ArrayList<>(x);  // same as always

 y.add(  );

 List z = Collections.immutableList(y);

 y = null;  // we promise `z` won't change again by clearing
the only path to mutating it!

The advantage would be that this information isn't part of the
type system where it can easily get lost. The actual
implementation knows best whether a copy must be made or not.

Of course, the immutableList wrapper can be used incorrectly and
the promise here can be broken by keeping a reference to the
original (mutable) list, but I think that's an acceptable trade-off.

--John

PS. Chosen names are just for illustration; there is some
discussion as what "unmodifiable" vs "immutable" means in the
context of collections that may contain elements that are mutable.
In this post, immutable refers to shallow immutability .

On 24/08/2022 03:24, Ethan McCue wrote:

Ah, I'm an idiot.

There is still a proposal here somewhere...maybe. right now non
jdk lists can't participate in the special casing?

On Tue, Aug 23, 2022, 9:00 PM Paul Sandoz
 wrote:

List.copyOf already does what you want.


https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/List.java#L1068

https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/ImmutableCollections.java#L168

Paul.

> On Aug 23, 2022, at 4:49 PM, Ethan McCue 
wrote:
>
> Hi all,
>
> I am running into an issue with the collections framework
where I have to choose between good semantics for users and
performance.
>
> Specifically I am taking a java.util.List from my users and
I need to choose to either
> * Not defensively copy and expose a potential footgun when
I pass that List to another thread
> * Defensively copy and make my users pay an unnecessary
runtime cost.
>
> What I would really want, in a nutshell, is for List.copyOf
to be a no-op when used on lists made with List.of().
>
> Below the line is a pitch I wrote up on reddit 7 months ago
for a mechanism I think could accomplish that. My goal is to
share the idea a bit more widely and to this specific
audience to get feedback.
>
>

https://www.reddit.com/r/java/comments/sf8qrv/comment/hv8or92/?utm_source=share_medium=web2x=3



>
> Important also for context is Ron Pressler's comment above.
> --
>
> What if the collections api added more marker interfaces
like RandomAccess?
 

Re: Proposal: Collection mutability marker interfaces

[Remi Forax]

> From: "Ethan McCue" 
> To: "John Hendrikx" 
> Cc: "core-libs-dev" 
> Sent: Wednesday, August 24, 2022 3:38:26 PM
> Subject: Re: Proposal: Collection mutability marker interfaces

> A use case that doesn't cover is adding to a collection.

> Say as part of a method's contract you state that you take ownership of a 
> List.
> You aren't going to copy even if the list is mutable.

> Later on, you may want to add to the list. Add is supported on ArrayList so 
> you
> don't need to copy and replace your reference, but you would if the list you
> were given was made with List.of or Arrays.asList

You can ask if the spliterator considers the collection as immutable or not, 
list.spliterator().hasCharacteristics(Spliterator.IMMUTABLE) 

sadly, List.of()/Map.of() does not report the spliterator characteristics 
correctly (the spliterator implementations are inherited from 
AbstracList/AbstractMap). 

so it's perhaps better to call add() inside a try/catch on 
UnsupportedOperationException. 

Rémi 

> On Wed, Aug 24, 2022, 8:13 AM John Hendrikx < [ 
> mailto:john.hendr...@gmail.com |
> john.hendr...@gmail.com ] > wrote:

>> Would it be an option to not make the receiver responsible for the decision
>> whether to make a copy or not? Instead put this burden (using default 
>> methods)
>> on the various collections?

>> If List/Set/Map had a method like this:

>> List immutableCopy(); // returns a (shallow) immutable copy if list is
>> mutable (basically always copies, unless proven otherwise)

>> Paired with methods on Collections to prevent collections from being 
>> modified:

>> Collections.immutableList(List)

>> This wrapper is similar to `unmodifiableList` except it implements
>> `immutableCopy` as `return this`.

>> Then for the various scenario's, where `x` is an untrusted source of List 
>> with
>> unknown status:

>> // Create a defensive copy; result is a private list that cannot be modified:

>> List y = x.immutableCopy();

>> // Create a defensive copy for sharing, promising it won't ever change:

>> List y = Collections.immutableList(x.immutableCopy());

>> // Create a defensive copy for mutating:

>> List y = new ArrayList<>(x); // same as always

>> // Create a mutable copy, modify it, then expose as immutable:

>> List y = new ArrayList<>(x); // same as always

>> y.add(  );

>> List z = Collections.immutableList(y);

>> y = null; // we promise `z` won't change again by clearing the only path to
>> mutating it!

>> The advantage would be that this information isn't part of the type system 
>> where
>> it can easily get lost. The actual implementation knows best whether a copy
>> must be made or not.

>> Of course, the immutableList wrapper can be used incorrectly and the promise
>> here can be broken by keeping a reference to the original (mutable) list, 
>> but I
>> think that's an acceptable trade-off.

>> --John

>> PS. Chosen names are just for illustration; there is some discussion as what
>> "unmodifiable" vs "immutable" means in the context of collections that may
>> contain elements that are mutable. In this post, immutable refers to shallow
>> immutability .
>> On 24/08/2022 03:24, Ethan McCue wrote:

>>> Ah, I'm an idiot.

>>> There is still a proposal here somewhere...maybe. right now non jdk lists 
>>> can't
>>> participate in the special casing?

>>> On Tue, Aug 23, 2022, 9:00 PM Paul Sandoz < [ mailto:paul.san...@oracle.com 
>>> |
>>> paul.san...@oracle.com ] > wrote:

 List.copyOf already does what you want.

 [
 https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/List.java#L1068
 |
 https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/List.java#L1068
 ]
 [
 https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/ImmutableCollections.java#L168
 |
 https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/ImmutableCollections.java#L168
 ]

 Paul.

> On Aug 23, 2022, at 4:49 PM, Ethan McCue < [ mailto:et...@mccue.dev |
 > et...@mccue.dev ] > wrote:

 > Hi all,

> I am running into an issue with the collections framework where I have to 
> choose
 > between good semantics for users and performance.

> Specifically I am taking a java.util.List from my users and I need to 
> choose to
 > either
> * Not defensively copy and expose a potential footgun when I pass that 
> List to
 > another thread
 > * Defensively copy and make my users pay an unnecessary runtime cost.

> What I would really want, in a nutshell, is for List.copyOf to be a no-op 
> when
 > used on lists made with List.of().

> Below the line is a pitch I wrote up on reddit 7 months ago for a 
> mechanism I
> think could accomplish that. My goal is to share the idea a bit more 
> widely and
 > to this specific audience to get feedback.

> [
> 

Re: Proposal: Collection mutability marker interfaces

[Ethan McCue]

A use case that doesn't cover is adding to a collection.

Say as part of a method's contract you state that you take ownership of a
List. You aren't going to copy even if the list is mutable.

Later on, you may want to add to the list. Add is supported on ArrayList so
you don't need to copy and replace your reference, but you would if the
list you were given was made with List.of or Arrays.asList

On Wed, Aug 24, 2022, 8:13 AM John Hendrikx  wrote:

> Would it be an option to not make the receiver responsible for the
> decision whether to make a copy or not?  Instead put this burden (using
> default methods) on the various collections?
>
> If List/Set/Map had a method like this:
>
>  List immutableCopy();  // returns a (shallow) immutable copy if
> list is mutable (basically always copies, unless proven otherwise)
>
> Paired with methods on Collections to prevent collections from being
> modified:
>
>  Collections.immutableList(List)
>
> This wrapper is similar to `unmodifiableList` except it implements
> `immutableCopy` as `return this`.
>
> Then for the various scenario's, where `x` is an untrusted source of List
> with unknown status:
>
>  // Create a defensive copy; result is a private list that cannot be
> modified:
>
>  List y = x.immutableCopy();
>
>  // Create a defensive copy for sharing, promising it won't ever
> change:
>
>  List y = Collections.immutableList(x.immutableCopy());
>
>  // Create a defensive copy for mutating:
>
>  List y = new ArrayList<>(x);  // same as always
>
>  // Create a mutable copy, modify it, then expose as immutable:
>
>  List y = new ArrayList<>(x);  // same as always
>
>  y.add(  );
>
>  List z = Collections.immutableList(y);
>
>  y = null;  // we promise `z` won't change again by clearing the only
> path to mutating it!
>
> The advantage would be that this information isn't part of the type system
> where it can easily get lost. The actual implementation knows best whether
> a copy must be made or not.
>
> Of course, the immutableList wrapper can be used incorrectly and the
> promise here can be broken by keeping a reference to the original (mutable)
> list, but I think that's an acceptable trade-off.
>
> --John
>
> PS. Chosen names are just for illustration; there is some discussion as
> what "unmodifiable" vs "immutable" means in the context of collections that
> may contain elements that are mutable. In this post, immutable refers to
> shallow immutability .
> On 24/08/2022 03:24, Ethan McCue wrote:
>
> Ah, I'm an idiot.
>
> There is still a proposal here somewhere...maybe. right now non jdk lists
> can't participate in the special casing?
>
> On Tue, Aug 23, 2022, 9:00 PM Paul Sandoz  wrote:
>
>> List.copyOf already does what you want.
>>
>>
>> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/List.java#L1068
>>
>> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/ImmutableCollections.java#L168
>>
>> Paul.
>>
>> > On Aug 23, 2022, at 4:49 PM, Ethan McCue  wrote:
>> >
>> > Hi all,
>> >
>> > I am running into an issue with the collections framework where I have
>> to choose between good semantics for users and performance.
>> >
>> > Specifically I am taking a java.util.List from my users and I need to
>> choose to either
>> > * Not defensively copy and expose a potential footgun when I pass that
>> List to another thread
>> > * Defensively copy and make my users pay an unnecessary runtime cost.
>> >
>> > What I would really want, in a nutshell, is for List.copyOf to be a
>> no-op when used on lists made with List.of().
>> >
>> > Below the line is a pitch I wrote up on reddit 7 months ago for a
>> mechanism I think could accomplish that. My goal is to share the idea a bit
>> more widely and to this specific audience to get feedback.
>> >
>> >
>> https://www.reddit.com/r/java/comments/sf8qrv/comment/hv8or92/?utm_source=share_medium=web2x=3
>> >
>> > Important also for context is Ron Pressler's comment above.
>> > --
>> >
>> > What if the collections api added more marker interfaces like
>> RandomAccess?
>> >
>> > It's already a common thing for codebases to make explicit null checks
>> at error boundaries because the type system can't encode null |
>> List.
>> >
>> > This feels like a similar problem.
>> > If you have a List in the type system then you don't know for sure
>> you can call any methods on it until you check that its not null. In the
>> same way, there is a set of methods that you don't know at the
>> type/interface level if you are allowed to call.
>> >
>> > If the List is actually a __
>> > Then you can definitely call
>> > And you know other reference holders might call
>> > And you can confirm its this case by
>> > null
>> > no methods
>> > no methods
>> > list == null
>> > List.of(...)
>> > get, size
>> > get, size
>> > ???
>> > Collections.unmodifiableList(...)
>> > get, size
>> > get, size, add, set
>> > ???
>> > 

Re: Proposal: Collection mutability marker interfaces

[John Hendrikx]

Would it be an option to not make the receiver responsible for the 
decision whether to make a copy or not?  Instead put this burden (using 
default methods) on the various collections?


If List/Set/Map had a method like this:

 List immutableCopy();  // returns a (shallow) immutable copy if 
list is mutable (basically always copies, unless proven otherwise)


Paired with methods on Collections to prevent collections from being 
modified:


 Collections.immutableList(List)

This wrapper is similar to `unmodifiableList` except it implements 
`immutableCopy` as `return this`.


Then for the various scenario's, where `x` is an untrusted source of 
List with unknown status:


 // Create a defensive copy; result is a private list that cannot 
be modified:


 List y = x.immutableCopy();

 // Create a defensive copy for sharing, promising it won't ever 
change:


 List y = Collections.immutableList(x.immutableCopy());

 // Create a defensive copy for mutating:

 List y = new ArrayList<>(x);  // same as always

 // Create a mutable copy, modify it, then expose as immutable:

 List y = new ArrayList<>(x);  // same as always

 y.add(  );

 List z = Collections.immutableList(y);

 y = null;  // we promise `z` won't change again by clearing the 
only path to mutating it!


The advantage would be that this information isn't part of the type 
system where it can easily get lost. The actual implementation knows 
best whether a copy must be made or not.


Of course, the immutableList wrapper can be used incorrectly and the 
promise here can be broken by keeping a reference to the original 
(mutable) list, but I think that's an acceptable trade-off.


--John

PS. Chosen names are just for illustration; there is some discussion as 
what "unmodifiable" vs "immutable" means in the context of collections 
that may contain elements that are mutable. In this post, immutable 
refers to shallow immutability .


On 24/08/2022 03:24, Ethan McCue wrote:

Ah, I'm an idiot.

There is still a proposal here somewhere...maybe. right now non jdk 
lists can't participate in the special casing?


On Tue, Aug 23, 2022, 9:00 PM Paul Sandoz  wrote:

List.copyOf already does what you want.


https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/List.java#L1068

https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/ImmutableCollections.java#L168

Paul.

> On Aug 23, 2022, at 4:49 PM, Ethan McCue  wrote:
>
> Hi all,
>
> I am running into an issue with the collections framework where
I have to choose between good semantics for users and performance.
>
> Specifically I am taking a java.util.List from my users and I
need to choose to either
> * Not defensively copy and expose a potential footgun when I
pass that List to another thread
> * Defensively copy and make my users pay an unnecessary runtime
cost.
>
> What I would really want, in a nutshell, is for List.copyOf to
be a no-op when used on lists made with List.of().
>
> Below the line is a pitch I wrote up on reddit 7 months ago for
a mechanism I think could accomplish that. My goal is to share the
idea a bit more widely and to this specific audience to get feedback.
>
>

https://www.reddit.com/r/java/comments/sf8qrv/comment/hv8or92/?utm_source=share_medium=web2x=3



>
> Important also for context is Ron Pressler's comment above.
> --
>
> What if the collections api added more marker interfaces like
RandomAccess?
>
> It's already a common thing for codebases to make explicit null
checks at error boundaries because the type system can't encode
null | List.
>
> This feels like a similar problem.
> If you have a List in the type system then you don't know for
sure you can call any methods on it until you check that its not
null. In the same way, there is a set of methods that you don't
know at the type/interface level if you are allowed to call.
>
> If the List is actually a __
> Then you can definitely call
> And you know other reference holders might call
> And you can confirm its this case by
> null
> no methods
> no methods
> list == null
> List.of(...)
> get, size
> get, size
> ???
> Collections.unmodifiableList(...)
> get, size
> get, size, add, set
> ???
> Arrays.asList(...)
> get, size, set
> get, size, set
> ???
> new ArrayList<>()
> get, size, add, set
> get, size, add, set
> ???
> While yes, there is no feasible way to encode these things in
the type system. Its not impossible to encode it at runtime though.
> interface FullyImmutable {
> // So you know the existence of this implies the 

Re: RFR: 8292698: Improve performance of DataInputStream

[Daniel Fuchs]

On Wed, 24 Aug 2022 06:17:19 GMT, Alan Bateman  wrote:

> I can't imagine a subclass of DataInputStream setting 'in' to null. If it 
> handles async close then it's possible that it replaces it with an input 
> stream where read throws IOException unconditionally. This is why the 
> original patch was problematic for the methods that call in.read in a loop. 
> The updated patch is probably okay, I'm just pointing out that we are forced 
> to think of subclasses when touching this code.

Thanks Alan - that makes sense to me.

-

PR: https://git.openjdk.org/jdk/pull/9956

Re: RFR: 8292698: Improve performance of DataInputStream

[Alan Bateman]

On Tue, 23 Aug 2022 18:57:07 GMT, Сергей Цыпанов  wrote:

> Suppose we have a scenario where `in` is replaced asynchronously in one of 
> implementations and the implementation is passed into constructor of 
> `DataInputStream`. In this case the patched code is less NPE-vulnerable, 
> isn't it?

I can't imagine a subclass of DataInputStream setting 'in' to null. If it 
handles async close then it's possible that it replaces it with an input stream 
where read throws IOException unconditionally. This is why the original patch 
was problematic for the methods that call in.read in a loop. The updated patch 
is probably okay, I'm just pointing out that we are forced to think of 
subclasses when touching this code.

-

PR: https://git.openjdk.org/jdk/pull/9956