Re: RFR: 8287522: StringConcatFactory: Add in prependers and mixers in batches [v7]

[Remi Forax]

- Original Message -
> From: "Claes Redestad" 
> To: "core-libs-dev" 
> Sent: Friday, June 3, 2022 2:19:34 PM
> Subject: Re: RFR: 8287522: StringConcatFactory: Add in prependers and mixers 
> in batches [v7]

> On Fri, 3 Jun 2022 11:19:04 GMT, ExE Boss  wrote:
> 
>>> You would think that, but javac doesn't do anything fancy once you store to 
>>> a
>>> local
>>> 
>>> javap output for lines 108 through 111:
>>> 
>>>449: ldc   #148// float 0.1f
>>>451: fstore31
>>>453: fload 31
>>>455: invokedynamic #149,  0// InvokeDynamic
>>>#4:makeConcatWithConstants:(F)Ljava/lang/String;
>>>460: astore32
>>>462: fload 31
>>>464: aload 4
>>>466: invokedynamic #152,  0// InvokeDynamic
>>>#7:makeConcatWithConstants:(FLjava/lang/String;)Ljava/lang/String;
>>>471: astore33
>>>473: aload 4
>>>475: fload 31
>>>477: invokedynamic #155,  0// InvokeDynamic
>>>#10:makeConcatWithConstants:(Ljava/lang/String;F)Ljava/lang/String;
>>
>> I guess it only happens for `final` locals.
> 
> Yes, looks like javac does constant fold when the local is explicitly final:
> 
>   final float f = 0.1f;
>   System.out.println("const folding? " + f);
> 
> javap:
> 
> 0: getstatic #2  // Field
> java/lang/System.out:Ljava/io/PrintStream;
> 3: ldc   #3  // String const folding? 0.1
> 5: invokevirtual #4  // Method
> java/io/PrintStream.println:(Ljava/lang/String;)V
> 
> 
> Maybe this constant folding is something javac could be enhanced to do on
> effectively final locals, though I think we can defer adjusting
> `HelloClasslist` to break that optimization when that time comes.

The JLS defines final variables (and effectively final) here 
https://docs.oracle.com/javase/specs/jls/se18/html/jls-4.html#jls-4.12.4

Constant expressions does not list effectively final variables as constant, 
only final variables
https://docs.oracle.com/javase/specs/jls/se18/html/jls-15.html#jls-15.29

> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/8855

regards,
Rémi

Stream.fromForEach(Consumer>)

[Remi Forax]

Hi all,
a stream is kind of push iterator so it can be created from any object that has 
a method like forEach(Consumer),
but sadly there is no static method to create a Stream from a Consumer of 
Consumer so people usually miss that creating a Stream from events pushed to a 
consumer is easy.

By example, let say i've a very simple parser like this, it calls the 
listener/callback for each tokens 

  record Parser(String text) {
void parse(Consumer listener) {
  for(var token: text.split(" ")) {
 listener.accept(token);
  }
}
  } 

Using the method Stream.fromForEach, we can create a Stream from the sequence 
of tokens that are pushed through the listener
  var parser = new Parser("1 2");
  var stream = Stream.fromForEach(parser::parse);

It can also works with an iterable, an optional or even a collection
  Iterable iterable = ...
  var stream = Stream.fromForEach(iterable::forEachRemaning);

  Optional optional = ...
  var stream = Stream.fromForEach(optional::ifPresent);

  List list = ...
  var stream = Stream.fromForEach(list::forEach);

I known the last two examples already have their own method stream(), it's just 
to explain how Stream.fromForEach is supposed to work.

In term of implementation, Stream.fromForEach() is equivalent to creating a 
stream using a mapMulti(), so it can be implemented like this

  static  Stream fromForEach(Consumer> forEach) {
return Stream.of((T) null).mapMulti((__, consumer) -> 
forEach.accept(consumer));
  }

but i think that Stream.fromForEach(iterable::forEachRemaning) is more readable 
than Stream.of(iterable).mapMult(Iterable::forEachRemaining).

The name fromForEach is not great and i'm sure someone will come with a better 
one.

regards,
Rémi

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline

[Remi Forax]

Ok,
let us know when it's out of draft,
i've several minor modifications about the code.

regards,
Rémi

- Original Message -
> From: "Jorn Vernee" 
> To: "core-libs-dev" , "hotspot-dev" 
> 
> Sent: Thursday, May 12, 2022 11:25:52 PM
> Subject: Re: RFR: 8286669: Replace MethodHandle specialization with ASM in 
> mainline

> On Thu, 12 May 2022 17:17:37 GMT, Jorn Vernee  wrote:
> 
>> Hi,
>> 
>> This PR brings over commits from the panama-foreign repo. These commits 
>> mostly
>> pertain to the switch to ASM and away from MethodHandle combinators for the
>> binding recipe specialization. But, there is one more commit which adds 
>> freeing
>> of downcall stubs, since those changes were mostly Java as well.
>> 
>> Thanks,
>> Jorn
> 
> Hmm, looks like there's an unexpected a tier1 failure on SysV in the GHA (I 
> only
> tested Windows locally, since these changes have been live in the panama repo
> for a while now...). Will convert back to draft and see what's up.
> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/8685

Re: HttpClient has no explicit way of releasing threads

[Remi Forax]

- Original Message -
> From: "Rafael Winterhalter" 
> To: "core-libs-dev" 
> Sent: Monday, May 9, 2022 11:43:49 PM
> Subject: HttpClient has no explicit way of releasing threads

> Hello,

Hello,

> 
> looking at thread dumps, I realized that the HttpClient implementation does
> not offer an explicit way to release its threads. Currently, the client:
> 
> 1. maintains a cached thread pool with a retention size of 60 seconds. If
> many such clients are created for short lived application, these threads
> pile up.
> 2. has a selector thread that only shuts down if the outer "facade"
> reference is collected via a weak reference. If an application is not
> running GC, this can take a while.
> 
> This is not a big problem but I have seen peaks with suddenly many, many
> threads (in test code) where many HttpClients were created for single use
> and I was wondering if it was ever considered to add a method for disposing
> the threads explicitly?

You can change the Executor (it's one parameter of the builder) to use whatever 
executors you want so you can shutdown that executor as you want.
This should fixed (1).

Also once you update to Java 19/21, it seems a good scenario to test the 
executor that spawn virtual threads instead of platform threads.

> 
> Alternatively, it might be an option to add a method like
> HttpClient.shared() which would return a singleton HttpClient (created on
> the first call, collected if no reference is kept anymore but reused in the
> meantime) to address such scenarios. I can of course add a singleton in my
> test project but I find this a bit awkward as it is something to remember
> and to repeat in all applications we maintain. Therefore, it might be
> convenient to add such methods for tests that usually aim to be decoupled.
> 
> Thanks for your consideration,
> best regards, Rafael

regards,
Rémi

Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

[Remi Forax]

- Original Message -
> From: "liach" 
> To: "core-libs-dev" , "security-dev" 
> 
> Sent: Tuesday, April 19, 2022 3:31:24 AM
> Subject: Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

> On Mon, 18 Apr 2022 20:42:48 GMT, Remi Forax  wrote:
> 
>> The third parameter of defineProxy() is a lambda which is called for each 
>> method
>> that can be overriden, either toString/equals/hashCode but also any default
>> methods,
> if the lambda return true, the method is overriden, otherwise the default
> implementation is used.
> 
> Not quite, I mean calling default implementations in the super interface,
> consider:
> 
> interface Root { void cleanUp(); }
> interface FeatureOne extends Root { default void cleanUp() { /* do something 
> */
> } }
> interface FeatureTwo extends Root { default void cleanUp() { /* do something 
> */
> } }
> 
> My proxy implements both feature one and two, but in your API, there is no way
> for my `cleanUp` to call both `FeatureOne.super.cleanUp();` and
> `FeatureTwo.super.cleanUp();`. You should probably expose the lookup in your
> linker too, especially that you enabled nest access and you can just use that
> lookup to resolve, say, an implementation static method in the proxy user
> class.

yes, you are right, i should send the Lookup too.

> 
> Also the "delegate" in your API would significantly benefit from
> https://bugs.openjdk.java.net/browse/JDK-8282798 (#7744), too.

I don't think i need the carrier API, the idea is to have only one field in the 
proxy, this field can be a value type (exactly a primitive class) in the 
future, so being expanded into multiple fields by the VM at runtime.

> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/8278

Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

[Remi Forax]

- Original Message -
> From: "liach" 
> To: "core-libs-dev" , "security-dev" 
> 
> Sent: Monday, April 18, 2022 10:01:39 PM
> Subject: Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

> On Sun, 17 Apr 2022 16:17:30 GMT, liach  wrote:
> 
>> Convert dynamic proxies to hidden classes. Modifies the serialization of 
>> proxies
>> (requires change in "Java Object Serialization Specification"). Makes the
>> proxies hidden in stack traces. Removes duplicate logic in proxy building.
>> 
>> The main compatibility changes and their rationales are:
>> 1. Modification to the serialization specification: In the "An instance of 
>> the
>> class is allocated... The contents restored appropriately" section, I propose
>> explicitly state that handling of proxies are unspecified as to allow
>> implementation freedom, though I've seen deliberate attempts for proxies to
>> implement interfaces with `readResolve` in order to control their 
>> serialization
>> behavior.
>>- This is for the existing generated constructor accessor is 
>> bytecode-based,
>>which cannot refer to hidden classes.
>>- An alternative is to preserve the behavior, where the serialization
>>constructor calls `invokespecial` on the closest serializable superclass'
>>no-arg constructor, like in #1830 by @DasBrain.
>>- My rationale against preservation is such super calls are unsafe and 
>> should be
>>discouraged in the long term. Calling the existing constructor with a 
>> dummy
>>argument, as in my implementation, would be more safe.
>> 2. The disappearance of proxies in stack traces.
>>- Same behavior exists in lambda expressions: For instance, in 
>> `((Runnable) ()
>>-> { throw new Error(); }).run();`, the `run` method, implemented by the
>>lambda, will not appear in the stack trace, and isn't too problematic.
>> 
>> A summary of the rest of the changes:
>> 1. Merged the two passes of determining module and package of the proxy into
>> one. This reduced a lot of code and allowed anchor class (for hidden class
>> creation) selection be done together as well.
>> 2. Exposed internal API for obtaining a full-privileged lookup to the rest of
>> `java.base`. This API is intended for implementation of legacy (pre
>> `MethodHandles.Lookup`) caller sensitive public APIs so they don't need more
>> complex tricks to obtain proper permissions as lookups.
>> 3. Implements [8229959](https://bugs.openjdk.java.net/browse/JDK-8229959):
>> passes methods computed by proxy generator as class data to the hidden proxy
>> class to reduce generated proxy class size and improve performance.
>> 
>> In addition, since this change is somewhat large, should we keep the old 
>> proxy
>> generator as well and have it toggled through a command-line flag (like the 
>> old
>> v49 proxy generator or the old reflection implementation)?
>> 
>> Please feel free to comment or review. This change definitely requires a CSR,
>> but I have yet to determine what specifications should be changed.
> 
> I strongly recommend a new API over revamping `Proxy` itself.
> https://github.com/forax/hidden_proxy would be a good prototype that serves
> most needs of the current Proxy API (except a few, like invoking default
> superinterface method). 

The third parameter of defineProxy() is a lambda which is called for each 
method that can be overriden, either toString/equals/hashCode but also any 
default methods,
if the lambda return true, the method is overriden, otherwise the default 
implementation is used.

> With hidden classes, I don't see much value in leaving
> the new API's produced instance in separate modules; what we have for hidden
> classes should be fine for the most part.
> 
> Imo the main obstacle is still the handling of serialization. The annotations
> are serializable, but currently hidden classes do not work with serialization
> at all and must use `writeReplace` and `readResolve`. And how to migrate
> annotations off proxies without breaking serialization is still a question as
> well. Maybe we can upgrade invocation handlers to allow them to declare custom
> `readResolve` logic for the proxy to facilitate migration away. How the new 
> API
> will treat serialization is still undetermined.

yes, i suppose that like with lambdas, we need a special overload of 
defineProxy that automatically implements writeReplace() and use a specific 
class SerializableProxy (mirroring how SerializableLambda works).

> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/8278

Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

[Remi Forax]

- Original Message -
> From: "Johannes Kuhn" 
> To: "Alan Bateman" , "core-libs-dev" 
> 
> Sent: Monday, April 18, 2022 12:53:45 PM
> Subject: Re: RFR: JDK-8242888: Convert dynamic proxy to hidden classes

> On 18-Apr-22 9:36, Alan Bateman wrote:
>> On 17/04/2022 17:24, liach wrote:
>>> Convert dynamic proxies to hidden classes. Modifies the serialization
>>> of proxies (requires change in "Java Object Serialization
>>> Specification"). Makes the proxies hidden in stack traces. Removes
>>> duplicate logic in proxy building.
>>>
>>> The main compatibility changes and their rationales are:
>>> 1. Modification to the serialization specification: In the "An
>>> instance of the class is allocated... The contents restored
>>> appropriately" section, I propose explicitly state that handling of
>>> proxies are unspecified as to allow implementation freedom, though
>>> I've seen deliberate attempts for proxies to implement interfaces with
>>> `readResolve` in order to control their serialization behavior.
>>>     - This is for the existing generated constructor accessor is
>>> bytecode-based, which cannot refer to hidden classes.
>>>     - An alternative is to preserve the behavior, where the
>>> serialization constructor calls `invokespecial` on the closest
>>> serializable superclass' no-arg constructor, like in #1830 by @DasBrain.
>>>     - My rationale against preservation is such super calls are unsafe
>>> and should be discouraged in the long term. Calling the existing
>>> constructor with a dummy argument, as in my implementation, would be
>>> more safe.
>>> 2. The disappearance of proxies in stack traces.
>>>     - Same behavior exists in lambda expressions: For instance, in
>>> `((Runnable) () -> { throw new Error(); }).run();`, the `run` method,
>>> implemented by the lambda, will not appear in the stack trace, and
>>> isn't too problematic.
>> 
>> It's great that you have time to experiment in this area but just to set
>> expectations that it will likely require significant discussion and
>> maybe prototyping of alternatives. It suspect the Reviewer effort will
>> end up being many times the effort required to do the actual work
>> because of the compatibility and security issues that will need to be
>> worked through.
>> 
>> I think you need to add non-discoverability to the list of compatibility
>> issues. Do we know for sure that there aren't frameworks and libraries
>> that use Class.forName on proxy classes? We've had issues in the past
>> with changes in this area.
>> 
>> It's too early to say but it might be that the compatibility risks may
>> nudge this one into creating a new API.
>> 
>> -Alan.
>> 
>> 
>> 
> 
> Proxies will have to be rethought at some future point - wrt Valhalla.
> 
> The current specification says:
> 
> > A proxy class implements exactly the interfaces specified at its
> creation, in the same order. Invoking getInterfaces on its Class object
> will return an array containing the same list of interfaces (in the
> order specified at its creation), [...]
> 
> In the current design Valhalla will add two interfaces - IdentityObject
> and ValueObject (afaik). One of them have to be implemented as well.
> Also, because the superclass is java.lang.reflect.Proxy, and that class
> has a protected final field, it can currently not implement ValueObject.

Recently, we (the Valhalla EG) have decided that IdentityObject/ValueObject 
were not the right way to represent identity and value class.
So no problem anymore on that side.

> 
> An other thing are annotations - currently they are implemented using
> Proxies. This implementation detail surfaces when serializing an
> annotation. Other implementation strategies are possible - for example
> spinning a record at runtime.
> But this leads to the question - how can one migrate away from
> serialized proxies in a compatible way?
> 
> In the end - a lot of stuff in the JDK depends on Proxies, and their
> limitations now begins to surface.
> 
> A new API may not be a bad idea. :)

Yes !
And we can leverage invokedynamic/method handles to avoid boxing/ bad perf.
The idea is that first time an implementation of an abstract method is 
required, an object (implementing an interface similar to InvocationHandler) 
acting as a bootstrap method is called to provide a method handle that will be 
used as implementation.

see https://github.com/forax/hidden_proxy for a prototype of that idea.

> 
> - Johannes

Rémi

Re: fast way to infer caller

[Remi Forax]

- Original Message -
> From: "Kasper Nielsen" 
> To: "Ceki Gülcü" 
> Cc: "core-libs-dev" 
> Sent: Thursday, April 7, 2022 1:53:33 PM
> Subject: Re: fast way to infer caller

>>
>> MethodHandles.lookup().lookupClass() looks very promising except that
>> there is no way to specify the depth.
>>
>> I am looking for a method to obtain the Nth caller at a cost of around
>> 100 to 200 nanoseconds of CPU time.  Do you think the JDK could cater
>> for this use case?
>>
> 
> Hi Ceki,
> 
> I don't think you will find the numbers you are looking for with
> StackWalker.
> Compared to something like Reflection.getCallerClass() which
> MethodHandles.lookup() uses.
> There is still a very large (>100x) performance gap between the two.
> 
> public class StackWalkerPerf {
> 
>static final StackWalker sw =
> StackWalker.getInstance(Option.RETAIN_CLASS_REFERENCE);
> 
>@Benchmark
>public Class stackWalkerCallerClass() {
>return sw.getCallerClass();
>}
> 
>@Benchmark
>public Class reflectionCallerClass() {
>return MethodHandles.lookup().lookupClass();
>}
> }
> 
> Benchmark   Mode  Cnt ScoreError  Units
> StackWalkerPerf.reflectionCallerClass   avgt   102,927 ± 0,012  ns/op
> StackWalkerPerf.stackWalkerCallerClass  avgt   10  915,287 ± 9,565  ns/op

The resulting class is not the same, so comparing the performance of two codes 
that does not do the same thing is dubious, no ?

> 
> /Kasper

Rémi

Re: fast way to infer caller

[Remi Forax]

- Original Message -
> From: "Ceki Gülcü" 
> To: "core-libs-dev" 
> Sent: Thursday, April 7, 2022 1:04:11 AM
> Subject: Re: fast way to infer caller

> Hi Jason,
> 
> Yes, the code was mentioned in SLF4J PR 271.
> 
> The benchmark can be found at
> 
> https://github.com/ceki/logback-perf/tree/master/src/main/java/ch/qos/logback/perf/caller
> 
> Here is the gist of the StackWalker variant:
> 
> public class CallerCompute {
> 
>static private StackWalker WALKER = StackWalker.getInstance();
> 
>static public String getCallerClass(int depth) {
> 
>List frames = WALKER.walk(s -> s.limit(4).toList());
> process the returned frames
>}
> }
> 
> The above executes at around 1.8 microseconds per call whereas the code
> in LogRecord.getSourceClassName() executes at 5 microseconds per call.
> The cost of LogRecord constructor is negligible at around 25 nanoseconds.
> 
> Anyway, thank you for the suggestion.

You can declare WALKER "final", it should help. This is also better to call 
getCallerClass ASAP and then pass the class to the other stack frames instead 
of having to skip those stack frames when calling getCallerClass. You may also 
try able to improve the speed by doing a == before calling equals(String) if 
you call String.intern() on THIS_CLASS_NAME.

> 
> --
> Ceki

Rémi

> 
> 
> On 4/7/2022 12:27 AM, Jason Mehrens wrote:
>> Ceki,
>> 
>> Looks like the benchmark code is from 
>> https://github.com/qos-ch/slf4j/pull/271
>> 
>> Looks like the benchmark code is doing a collection so perhaps that is some 
>> of
>> the performance hit?
>> Have you benchmarked java.util.LogRecord.getSourceClassName() to compare with
>> your StackWalker benchmark?
>> 
>> https://github.com/openjdk/jdk/blob/master/src/java.logging/share/classes/java/util/logging/LogRecord.java#L754
>> 
>> Jason
>> 
>> 
>> From: core-libs-dev  on behalf of Ceki
>> Gülcü 
>> Sent: Wednesday, April 6, 2022 4:26 PM
>> To: core-libs-dev
>> Subject: Re: fast way to infer caller
>> 
>> 
>> Hi Rémi,
>> 
>> Thank you for your answer.
>> 
>> According to some benchmarks on a i7-8565U Intel CPU (quite average
>> CPU), here are the costs of computing the caller class via different
>> methods:
>> 
>> using new Throwable().getStackTrace: 11 microseconds per call
>> using StackWalker API: 1.8 microseconds per call
>> using SecurityManager: 0.9 microseconds per call
>> 
>> While a six fold improvement (StackWalker compared to new Thorowable) is
>> nothing to sneeze at, the performance of StackWalker is not as good as
>> with a custom SecurityManager.
>> 
>> I have not said so explicitly but the aim here is to allow the user to
>> obtain a new logger by calling LoggerFactory.getLogger() with no
>> arguments with the returned logger named after the caller class.
>> 
>> Spending 1 or 2 microseconds for this call is OK if the logger is a
>> static field. However, if the logger is an instance field, then spending
>> 2 microseconds per host object instance just to obtain a logger might
>> have a noticeable impact on performance. It follows that the performance
>> of LoggerFactory.getLogger() must be exceptionally good, assuming we
>> wish to avoid having the user accidentally shooting herself on the foot,
>> ergo the 100 nanosecond performance per call requirement.
>> 
>> Noting that invoking MethodHandles.lookup().lookupClass() seems very
>> fast (about 2 nanoseconds), I would be very interested if  new
>> lookup(int index) method were added to java.lang.invoke.MethodHandles
>> class, with index designating the desired index on the call stack.
>> 
>> Does the above make sense? How difficult would it be to add such a method?
>> 
>> --
>> Ceki Gülcü
>> 
>> 
>> On 4/6/2022 5:52 PM, Remi Forax wrote:
>>> - Original Message -
>>>> From: "Ceki Gülcü" 
>>>> To: "core-libs-dev" 
>>>> Sent: Wednesday, April 6, 2022 5:30:51 PM
>>>> Subject: fast way to infer caller
>>>
>>>> Hello,
>>>
>>> Hello,
>>>
>>>>
>>>> As you are probably aware, one of the important primitives used in
>>>> logging libraries is inferring the caller of a given logging statement.
>>>> The current common practice is to create a throwable and process its
>>>> stack trace. This is rather wasteful and rather slow. As an alterna

Re: fast way to infer caller

[Remi Forax]

- Original Message -
> From: "Ceki Gülcü" 
> To: "core-libs-dev" 
> Sent: Wednesday, April 6, 2022 11:26:39 PM
> Subject: Re: fast way to infer caller

> Hi Rémi,
> 
> Thank you for your answer.
> 
> According to some benchmarks on a i7-8565U Intel CPU (quite average
> CPU), here are the costs of computing the caller class via different
> methods:
> 
> using new Throwable().getStackTrace: 11 microseconds per call
> using StackWalker API: 1.8 microseconds per call
> using SecurityManager: 0.9 microseconds per call
> 
> While a six fold improvement (StackWalker compared to new Throwable) is
> nothing to sneeze at, the performance of StackWalker is not as good as
> with a custom SecurityManager.
> 
> I have not said so explicitly but the aim here is to allow the user to
> obtain a new logger by calling LoggerFactory.getLogger() with no
> arguments with the returned logger named after the caller class.
> 
> Spending 1 or 2 microseconds for this call is OK if the logger is a
> static field. However, if the logger is an instance field, then spending
> 2 microseconds per host object instance just to obtain a logger might
> have a noticeable impact on performance. It follows that the performance
> of LoggerFactory.getLogger() must be exceptionally good, assuming we
> wish to avoid having the user accidentally shooting herself on the foot,
> ergo the 100 nanosecond performance per call requirement.
> 
> Noting that invoking MethodHandles.lookup().lookupClass() seems very
> fast (about 2 nanoseconds), I would be very interested if  new
> lookup(int index) method were added to java.lang.invoke.MethodHandles
> class, with index designating the desired index on the call stack.
> 
> Does the above make sense? How difficult would it be to add such a method ?

I think that for MethodHandles.lookup() and all other caller sensitive methods, 
the VM uses a trick, it inlines the call into the caller method, once this is 
done, the caller class is know statically and can be replaced by a constant. In 
order to use the same trick, you need a way to force the inlining through 
getLogger(), the is something the JDK can do but that is not available to user 
code.

And you do not want lookup(-1) because getLogger() can be called by reflection, 
java.lang.invoke or a lambda, in all these cases you have "hidden" stack frames 
in between the user code code and LoggerFactory.getLogger(), you need to skip 
those stack frames, this is what the StackWalker does.

Now, i don't think there is a real solution to you issue, worst i will try to 
convince you that this is not a real problem :)
You are offering convenience using magic to your user, this has a cost.

For me this is very similar to the trade off you have by offering to change the 
logger configuration at runtime.
This is convenient but it requires a volatile read (even when the logger is 
de-activated) which destroy performance in tight loop (you loose hoisting).
I believe that if your users are fine with that, they are also fine with a call 
to LoggerFactory.getLogger() being a little slow.

> 
> --
> Ceki Gülcü

Rémi

> 
> 
> On 4/6/2022 5:52 PM, Remi Forax wrote:
>> - Original Message -
>>> From: "Ceki Gülcü" 
>>> To: "core-libs-dev" 
>>> Sent: Wednesday, April 6, 2022 5:30:51 PM
>>> Subject: fast way to infer caller
>> 
>>> Hello,
>> 
>> Hello,
>> 
>>>
>>> As you are probably aware, one of the important primitives used in
>>> logging libraries is inferring the caller of a given logging statement.
>>> The current common practice is to create a throwable and process its
>>> stack trace. This is rather wasteful and rather slow. As an alternative,
>>> I have tried using the StackWalker API to infer the caller but was
>>> unsatisfied with the performance.
>>>
>>> MethodHandles.lookup().lookupClass() looks very promising except that
>>> there is no way to specify the depth.
>>>
>>> I am looking for a method to obtain the Nth caller at a cost of around
>>> 100 to 200 nanoseconds of CPU time.  Do you think the JDK could cater
>>> for this use case?
>> 
>> We have designed the StackWalker with that in mind
>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html
>> 
>> see the discussion on StackWalker.getCallerClass()
>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html#getCallerClass()

Re: fast way to infer caller

[Remi Forax]

- Original Message -
> From: "Ceki Gülcü" 
> To: "core-libs-dev" 
> Sent: Wednesday, April 6, 2022 5:30:51 PM
> Subject: fast way to infer caller

> Hello,

Hello,

> 
> As you are probably aware, one of the important primitives used in
> logging libraries is inferring the caller of a given logging statement.
> The current common practice is to create a throwable and process its
> stack trace. This is rather wasteful and rather slow. As an alternative,
> I have tried using the StackWalker API to infer the caller but was
> unsatisfied with the performance.
> 
> MethodHandles.lookup().lookupClass() looks very promising except that
> there is no way to specify the depth.
> 
> I am looking for a method to obtain the Nth caller at a cost of around
> 100 to 200 nanoseconds of CPU time.  Do you think the JDK could cater
> for this use case?

We have designed the StackWalker with that in mind
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html

see the discussion on StackWalker.getCallerClass()
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html#getCallerClass()

> 
> --
> Ceki Gülcü
> 
> Sponsoring SLF4J/logback/reload4j at https://github.com/sponsors/qos-ch

regards,
Rémi

Re: When to initialize the method's class for MethodHandles.Lookup.findStatic()?

[Remi Forax]

- Original Message -
> From: "Cheng Jin" 
> To: "core-libs-dev" 
> Sent: Thursday, March 17, 2022 5:42:57 PM
> Subject: When to initialize the method's class for 
> MethodHandles.Lookup.findStatic()?

> Hi there,
> 
> The document of
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#findStatic(java.lang.Class,java.lang.String,java.lang.invoke.MethodType)
> in the Java API is ambiguous in terms of when to initialize the method's class
> as follows (the same description as in other OpenJDK versions)
> 
> If the returned method handle is invoked, the method's class will be
> initialized, if it has not already been initialized.
> 
> 
> It occurs to me that the method's class should be initialized when invoking 
> the
> method handle but OpenJDK actually chooses to do the initialization in
> lookup.findStatic() rather than in mh.invoke()
> e.g.
> import java.lang.invoke.*;
> 
> public class Test_1 {
>static MethodHandle mh = MethodHandles.lookup().findStatic(Test_2.class,
>"testMethod", MethodType.methodType(int.class, int.class)); <---
>Test_2.class gets initialized and verified.
> 
>public static void main(String[] args) throws Throwable {
>Test_1.mh.invoke();
>}
> }
> 
> public class Test_2 {
>static int testMethod(int value) { return (value + 1); }
> }
> 
> So there should be more clear explanation what is the correct or expected
> behaviour at this point and why OpenJDK doesn't comply with the document to
> delay the initialization of the method's class to mh.invoke().

Hi,
if by initialization you mean running the static block, then it's a bug.

As far as i remember, the JVMS spec cleanly separate the Linking exceptions 
from the Runtime exceptions.
https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-6.html#jvms-6.5.invokestatic

The linking exceptions occurs when calling findStatic() while the runtime 
exceptions will appear at runtime when calling invokeExact()/invoke().

> 
> Best Regards
> Cheng Jin

regards,
Rémi

Re: Making enum hashcodes consistent across JVM's

[Remi Forax]

- Original Message -
> From: "dfranken jdk" 
> To: "core-libs-dev" 
> Sent: Thursday, March 17, 2022 1:49:08 PM
> Subject: Making enum hashcodes consistent across JVM's

> Dear all,
> 
> Currently enums do not have a well-defined hashCode() implementation so
> they defer to Object.hashCode() which just uses an internal mechanism
> to determine the hashcode, likely based on the object's place in the
> heap.

No, and no.

Enum.hashCode() is well defined, it is equivalent to super.hashCode() and it 
can not be overridden by a specific enum (see the javadoc).

> 
> This may confuse a lot of developers as other classes such as String do
> have a well-defined hashcode which is consistent across multiple JVM's.

One of the reasons why String.hashCode() is defined is because the compiler 
(javac) compiles a switch on String using String.hashCode() so the hashCode at 
compile time and at runtime has to be the same.

Having a well defined hashCode was a mistake, first it means that we can not 
change the implementation later (the current hashCode of String has too many 
collisions) and it also make DDOS attacks easier because you can pre-compute a 
list of strings with the same hashCode.

> 
> Could it be a good idea to make enums' hashCode() method more reliably
> and predictable? For instance by using the hashcode of the name of its
> value?

Enum already have a kind of perfect hash, ordinal() which is reliable and 
predictable but it's not a great value as hashCode.

> 
> For example:
> 
> class MyEnum { A, B }
> -> A.hashCode() == A.name().hashCode()

This is not a good hashCode,

  enum Foo { Aa, BB }
  Foo.Aa.hashCode() == Foo.BB.hashCode()


> 
> Kind regards,
> 
> Dave Franken

regards,
Rémi

Re: Behavior change in the jar tool JDK11 vs JDK8.

[Remi Forax]

- Original Message -
> From: "Pasam Soujanya1" 
> To: "core-libs-dev" 
> Sent: Monday, March 14, 2022 12:48:42 PM
> Subject: Behavior change in the jar tool JDK11 vs JDK8.

> There is a significant difference in the way the JAR tool (starting JDK11) 
> seems
> to be responding to target files that are not present , when compared to JDK8.
> With JDK 8, the jar tool just reports about target files that are absent, and
> creates the jar file with whatever targets are available.
> 
> With JDK8, If I try to create a jar file with one existing file and another
> invalid file name I can see achieve getting created with the file that exists.
> Only the warning seems to be printed for the target that doesn't exists:
> 
> $ jar -cvf sample.jar exists.txt does_not_exist.txt
> does_not_exist.txt : no such file or directory
> added manifest
> adding: exists.txt(in = 0) (out= 0)(stored 0%)
> 
> From JDK11 onward upto the latest, though I can see the same verbose output 
> the
> jar file(sample.jar) is not created.
> 
> Looking at the code, the targets that do exist are written to a temporary JAR
> file, but the following "validation" code, which runs once the tool has
> finished writing the temporary JAR file, only moves/renames the temporary file
> to the specified JAR location if "ok" is true. If "ok" is false the temporary
> file is simply deleted:
> https://github.com/openjdk/jdk11u/blob/22186cb1fe22b4b30fc72c67ce9946cd4f03199d/src/jdk.jartool/share/classes/sun/tools/jar/Main.java#L451
> 
> Is this behavior change intentional? I couldn't find anything documented in
> release notes of JDK 9,10,11. If not intentional, can someone help me create
> bug report on OpenJDK jira? Thank you.

I believe it's a bugfix, since the creation of the command "jar", jar follows 
the semantics of "tar", so the behavior you are seeing now is the correct one.
But i was not able to pinpoint the bug corresponding to that issue.

> 
> Regards,
> Pasam Soujanya.

regards,
Rémi

Re: Questions about enhancement and Correction to Java OpenJDK Floating Point?

[Remi Forax]

- Original Message -
> From: "A Z" 
> To: "core-libs-dev" 
> Sent: Monday, March 14, 2022 7:49:04 AM
> Subject: Questions about enhancement and Correction to Java OpenJDK Floating 
> Point?

Hi Terry,
if you want to have the same output as C, instead of println() use printf().

In your example, using
  out.printf("%f\n", c);

prints
  0.01

  0.01

regards,
Rémi

> To whom it may concern,
> 
> Having noticed
> 
> https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8190947
> https://bugs.openjdk.java.net/browse/JDK-8190991
> 
> and similar, at
> https://community.oracle.com/tech/developers/discussion/4126262/big-issue-with-float-double-java-floating-point
> 
> I have been referred on to the core-libs-dev area.
> 
> The software development company I represent wishes to keep its name
> confidential,
> and no-mentioned, at this time.
> 
> A number of us at our end have found that floating point and StrictMath
> arithmetic
> on both float and double does not result in range accuracy, but produces
> denormal
> and pronormal values.
> 
> We are aware of the Java Language Specification, and IEEE 754 specifications,
> to these issues, but are still finding that they are not the most relevant or
> great issue.
> 
> While we are aware of the BigDecimal and BigInteger workarounds, and
> furthermore, the calculator class including big-math
> https://github.com/eobermuhlner,
> we are finding in the development, debugging, and editing of our Java 
> programs,
> that using other classes to operate and exchange for the lack of range 
> accuracy
> from float,
> double and java.lang.StrictMath, that we are getting bogged down with what 
> turns
> into
> more inferior software.  The known and available workaround approaches are
> becoming
> stop-gap measures, perforcedly put in place, while introducing other problems
> into OpenJDK or Java software that don't have any particular, immediate,
> solutions.
> 
> Substituting float and double data in and out of BigDecimal and BigInteger
> produces
> source code which is much messier, complicated, error prone, difficult to
> understand
> and to change, is definitely slower, and is an inferior substitute when float
> and double are more than enough in the overwhelming majority of corresponding
> cases.
> This is particularly showing up in 2D and 3D Graphics software, by the default
> OpenJDK Libraries, but also through JMonkeyEngine 3.5.
> 
> Possessing the option to immediately deal with the precondition, postcondition
> and field types of float and double is far superior and more than ideal.
> All this is before the massive advantage of being able to use operators,
> but the change case becomes overwhelming when along a range accurate,
> double (or maybe float, also) supporting Scientific Calculator class.
> 
> If I want to discuss (at least OpenJDK) change in this area, I have
> been pointed to the core-libs area, by one of the respondents
> of the article:
> 
> https://community.oracle.com/tech/developers/discussion/4126262/big-issue-with-float-double-java-floating-point.
> 
> Is there anyone here, at core-libs-dev, who can point
> me in a better Oracle or OpenJDK direction, to discuss further
> and see about Java float and double and StrictMath floating point
> arithmetic denormal and pronormal values being repaired away
> and being made range accurate for all evaluation operations
> with them?
> 
> Certainly since other languages already have, that are open source
> and open resource file ones.  It is a mathematical fact that, for
> consistent, necessary and even fast term, 10% of 10% must
> always precisely be 1%, and by no means anything else.
> 
> Consider these three different language API evaluations,
> using their equivalents of float and double to perform
> the floating point equivalent of that precise evaluation:
> 
> //--
> //The C Language.
> #include 
> 
> int main()
> {
>printf("Program has started...");
>printf("\n");
>printf("\n");
>double a = 0.1D;
>double b = 0.1D;
>double c = a*b;
>printf("%lf",c);
>printf("\n");
>printf("\n");
>float d = 0.1F;
>float e = 0.1F;
>float f = d*e;
>printf("%lf",f);
>printf("\n");
>printf("\n");
>printf("Program has Finished.");
>return 0;
> }
> 
> /*
> Program has started...
> 
> 0.01
> 
> 0.01
> 
> Program has Finished.
> */
> //--
> //The C++ Language.
> 
> #include 
> 
> using namespace std;
> 
> int main()
> {
>cout << "Program has started..." << endl;
>double a = 0.1D;
>double b = 0.1D;
>double c = a*b;
>cout << endl << c << endl << endl;
>float d = 0.1F;
>float e = 0.1F;
>float f = d*e;
>cout << f << endl << endl;
>cout << "Program has Finished.";
>return 0;
> }
> 
> /*
> Program has started...
> 
> 0.01
> 
> 0.01
> 
> Program has Finished.
> */
> 
> 

Re: RFR: JDK-8282798 java.lang.runtime.Carrier

[Remi Forax]

- Original Message -
> From: "John R Rose" 
> To: "core-libs-dev" 
> Sent: Tuesday, March 8, 2022 5:55:00 PM
> Subject: Re: RFR: JDK-8282798 java.lang.runtime.Carrier

> On Tue, 8 Mar 2022 15:59:59 GMT, Maurizio Cimadamore 
> wrote:
> 
>>> We propose to provide a runtime anonymous carrier class object generator;
>>> java.lang.runtime.Carrier. This generator class is designed to share 
>>> anonymous
>>> classes when shapes are similar. For example, if several clients require
>>> objects containing two integer fields, then Carrier will ensure that each
>>> client generates carrier objects using the same underlying anonymous class.
>>> 
>>> See JBS for details.
>>
>> src/java.base/share/classes/java/lang/runtime/Carrier.java line 48:
>> 
>>> 46:
>>> 47: /**
>>> 48:  * This  class is used to create objects that have number and types of
>> 
>> The class javadoc seems a bit on the thin side. I would say something on the
>> fact that the shape of the carrier is determined by a MethodType, etc, and 
>> add
>> an example to illustrate basic usage.
> 
> Agreed.  Also, this class is unusual in that it is not instantiated; like
> `Arrays` or `Collections` it is a (small) bundle of static factories (or are
> they algorithms?).  I think as such it should be called `Carriers`.
> 
> I'm slightly surprised the MH factories are not factored through a metaobject 
> of
> the form
> 
> record CarrierBinding(
>MethodType methodType,
>MethodHandle constructor,
>List> componentTypes,
>List components)
> { … }

Yes, i've done something very similar
https://github.com/forax/switch-carrier/blob/master/src/main/java/com/github/forax/carrier/java/lang/runtime/Patterns.java#L172

Note that constructor.type() == methodType and constructor.parameterList() == 
componenetTypes,
so the record can be reduced to a tuple (constructor / components).

> 
> 
> The presupposition here, I suppose, is that carriers will only be used by 
> condy
> or similar quasi-statically configured clients, so having the multiple lookups
> through a hidden table is no burden, and the clients can always keep the
> associations correct (between constructors and various component accessors).
> 
> **But** if I were to use carriers to manage intermediate structures in (say) a
> serialization package (for instance) I would need to make my own records like
> the above for my own bookkeeping, and I would be slightly miffed that the
> Carrier API insisted on doing a de-novo lookup twice on each MT key (to say
> nothing of me having to create the MT key first).

It depends if you want to use the same carrier for several cases or not.

> 
> **And** if I were to use carriers in that way (away from condy), **then** I
> would want to skip the step of building the MethodType, and wish for a factory
> method for CarrierBinding instances that took a plain List, as well as 
> a
> factory method that took the MethodType (which is convenient for condy).

As i said above, constructor.type() == methodType so anyway the MethodType has 
to be created.

> 
> BTW, it would be normal to give the name `Carrier` (which is a good name BTW) 
> to
> the record type that embodies the group of method handles, so `record
> Carrier(…constructor…components…) {…factories…}`.
> 
> I suppose stuff like this could be added later.  But it's worth considering 
> now,
> simply because there is an early decision point between a class named 
> `Carrier`
> and a static-only class named `Carriers`.

Rémi

Re: RFR: JDK-8282798 java.lang.runtime.Carrier

[Remi Forax]

Hi Jim,
I believe that there is a mismatch about what is needed for the pattern 
matching and the API you propose.
The Carrier API allows to map one tuple of types to one storage representation 
based on ints, longs and references.

But what we need is to have several shapes for the same storage, mapping is not 
one to one but many to one, more like how unions are mapped in C.
For a switch, if we have several cases, each one need it's own shape to store 
the bindings but at the same time we also need one field to be the same for all 
shapes (the field of type int indicating which case match) so one storage but 
many shapes.

Rémi

- Original Message -
> From: "Jim Laskey" 
> To: "core-libs-dev" 
> Sent: Tuesday, March 8, 2022 3:11:39 PM
> Subject: RFR: JDK-8282798 java.lang.runtime.Carrier

> We propose to provide a runtime anonymous carrier class object generator;
> java.lang.runtime.Carrier. This generator class is designed to share anonymous
> classes when shapes are similar. For example, if several clients require
> objects containing two integer fields, then Carrier will ensure that each
> client generates carrier objects using the same underlying anonymous class.
> 
> See JBS for details.
> 
> -
> 
> Commit messages:
> - Introduce java.lang.runtime.Carrier
> 
> Changes: https://git.openjdk.java.net/jdk/pull/7744/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7744=00
>  Issue: https://bugs.openjdk.java.net/browse/JDK-8282798
>  Stats: 1085 lines in 2 files changed: 1085 ins; 0 del; 0 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/7744.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7744/head:pull/7744
> 
> PR: https://git.openjdk.java.net/jdk/pull/7744

Re: Replace simple iterations of Map.entrySet with Map.forEach calls

[Remi Forax]

- Original Message -
> From: "Stuart Marks" 
> To: "-" , "core-libs-dev" 
> 
> Sent: Saturday, March 5, 2022 1:58:30 AM
> Subject: Re: Replace simple iterations of Map.entrySet with Map.forEach calls

> Hi, I have to say I'm pretty skeptical of this change overall.
> 
> It sounds like the main issue is iterating over the entrySet of a map that 
> might
> be
> modified concurrently. The bug report says
> 
> > for concurrent maps, regular iteration of entrySet may fail spontaneously
> 
> This isn't true for ConcurrentHashMap or ConcurrentSkipListMap, which have
> weakly
> consistent iterators that won't error out. It *is* true for synchronizedMap,
> which
> can throw ConcurrentModificationException if the map is modified during
> iteration. I
> guess that's mostly what you're referring to.
> 
> The problem with synchronizedMap (and the other synchronized wrappers) is that
> they
> require external synchronization around iteration, and it's pretty much
> impossible
> to cover all the cases where iteration occurs. It's used everywhere outside 
> the
> JDK.
> Even within the JDK, it appears quite frequently and is hard to avoid. For
> example,
> consider
> 
> collection.addAll(synchronizedMap.keySet())
> 
> AbstractCollection::addAll iterates its argument, and this is inherited in a
> bunch
> of places.
> 
> The root of the problem is that the synchronized wrappers support concurrent
> updating only in a very narrow range of use cases. If people have trouble with
> concurrent map updating, they need to use a real concurrent data structure, or
> they
> need to put everything inside their own class and have it do locking around
> higher-level operations. Replacing a few entrySet iteration cases with
> Map::forEach
> doesn't help anything.

Hi DrDeprecator,
i wonder if we should not deprecate the method synchronized*, usually they 
cause more harm than good for the reason you explain.

> 
> s'marks

Rémi

> 
> 
> 
> On 2/22/22 3:09 PM, - wrote:
>> Hello,
>> In the patch for 8281631 ,
>> xenoamess intentionally avoided
>>  repeatedly
>> calling Map::size, for it may not be constant-timed due to being
>> concurrent. This alerted me that the for loops on Map::entrySet may be
>> similarly not thread-safe.
>> 
>> I believe that we should migrate for loop on Map::entrySet to Map::forEach
>> calls; concurrent map callees usually have dedicated logic
>> 
>> to mitigate such thread-safety issues. Synchronized map
>> 
>> callees are benefitted too.
>> 
>> Another lesser benefit is reduced object allocation for iteration. While
>> the most popular implementations (HashMap, LinkedHashMap, WeakHashMap,
>> TreeMap) don't need to allocate entries for iteration, many other (EnumMap,
>> IdentityHashMap, concurrent maps) do, and iterating those maps with forEach
>> is less costly. (Note that 8170826
>>  takes care of
>> implementing proper forEach in EnumMap)
>> 
>> A JBS issue has already been submitted at
>> https://bugs.openjdk.java.net/browse/JDK-8282178, and I have prepared a
>> patch. This patch modifies the putAll implementations of a few JDK maps to
>> let the callee feed entries through Map::forEach to be put into the maps.
>> Two places of Map::entrySet calls in Collectors has also been updated.
>> 
>> For most other existing usages in the JDK, I don't think they will benefit
>> from such a migration: They mostly iterate on the entryset of the popular
>> implementations that already host entries and are single-threaded, and I
>> don't think it's in our best interest to touch those use cases.
>> 
>> So, here are my questions:
>> 1. Is such a concept of replacing Map::entrySet calls with Map::forEach
>> calls reasonable, or is there any fundamental flaw?
>> If the concept sounds good, I will submit a patch, so we can answer
>> 2. Is the changes implemented correct for this concept, i.e. targeting code
>> where users supply callee maps?
>> 
>> Best regards

Re: RFR: 8282143: Objects.requireNonNull should be ForceInline

[Remi Forax]

- Original Message -
> From: "Paul Sandoz" 
> To: "core-libs-dev" 
> Sent: Tuesday, March 1, 2022 1:48:02 AM
> Subject: Re: RFR: 8282143: Objects.requireNonNull should be ForceInline

> On Sat, 19 Feb 2022 05:51:52 GMT, Quan Anh Mai  wrote:
> 
>> Hi,
>> 
>> `Objects.requireNonNull` may fail to be inlined. The call is expensive and 
>> may
>> lead to objects escaping to the heap while the null check is cheap and is 
>> often
>> elided. I have observed this when using the vector API when a call to
>> `Objects.requireNonNull` leads to vectors being materialised in a hot loop.
>> 
>> Should the other `requireNonNull` be `ForceInline` as well?
>> 
>> Thank you very much.
> 
> `Objects.requireNonNull` is also used on the critical path of VarHandles, and
> other places in `j.l.invoke` so I think this is generally beneficial beyond 
> use
> by the Vector API.

It is also use by javac when the JLS requires to emit a nullcheck, like for 
example when creating a method reference.

> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/7543

Re: Should System.exit be controlled by a Scope Local?

[Remi Forax]

Hi Ethan,
there is a far simpler solution, call org.apache.ivy.run(args, true) instead of 
org.apache.ivy.main(args) in your tool provider.

regards,
Rémi

- Original Message -
> From: "Ethan McCue" 
> To: "core-libs-dev" 
> Sent: Saturday, February 26, 2022 11:14:19 PM
> Subject: Should System.exit be controlled by a Scope Local?

> I have a feeling this has been considered and I might just be articulating
> the obvious - but:
> 
> As called out in JEP 411, one of the remaining legitimate uses of the
> Security Manager is to intercept calls to System.exit. This seems like a
> decent use case for the Scope Local mechanism.
> 
> 
>public class Runtime {
>...
>private final ScopeLocal EXIT =
> ScopeLocal.newInstance();
> 
>...
> 
>public void overridingExitBehavior(IntConsumer exit, Runnable run) {
>ScopeLocal.with(EXIT, exit).run(run);
>}
> 
>...
> 
>public void exit(int status) {
>if (EXIT.isBound()) {
>EXIT.get().accept(status);
>}
>else {
>Shutdown.exit(status);
>}
>}
>}
> 
> 
> One of the likely minor benefits in the scope of things, but related to the
> parts of the ecosystem I am doodling with so I'll mention it, is that it
> would become possible to wrap "naive" cli programs with the ToolProvider
> SPI without rewriting their code if this System.out, and System.err all
> became reliably configurable.
> 
> For instance, Apache Ivy's CLI has a main class that looks like this
> 
> https://github.com/apache/ant-ivy/blob/424fa89419147f50a41b4bdc665d8ea92b5da516/src/java/org/apache/ivy/Main.java
> 
>package org.apache.ivy;
> 
>public final class Main {
>...
> 
>public static void main(String[] args) throws Exception {
>try {
>run(args, true);
>System.exit(0);
>} catch (ParseException ex) {
>System.err.println(ex.getMessage());
>System.exit(1);
>}
>}
> }
> 
> Making these otherwise static parts of the system configurable would enable
> a third party library to write
> 
>public final class IvyToolProvider implements ToolProvider {
>@Override
>public String name() {
>return "ivy";
>}
> 
>@Override
>public int run(PrintWriter out, PrintWriter err, String... args) {
>var exit = new AtomicInteger(0);
>Runtime.getRuntime().overridingExitBehavior(exit::set, () -> {
>System.overridingOut(out, () -> {
> System.overridingErr(err, Main::main);
>}
>};
>return exit.get();
>}
>}
> 
> Whether that would be enough to make it so that people other than Christian
> Stein use the mechanism is anyone's guess, but might be worth a shot.
> 
> https://grep.app/search?q=java.util.spi.ToolProvider

Sequenced Collections

[Remi Forax]

I've read the draft of the JEP on sequenced collection, and i think the 
proposed design can be improved.
  https://bugs.openjdk.java.net/browse/JDK-8280836

I agree with the motivation, there is a need for an API to consider the element 
of a list, a sorted set and a linked hash set as an ordered sequence of 
elements with a simple way to access/add/remove the first/last element and also 
reverse the elements as view.

I disagree about the conclusion that we need to introduce 4 new interfaces for 
that matter.

Here are the reasons
1/ Usually an ordered collection is called a list. Introducing an interface 
SequencedCollection for something which is usually called a list will cause 
more harm than good. Or maybe we should rename LISP to SEQP :)

2/ There is already an interface List in Java, that represents an ordered 
sequence of elements, with LinkedList being the name of the the double linked 
list implementation. You can argue that there is a slight difference between 
the semantics of java.util.List and the proposed syntax of 
java.util.SequencedCollection, but given that people already have difficulties 
to understand basic data structure concepts, as a teacher i dread to have a 
discussion on those slight differences that are only true in Java.

If the collection API was not already existing, we may discuss about having the 
same interface java.util.List to both indexed collection and ordered 
collection, but that boat has sailed a long time ago.

So in first approach, we should refactor sorted set and linked hash set to 
directly implement java.util.List and all the proposed methods into 
java.util.List. But as you hint in the Risks and Assumptions section, this will 
cause regression due to inference and also we will have trouble with 
LinkedHashMap (see below).

3/ LinkedHashMap mixes 3 implementations in one class, some of these 
implementations does not conform to the semantics of SequencedMap.
- You can opt-out having the key sequentially ordered as defined by 
SequencedMap by using the constructor LinkedHashMap(int initialCapacity, float 
loadFactor, boolean accessOrder) and passing true as last parameter. 
- You can opt-out having the key sequentially ordered as defined by 
SequencedMap by overriding removeEldestEntry(), removing the first entry at the 
same time you add a new one.

Because all these reasons, i think we should move to another design, using 
delegation instead of inheritance, which for the collection framework means 
exposing new way to access/modify sorted set and linked hash set through 
java.util.List views.

The concept of views is not a new concept, it's used in Arrays.asList(), 
List.subList() or Map.keySet()/values()/entrySet() (and more). The idea is not 
that a sorted set is a list but that it provides a method to see it as a list. 
It solves our problem of compatibility by not adding super types to existing 
type and also the problem of the semantics of LinkedHashMap because a view 
keeps the semantics of the data structure it originated.

Here is the proposed new methods in List, SortedSet and SortedMap.

interface List extends Collection {
  // new methods
  void addFirst();
  void addLast();
  E getFirst();
  E getLast();
  E removeFirst();
  E removeLast();
  List reversedList(); // or descendingList() ??
}

interface SortedSet implements Set {
  // new methods
  List asList(); 
}

interface SortedMap implements Map {
  // new methods
  List keyList();  // do not use covariant return type
  List> entryList();  // same
}

I believe this design is objectively better than the one proposed because as a 
user being able to use new interfaces is a slow process, the 
libraries/dependencies must be updated to take the new interfaces as parameter 
before the new types can be used. By contrast, the proposed design only enhance 
existing interfaces so people will enjoy the new methods directly when 
introduced.

Rémi

Re: Thread.dispatchUncaughtException possible NPE?

[Remi Forax]

- Original Message -
> From: "Andrey Turbanov" 
> To: "core-libs-dev" 
> Sent: Monday, January 17, 2022 10:37:04 AM
> Subject: Thread.dispatchUncaughtException possible NPE?

> Hello.

Hello Andrey,

> I see that Thread.dispatchUncaughtException calls
> getUncaughtExceptionHandler() which reads volatile field twice:
> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/Thread.java#L1978
> 
> private volatile UncaughtExceptionHandler uncaughtExceptionHandler;
> 
> public UncaughtExceptionHandler getUncaughtExceptionHandler() {
>return uncaughtExceptionHandler != null ?
>uncaughtExceptionHandler : group;
> }
> 
> private void dispatchUncaughtException(Throwable e) {
>getUncaughtExceptionHandler().uncaughtException(this, e);
> }
> 
> 
> I wonder if it's possible to get a NPE here? Another thread could
> change uncaughtExceptionHandler between 2 volatile reads.
> Or JVM somehow forbid this?

yes, it's a bug, the field should be read only once.

> 
> 
> 
> 
> Andrey Turbanov

Rémi

Re: LambdaMetafactory requires full privilege access, but doesn't seem to actually restrict functionality

[Remi Forax]

- Original Message -
> From: "Steven Schlansker" 
> To: "core-libs-dev" 
> Sent: Wednesday, January 12, 2022 9:56:30 PM
> Subject: LambdaMetafactory requires full privilege access, but doesn't seem 
> to actually restrict functionality

> Hi core-libs-dev,
> 
> I am maintaining a module for the popular Jackson JSON library that attempts 
> to
> simplify code-generation code without losing performance.
> Long ago, it was a huge win to code-generate custom getter / setter / field
> accessors rather than use core reflection. Now, the gap is closing a lot with
> MethodHandles, but there still seems to be some benefit.
> 
> The previous approach used for code generation relied on the CGLib + ASM
> libraries, which as I am sure you know leads to horrible-to-maintain code 
> since
> you essentially write bytecode directly.

I don't see the issue here, writing bytecodes in not that hard :)

> Feature development basically stopped because writing out long chains of
> `visitVarInsn(ASTORE, 3)` and the like scares off most contributors, myself
> included.

yes, sadly known issue, generating assembly code even a highlevel one like the 
Java byetcode by hands require a niche knowledge which sadly is rarely teached 
at university anymore (and let's not talked about bootcamp).  

> 
> As an experiment, I started to port the custom class generation logic to use
> LambdaMetafactory. The idea is to use the factory to generate `Function T>` getter and `BiConsumer` setter implementations.

If you want to make the serailization/deserialization to JSON fast you will 
mostly battle two things,
 - polymorphic call site, a call that can call some many different 
implementations that the VM will not try to inline,
   usually the trick is to just have one of such call to take care of the 
different kind of objects.
 - boxing, that kind of code tends to abstract over any values, but boxing 
means allocation if there is no inlining.  

For me, it means that you want two specialized codes, the decoder that switches 
over the keys and call the correct setter and the encoder that calls the 
getters in sequence.

Trying to abstract over getters and setters is a step too far because you loose 
the benefit to write a code specific to a peculiar class.

I suppose you try to use the method handles directly with invokeExact() if you 
are able to remove the boxing
or with invoke() if not ?

Because it's not clear to me why you want to use the LambdaMetafactory ?

[...]

> 
> I'm also curious for any feedback on the overall approach of using the
> Metafactory, perhaps I am way off in the weeds, and should just trust
> MethodHandles to perform well if you use invokeExact :) JMH does seem to show
> some benefit though especially with Graal compiler.
> 
> Thanks a bunch for any thoughts,
> Steven Schlansker

Rémi

Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger [v3]

[Remi Forax]

- Original Message -
> From: "Bernd Eckenfels" 
> To: "core-libs-dev" 
> Sent: Jeudi 18 Novembre 2021 12:07:19
> Subject: Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger 
> [v3]

> What about a new API multiply method which takes an forkjoinpool, and only if
> that is used/specified it will use the parallel mode (and only if Notsitzes
> threshold applies?). Most of the pool tuning can then done with this argument.
> It also avoids surprise threads.

You don't need it, here is the usual trick if you want to specify a specific 
fork join pool
https://stackoverflow.com/questions/21163108/custom-thread-pool-in-java-8-parallel-stream

> 
> Gruss
> Bernd

regards,
Rémi

> --
> http://bernd.eckenfels.net
> 
> Von: core-libs-dev  im Auftrag von kabutz
> 
> Gesendet: Thursday, November 18, 2021 11:41:40 AM
> An: core-libs-dev@openjdk.java.net 
> Betreff: Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger
> [v3]
> 
> On Thu, 18 Nov 2021 07:26:45 GMT, David Holmes  wrote:
> 
>> To add my 2c IMO a parallel version of this type absolutely **must** be 
>> opt-in.
>> There are simply far too many side-effects of using the FJP and multiple
>> threads to perform the calculation in parallel as if it is just a minor
>> implementation detail. A clear API is 1000x better than a "kill switch".
>>
>> And yes you may still need to expose some kind of tuning knob.
>>
>> David
> 
> Yes, it **must** be opt-in. However I'm not sure that a tuning knob will be
> necessary. BigInteger has thresholds for using different multiply algorithms
> and these are also not configurable.
> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/6409

Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources

[Remi Forax]

- Original Message -
> From: "Glavo" 
> To: "Tagir F.Valeev" 
> Cc: "core-libs-dev" 
> Sent: Samedi 16 Octobre 2021 06:25:40
> Subject: Re: RFR: 8274412: Add a method to Stream API to consume and close 
> the stream without using try-with-resources

> I don't think it is a perfect solution to combine the close operation
> with the terminator operation, because there are similar issues
> in the intermediate operation.
> 
> Consider this use case (for example only):
> 
> try (var stream = Files.list(path)
>.flatMap(dir -> {
>try {
>return Files.list(dir);
>} catch (IOException e) {
>return Stream.empty();
>}
>})) {
>// ...
> }
> 
> It looks closed, but it doesn't. Closing the Stream generated by flatMap
> will not close all member Stream. 

The stream is closed, from the javadoc
"Each mapped stream is closed after its contents have been placed into this 
stream."


> Further consideration is needed to
> deal with the problem of closing the Stream. Perhaps it is a withCleaner
> method that registers the Stream with the cleaner, or other solutions.
> 
> 

regards,
Rémi

> 
> 
> Tagir F.Valeev  于2021年10月4日周一 下午2:52写道：
> 
>> Currently, when the stream holds a resource, it's necessary to wrap it
>> with try-with-resources. This undermines the compact and fluent style of
>> stream API calls. For example, if we want to get the `List` of files inside
>> the directory and timely close the underlying filehandle, we should use
>> something like this:
>>
>>
>> List paths;
>> try (Stream stream = Files.list(Path.of("/etc"))) {
>> paths = stream.toList();
>> }
>> // use paths
>>
>>
>> I suggest to add a new default method to Stream interface named
>> `consumeAndClose`, which allows performing terminal stream operation and
>> closing the stream at the same time. It may look like this:
>>
>>
>> default  R consumeAndClose(Function, ? extends R>
>> function) {
>> Objects.requireNonNull(function);
>> try(this) {
>> return function.apply(this);
>> }
>> }
>>
>>
>> Now, it will be possible to get the list of the files in the fluent manner:
>>
>>
>> List list =
>> Files.list(Path.of("/etc")).consumeAndClose(Stream::toList);
>>
>> -
>>
>> Commit messages:
>>  - Fix tests
>>  - 8274412: Add a method to Stream API to consume and close the stream
>> without using try-with-resources
>>
>> Changes: https://git.openjdk.java.net/jdk/pull/5796/files
>>  Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5796=00
>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8274412
>>   Stats: 140 lines in 5 files changed: 135 ins; 0 del; 5 mod
>>   Patch: https://git.openjdk.java.net/jdk/pull/5796.diff
>>   Fetch: git fetch https://git.openjdk.java.net/jdk
>> pull/5796/head:pull/5796
>>
>> PR: https://git.openjdk.java.net/jdk/pull/5796

Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources

[Remi Forax]

I agree with the idea of a try() syntax, but i don't think we need more 
interfaces.

Yes, John is right about the fact that the TWR Aucloseable does not work well 
with checked exceptions,
but the issue is more that there is nothing that works well with checked 
exceptions because Java has no way to compose them correctly
(remainder: we should remove the idea of checked exceptions from the language 
given that it's a backward compatible change and Kotlin, C# are safer than Java 
because users of those languages are not used to write a 
try/catch/printStackTrace in those languages unlike in Java).

So for me, AutoCloseable is enough.

The problem is more than a fluent API are expression oriented and a 
try-with-resources is statement oriented, hence the mismatch.

Like we have introduced the switch expression, here we need a 
try-with-resources expression.

There are good reasons to have a try-with-resources expression
1) it can be used to track a fluent chain that should close() the resources
2) the compiler can guarantee that the stream/reader/etc around the resources 
does not leak outside.

In term of syntax, i don't think that the arrow syntax (the one used for a case 
of a switch case or a lambda) should be used here, because using a block 
expression seems to be always a mistake.

I like the proposal from John, a try(expression) or a try expression (without 
the parenthesis).

regards,
Rémi


- Original Message -
> From: "John Rose" 
> To: "Paul Sandoz" , "Brian Goetz" 
> , "Tagir F.Valeev"
> 
> Cc: "core-libs-dev" 
> Sent: Lundi 11 Octobre 2021 20:42:20
> Subject: Re: RFR: 8274412: Add a method to Stream API to consume and close 
> the stream without using try-with-resources

> So the purpose of TWR is to hold an object with a “close-debt”
> (debt of a future call to close) and pay it at the end of a block,
> sort of like C++ RAII (but also sort of not).
> 
> But fluent syntaxes (which I like very much and hope to see
> more of in the future!) don’t play well with blocks, so if a
> fluent chain (any part of that chain:  It’s multiple objects)
> incurs a “close-debt”, it’s hard to jam a TWR block into it.
> 
> Hence the current proposal.  I agree with Brian and Paul
> that we haven’t examined all the corners of this problem
> yet.  And I’d like to poke at the concept of “close-debt” to
> help with the examination.
> 
> Just for brain storming, I think we could model “close-debt”
> outside either fluent API structure or TWR block structure.
> Java O-O APIs are the pre-eminent way to model things in
> Java, and they work exceedingly well, when used with skill.
> 
> AutoCloseable models close-debt of course.  But it has two
> weaknesses:  It doesn’t model anything *other* than the
> debt, and its (sole) method skates awkwardly around the
> issue of checked exceptions.  (It requires an override with
> exception type narrowing to be used in polite company.)
> AC is more of an integration hook with TWR, rather than
> a general-purpose model for close-debt.  Therefore it doesn’t
> teach us much about close-debt in a fluent setting.
> 
> Surely we can model close-debt better.  Let’s say that an
> operation (expression) with close-debt *also* has a return
> value and (for grins) *also* has an exception it might throw.
> This gets us to an API closer to Optional.  (If you hear the
> noise of a monad snuffling around in the dark outside
> your tent, you are not wrong.)
> 
> interface MustClose_1 {
>   T get() throws X;  //or could throw some Y or nothing at all
>   void close() throws X;
> }
> 
> (I wish we had an easier way to associate such an X
> with such a T, so that Stream could be more
> interoperable with simple Stream.  It’s a pain to
> carry around those X arguments.  So I’ll drop X now.)
> 
> interface MustClose_2 {
>   T get();
>   void close() throws Exception;
> }
> 
> An expression of this type requires (in general) two
> operations to finish up:  It must be closed, and the result
> (type T) must be gotten.  There’s an issue of coupling between
> the two methods; I would say, decouple their order, so that
> the “get” call, as with Optional, can be done any time,
> and the “close” call can be done in any order relative
> to “get”.  Both calls should be idempotent, I think.
> But that’s all second-order detail.
> 
> A first-order detail is the apparent but incorrect 1-1 relation
> between T values and close-debts.  That’s very wrong;
> a closable stream on 1,000 values has one close-debt,
> not 1,000.  So maybe we need:
> 
> interface MustClose_3 {
>   S map(Function value);
>   void close() throws Exception;
> }
> 
> That “map” method looks a little like Remi’s apply
> method.  Did I mention this design requires skill
> (as well as flexibility, with one hand already tied
> by checked exceptions)?  I’m at the edge of my own
> skill here, but I think there’s good ground to explore
> here.
> 
> In a fluent setting, a Stream that incurs a close-debt
> might be typed (after incurring the 

Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources

[Remi Forax]

- Original Message -
> From: "Tagir F.Valeev" 
> To: "core-libs-dev" 
> Sent: Lundi 4 Octobre 2021 08:51:55
> Subject: RFR: 8274412: Add a method to Stream API to consume and close the 
> stream without using try-with-resources

> Currently, when the stream holds a resource, it's necessary to wrap it with
> try-with-resources. This undermines the compact and fluent style of stream API
> calls. For example, if we want to get the `List` of files inside the directory
> and timely close the underlying filehandle, we should use something like this:
> 
> 
> List paths;
> try (Stream stream = Files.list(Path.of("/etc"))) {
>paths = stream.toList();
> }
> // use paths
> 
> 
> I suggest to add a new default method to Stream interface named
> `consumeAndClose`, which allows performing terminal stream operation and
> closing the stream at the same time. It may look like this:
> 
> 
>default  R consumeAndClose(Function, ? extends R> 
> function)
>{
>Objects.requireNonNull(function);
>try(this) {
>return function.apply(this);
>}
>}
> 
> 
> Now, it will be possible to get the list of the files in the fluent manner:
> 
> 
> List list = Files.list(Path.of("/etc")).applyAndClose(Stream::toList);


I would prefer the method to be called applyAndClose() because it is what the 
method does, it applies the function and closes the stream.

There are two missing information in the javadoc
- the function taken as parameter should not return a stream, because the 
stream will be closed
  This is not okay
List list = Files.list(Path.of("/etc")).applyAndClose(s -> 
s).toList();


- if there are intermediary operations, they have to be done in the function 
taken as parameter and not before calling applyAndClose()
  This is okay
List list = Files.list(Path.of("/etc")).applyAndClose(s -> 
s.map(path -> Integer.parseInt(path.toString())).toList());

  This is not okay
List list = Files.list(Path.of("/etc")).map(path -> 
Integer.parseInt(path.toString())).applyAndClose(Stream::toList);


In both case, IDEs can help, but i think it should be written explicitly in the 
javadoc.


> 
> -
> 
> Commit messages:
> - Fix tests
> - 8274412: Add a method to Stream API to consume and close the stream without
> using try-with-resources
> 
> Changes: https://git.openjdk.java.net/jdk/pull/5796/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5796=00
>  Issue: https://bugs.openjdk.java.net/browse/JDK-8274412
>  Stats: 140 lines in 5 files changed: 135 ins; 0 del; 5 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/5796.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5796/head:pull/5796
> 
> PR: https://git.openjdk.java.net/jdk/pull/5796

Re: Discussion: easier Stream closing

[Remi Forax]

At that point, the real question is why not call close() at the end of all 
terminal operations by wrapping each implementation in a try-with-resources ?

Rémi

- Original Message -
> From: "Brian Goetz" 
> To: "Tagir Valeev" , "core-libs-dev" 
> 
> Sent: Lundi 27 Septembre 2021 22:41:00
> Subject: Re: Discussion: easier Stream closing

> In Java 8, I think we were reluctant to lean on the idiom of "pass me a
> lambda and I'll pass it the confined data"), because Java developers
> were already struggling to understand lambdas.  But now that we're
> mostly over that hurdle, API points that accept Consumer are
> a powerful way to gain confinement (as we've seen in StackWalker, and
> also in the explorations of ScopeLocal in Loom.)  So I have no objection
> to this idiom.
> 
> I have some concern that putting these side-by-side with existing
> Files.walk(...) methods will be a source of confusion, creating two
> different ways to do the same thing.
> 
> I would rather not add anything new to BaseStream; it was a mistake to
> expose at all, and I'd rather see it deprecated than add more to it.
> However, adding it individually to the Stream implementations is
> equivalent, and doesn't have this problem.
> 
> The consumeAndClose approach is clever, in that it adds one API point
> that works for all streams, rather than having to add a new API point
> for every factory of a closeable stream; on the other hand, it is
> dramatically less discoverable, and so requires more education to get
> people to use it (and, as you say, has the exception problem.)
> 
> On 9/26/2021 5:27 AM, Tagir Valeev wrote:
>> Hello!
>>
>> With current NIO file API, a very simple problem to get the list of
>> all files in the directory requires some ceremony:
>>
>> List paths;
>> try (Stream stream = Files.list(Path.of("/etc"))) {
>>  paths = stream.toList();
>> }
>>
>> If we skip try-with-resources, we may experience OS file handles leak,
>> so it's desired to keep it. Yet, it requires doing boring stuff. In
>> this sense, classic new File("/etc").list() was somewhat more
>> convenient (despite its awful error handling).
>>
>> I like how this problem is solved in StackWalker API [1]: it limits
>> the lifetime of the Stream by requiring a user-specified function to
>> consume it. After the function is applied, the stream is closed
>> automatically. We could add a similar overload to the Files API. As an
>> additional feature, we could also translate all UncheckedIOException's
>> back to IOException for more uniform exception processing:
>>
>> /**
>>   * @param dir  The path to the directory
>>   * @param function  function to apply to the stream of directory files
>>   * @param  type of the result
>>   * @return result of the function
>>   * @throws IOException if an I/O error occurs when opening the directory, or
>>   * UncheckedIOException is thrown by the supplied function.
>>   */
>> public static  T list(Path dir, Function, ?
>> extends T> function) throws IOException {
>>  try (Stream stream = Files.list(dir)) {
>>  return function.apply(stream);
>>  }
>>  catch (UncheckedIOException exception) {
>>  throw exception.getCause();
>>  }
>> }
>>
>> In this case, getting the List of all files in the directory will be
>> as simple as
>>
>> List paths = Files.list(Path.of("/etc"), Stream::toList);
>> This doesn't limit the flexibility. For example, if we need only file
>> names instead of full paths, we can do this:
>> List paths = Files.list(Path.of("/etc"), s ->
>> s.map(Path::getFileName).toList());
>>
>> Alternatively, we could enhance the BaseStream interface in a similar
>> way. It won't allow us to translate exceptions, but could be useful
>> for every stream that must be closed after consumption:
>>
>> // in BaseStream:
>>
>> /**
>>   * Apply a given function to this stream, then close the stream.
>>   * No further operation on the stream will be possible after that.
>>   *
>>   * @param function function to apply
>>   * @param  type of the function result
>>   * @return result of the function
>>   * @see #close()
>>   */
>> default  R consumeAndClose(Function function) {
>>  try(@SuppressWarnings("unchecked") S s = (S) this) {
>>  return function.apply(s);
>>  }
>> }
>>
>> The usage would be a little bit longer but still more pleasant than
>> explicit try-with-resources:
>>
>> List list = 
>> Files.list(Path.of("/etc")).consumeAndClose(Stream::toList);
>>
>> On the other hand, in this case, we are free to put intermediate
>> operations outside of consumeAndClose, so we won't need to nest
>> everything inside the function. Only terminal operation should be
>> placed inside the consumeAndClose. E.g., if we need file names only,
>> like above, we can do this:
>>
>> List list =
>> Files.list(Path.of("/etc")).map(Path::getFileName).consumeAndClose(Stream::toList);
>>
>> What do you think?
>>
>>
>> [1]
> > 

Re: Discussion: easier Stream closing

[Remi Forax]

- Original Message -
> From: "Tagir Valeev" 
> To: "core-libs-dev" 
> Sent: Dimanche 26 Septembre 2021 11:27:58
> Subject: Discussion: easier Stream closing

> Hello!
> 
> With current NIO file API, a very simple problem to get the list of
> all files in the directory requires some ceremony:
> 
> List paths;
> try (Stream stream = Files.list(Path.of("/etc"))) {
>paths = stream.toList();
> }
> 
> If we skip try-with-resources, we may experience OS file handles leak,
> so it's desired to keep it. Yet, it requires doing boring stuff. In
> this sense, classic new File("/etc").list() was somewhat more
> convenient (despite its awful error handling).
> 
> I like how this problem is solved in StackWalker API [1]: it limits
> the lifetime of the Stream by requiring a user-specified function to
> consume it. After the function is applied, the stream is closed
> automatically. We could add a similar overload to the Files API. As an
> additional feature, we could also translate all UncheckedIOException's
> back to IOException for more uniform exception processing:
> 
> /**
> * @param dir  The path to the directory
> * @param function  function to apply to the stream of directory files
> * @param  type of the result
> * @return result of the function
> * @throws IOException if an I/O error occurs when opening the directory, or
> * UncheckedIOException is thrown by the supplied function.
> */
> public static  T list(Path dir, Function, ?
> extends T> function) throws IOException {
>try (Stream stream = Files.list(dir)) {
>return function.apply(stream);
>}
>catch (UncheckedIOException exception) {
>throw exception.getCause();
>}
> }
> 
> In this case, getting the List of all files in the directory will be
> as simple as
> 
> List paths = Files.list(Path.of("/etc"), Stream::toList);
> This doesn't limit the flexibility. For example, if we need only file
> names instead of full paths, we can do this:
> List paths = Files.list(Path.of("/etc"), s ->
> s.map(Path::getFileName).toList());


It seems a nice addition, forgetting the try-with-resources is a very common 
source of bugs in my students code.
Part of the issue is that there is no way to easily test if there are still 
open descriptors during tests.
I wonder if the VM can not offer such test using a special flag 
(-XX:+CheckPendingFileDescriptor) or something like that.

> 
> Alternatively, we could enhance the BaseStream interface in a similar
> way. It won't allow us to translate exceptions, but could be useful
> for every stream that must be closed after consumption:
> 
> // in BaseStream:
> 
> /**
> * Apply a given function to this stream, then close the stream.
> * No further operation on the stream will be possible after that.
> *
> * @param function function to apply
> * @param  type of the function result
> * @return result of the function
> * @see #close()
> */
> default  R consumeAndClose(Function function) {
>try(@SuppressWarnings("unchecked") S s = (S) this) {
>return function.apply(s);
>}
> }
> 
> The usage would be a little bit longer but still more pleasant than
> explicit try-with-resources:
> 
> List list = Files.list(Path.of("/etc")).consumeAndClose(Stream::toList);
> 
> On the other hand, in this case, we are free to put intermediate
> operations outside of consumeAndClose, so we won't need to nest
> everything inside the function. Only terminal operation should be
> placed inside the consumeAndClose. E.g., if we need file names only,
> like above, we can do this:
> 
> List list =
> Files.list(Path.of("/etc")).map(Path::getFileName).consumeAndClose(Stream::toList);
> 
> What do you think?

This one does not work because if Path::getFileName fails with an exception, 
close() will not be called.
What you are proposing here is equivalent to
  try(var stream = Files.list(Path.of("/etc")).map(Path::getFileName)) {
stream.toList()
  }

instead of
  try(var stream = Files.list(Path.of("/etc"))) {
stream..map(Path::getFileName).toList()
  }

regards,
Rémi

> 
> 
> [1]
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html#walk(java.util.function.Function)

Re: StringBuilder OOMs earlier with JRE 11 compared to JRE 8

[Remi Forax]

- Original Message -
> From: "David Holmes" 
> To: "S" , "core-libs-dev" 
> 
> Cc: "Andrey Loskutov" 
> Sent: Vendredi 27 Août 2021 15:25:25
> Subject: Re: StringBuilder OOMs earlier with JRE 11 compared to JRE 8

> Hi Simeon,
> 
> Redirecting this to core-libs-dev as it is not a serviceability issue.
> (bcc serviceabillity-dev)

Hi Simeon,
in Java 9, the String representation was changed to use less memory if the 
characters can be encoded in ISO Latin 1,
a side effect is that StringBuilder now uses a byte[] instead of a char[], 
hence the maximum size being half the one it was previously.

> 
> Thanks,
> David

[1] 
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/AbstractStringBuilder.java#L63

> 
> On 27/08/2021 8:53 pm, S A wrote:
>> Hi all,
>> 
>> while working on https://bugs.eclipse.org/bugs/show_bug.cgi?id=575641
>> , I noticed that
>> StringBuilder throws an OOM "half as early" in OpenJDK 11 (and 17 early
>> access), when compared to OpenJDK 8.
>> 
>> In particular, I ran the following snippet to see where the limit of
>> appending to a StringBuilder is:
>> 
>> StringBuilder sb = new StringBuilder();
>> long n = 8L * 1024L * 1024L;
>> for (long i = 0; i < n; ++i) {
>> int m = 1024;
>> for (int j = 0; j < m; ++j) {
>> int length = sb.length();
>> try {
>> sb.append((char) j);
>> } catch (Error e) {
>> System.out.println("Error occurred at length=" + length + " [i=" + i +
>> ", j=" + j + "]");
>> throw e;
>> }
>> 
>> }
>> }
>> 
>> JRE 8:
>> 
>> Error occurred at length=2147483645 [i=2097151, j=1021]
>> Exception in thread "main" java.lang.OutOfMemoryError: Requested array
>> size exceeds VM limit
>> at java.util.Arrays.copyOf(Arrays.java:3332)
>> at
>> java.lang.AbstractStringBuilder.ensureCapacityInternal(AbstractStringBuilder.java:124)
>> at java.lang.AbstractStringBuilder.append(AbstractStringBuilder.java:649)
>> at java.lang.StringBuilder.append(StringBuilder.java:202)
>> at
>> test.stringbuilder.TestStringbuilderOOM.main(TestStringbuilderOOM.java:13)
>> 
>> JRE 11:
>> 
>> Error occurred at length=1073741822 [i=1048575, j=1022]
>> Exception in thread "main" java.lang.OutOfMemoryError: Requested array
>> size exceeds VM limit
>> at java.base/java.util.Arrays.copyOf(Arrays.java:3745)
>> at
>> java.base/java.lang.AbstractStringBuilder.ensureCapacityInternal(AbstractStringBuilder.java:172)
>> at
>> java.base/java.lang.AbstractStringBuilder.append(AbstractStringBuilder.java:748)
>> at java.base/java.lang.StringBuilder.append(StringBuilder.java:241)
>> at
>> TestJava11/test.stringbuilder.TestStringbuilderOOM.main(TestStringbuilderOOM.java:13)
>> 
>> While StringBuilder is not a good choice for holding GBs of text, I
>> wonder that no effort is made to clamp the capacity to its limit when (I
>> assume) it is being doubled upon array expansion.
>> 
>> Is this something that should be looked into or it can be safely ignored
>> (from JDK users POV)?
>> 
>> Best regards,
> > Simeon

Re: Implementing MethodHandleProxies#asInterfaceInstance with hidden classes

[Remi Forax]

- Original Message -
> From: "-" 
> To: "Brian Goetz" , "core-libs-dev" 
> 
> Sent: Lundi 23 Août 2021 08:34:17
> Subject: Re: Implementing MethodHandleProxies#asInterfaceInstance with hidden 
> classes

> Thanks for the quick reply!
> 
> The main drawback, API wise, with LMF is that it does not accept
> non-direct method handles [1], while the MHP is able accept any method
> handle, such as native ones from JEP 389 [2]. LMF also lacks the
> ability to restore a method handle from a wrapper instance. Using LMF
> also requires user discretion on which methods to implement, and the
> lookup's required privileges may change too. In general, MHP would be
> more runtime-oriented and transient compared to LMF.
> 
> I am inclined toward a separate bytecode generation implementation for
> the improved MHP asInterfaceInstance, somewhat similar to what's in
> JEP 416's implementation [3]. The extra bridges for all compatible
> methods, accessors to backing method handle/interface, potential to
> expand to abstract classes, different hidden class modes would reduce
> the commonalities between the two implementations and make
> refactorings on LMF to support these MHP functionalities costly.

In my opinion, what is missing is a java.lang.invoke.Proxy, the equivalent of 
java.lang.reflect.Proxy but using defineHiddenClass + a bootstrap method + 
method handles instead of the InvocationHandler + j.l.r.Method. With that, 
implementing java.lang.invoke.MethodHandleProxies on top of of 
java.lang.invoke.Proxy is just a matter of wiring.

As you said, there is a need for a more dynamic version of the 
LambdaMetafactory.
I believe it should work with any interfaces not only functional interfaces.
This would avoid statically typed langages like Kotlin or Scala, or injection 
frameworks like Weld, Spring or Guice to generate a lot of proxy classes and it 
should be straightforward enough so static compilers like graal native image or 
Android d8 can generate the proxy classes at compile time to support "native" 
compilation (the same way lambdas are currently supported).
So we get best of both worlds, efficient dynamic proxies at runtime AND 
supports of "native" compilation.

Rémi

> 
> [1]
> https://github.com/openjdk/jdk/blob/b690f29699180149d33b6a83de10697790587a87/src/java.base/share/classes/java/lang/invoke/AbstractValidatingLambdaMetafactory.java#L141
> [2] https://openjdk.java.net/jeps/389#The-C-linker
> [3]
> https://github.com/openjdk/jdk/blob/cff856f9df89293cbc8f2f1e977148cd6ece4f85/src/java.base/share/classes/jdk/internal/reflect/ClassByteBuilder.java
> 
> 
> On Sun, Aug 22, 2021 at 9:26 PM Brian Goetz  wrote:
>>
>> This was an early attempt at the functionality provided by LambdaMetafactory.
>> It could probably be reimplemented on top of that, but probably could be
>> deprecated in favor of LMF as well.
>>
>> Sent from my iPad
>>
>> > On Aug 22, 2021, at 10:08 PM, liangchenb...@gmail.com wrote:
>> >
>> > Currently, java.lang.invoke.MethodHandleProxies#asInterfaceInstance [1] is
>> > implemented with java.lang.reflect.Proxy. After looking at its public API,
>> > including Javadoc, it seems that MethodHandleProxies would benefit from a
>> > hidden class implementation without changing its public API definition
>> > (including Javadocs).
>> >
>> > Recently, there is JEP 416 [2] for reimplementing reflection based on
>> > method handles. This implementation utilizes hidden classes with method
>> > handles passed in classdata and retrieved to condy, which allows generic
>> > method handles (beyond regular constable ones) to be optimized in method
>> > calls. Similarly, for MethodHandleProxies, hidden classes allow the
>> > implementations to detach from classloaders and be freely recyclable; they
>> > can use class data to store the adapted method handles (which can
>> > significantly speed up invocations); and it can allow it to support
>> > implementing single-abstract-method abstract classes in the future (as
>> > discussed in its Javadoc) as well, with only minor tweaks.
>> >
>> > Does this sound feasible? I want to ensure it is a good idea before any
>> > implementation is attempted. If there is any issue with this vision, please
>> > don't hesitate to point it out. Feel free to comment, too! If this looks
>> > good, I hope an issue can be created for it.
>> >
>> > Best
>> >
>> > [1]
>> > https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/lang/invoke/MethodHandleProxies.html
> > > [2] https://openjdk.java.net/jeps/416

Re: [jdk17] RFR: 8269096: Add java.util.Objects.newIdentity method [v5]

[Remi Forax]

- Original Message -
> From: "Roger Riggs" 
> To: "core-libs-dev" 
> Sent: Mardi 29 Juin 2021 18:44:04
> Subject: Re: [jdk17] RFR: 8269096: Add java.util.Objects.newIdentity method 
> [v5]

> On Tue, 29 Jun 2021 05:47:03 GMT, Tagir F. Valeev  wrote:
> 
>>> Roger Riggs has updated the pull request incrementally with one additional
>>> commit since the last revision:
>>> 
>>>   Add test synchronizing on return value of Objecst.newIdentity()
>>
>> Probably it would be better to have an inner class named like `Identity` 
>> instead
>> of anonymous class? When debugging or analyzing memory dumps, it would be 
>> more
>> user-friendly to see `Objects$Identity` than `Objects$1`.
>> 
>> Probably, not the part of this feature request, but it would be nice to add
>> another method with string parameter, like `Objects.newIdentity("MY
>> SENTINEL")`. The string should be stored in the field and returned from
>> toString(). Again, this would make it easier to find where the object comes
>> from during debugging or memory dump analysis. For the record, here's what we
>> have in IntelliJ IDEA sources (Apache 2.0 licensed):
>> 
>> 
>> public final class ObjectUtils {
>>   private ObjectUtils() { }
>> 
>>   ...
>> 
>>   /**
>>* Creates a new object which could be used as sentinel value (special 
>> value to
>>distinguish from any other object). It does not equal
>>* to any other object. Usually should be assigned to the static final 
>> field.
>>*
>>* @param name an object name, returned from {@link #toString()} to 
>> simplify the
>>debugging or heap dump analysis
>>* (guaranteed to be stored as sentinel object field). If 
>> sentinel is
>>assigned to the static final field,
>>* it's recommended to supply that field name (possibly 
>> qualified
>>with the class name).
>>* @return a new sentinel object
>>*/
>>   public static @NotNull Object sentinel(@NotNull @NonNls String name) {
>> return new Sentinel(name);
>>   }
>> 
>>   private static final class Sentinel {
>> private final String myName;
>> 
>> Sentinel(@NotNull String name) {
>>   myName = name;
>> }
>> 
>> @Override
>> public String toString() {
>>   return myName;
>> }
>>   }
> 
> @amaembo Good suggestion for the Valhalla API and implementation.  Today, only
> the hashCode() identifies an Object.
> For JDK17, it will continue to be a raw Object() instance.

We should try to have a very limited API surface for now.
We want to let the door open to having Object being both an identity class 
(when used with "new") and an abstract class (when inherited) by using two 
different generic specialization of the same class or whatever other scheme we 
come before Valhalla is complete.

> 
> -
> 
> PR: https://git.openjdk.java.net/jdk17/pull/112

Rémi

Release of ASM 9.2

[Remi Forax]

Hi everybody,
we are happy to announce the release of ASM 9.2 which support Java 18 
(available via Maven Central).

I still hope to find the time this summer to work of the support of Valhalla 
new bytecodes.

Rémi

Re: JEP 411: Deprecation with removal would break most existing Java libraries

[Remi Forax]

- Mail original -
> De: "Rafael Winterhalter" 
> À: "core-libs-dev" 
> Envoyé: Dimanche 13 Juin 2021 22:28:33
> Objet: JEP 411: Deprecation with removal would break most existing Java 
> libraries

> I am currently looking into how I should address JEP 411 in my library Byte
> Buddy and I find it rather challenging. The problem I am facing is that I
> know of several users who rely on the security manager in their Java 8/11
> applications. I would like to continue to support those users' use cases as
> long as I support Java versions that contain the security manager, which
> will be for many years to come. At the same time, I would like to address
> the announced removal of the API and make sure that Byte Buddy can work
> without it prior to the deadline when the library in its current state
> would no longer link.

Can you tell us about more about the scenarii where people are relying on the 
uses of doPrivileged() by ByteBuddy ?

> 
> From my understanding of the intention of JEP 411, the API was supposed to
> be stubbed – similar to Android’s stubbing of the API - rather than being
> removed. However, with the announced deprecation for removal of
> AccessController and SecurityManager, I understand that I would need to
> fully remove the dispatching to work with future Java versions.
> 
> Furthermore, it is difficult to create a working facade for dispatching to
> the security manager only if it is available. Methods like
> AccessController.doPrivileged are caller sensitive and by adding a utility
> to a library, this utility would leak to any potential user. It would
> therefore require package-private dispatchers for any relevant package,
> which would lead to a lot of copy-paste to retain backwards compatibility
> (given that a library cannot assume to be run as a module).
> 
> Finally, removing the API would mean that Byte Buddy versions of the last
> ten years would no longer link in future JDKs. For Byte Buddy where new
> Java versions often require an update, that might not be a big issue but
> many other libraries do support the API, I don’t feel it would be a rather
> severe restriction and cause unnecessary breakage if API is removed, rather
> than stubbed. I am thinking of libraries like Netty here which are rather
> omnipresent and would suddenly no longer link, a concept that is unlikely
> intuitive to a lot of developers.

In my opinion, the best way to deal with that is to have two codes one which is 
compatible with an AccessController.doPrivileged and one without it, using a 
multi release jar.
I can see a Maven/Gradle plugin that takes a code that uses an AccessController 
and generate a new code with a higher Java version that remove the calls to 
AccessController,
as a kind of forward port.

But we are not yet there, AccessController has to be removed first.

> 
> Therefore, my question is: should SecurityManager, AccessController and the
> Policy APIs really be deprecated for removal? Rather, I think that the APIs
> should be deprecated, but be retained with stubbed implementations.
> System.getSecurityMananger would then always return null.
> System.setSecurityManager on the other hand could be deprecated for
> removal. This way, existing code could continue to work as if the security
> manager is not active, which already is the common scenario and would not
> cause any disruption at the small price of keeping a handful of some
> stubbed classes.

I guess the real question is when those classes will be removed.
If it's after several LTS releases, nobody will care. If it's tomorrow, that's 
another story.

> 
> Thanks for advice on how this is intended to be handled by library
> developers like me.
> Best regards, Rafael

regards,
Rémi

javac throws an AssertionError while compiling a switch on types

[Remi Forax]

Hi all,
javac does like this code, there is a check missing because javac should not 
reach Gen with a code like this.

Object o = null;
var value = switch(o) {
  default -> 0;
  case String s -> 0;
};
System.out.println(value);

An exception has occurred in the compiler (17-internal). Please file a bug 
against the Java compiler via the Java bug reporting page 
(http://bugreport.java.com) after checking the Bug Database 
(http://bugs.java.com) for duplicates. Include your program, the following 
diagnostic, and the parameters passed to the Java compiler in your report. 
Thank you.
java.lang.AssertionError
at jdk.compiler/com.sun.tools.javac.util.Assert.error(Assert.java:155)
at jdk.compiler/com.sun.tools.javac.util.Assert.check(Assert.java:46)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.handleSwitch(Gen.java:1310)
at 
jdk.compiler/com.sun.tools.javac.jvm.Gen.doHandleSwitchExpression(Gen.java:1238)
at 
jdk.compiler/com.sun.tools.javac.jvm.Gen.visitSwitchExpression(Gen.java:1202)
at 
jdk.compiler/com.sun.tools.javac.tree.JCTree$JCSwitchExpression.accept(JCTree.java:1381)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.genExpr(Gen.java:877)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.visitLetExpr(Gen.java:2376)
at 
jdk.compiler/com.sun.tools.javac.tree.JCTree$LetExpr.accept(JCTree.java:3288)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.genExpr(Gen.java:877)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.visitVarDef(Gen.java:1081)
at 
jdk.compiler/com.sun.tools.javac.tree.JCTree$JCVariableDecl.accept(JCTree.java:1028)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.genDef(Gen.java:610)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.genStat(Gen.java:645)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.genStat(Gen.java:631)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.genStats(Gen.java:682)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.visitBlock(Gen.java:1097)
at 
jdk.compiler/com.sun.tools.javac.tree.JCTree$JCBlock.accept(JCTree.java:1092)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.genDef(Gen.java:610)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.genStat(Gen.java:645)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.genMethod(Gen.java:967)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.visitMethodDef(Gen.java:930)
at 
jdk.compiler/com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:922)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.genDef(Gen.java:610)
at jdk.compiler/com.sun.tools.javac.jvm.Gen.genClass(Gen.java:2415)
at 
jdk.compiler/com.sun.tools.javac.main.JavaCompiler.genCode(JavaCompiler.java:737)
at 
jdk.compiler/com.sun.tools.javac.main.JavaCompiler.generate(JavaCompiler.java:1617)
at 
jdk.compiler/com.sun.tools.javac.main.JavaCompiler.generate(JavaCompiler.java:1585)
at 
jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:946)
at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:317)
at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:176)
at jdk.compiler/com.sun.tools.javac.Main.compile(Main.java:64)
at jdk.compiler/com.sun.tools.javac.Main.main(Main.java:50)

regards,
Rémi

case null vs case dominance

[Remi Forax]

Hi all,
the first part of the message is about javac error message that could be 
improved,
the second part is about the current spec being not very logical.

With this code

Object o = null;
var value = switch(o) {
  //case null -> 0;
  case Object __ -> 0;
  case null -> 0;
};
System.out.println(value);

The error message is
  PatternMatching101.java:70: error: this case label is dominated by a 
preceding case label
  case null -> 0;
  ^

The error message is wrong here, because it's 'case null' and you can put a 
case null where you want but below a total pattern, so the error mesage should 
reflect that.

Here is an example that compiles showing that case null can be below a case 
String, which it dominates

Object o = null;
var value = switch(o) {
  case String s -> 0;
  case null -> 0;
  default -> 0;
};

Also with default, the spec says that the code below should compile (and it 
works with javac),
because default and case null are disjoint in term of type, but it feels wrong 
to me.

Object o = null;
var value = switch(o) {
  default -> 0;
  case null -> 0;
};
System.out.println(value);

The problem is that sometimes 'default' acts as a total pattern sometimes it 
does not, if there is a case null. I wonder if it's not better to ask users to 
put the case null on top.

Rémi

Pattern matching, not informative error message when the binding name is missing

[Remi Forax]

Hi all,
with this code

  sealed interface Vehicle {}
  record Car(String owner, String color) implements Vehicle {}
  record Bus(String owner) implements Vehicle {}

  public static void example3() {
var vehicles = List.of(
new Car("Bob", "red"),
new Bus("Ana")
);
var tax = vehicles.stream()
.mapToInt(v -> switch(v) {
case Car -> 100;
case Bus -> 200;
default -> throw new AssertionError();
}).sum();
System.out.println("tax " + tax);
  }

If there is no binding name in the case,
  case Car
instead of
  case Car car
the error message is not very explicit

PatternMatching101.java:40: error: cannot find symbol
 case Car -> 100;
  ^
  symbol:   variable Car
  location: class PatternMatching101

I believe that in this case, it's possible to know that this is a switch on 
types, so provide a better error message.

regards,
Rémi

Pattern matching - exhaustiveness is botched

[Remi Forax]

I don't know if you know but using the latest version of the source,
this code does not compile.

  sealed interface Vehicle {}
  record Car(String owner, String color) implements Vehicle {}
  record Bus(String owner) implements Vehicle {}

  public static void example2() {
var vehicles = List.of(
new Car("Bob", "red"),
new Bus("Ana")
);
for(var vehicle: vehicles) {
  switch(vehicle) {
case Car car -> System.out.println("car !");
case Bus bus -> System.out.println("bus !");
//default -> throw new AssertionError();
  }
}
  }


PatternMatching101.java:25: error: the switch statement does not cover all 
possible input values
  switch(vehicle) {
  ^

same issue with a switch expression.

regards,
Rémi

Re: RFR: 8199318: add idempotent copy operation for Map.Entry

[Remi Forax]

- Mail original -
> De: "Peter Levart" 
> À: "Rémi Forax" , "core-libs-dev" 
> 
> Envoyé: Jeudi 3 Juin 2021 20:49:05
> Objet: Re: RFR: 8199318: add idempotent copy operation for Map.Entry

> On 02/06/2021 19:24, Rémi Forax wrote:
>> I foolishly hoped that nobody would subclass a class with `Immutable` in its
>> name,
>> oh boy i was wrong:)
> 
> 
> There's nothing wrong in subclassing an immutable class. As long as the
> subclass keeps being immutable. Was it subclassed and made mutable by that?

It has to be immutable all the way up, you have the same issue if the subclass 
is not final.

If you filter out guava an google cache, github get you 12 pages of result and 
only one stupid code
https://github.com/klayders/interlude/blob/95fd59a911d2baa8ac36ae6b877955aa4fbd095e/src/main/java/l2/gameserver/skills/SkillEntry.java#L12

A lot of code uses SimpleImmutableEntry as a pair, my hope is that the 
introduction of records will clean that practice.

That said, there is also several broken codes, mostly two issues,
either a.equals(n) is not equivalent to b.equals(a) or a.equals(b) is not 
equivalent to a.compareTo(b) == 0.

I kind of regret that the compiler does not provide automatically an 
implementation of compareTo if the record implements Comparable.
People sucks at writing compareTo and the resulting bugs are hard to 
find/reproduce.

> 
> 
> Peter

Rémi

Re: RFR: 8262891: Compiler implementation for Pattern Matching for switch (Preview) [v4]

[Remi Forax]

- Mail original -
> De: "Evgeny Mandrikov" 
> À: "build-dev" , "compiler-dev" 
> , "core-libs-dev"
> , "javadoc-dev" 
> Envoyé: Mardi 25 Mai 2021 11:32:03
> Objet: Re: RFR: 8262891: Compiler implementation for Pattern Matching for 
> switch (Preview) [v4]

> On Mon, 17 May 2021 19:01:01 GMT, Jan Lahoda  wrote:
> 
>>> Good work. There's a lot to take in here. I think overall, it holds up well 
>>> - I
>>> like how case labels have been extended to accommodate for patterns. In the
>>> front-end, I think there are some questions over the split between Attr and
>>> Flow - maybe it is unavoidable, but I'm not sure why some control-flow 
>>> analysis
>>> happens in Attr instead of Flow and I left some questions to make sure I
>>> understand what's happening.
>>> 
>>> In the backend it's mostly good work - but overall the structure of the 
>>> code,
>>> both in Lower and in TransPattern is getting very difficult to follow, given
>>> there are many different kind of switches each with its own little twist
>>> attached to it. It would be nice, I think (maybe in a separate cleanup?) if 
>>> the
>>> code paths serving the different switch kinds could be made more separate, 
>>> so
>>> that, when reading the code we can worry only about one possible code shape.
>>> That would make the code easier to document as well.
>>
>> @mcimadamore, @forax, thanks a lot for comments! I tried to apply the 
>> requested
>> changes in recent commits
>> (https://github.com/openjdk/jdk/pull/3863/commits/3fc2502a33cec20f39fe571eb25538ee3b459a9b
>> https://github.com/openjdk/jdk/pull/3863/commits/aeddb85e65bf77ed62dc7fa1ad00c29646d02c99
>> ).
>> 
>> I've also tried to update the implementation for the latest spec changes 
>> here:
>> https://github.com/openjdk/jdk/pull/3863/commits/54ba974e1aac00bbde1c3bd2627f01caaaeda09b
>> 
>> The spec changes are: total patterns are nullable; pattern matching ("new")
>> statement switches must be complete (as switch expressions).
>> 
>> Any further feedback is welcome!
> 
> Hi @lahodaj   ,
> 
> I tried `javac` built from this PR and for the following `Example.java`
> 
> 
> class Example {
>void example(Object o) {
>switch (o) {
>case String s && s.length() == 0 ->
>System.out.println("1st case");
>case String s && s.length() == 1 ->  // line 6
>System.out.println("2nd case");  // line 7
>case String s -> // line 8
>System.out.println("3rd case");  // line 9
>default ->   // line 10
>System.out.println("default case");  // line 11
>}
>}
> }
> 
> 
> execution of
> 
> 
> javac --enable-preview --release 17 Example.java
> javap -v -p Example.class
> 
> 
> produces
> 
> 
>  void example(java.lang.Object);
>descriptor: (Ljava/lang/Object;)V
>flags: (0x)
>Code:
>  stack=2, locals=7, args_size=2
> 0: aload_1
> 1: dup
> 2: invokestatic  #7  // Method
> 
> java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
> 5: pop
> 6: astore_2
> 7: iconst_0
> 8: istore_3
> 9: aload_2
>10: iload_3
>11: invokedynamic #13,  0 // InvokeDynamic
>#0:typeSwitch:(Ljava/lang/Object;I)I
>16: tableswitch   { // 0 to 2
>   0: 44
>   1: 74
>   2: 105
> default: 122
>}
>44: aload_2
>45: checkcast #17 // class java/lang/String
>48: astore4
>50: aload 4
>52: invokevirtual #19 // Method 
> java/lang/String.length:()I
>55: ifeq  63
>58: iconst_1
>59: istore_3
>60: goto  9
>63: getstatic #23 // Field
>java/lang/System.out:Ljava/io/PrintStream;
>66: ldc   #29 // String 1st case
>68: invokevirtual #31 // Method
>java/io/PrintStream.println:(Ljava/lang/String;)V
>71: goto  133
>74: aload_2
>75: checkcast #17 // class java/lang/String
>78: astore5
>80: aload 5
>82: invokevirtual #19 // Method 
> java/lang/String.length:()I
>85: iconst_1
>86: if_icmpeq 94
>89: iconst_2
>90: istore_3
>91: goto  9
>94: getstatic #23 // Field
>java/lang/System.out:Ljava/io/PrintStream;
>97: ldc   #37 // String 2nd case
>99: invokevirtual #31 // Method
>java/io/PrintStream.println:(Ljava/lang/String;)V
>   102: goto  133
>  

Re: JEP draft: Scope Locals

[Remi Forax]

Apart the performance, threadLocal has currently two issues that are addressed 
by scope locals.

ThreadLocal let the programmer defines the lifecycle, while it seems great on 
paper, it's a huge mess in reality.
Because not everybody wraps the calls to set()/remove() in a try/finally, so 
the values inside thread locals are not reclaimed by the GC.
It was less an issue before pooling because all the values are reclaimed when 
the thread die, but with pooling, threads do not die.
(I think this is the point made by Brian about ThreadLocal not interacting 
smoothly with ThreadLocal)

InheritableThreadLocal copies everything each time a new thread is created, 
again this lead to memory leaks because
if an unexpected threads is created, sadly some libraries do that, all values 
are now referenced by this new thread that may never die.

If you want more
https://www.google.com/search?hl=en=ThreadLocal%20memory%20leak

I think the JEP should be more explicit about the shortcoming of ThreadLocal 
and how the design of scope local fix both issues.

Rémi

- Mail original -
> De: "Brian Goetz" 
> À: "Andrew Haley" , "core-libs-dev" 
> , "loom-dev"
> 
> Envoyé: Mercredi 12 Mai 2021 20:57:33
> Objet: Re: JEP draft: Scope Locals

> Scope locals have come together nicely.
> 
> I have some vague thoughts on the presentation of the JEP draft.  There
> are (at least) three intertwined things in the motivation:
> 
>  - ThreadLocal (and ITL especially) were always compromises, and with
> the advent of Loom, have become untenable -- but the need for implicit
> parameters has not gone away
>  - Scoped locals, because of their effective finality and dynamic
> scoping, offer a programming model that is a better fit for virtual
> threads, but, even in the absence of virtual threads, are an enabler for
> structured concurrency
>  - The programming model constraints enable a better-performing
> implementation
> 
> In reading the draft, these separate motivations seem somewhat tangled.
> All the words make sense, but a reader has a hard time coming away with
> a clear picture of "so, why did we do this exactly, besides that its
> cool and faster?"
> 
> A possible way to untangle this is:
> 
>  - the underlying use cases: various forms of implicit context
> (transaction context, implicit parameters, leaving breadcrumbs for your
> future self.)
>  - the existing solution: thread locals.  ThreadLocals are effectively
> mutable per-thread globals.  The unconstrained mutability makes them
> hard to optimize.  ThreadLocals interact poorly with pooled threads.
>  - Here comes Loom!  We no longer need to pool threads.  So, why are
> TLs not good enough?
>  - The more constrained programming model of SLs enables two big new
> benefits:
>    - structured concurrency, which is applicable to virtual and
> non-virtual threads alike
>    - better optimization of inheritance and get operations
> 
> 
> 
> 
> 
> 
> 
> On 5/12/2021 10:42 AM, Andrew Haley wrote:
>> There's been considerable discussion about scope locals on the loom-dev list,
>> and it's now time to open this to a wider audience. This subject is important
>> because. although scope locals were motivated by the the needs of Loom, they
>> have many potential applications outside that project.
>>
>> The draft JEP is at
>>
>> https://bugs.openjdk.java.net/browse/JDK-8263012
>>
>> I've already received some very helpful suggestions for enhancements to
>> the API, and it'll take me a while to work through them all. In particular,
>> Paul Sandoz has suggested that I unify the classes Snapshot and Carrier,
>> and it will take me some time to understand the consequences of that.
>>
>> In the meantime, please have a look at the JEP and comment here.
>>
>>
>> For reference, earlier discussions are at
>>
>> https://mail.openjdk.java.net/pipermail/loom-dev/2021-March/002268.html
>> https://mail.openjdk.java.net/pipermail/loom-dev/2021-April/002287.html
>> https://mail.openjdk.java.net/pipermail/loom-dev/2021-May/002427.html

Re: [External] : Re: ReversibleCollection proposal

[Remi Forax]

- Mail original -
> De: "Peter Levart" 
> À: "Stuart Marks" 
> Cc: "core-libs-dev" 
> Envoyé: Mercredi 12 Mai 2021 08:28:07
> Objet: Re: [External] : Re: ReversibleCollection proposal

> On 5/12/21 2:55 AM, Stuart Marks wrote:
>> As you point out, add() is kind of like addLast(), except without the
>> reordering semantics for LinkedHashSet. And reversed().add() is a
>> roundabout way of saying addFirst() -- also without the reordering
>> semantics for LinkedHashSet. I think most people's reactions would be
>> "Why didn't they just provide addFirst/addLast?" Plus the reordering
>> would be missing for LHS.
>>
>> A second-order issue is performance. I'd expect that implementations
>> would want to provide a fast-path for addFirst() that is amenable to
>> JIT optimization; this seems harder to achieve with reversed().add().
> 
> 
> The allocation of a reversed view instance typically goes away when C2
> compiles the method (if the instance isn't cached like in
> AbstractMap.keySet/values) so this can be as performant as specialized
> addFirst(), but lack of reordering of existent element in LinkedHashSet
> is a different problem I haven thought about.

Don't forget that the default method implementation is just that a default 
method,
at least the JDK implementations like LinkedHashSet can/should provide a better 
implementation.

> 
> 
> Regards, Peter

regards,
Rémi

Re: Draft JEP: Reimplement Core Reflection on Method Handles

[Remi Forax]

Hi Mandy,
impressive work !

I think that the method that are a caller-sensitive adapter (the one that takes 
a supplementary class as last parameter) should be annotated with a specific 
JDK internal annotation,
so the link between the caller sensitive method and it's adapter is obvious for 
the humans that read the code.

Otherwise, i've only taken a look to the parts of the code that are using ASM.

This line is weird, it uses 52 which is Java 8
https://github.com/mlchung/jdk/commit/320efd2e5697627243f6fe058485fb8708a0cd41#diff-4e4fca8bb2eb6320ff485ee724248e1641b4bb3f6dbae8526e87c5cf15905d9aR1262

Perhaps all versions should be updated to 61 (Java 17), unit the internal 
version of ASM is refreshed so the constant V17 can be used.

Rémi

- Mail original -
> De: "mandy chung" 
> À: "core-libs-dev" , "hotspot compiler" 
> 
> Envoyé: Mardi 11 Mai 2021 22:42:01
> Objet: Draft JEP: Reimplement Core Reflection on Method Handles

> This draft JEP is a proposal to reimplement core reflection on top of
> method handles:
>    https://bugs.openjdk.java.net/browse/JDK-8266010
> 
> Feedback is welcome.  The prototype is at [1].
> 
> Mandy
> [1] https://github.com/mlchung/jdk/tree/reimplement-method-invoke

Re: Review CSR JDK-8266760: Remove sun.misc.Unsafe::defineAnonymousClass

[Remi Forax]

- Mail original -
> De: "mandy chung" 
> À: "core-libs-dev" , "valhalla-dev" 
> 
> Envoyé: Lundi 10 Mai 2021 21:26:33
> Objet: Review CSR JDK-8266760: Remove sun.misc.Unsafe::defineAnonymousClass

> Hidden classes were added in Java SE 15. Class data support was added in
> 16. `sun.misc.Unsafe::defineAnonymousClass` was deprecated in JDK 15 and
> deprecated for terminally removal in JDK 16.
> 
> I propose to remove `sun.misc.Unsafe::defineAnonymousClass` from JDK 17:
> CSR: https://bugs.openjdk.java.net/browse/JDK-8266760
> 
> Comments?

Do it.

I will be brave and not cry too much :)

> 
> Mandy

Rémi

Re: Collection::getAny discussion

[Remi Forax]

- Mail original -
> De: "Stephen Colebourne" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 30 Avril 2021 23:15:45
> Objet: Re: Collection::getAny discussion

> On Fri, 30 Apr 2021 at 19:50, Stuart Marks  wrote:
>> You're asking for something that's somewhat different, which you called the
>> "find
>> the first element when there is only one" problem. Here, there's a 
>> precondition
>> that
>> the collection have a single element. (It's not clear to me what should 
>> happen
>> if
>> the collection has zero or more than one element.)
> 
> I think any get() or getAny() method on Collection is semantically
> equivalent to iterator.next(). I'm not sure there is another viable
> option.

Thinking a little more about conflating "first" and "any".
I wonder if we have not already cross the Rubicon on that matter,

If we have a HashSet or any collections and using Stream.findFirst()
  var collection = new HashSet<>(...); 
  var result = collection.stream().findFirst().orElseThrow();

We will get the result of collection.iterator().next()

So adding a default method getFirst() on Collection that returns the result of 
iterator().next() is pretty coherent, no ?

[...]

> 
> Stephen

Rémi

Re: ReversibleCollection proposal

[Remi Forax]

- Mail original -
> De: "Stuart Marks" 
> À: "Peter Levart" 
> Cc: "core-libs-dev" , "Stephen Colebourne" 
> 
> Envoyé: Mercredi 28 Avril 2021 02:04:22
> Objet: Re: ReversibleCollection proposal

> On 4/27/21 2:25 AM, Peter Levart wrote:
>> Right, I'm persuaded. Now if only the problems of transition to the usage of 
>> new
>> type that Remi is worried about were mild enough. Source (in)compatibility is
>> not
>> that important if workarounds exist that are also backwards source 
>> compatible so
>> that the same codebase can be compiled with different JDKs. Binary 
>> compatibility
>> is
>> important. And I guess there is a variant of the proposal that includes
>> ReversibleCollection and ReversibleSet and is binary compatible.
> 
> Yes, the source incompatibilities with the types seem to involve type 
> inference
> (e.g., use of var choosing differently based on new types in the library) so 
> it
> should be possible to make minor source code changes to adjust or override the
> type
> that's inferred.
> 
> On binary compatibility, that comes mostly from the newly introduced methods
> (reversed, addFirst etc.) and from adding covariant overrides to 
> LinkedHashSet.
> I
> guess the "variant" you mention is adding new methods with the new return 
> types
> (e.g., reversibleEntrySet) instead of covariant overrides. Or was it something
> else?

Yes, you need type inference (compute lub), var is not necessary,
after all
  var a = f();
  g(a)
is equivalent to
  g(f())

So you have an issue with varargs like T..., any method that takes two T like 
m(T, T), or with the ternary operator ?:

At best you have a source compatibility issue, at worst, you have a runtime 
error or a runtime slowdown.
This is something that have not been study well but the worst scenario is when 
a VarHandle or a MethodHandle is involved IMO,
because a call like methodHandle.invokeExact() or VarHandle.compareAndSet will 
still compile if the inferred type changed but at runtime at best you will have 
a WrongMethodTypeException, at worst it will still work but performance will 
fall of the cliff.

But maybe nobody has ever written a code like
  methodHandle.invoke(flag? list: linkedHashMap)

Also the source compatibility issues are not only restricted to Java
- source compatiblity can also be a bigger issue than it was before for people 
that are using jshell/java as a shell script. 
- and what about the other JVM languages that are using the collection API, 
mostly Groovy and Kotlin because they are leveraging inference far more than in 
Java.
  Scala tends to use Scala collection, and Clojure or JRuby are dynamically 
typed so it should not be less an issue.

> 
> s'marks

Rémi

Re: New Collections interface - Sized

[Remi Forax]

- Mail original -
> De: "Stephen Colebourne" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 23 Avril 2021 11:23:03
> Objet: New Collections interface - Sized

> Hi all,
> While a discussion on ReversibleCollection is going on, I'd like to
> raise the interface I've always found most missing from the framework
> - Sized
> 
> public interface Sized {
>   int size();
>   default boolean isEmpty() {
> return size() == 0;
>   }
>   default boolean isNotEmpty() {
> return !isEmpty();
>   }
> }
> 
> This would be applied to Collection and Map, providing a missing
> unification. Anything else that has a size could also be retrofitted,
> such as String. Ideally arrays too, but that could come later. Note
> that Iterable would not implement it.
> 
> WDYT?

There are 3 ways to have instances of different classes to have a common 
behavior in Java,
- either thy implement a common super-type
- or you use a structural type conversion (a method ref on an instance)
  here:
Sized sized = list::size;
- or you use pattern matching (or a cascade of if/else if you are like the old 
fashion)
int size(Object o) {
  return switch(o) {
case List list -> list.size();
case Object[] array -> array.length();
case String s -> s.length();
default -> throw ...
  };
}

The advantage of pattern matching is that it does not have a global impact, out 
of where you want to use it,
by example, here, you can use s.length() instead of s.codepoints().count() 
without having to think what the default size of a String means for all program 
that can be written. 

> 
> Stephen

Rémi

Re: New Collections interface - Sized

[Remi Forax]

- Mail original -
> De: "Stephen Colebourne" 
> À: "core-libs-dev" 
> Envoyé: Samedi 24 Avril 2021 00:14:51
> Objet: Re: New Collections interface - Sized

> On Fri, 23 Apr 2021 at 23:07, Brian Goetz  wrote:
>> >> Is there a compelling example of where this would be used by clients?
>> > Here are some examples:
>> > https://stackoverflow.com/questions/10988634/java-global-isempty-method
>> Without passing judgment on the sort of dynamically typed programs that
>> need a method like this
> 
> The other example is better as it benefits from declaring an API that
> only accepts instances of `Sized` and does not need to get the
> contents.
> 
>> But again, if you are treating these things as containers, then a Sized
>> doesn't get you very far, because if you conclude the thing isn't empty,
>> you're going to want to get stuff out, and Sized has no methods for
>> that.  So presumably there's some companion to Sized for accessing
>> elements by index:
>>
>>  interface HasStuff extends Sized {
>>  T get(int index);
>>  }
> 
> I don't think there has to be. the more useful interface would be this
> one, but to date there has been strong resistance in unifying the
> Collection and Map interfaces:
> 
> interface Stuff extends Sized {
> int size();
> int isEmpty();
> int isNotEmpty();
>Iterator iterator();
> }

This is basically Spliterator, an iterator + a size, with the iterator is 
"push" instead of "pull" because it's more efficient.

In details a Spliterator is either
- an Iterable (with no SIZED characteristic)
- an Iterable + size (if SIZED and estimateSize() != Long.MAX_VALUE)
- an Iterable + comparator (if SORTED and comparator != null)
- an Iterable + split (if trySplit != null)

and all combinations of the above.

> 
> Stephen

Rémi

Re: ReversibleCollection proposal

[Remi Forax]

- Mail original -
> De: "Stephen Colebourne" 
> À: "core-libs-dev" 
> Envoyé: Jeudi 22 Avril 2021 00:14:10
> Objet: Re: ReversibleCollection proposal

> On Wed, 21 Apr 2021 at 18:39, Stuart Marks  wrote:
>> The value being provided here is that the ReversibleCollection interface
>> provides a
>> context within which specs no longer need to hedge about "if the collection 
>> has
>> an
>> ordering". APIs that produce or consume ReversibleCollection no longer need 
>> to
>> include this hedge, or have disclaimers about ordering. Potential new
>> ReversibleCollection implementations also need not worry about this issue in 
>> the
>> same way they did when they were forced to implement Collection directly.
> 
> Having thought about this for a few days my "interesting thought" is
> that adding a `ReversibleCollection` interface and adding additional
> methods can be orthogonal choices.
> 
> Specifically, I do rather like Peter's suggestion of adding more
> methods to Collection. Adding these methods would be pretty useful in
> general and give the proposed change much greater reach:
> 
> public interface Collection  {
> default void addFirst(E e) { throw new
> UnsupportedOperationException(); }
> default E getFirst() { return iterator().next(); }
> default E removeFirst() { var i = iterator(); i.next();
> i.remove(); }
> }
> 
> My view being that every collection has some notion of "first", even
> if that notion is "undefined". If that was the only change made, I
> think that would be a good move forward.
> 
> These would be the various options:
> 
> - 7 new methods on ReversibleCollection (Stuart's suggestion)
> - 7 new methods on Collection, no ReversibleCollection (Peter's suggestion)
> - 3 new methods on Collection, no ReversibleCollection (my suggestion #1)
> - 3 new methods on Collection, 4 methods on ReversibleCollection (my
> suggestion #2)
> - 7 new methods on Collection, 0 methods on ReversibleCollection (my
> suggestion #3)
> 
> FWIW, I think I prefer my suggestion #2

I would like to preserve the invariant that, when calling a method on a 
Collection/iterator, an UnsupportedOperationException only occurs when trying 
to mutate that collection.
If we are ok with that, this means that addFirst can not be a default method of 
Collection.

getFirst() on Collection is a nice addition has I already said.

And i'm not a big fan of removeFirst() on Collection, mostly because i think 
that the sequence this.iterator + next() + remove() is pretty rare on a 
Collection or a Set
(i would also like to keep the invariant that Set and Collection have the same 
set of methods).

So addFirst/addLast/removeFirst/removeLast should be added to 
SortedSet/NavigableSet, Deque, List and LinkedHashMap (if there are not already 
present).

For reverse/descending, I still prefer to have descendingSet() on Set and 
descendingList() on List, the same way we have keySet() or subList() (the 
method names have the resulting interface has suffix).
This also solve the issue with LinkedList implementing both List and Deque.

I believe that the cost of introducing ReversibleCollection is too high
- it's not source backward compatible change
- replacing Collection/Set by ReversibleCollection/ReversibleSet has return 
type of an existing interface/class is not a source backward compatible change
- to be able to use ReversibleCollection parameter of a public method of a 
library, we have to wait the whole Java ecosystem to have been updated to 
implement ReversibleCollection (Python 2/3 like migration).

> 
> Stephen

Rémi

Re: ReversibleCollection proposal

[Remi Forax]

- Mail original -
> De: "Stuart Marks" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 16 Avril 2021 19:40:55
> Objet: ReversibleCollection proposal

> This is a proposal to add a ReversibleCollection interface to the Collections
> Framework. I'm looking for comments on overall design before I work on 
> detailed
> specifications and tests. Please send such comments as replies on this email
> thread.
> 
> Here's a link to a draft PR that contains the code diffs. It's prototype
> quality,
> but it should be good enough to build and try out:
> 
> https://github.com/openjdk/jdk/pull/3533
> 
> And here's a link to a class diagram showing the proposed additions:
> 
> 
> https://cr.openjdk.java.net/~smarks/ReversibleCollection/ReversibleCollectionDiagram.pdf
> 
> Thanks,
> 
> s'marks

Thinking a little bit about your proposal,
introducing an interface right in the middle of a hierarchy is not a backward 
compatible change
(you have an issue when the compiler has to use the lowest upper bound).

By example
  void m(List> list) { ... }

  var list = List.of(new LinkedHashSet(), List.of("foo"));
  m(list);  // does not compile anymore

currently the type of list is List> but with your proposal, 
the type will be List>

Rémi

Re: ReversibleCollection proposal

[Remi Forax]

- Mail original -
> De: "Stuart Marks" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 16 Avril 2021 19:40:55
> Objet: ReversibleCollection proposal

> This is a proposal to add a ReversibleCollection interface to the Collections
> Framework. I'm looking for comments on overall design before I work on 
> detailed
> specifications and tests. Please send such comments as replies on this email
> thread.
> 
> Here's a link to a draft PR that contains the code diffs. It's prototype
> quality,
> but it should be good enough to build and try out:
> 
> https://github.com/openjdk/jdk/pull/3533
> 
> And here's a link to a class diagram showing the proposed additions:
> 
> 
> https://cr.openjdk.java.net/~smarks/ReversibleCollection/ReversibleCollectionDiagram.pdf
> 
> Thanks,
> 
> s'marks
> 
> 
> # Ordering and Reversibility
> 
> 
> A long-standing concept that's been missing from collections is that of the
> positioning, sequencing, or arrangement of elements as a structural property 
> of
> a
> collection. (This is sometimes called the "iteration order" of a collection.)
> For
> example, a HashSet is not ordered, but a List is. This concept is mostly not
> manifested in the collections API.
> 
> Iterating a collection produces elements one after another in *some* sequence.
> The
> concept of "ordered" determines whether this sequence is defined or whether 
> it's
> a
> coincidence of implementation. What does "having an order" mean? It implies 
> that
> there is a first element and that each element has a successor. Since
> collections
> have a finite number of elements, it further implies that there is a last
> element
> that has no successor. However, it is difficult to discern whether a 
> collection
> has
> a defined order. HashSet generally iterates its elements in the same undefined
> order, and you can't actually tell that it's not a defined order.
> 
> Streams do have a notion of ordering ("encounter order") and this is 
> preserved,
> where appropriate, through the stream pipeline. It's possible to detect this 
> by
> testing whether its spliterator has the ORDERED characteristic. Any collection
> with
> a defined order will have a spliterator with this characteristic. However, 
> this
> is
> quite a roundabout way to determine whether a collection has a defined order.
> Furthermore, knowing this doesn't enable any additional operations. It only
> provides
> constraints on the stream's implementations (keeping the elements in order) 
> and
> provides stronger semantics for certain operations. For example, findFirst() 
> on
> an
> unordered stream is the same as findAny(), but actually finds the first 
> element
> if
> the stream is ordered.
> 
> The concept of ordering is thus present in the system but is surfaced only in 
> a
> fairly indirect way. We can strengthen abstraction of ordering by making a few
> observations and considering their implications.
> 
> Given that there is a first element and a last element, the sequence of 
> elements
> has
> two ends. It's reasonable to consider operations (add, get, remove) on either
> end.
> Indeed, the Deque interface has a full complement of operations at each end.
> This is
> an oft-requested feature on various other collections.
> 
> Each element except for the last has a successor, implying that each element
> except
> for the first has a predecessor. Thus it's reasonable to consider iterating 
> the
> elements from first to last or from last to first (that is, in forward or
> reverse
> order). Indeed, the concept of iterating in reverse order appears already in
> bits
> and pieces in particular places around the collections:
> 
>  - List has indexOf() and lastIndexOf()
>  - Deque has removeFirstOccurrence() and removeLastOccurrence()
>  - List has a ListIterator with hasPrevious/previous methods
>  - Deque and NavigableSet have descendingIterator methods
> 
> Given an ordered collection, though, there's no general way to iterate it in
> reverse
> order. Reversed iteration isn't the most common operation, but there are some
> frequent use cases, such as operating on elements in most-recently-added 
> order.
> Questions and bug reports about this have come up repeatedly over the years.
> 
> Unfortunately, iterating in reverse order is much harder than iterating in
> forward
> order. There are a variety of ways to iterate in forward order. For example,
> given a
> List, one can do any of the following:
> 
> for (var e : list) { ... }
> list.forEach(...)
> list.stream()
> list.toArray()
> 
> However, to iterate a list in reverse order, one must use an explicit loop 
> over
> ListIterator:
> 
> for (var it = list.listIterator(list.size()); it.hasPrevious(); ) {
> var e = it.previous();
> ...
> }
> 
> Streaming the elements of a List in reverse order is even worse. One approach
> would
> be to implement a reverse-ordered Iterator that wraps a ListIterator and
> delegates
> hasNext/next calls to the ListIterator's 

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxilary classes

[Remi Forax]

- Mail original -
> De: "Rafael Winterhalter" 
> À: "core-libs-dev" , "serviceability-dev" 
> 
> Envoyé: Vendredi 16 Avril 2021 15:52:07
> Objet: RFR: 8200559: Java agents doing instrumentation need a means to define 
> auxilary classes

> To allow agents the definition of auxiliary classes, an API is needed to allow
> this. Currently, this is often achieved by using `sun.misc.Unsafe` or
> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed 
> from
> `sun.misc.Unsafe`.

You can already use Lookup.defineClass() + privateLookupIn() + 
Instrumentation.redefineModule() for that ?

Rémi

> 
> -
> 
> Commit messages:
> - 8200559: Java agents doing instrumentation need a means to define auxiliary
> classes
> 
> Changes: https://git.openjdk.java.net/jdk/pull/3546/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3546=00
>  Issue: https://bugs.openjdk.java.net/browse/JDK-8200559
>  Stats: 185 lines in 4 files changed: 185 ins; 0 del; 0 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/3546.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3546/head:pull/3546
> 
> PR: https://git.openjdk.java.net/jdk/pull/3546

Re: ObjectMethods seems generating wrong methods for array-type field

[Remi Forax]

- Mail original -
> De: "Kengo TODA" 
> À: "core-libs-dev" 
> Envoyé: Mercredi 14 Avril 2021 11:03:14
> Objet: Re: ObjectMethods seems generating wrong methods for array-type field

Hello,

> I found that the JLS 16 does not cover the array type record component:
> https://docs.oracle.com/javase/specs/jls/se16/html/jls-8.html#jls-8.10.3
> 
> So it could be not a core-lib issue but a JLS issue.

Nope, the implementation of a record is covered in the javadoc of 
java.lang.Record
for equals, see 
https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/lang/Record.html#equals(java.lang.Object)

As you can see the spec clearly says that for any fields which is an object 
(arrays are objects), it should be semantically equivalent to calling 
java.util.Objects.equals() on those fields.

So it's not a bug, it's how record.equals() works, as Raffaello said, records 
are dumb containers, this is how we have specified them.

Records have been specified as part of the project Amber, you can take a look 
to the mailing list [1] for more if you want.

> 
> Do I need to forward this thread to another mailing list?
> If so, please let me know which is the preferred one.
> I've checked https://mail.openjdk.java.net/mailman/listinfo but not so sure.
> 
> 
> Thank you very much.

regards,
Rémi

[1] https://mail.openjdk.java.net/mailman/listinfo/amber-spec-experts

> 
> ***
> Kengo TODA
> skypen...@gmail.com
> 
> 
> On Wed, Apr 14, 2021 at 9:04 AM Kengo TODA  wrote:
> 
>> Hello there,
>>
>>
>> I'm Kengo TODA, an engineer learning about the Record feature.
>> Today I found a nonintentional behavior, and it seems that the bug
>> database has no info about it.
>> Let me ask here to confirm it's by-design or not. If it's a bug, I want to
>> try to send a patch to OpenJDK.
>>
>> Here is the code reproducing the nonintentional behavior:
>> https://gist.github.com/KengoTODA/4d7ef6a5226d347ad9385241fee6bc63
>>
>> In short, the ObjectMethods class in OpenJDK v16 generates code
>> that invokes the fields' method, even when the field is an array.
>>
>> Please help me to understand this behavior, or
>> make an entry in the bug database to propose a patch.
>> Thanks in advance! :)
>>
>> ***
>> Kengo TODA
>> skypen...@gmail.com

Re: RFR: 8263087: Add a MethodHandle combinator that switches over a set of MethodHandles

[Remi Forax]

- Mail original -
> De: "Jorn Vernee" 
> À: "core-libs-dev" 
> Envoyé: Mardi 13 Avril 2021 16:59:58
> Objet: Re: RFR: 8263087: Add a MethodHandle combinator that switches over a 
> set of MethodHandles

> On Thu, 8 Apr 2021 18:51:21 GMT, Jorn Vernee  wrote:
> 
>> This patch adds a `tableSwitch` combinator that can be used to switch over a 
>> set
>> of method handles given an index, with a fallback in case the index is out of
>> bounds, much like the `tableswitch` bytecode. Here is a description of how it
>> works (copied from the javadoc):
>> 
>>  Creates a table switch method handle, which can be used to switch over 
>> a set of
>>  target
>>  method handles, based on a given target index, called selector.
>> 
>>  For a selector value of {@code n}, where {@code n} falls in the range 
>> {@code [0,
>>  N)},
>>  and where {@code N} is the number of target method handles, the table 
>> switch
>>  method
>>  handle will invoke the n-th target method handle from the list of 
>> target method
>>  handles.
>> 
>>  For a selector value that does not fall in the range {@code [0, N)}, 
>> the table
>>  switch
>>  method handle will invoke the given fallback method handle.
>> 
>>  All method handles passed to this method must have the same type, with 
>> the
>>  additional
>>  requirement that the leading parameter be of type {@code int}. The 
>> leading
>>  parameter
>>  represents the selector.
>> 
>>  Any trailing parameters present in the type will appear on the returned 
>> table
>>  switch
>>  method handle as well. Any arguments assigned to these parameters will 
>> be
>>  forwarded,
>>  together with the selector value, to the selected method handle when 
>> invoking
>>  it.
>> 
>> The combinator does not support specifying the starting index, so the switch
>> cases always run from 0 to however many target handles are specified. A
>> starting index can be added manually with another combination step that 
>> filters
>> the input index by adding or subtracting a constant from it, which does not
>> affect performance. One of the reasons for not supporting a starting index is
>> that it allows for more lambda form sharing, but also simplifies the
>> implementation somewhat. I guess an open question is if a convenience 
>> overload
>> should be added for that case?
>> 
>> Lookup switch can also be simulated by filtering the input through an 
>> injection
>> function that translates it into a case index, which has also proven to have
>> the ability to have comparable performance to, or even better performance 
>> than,
>> a bytecode-native `lookupswitch` instruction. I plan to add such an injection
>> function to the runtime libraries in the future as well. Maybe at that point 
>> it
>> could be evaluated if it's worth it to add a lookup switch combinator as 
>> well,
>> but I don't see an immediate need to include it in this patch.
>> 
>> The current bytecode intrinsification generates a call for each switch case,
>> which guarantees full inlining of the target method handles. Alternatively we
>> could only have 1 callsite at the end of the switch, where each case just 
>> loads
>> the target method handle, but currently this does not allow for inlining of 
>> the
>> handles, since they are not constant.
>> 
>> Maybe a future C2 optimization could look at the receiver input for 
>> invokeBasic
>> call sites, and if the input is a phi node, clone the call for each constant
>> input of the phi. I believe that would allow simplifying the bytecode without
>> giving up on inlining.
>> 
>> Some numbers from the added benchmarks:
>> 
>> Benchmark(numCases)  (offset)  
>> (sorted)
>> Mode  Cnt   Score   Error  Units
>> MethodHandlesTableSwitchConstant.testSwitch   5 0   
>> N/A
>> avgt   30   4.186 � 0.054  ms/op
>> MethodHandlesTableSwitchConstant.testSwitch   5   150   
>> N/A
>> avgt   30   4.164 � 0.057  ms/op
>> MethodHandlesTableSwitchConstant.testSwitch  10 0   
>> N/A
>> avgt   30   4.124 � 0.023  ms/op
>> MethodHandlesTableSwitchConstant.testSwitch  10   150   
>> N/A
>> avgt   30   4.126 � 0.025  ms/op
>> MethodHandlesTableSwitchConstant.testSwitch  25 0   
>> N/A
>> avgt   30   4.137 � 0.042  ms/op
>> MethodHandlesTableSwitchConstant.testSwitch  25   150   
>> N/A
>> avgt   30   4.113 � 0.016  ms/op
>> MethodHandlesTableSwitchConstant.testSwitch  50 0   
>> N/A
>> avgt   30   4.118 � 0.028  ms/op
>> MethodHandlesTableSwitchConstant.testSwitch  50   150   
>> N/A
>> avgt   30   4.127 � 0.019  ms/op
>> MethodHandlesTableSwitchConstant.testSwitch 100 0   
>> N/A
>> avgt   30   4.116 � 0.013  ms/op
>> MethodHandlesTableSwitchConstant.testSwitch 100   150   

Re: [External] : Re: RFR: 8263087: Add a MethodHandle combinator that switches over a set of MethodHandles

[Remi Forax]

> De: "John Rose" 
> À: "Remi Forax" 
> Cc: "Jorn Vernee" , "core-libs-dev"
> 
> Envoyé: Samedi 10 Avril 2021 01:43:49
> Objet: Re: [External] : Re: RFR: 8263087: Add a MethodHandle combinator that
> switches over a set of MethodHandles

> On Apr 9, 2021, at 4:00 PM, John Rose < [ mailto:john.r.r...@oracle.com |
> john.r.r...@oracle.com ] > wrote:

>> The MH combinator for lookupswitch can use a data-driven
>> reverse lookup in a (frozen/stable) int[] array, using binary
>> search. The bytecode emitter can render such a thing as
>> an internal lookupswitch, if that seems desirable. But
>> the stable array with binary search scales to other types
>> besides int, so it’s the right primitive.

> This may be confusing on a couple of points.
> First, the mapping function I’m talking about is not
> a MH combinator, but rather a MH factory, which takes
> a non-MH argument, probably an unsorted array or List
> of items of any type. It then DTRT (internal hash map
> or tree map or flat binary search or flat table lookup or
> lookupswitch or any combination thereof) to get
> an algorithm to classify the array or List elements
> into a compact enumeration [0,N).

> Second, when the input array or List is of int (or
> Integer) then it *might* be a lookupswitch internally,
> and I’m abusing the terminology to call this particular
> case a lookupswitch. But it’s really just a classifier
> factory, whose output is a MH of type K → [0,N) for
> some K. The output might also be ToIntFunction
> for all I care; that can be inter-converted with a MH.

As you said, the classifier is either a lookup switch or a hashmap.get() or a 
perfect hash function like ordinal(). 
The last two can be already be seen as MH, that you can already compose. 
The only one we can not currently, without generating bytecode, is the lookup 
switch, so we should have a lookupswitch combinator. 

This does not mean we do not need the tableswitch combinator, it means we need 
both. 

Firthermore, if we do have both combinators, there is no need to a special 
mechanism, or am i missing something ? 

Rémi 

Re: RFR: 8263087: Add a MethodHandle combinator that switches over a set of MethodHandles

[Remi Forax]

- Mail original -
> De: "Jorn Vernee" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 9 Avril 2021 12:51:53
> Objet: RFR: 8263087: Add a MethodHandle combinator that switches over a set 
> of MethodHandles

> This patch adds a `tableSwitch` combinator that can be used to switch over a 
> set
> of method handles given an index, with a fallback in case the index is out of
> bounds, much like the `tableswitch` bytecode.
> 
> The combinator does not support specifying the starting index, so the switch
> cases always run from 0 to however many target handles are specified. A
> starting index can be added manually with another combination step that 
> filters
> the input index by adding or subtracting a constant from it, which does not
> affect performance. One of the reasons for not supporting a starting index is
> that it allows for more lambda form sharing, but also simplifies the
> implementation somewhat. I guess an open question is if a convenience overload
> should be added for that case?

I think the combinator should be lookupswitch which is more general than 
tableswitch with a special case when generating the bytecode to generate a 
tableswitch instead of a lookupswitch if the indexes are subsequent.

> 
> Lookup switch can also be simulated by filtering the input through an 
> injection
> function that translates it into a case index, which has also proven to have
> the ability to have comparable performance to, or even better performance 
> than,
> a bytecode-native `lookupswitch` instruction. I plan to add such an injection
> function to the runtime libraries in the future as well. Maybe at that point 
> it
> could be evaluated if it's worth it to add a lookup switch combinator as well,
> but I don't see an immediate need to include it in this patch.
> 

As i said in the bug when we discuss about that the filtering function,
i believe that the filtering function for emulating lookupswitch is 
lookupswitch itself.

> The current bytecode intrinsification generates a call for each switch case,
> which guarantees full inlining of the target method handles. Alternatively we
> could only have 1 callsite at the end of the switch, where each case just 
> loads
> the target method handle, but currently this does not allow for inlining of 
> the
> handles, since they are not constant.

This scheme also allows to never JIT compile a branch which is never used.

> 
> Maybe a future C2 optimization could look at the receiver input for 
> invokeBasic
> call sites, and if the input is a phi node, clone the call for each constant
> input of the phi. I believe that would allow simplifying the bytecode without
> giving up on inlining.
> 
> Some numbers from the added benchmarks:
> Benchmark(numCases)  (offset)  
> (sorted)
> Mode  Cnt   Score   Error  Units
> MethodHandlesTableSwitchConstant.testSwitch   5 0   
> N/A
> avgt   30   4.186 � 0.054  ms/op
> MethodHandlesTableSwitchConstant.testSwitch   5   150   
> N/A
> avgt   30   4.164 � 0.057  ms/op
> MethodHandlesTableSwitchConstant.testSwitch  10 0   
> N/A
> avgt   30   4.124 � 0.023  ms/op
> MethodHandlesTableSwitchConstant.testSwitch  10   150   
> N/A
> avgt   30   4.126 � 0.025  ms/op
> MethodHandlesTableSwitchConstant.testSwitch  25 0   
> N/A
> avgt   30   4.137 � 0.042  ms/op
> MethodHandlesTableSwitchConstant.testSwitch  25   150   
> N/A
> avgt   30   4.113 � 0.016  ms/op
> MethodHandlesTableSwitchConstant.testSwitch  50 0   
> N/A
> avgt   30   4.118 � 0.028  ms/op
> MethodHandlesTableSwitchConstant.testSwitch  50   150   
> N/A
> avgt   30   4.127 � 0.019  ms/op
> MethodHandlesTableSwitchConstant.testSwitch 100 0   
> N/A
> avgt   30   4.116 � 0.013  ms/op
> MethodHandlesTableSwitchConstant.testSwitch 100   150   
> N/A
> avgt   30   4.121 � 0.020  ms/op
> MethodHandlesTableSwitchOpaqueSingle.testSwitch   5 0   
> N/A
> avgt   30   4.113 � 0.009  ms/op
> MethodHandlesTableSwitchOpaqueSingle.testSwitch   5   150   
> N/A
> avgt   30   4.149 � 0.041  ms/op
> MethodHandlesTableSwitchOpaqueSingle.testSwitch  10 0   
> N/A
> avgt   30   4.121 � 0.026  ms/op
> MethodHandlesTableSwitchOpaqueSingle.testSwitch  10   150   
> N/A
> avgt   30   4.113 � 0.021  ms/op
> MethodHandlesTableSwitchOpaqueSingle.testSwitch  25 0   
> N/A
> avgt   30   4.129 � 0.028  ms/op
> MethodHandlesTableSwitchOpaqueSingle.testSwitch  25   150   
> N/A
> avgt   30   4.105 � 0.019  ms/op
> MethodHandlesTableSwitchOpaqueSingle.testSwitch  50 0   
> N/A
> avgt   30   4.097 � 0.021  ms/op
> MethodHandlesTableSwitchOpaqueSingle.testSwitch  50   150   
> N/A
> avgt   30   4.131 � 0.037  

Re: Instrumenting Executors - issues in Spring Cloud Sleuth and JDK16

[Remi Forax]

- Mail original -
> De: "Marcin Grzejszczak" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 9 Avril 2021 16:29:32
> Objet: Instrumenting Executors - issues in Spring Cloud Sleuth and JDK16

> Hi!
> 
> I'm the lead of Spring Cloud Sleuth [1], a project dedicated to working with
> distributed tracing. We're propagating the tracing context e.g. through
> threads. That means that when a user spawns a new thread we need to pass the
> context from the old thread to the new one. Example - if the user uses an
> Executor or an ExecutorService (e.g. via calling the execute(Runnable r)
> method) then we need to wrap the Runnable in its trace representation. That
> means that we retrieve the context from Thread A , pass it in the constructor
> of the TraceRunnable and then restore it once the run method is called in
> Thread B.
> 
> The problem in Sleuth that we have with JDK16 is that we can't use reflection 
> to
> ensure that we're wrapping all methods of any Executors [2]. In other words we
> want to create a proxy around an existing Executor and wrap all methods.
> Currently, we're using reflection cause Executor implementations such as
> `ScheduledThreadPoolExecutor` have quite a few protected methods that we can't
> access. What we would like to achieve is a delegation mechanism that we can
> wrap all objects that the given Executor is using within their API (such as
> Runnables, Callables, other Executors) in their trace representation and then
> delegate calls for all methods to the wrapped object. That would also mean the
> delegation to currently protected methods.
> 
> If there's another way to achieve this other than opening the
> java.util.concurrent API then I would very much like to use it. Currently with
> JDK16 it's not possible to instrument that code so context propagation might 
> be
> buggy when dealing with executors.

I'm not sure if you are using an agent or not, if you are using an agent, you 
can redefine a module
https://docs.oracle.com/en/java/javase/16/docs/api/java.instrument/java/lang/instrument/Instrumentation.html#redefineModule(java.lang.Module,java.util.Set,java.util.Map,java.util.Map,java.util.Set,java.util.Map)

regards,
Rémi

> 
> Marcin Grzejszczak
> Staff Engineer, Spring Cloud

Re: RFR: 8263668: Update java.time to use instanceof pattern variable

[Remi Forax]

- Mail original -
> De: "Michael Kuhlmann" 
> À: "core-libs-dev" 
> Envoyé: Mercredi 24 Mars 2021 13:23:08
> Objet: Re: RFR: 8263668: Update java.time to use instanceof pattern variable

> On 3/24/21 12:09 PM, Rémi Forax wrote:
>> On Wed, 24 Mar 2021 09:56:16 GMT, Patrick Concannon 
>> wrote:
>> 
>>> Hi,
>>>
>>> Could someone please review my code for updating the code in the `java.time`
>>> package to make use of the `instanceof` pattern variable?
>>>
>>> Kind regards,
>>> Patrick
>> 
>> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java 
>> line
>> 168:
>> 
>>> 166: private static final TemporalQuery QUERY_REGION_ONLY =
>>> (temporal) -> {
>>> 167: ZoneId zone = temporal.query(TemporalQueries.zoneId());
>>> 168: return (zone != null && (!(zone instanceof ZoneOffset)) ? zone 
>>> :
>>> null);
>> 
>> i find this code hard to read
>> `return (zone != null && (!(zone instanceof ZoneOffset))) ? zone : null;`
>> seems better`
> 
> The whole null check is not necessary.
> 
> `return zone instanceof ZoneOffset ? null : zone;`

yes,
you are right !

> 
>> -
>> 
>> PR: https://git.openjdk.java.net/jdk/pull/3170

Rémi

Re: New candidate JEP: 400: UTF-8 by Default

[Remi Forax]

- Mail original -
> De: "Bernd Eckenfels" 
> À: "core-libs-dev" 
> Cc: "jdk-dev" 
> Envoyé: Jeudi 11 Mars 2021 02:12:49
> Objet: Re: New candidate JEP: 400: UTF-8 by Default

> I like it. The only thing which I feel is missing would be an official API to
> get the operating environments default encoding (essentially to get the value
> used if COMPAT would have been specified).
> 
> For example, in our server application, we do have some code which is 
> specified
> as using exactly this charset  (I.e. if user configures 
> targetEncoding=PLATFORM
> we are using intentionally the no-arg APIs). We can change that code to 
> specify
> a Charset, but then we need a way to retrieve that - without poking into
> unsupported system properties or environment properties. For example
> System.platformCharset().
> 
> I understand that this might have it’s own complications - as not all OS have
> this concept (for example on Windows there might be different codepages
> depending on the unicode status of an application). But falling back to 
> today’s
> file.encoding code would at least be consistent and the behavior most
> implementer would desire when adapting legacy code to this JEP.

Hi,
FileReader has a method named getEncoding()

> 
> Gruss
> Bernd
> --
> http://bernd.eckenfels.net

regards,
Rémi

Re: Inconsistency in Constructor.getGenericParameterTypes()

[Remi Forax]

Hi Joe,
i think the overview of the package java.lang.reflect should discuss the fact 
that the reflection view is what is stored in the classfile, not exactly a 
reflection of the java code.

So depending on what you are requesting, you can see synthetic parameters 
generated by javac or only the Java view because the Java view is directly 
stored in an attributes like with generics

Rémi

- Mail original -
> De: "joe darcy" 
> À: "Oliver Drotbohm" 
> Cc: "core-libs-dev" 
> Envoyé: Lundi 1 Mars 2021 22:35:59
> Objet: Re: Inconsistency in Constructor.getGenericParameterTypes()

> Hi Oliver,
> 
> Perhaps the time has come to make a run at discussing this situation in
> the javadoc. One challenge in writing this material up is to phrase and
> structure the text so it offers a net-clarification of the situation. In
> other words, to not distract or confuse most readers on what is usually
> a tricky detail.
> 
> The @apiNote javadoc tag offers a mechanism to separate such discussion.
> 
> Thanks,
> 
> -Joe
> 
> On 2/26/2021 2:20 PM, Oliver Drotbohm wrote:
>> Hi Joe,
>>
>> thanks for the explanation. We switched to rather iterating over
>> ….getParameters() and take it from there. Do you think it makes sense to 
>> leave
>> a note about this in the Javadoc?
>>
>> Cheers,
>> Ollie
>>
>>> Am 26.02.2021 um 22:38 schrieb Joe Darcy :
>>>
>>> Hello Oliver,
>>>
>>> This is long-standing if surprising and under-documented behavior.
>>>
>>> The getGenericFoo methods, when generic type information is present, give a
>>> source-level view of the element. At a source level, the implicit outer this
>>> parameter is not present and thus omitted by
>>> constructor.getGenericParameterTypes for the constructor in question.
>>>
>>> HTH,
>>>
>>> -Joe
>>>
>>> On 2/26/2021 5:41 AM, Oliver Drotbohm wrote:
 Previously sent to the wrong list. Sorry for the double post.

 Von: Oliver Drotbohm 
 Betreff: Inconsistency in Constructor.getGenericParameterTypes()
 Datum: 25. Februar 2021 um 10:03:12 MEZ
 An: jdk-...@openjdk.java.net

 Hi all,

 we've just ran into the following issue: for a non-static, generic inner 
 class
 with a constructor declaring a generic parameter, a call to
 constructor.getGenericParameterTypes() does not return the enclosing class
 parameter type. Is that by intention? If so, what's the reasoning behind 
 that?

 Here's a the output of a reproducer (below):

 static class StaticGeneric - names: value, string
 static class StaticGeneric - parameters: [class java.lang.Object, class
 java.lang.String]
 static class StaticGeneric - generic parameters: [T, class 
 java.lang.String]

 class NonStaticGeneric - names: this$0, value, String
 class NonStaticGeneric - parameters: [class Sample, class 
 java.lang.Object,
 class java.lang.String]
 class NonStaticGeneric - generic parameters: [T, class java.lang.String]

 class NonStaticNonGeneric - names: this$0, String
 class NonStaticNonGeneric - parameters: [class Sample, class 
 java.lang.String]
 class NonStaticNonGeneric - generic parameters: [class Sample, class
 java.lang.String]

 Note how the constructor of the NonStaticGeneric type exposes three 
 parameter
 names, three parameter types but omits the enclosing class parameter in the
 list of generic parameter types.

 Tested on JDK 8 to 15. Same behavior.

 Cheers,
 Ollie


 class Sample {

public static void main(String[] args) {

Constructor first = 
 StaticGeneric.class.getDeclaredConstructors()[0];

System.out.println("static class StaticGeneric - names: "
+

 Arrays.stream(first.getParameters()).map(Parameter::getName).collect(Collectors.joining(",
")));
System.out.println("static class StaticGeneric - parameters: 
 " +
Arrays.toString(first.getParameterTypes()));
System.out.println(
"static class StaticGeneric - generic 
 parameters: " +

 Arrays.toString(first.getGenericParameterTypes()));

System.out.println();

Constructor second = 
 NonStaticGeneric.class.getDeclaredConstructors()[0];
System.out.println("class NonStaticGeneric - names: "
+

 Arrays.stream(second.getParameters()).map(Parameter::getName).collect(Collectors.joining(",
")));
System.out.println("class NonStaticGeneric - parameters: " +
Arrays.toString(second.getParameterTypes()));
System.out
.println(

Re: Class.getRecordComponents security checks

[Remi Forax]

- Mail original -
> De: "Attila Szegedi" 
> À: "core-libs-dev" 
> Envoyé: Dimanche 21 Février 2021 21:14:48
> Objet: Class.getRecordComponents security checks

> Hey folks,
> 
> Why are security checks for Class.getRecordComponents as strict as those for
> e.g. getDeclaredMethods? I would’ve expected they’d be as strict as those for
> e.g. getMethods. Specifically, the difference is the:
> 
>> “the caller's class loader is not the same as the class loader of this class 
>> and
>> invocation of s.checkPermission method with
>> RuntimePermission("accessDeclaredMembers") denies access to the declared
>> methods within this class”

Good question, getRecordComponents() list the record components not the record 
accessors,
while each record component has a corresponding accessor method, the reverse is 
not true.

so here, what you are asking is more like asking fields than methods, so it's 
more like getDeclaredFields() than getMethods(),
hence the runtime check if there is a SecurityManager enabled.

> 
> step. Aren’t record accessors supposed to be public?

yes, at least for the code generated by javac, accessors are always public
(the class itself may be non public, and the package may not be exported).

> 
> Attila.

Rémi

Re: System.getEnv(String name, String def)

[Remi Forax]

- Mail original -
> De: "Michael Kuhlmann" 
> À: "core-libs-dev" 
> Envoyé: Mardi 16 Février 2021 13:34:30
> Objet: Re: System.getEnv(String name, String def)

> Hi Rémi,
> 
> I don't want to be pedantic, but I see this as an anti-pattern. You
> would create an Optional just to immediately call orElse() on it. That's
> not how Optional should be used. (But you know that.)
> 
> It's described in Recipe 12 of this Java Magazine article, for instance:
> https://blogs.oracle.com/javamagazine/12-recipes-for-using-the-optional-class-as-its-meant-to-be-used

yep, you are right.
Optional.ofNullable(...).orElse(...) is not the best pattern in term of 
readability. 

> 
> Best,
> Michael

regards,
Rémi

> 
> On 2/15/21 3:09 PM, Remi Forax wrote:
>> Hi Loic,
>> You can use Optional.OfNullable() which is a kind of the general bridge 
>> between
>> the nullable world and the non-nullable one.
>> 
>>var fooOptional = Optional.ofNullable(System.getenv("FOO"));
>>var fooValue = fooOptional.orElse(defaultValue);
>> 
>> regards,
>> Rémi Forax
>> 
>> - Mail original -
>>> De: "Loïc MATHIEU" 
>>> À: "core-libs-dev" 
>>> Envoyé: Lundi 15 Février 2021 14:59:42
>>> Objet: System.getEnv(String name, String def)
>> 
>>> Hello,
>>>
>>> I wonder if there has already been some discussion to provide
>>> a System.getEnv(String name, String def) method that allows to return a
>>> default value in case the env variable didn't exist.
>>>
>>> When using system properties instead of env variable, we do have a
>>> System.getProperty(String key, String def) variant.
>>>
>>> Stating the JavaDoc of System.getEnv():
>>> *System properties and environment variables are both conceptually mappings
>>> between names and values*
>>>
>>> So if system properties and environment variables are similar concepts,
>>> they should provide the same functionalities right ?
>>>
>>> This would be very convenient as more and more people rely on
>>> environment variables these days to configure their applications.
>>>
>>> Regards,
>>>
> >> Loïc

Re: System.getEnv(String name, String def)

[Remi Forax]

Hi Loic,
You can use Optional.OfNullable() which is a kind of the general bridge between 
the nullable world and the non-nullable one.

  var fooOptional = Optional.ofNullable(System.getenv("FOO")); 
  var fooValue = fooOptional.orElse(defaultValue);

regards,
Rémi Forax

- Mail original -
> De: "Loïc MATHIEU" 
> À: "core-libs-dev" 
> Envoyé: Lundi 15 Février 2021 14:59:42
> Objet: System.getEnv(String name, String def)

> Hello,
> 
> I wonder if there has already been some discussion to provide
> a System.getEnv(String name, String def) method that allows to return a
> default value in case the env variable didn't exist.
> 
> When using system properties instead of env variable, we do have a
> System.getProperty(String key, String def) variant.
> 
> Stating the JavaDoc of System.getEnv():
> *System properties and environment variables are both conceptually mappings
> between names and values*
> 
> So if system properties and environment variables are similar concepts,
> they should provide the same functionalities right ?
> 
> This would be very convenient as more and more people rely on
> environment variables these days to configure their applications.
> 
> Regards,
> 
> Loïc

Re: RFR: 8252399: Update mapMulti documentation to use type test pattern instead of instanceof once JEP 375 exits preview [v3]

[Remi Forax]

Hi Patrick,

"Iterable" is Ok as runtime type of the classical instanceof but not in a type 
pattern where it is a raw type,
so
  if (e instanceof Iterable elements) {
should be
  if (e instanceof Iterable elements) {

regards,
Rémi

- Mail original -
> De: "Patrick Concannon" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 12 Février 2021 13:16:00
> Objet: Re: RFR: 8252399: Update mapMulti documentation to use type test 
> pattern instead of instanceof once JEP 375 exits
> preview [v3]

>> Hi,
>> 
>> Could someone please review my changeset for JDK-8252399: 'Update mapMulti
>> documentation to use type test pattern instead of instanceof once JEP 375 
>> exits
>> preview' ?
>> 
>> This change updates the example code displayed in the API documentation for
>> mapMulti to use the type test pattern introduced in
>> [JEP375](https://openjdk.java.net/jeps/375).
>> 
>> Kind regards,
>> Patrick
> 
> Patrick Concannon has updated the pull request incrementally with one 
> additional
> commit since the last revision:
> 
>  8252399: Added more appropriate test stream for Expand Iterable example
> 
> -
> 
> Changes:
>  - all: https://git.openjdk.java.net/jdk/pull/2544/files
>  - new: https://git.openjdk.java.net/jdk/pull/2544/files/1b8ca544..8f8dfe5c
> 
> Webrevs:
> - full: https://webrevs.openjdk.java.net/?repo=jdk=2544=02
> - incr: https://webrevs.openjdk.java.net/?repo=jdk=2544=01-02
> 
>  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/2544.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2544/head:pull/2544
> 
> PR: https://git.openjdk.java.net/jdk/pull/2544

Re: JDK-6824466 java.lang.reflect.Method should use java.lang.invoke.MethodHandle

[Remi Forax]

- Mail original -
> De: "Johannes Kuhn" 
> À: "core-libs-dev" 
> Envoyé: Lundi 1 Février 2021 15:50:51
> Objet: JDK-6824466 java.lang.reflect.Method should use 
> java.lang.invoke.MethodHandle

> While implementing a prototype for JDK-8242888 (Convert dynamic proxy to
> hidden classes) I came across the problem that hidden classes can't be
> mentioned in the constant pool, breaking the constructor for serialization.
> 
> To remedy that problem, I used a MHConstructorAccessor which delegates
> the actual work to MethodHandles - not unlike what JDK-6824466 suggests.
> 
> As there has been previous work in that area, (I am aware of at least 3
> independently developed prototypes for that bug/enchantment) I would
> like to ask a few questions around it:
> 
> 
> 
> 1) What are the challenges?
> 
> From the bug I could infer, that it's cold start is slower than
> NativeMethodAccessor, but still faster than the generated (bytecode
> spinning) accessors.
> 
> 2) Are there any roadblocks that prevent replacing the
> MethodAccessorGenerator with accessors that use MethodHandles?
> 
> From my limited tests, it appears to work well enough.
> 
> 3) Should I try to implement it?
> 
> 
> 
> From my POV, replacing MethodAccessorGenerator with accessors that
> delegate to MethodHandles has a few benefits:
> 
> * Support for hidden classes. (Currently fallback to native accessors)
> * Removal of MethodAccessorGenerator (which is old and clunky)

Hi Johannes,
the native accessors doesn't play with loom (a C stack frame in the middle of 
the Java stack can not be moved) so it doesn't seem to be a good idea to rely 
on the native accessors for hidden classes.

> 
> - Johannes

Rémi

Re: Why does Set.of disallow duplicate elements?

[Remi Forax]

Set.of() is the closest way we've got to a literal Set without having 
introduced a special syntax for that in the language.

The idea is that if you conceptually want to write
  Set set = { "hello", "world" };
instead, you write
  Set set = Set.of("hello", "world");

In that context, it makes sense to reject Set constructed with the same element 
twice because this is usually a programming error.
So
  Set.of("hello", "hello")
throws an IAE.

If you want a Set from a collection of elements, you can use
  Set.copyOf(List.of("hello", "hello"))

regards,
Rémi

- Mail original -
> De: "dfranken jdk" 
> À: "core-libs-dev" 
> Envoyé: Samedi 30 Janvier 2021 19:30:06
> Objet: Why does Set.of disallow duplicate elements?

> Dear users,
> 
> While looking at the implementation of Set.of(...) I noticed that
> duplicate elements are not allowed, e.g. Set.of(1, 1) will throw an
> IllegalArgumentException. Why has it been decided to do this?
> 
> My expectation was that duplicates would simply be removed.
> 
> If I do for instance new HashSet<>()
> it works and duplicates are removed. To me, it looks a bit inconsistent
> to have duplicates removed for a collection passed in the constructor,
> but not for a collection (even though it is a vararg array) passed to a
> static factory method.
> 
> Kind regards,
> 
> Dave Franken

It's not a bug but it's not user friendly

[Remi Forax]

A student of mine send me a code that can be reduced to this code
 
---
import java.lang.invoke.MethodHandles;
import java.lang.invoke.VarHandle;

public class ThereIsABugButWhere {
  private static final VarHandle TEXT;
  static {
try {
  TEXT = MethodHandles.lookup().findVarHandle(ThereIsABugButWhere.class, 
"text", String.class);
} catch (NoSuchFieldException | IllegalAccessException e) {
  throw new AssertionError(e);
}
  }


  private final String text;

  ThereIsABugButWhere() {
text = "FOO";
  }

  public void update(String s) {
TEXT.compareAndSet(this, "FOO", s);
  }

  public static void main(String[] args) {
new ThereIsABugButWhere().update("BAR");
  }
}

---
If you execute it, you get
Exception in thread "main" java.lang.UnsupportedOperationException
at java.base/java.lang.invoke.VarForm.getMemberName(VarForm.java:99)
at 
java.base/java.lang.invoke.VarHandleGuards.guard_LLL_Z(VarHandleGuards.java:77)
at ThereIsABugButWhere.update(ThereIsABugButWhere.java:22)
at ThereIsABugButWhere.main(ThereIsABugButWhere.java:26)

It takes me 20 mins to find the issue ...
I think we can improve the error message or even better report the issue at the 
right location :)

regards,
Rémi

Re: RFR: 8257596: Clarify trusted final fields for record classes

[Remi Forax]

- Mail original -
> De: "Mandy Chung" 
> À: "core-libs-dev" , "hotspot-runtime-dev" 
> 
> Envoyé: Mercredi 9 Décembre 2020 01:43:34
> Objet: Re: RFR: 8257596: Clarify trusted final fields for record classes

> On Tue, 8 Dec 2020 22:52:37 GMT, Mandy Chung  wrote:
> 
>> This is a follow-up on JDK-8255342 that removes non-specified JVM checks on
>> classes with Record attributes.  That introduces a regression in
>> `InstanceKlass::is_record` that returns true on a non-record class which has
>> `RecordComponents` attribute present.   This causes unexpected semantics in
>> `JVM_IsRecord` and `JVM_GetRecordComponents` and also a regression to trust
>> final fields for non-record classes.
>> 
>> I propose to change `InstanceKlass::is_record` to match the JLS semantic of a
>> record class, i.e. final direct subclass of `java.lang.Record` with the
>> presence of `RecordComponents` attribute.  There is no change to JVM class 
>> file
>> validation.   Also I propose clearly define:
>> - `JVM_IsRecord` returns true if the given class is a record i.e. final 
>> and
>> direct subclass of `java.lang.Record` with `RecordComponents` attribute
>> - `JVM_GetRecordComponents` returns an `RecordComponents` array  if
>> `RecordComponents` attribute is present; otherwise, returns NULL.  This 
>> does
>> not check if it's a record class or not.  So it may return non-null on a
>> non-record class if it has `RecordComponents` attribute.  So
>> `JVM_GetRecordComponents` returns the content of the classfile.
> 
> Hi Remi,
> 
>> It's not an issue, the JVM view of what a record is and the JLS view of what 
>> a
>> record is doesn't have to be identical,
>> only aligned. It's fine for the VM to consider any class that have a record
>> Attribute is a record.
>> 
>> We already had this discussion on amber-spec-expert list,
>> see
>> https://mail.openjdk.java.net/pipermail/amber-spec-experts/2020-November/002630.html
> 
> What is the conclusion (sorry it was unclear to me)?  Drop TNSFF for records?
> 
> This issue is to fix the regression introduced by JDK-8255342.   I expect
> someone else will file a new JBS issue and implement what amber-spec-experts
> decided.
> 
>> We already know that the JLS definition of a record will have to be changed 
>> for
>> inline record (an inline record is not direct subclass of j.l.Record because
>> you have the reference projection in the middle).
> 
> Yes I saw that.   I updated
> [JDK-8251041](https://bugs.openjdk.java.net/browse/JDK-8251041) to follow up.
> 
>> The real issue is that the JIT optimisation and Field.set() should be 
>> aligned,
>> VarHandle deosn't let you change a final field and Unsafe is unsafe, so the
>> current problem is that Field.set() relies on the reflection api by calling
>> Class.isRecord() which is not good because Classs.isRecord() returns the JLS
>> view of the world not the JVM view of the world.
>>
>> As said in the mail chain above, for the JIT optimization, instead of listing
>> all the new constructs, record, inline, etc, i propose to check the other 
>> way,
>> only allow to set a final field is only allowed for plain old java class, so
>> the VM should not have a method InstanceKlass::is_record but a method that
>> return if a class is a plain class or not and this method should be called by
>> the JIT and by Field.set (through a JVM entry point)
>> so the fact the optimization will be done or not is not related to what the 
>> JLS
>> think a record is, those are separated concern.
> 
> I agree the current situation is not ideal which requires to declare all the 
> new
> constructs explicitly for TNSFF.   However, it's a reasonable tradeoff to get
> the JIT optimization for records in time while waiting for true TNSFF
> investigation like JMM and other relevant specs.   I see this just a stop-gap
> solution.  When the long-term TNSFF is in place, the spec can be updated to
> drop the explicit list of record, inline, etc.
> 
> A related issue is
> [JDK-8233873](https://bugs.openjdk.java.net/browse/JDK-8233873).   I'm happy 
> if
> we can do TNSFF in a different solution.
> 
> Again this PR intends to fix the regression.  Two options:
> 1. Keep [JDK-8247444](https://bugs.openjdk.java.net/browse/JDK-8247444) and
> implement as this proposed patch
> 2. Backout [JDK-8247444](https://bugs.openjdk.java.net/browse/JDK-8247444)
> 
> I expect Chris and/or others will follow up the decision made by the
> amber-spec-experts w.r.t. trusting finals in records.   I'm okay with either
> option.

For me, it's backout JDK-8247444, as i said on the amber-spec-expert, i prefer 
VM to be oblivious about java.lang.Record.
And in the future, the real fix is to change the spec of Field.set() to say 
that it's only allowed for plain java classes and have the implementation to 
directly asks the VM is a non static field is trusted or not.

And there are also a related issue with the validation of the 
InnerClass/Enclosing attribute were the VM does a 

Re: RFR: 8257596: Clarify trusted final fields for record classes

[Remi Forax]

- Mail original -
> De: "Mandy Chung" 
> À: "core-libs-dev" , "hotspot-dev" 
> 
> Envoyé: Mardi 8 Décembre 2020 23:57:39
> Objet: RFR: 8257596: Clarify trusted final fields for record classes

Hi Mandy,

> This is a follow-up on JDK-8255342 that removes non-specified JVM checks on
> classes with Record attributes.  That introduces a regression in
> `InstanceKlass::is_record` that returns true on a non-record class which has
> `RecordComponents` attribute present.   This causes unexpected semantics in
> `JVM_IsRecord` and `JVM_GetRecordComponents` and also a regression to trust
> final fields for non-record classes.

It's not an issue, the JVM view of what a record is and the JLS view of what a 
record is doesn't have to be identical,
only aligned. It's fine for the VM to consider any class that have a record 
Attribute is a record.

We already had this discussion on amber-spec-expert list,
see 
https://mail.openjdk.java.net/pipermail/amber-spec-experts/2020-November/002630.html

We already know that the JLS definition of a record will have to be changed for 
inline record (an inline record is not direct subclass of j.l.Record because 
you have the reference projection in the middle).

The real issue is that the JIT optimisation and Field.set() should be aligned, 
VarHandle deosn't let you change a final field and Unsafe is unsafe, so the 
current problem is that Field.set() relies on the reflection api by calling 
Class.isRecord() which is not good because Classs.isRecord() returns the JLS 
view of the world not the JVM view of the world.

As said in the mail chain above, for the JIT optimization, instead of listing 
all the new constructs, record, inline, etc, i propose to check the other way, 
only allow to set a final field is only allowed for plain old java class, so 
the VM should not have a method InstanceKlass::is_record but a method that 
return if a class is a plain class or not and this method should be called by 
the JIT and by Field.set (through a JVM entry point)
so the fact the optimization will be done or not is not related to what the JLS 
think a record is, those are separated concern.

The more we inject the Java (the language) semantics in the JVM the less it is 
useful for other languages that run one the JVM.

Rémi

> 
> I propose to change `InstanceKlass::is_record` to match the JLS semantic of a
> record class, i.e. final direct subclass of `java.lang.Record` with the
> presence of `RecordComponents` attribute.  There is no change to JVM class 
> file
> validation.   Also I propose clearly define:
>- `JVM_IsRecord` returns true if the given class is a record i.e. final and
>direct subclass of `java.lang.Record` with `RecordComponents` attribute
>- `JVM_GetRecordComponents` returns an `RecordComponents` array  if
>`RecordComponents` attribute is present; otherwise, returns NULL.  This 
> does
>not check if it's a record class or not.  So it may return non-null on a
>non-record class if it has `RecordComponents` attribute.  So
>`JVM_GetRecordComponents` returns the content of the classfile.

Rémi

> 
> -
> 
> Commit messages:
> - 8257596: Clarify trusted final fields for record classes
> 
> Changes: https://git.openjdk.java.net/jdk/pull/1706/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1706=00
>  Issue: https://bugs.openjdk.java.net/browse/JDK-8257596
>  Stats: 60 lines in 4 files changed: 30 ins; 10 del; 20 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/1706.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1706/head:pull/1706
> 
> PR: https://git.openjdk.java.net/jdk/pull/1706

Re: Has it been considered to add inverse methods to collections which are in Optional?

[Remi Forax]

- Mail original -
> De: "Dave Franken" 
> À: "core-libs-dev" 
> Envoyé: Mardi 8 Décembre 2020 13:17:25
> Objet: Has it been considered to add inverse methods to collections which are 
> in Optional?

> Dear all,

Hi Dave,

> 
> The class java.util.Optional could be considered a collection with a limit
> on how many values it can hold, minimum 0, maximum 1.
> Likewise, any class implementing the regular java.util.Collection has a
> minimum, 0 and a maximum > 1.

Optional is more considered as a Stream with 0 or 1 element.
The primary usage of Optional is to temporary wraps a value (or not) as the 
result of a computation so it's not really like a collection which is a more 
"permanent" storage. 

> 
> While Optional does not implement Collection, it could be seen as a
> pseudo-collection in this regard.
> Optional has a convenience method for which there is an inverse,
> isPresent() which is the inverse of isEmpty().
> 
> Has it been considered to add such a method (or other convenience methods)
> to collections as well?
> For instance, java.util.Collection has the method isEmpty() which behaves
> exactly like Optional's isEmpty(), but it does not have an inverse
> counterpart.

Adding methods like notContains or notFilter have been considered several times 
in the past and rejected because it's too many methods with no real gain. I'm 
sure you can dig some old emails on lambda-dev and see the first iteration of 
the Stream API, it was full of convenient methods like that.
I'm sure Stuart or Brian have a template answer for that :)

> 
> I think it would be logical to add such as convenience method to
> Collection, because we have a precedent with Optional.
> We could add `bikeshed name here, as an example I'm going to use
> hasElements()' to java.util.Collection and, for simplicity's sake make the
> default implementation just call !isEmpty(); obviously implementations
> could offer optimized versions if necessary.
> 
> It would improve readability where we have the negation of isEmpty(), e.g.:
> 
> if (!myList.isEmpty()) {
> }
> 
> // vs.
> if (myList.hasElements()) {
> }
> 
> Note that in the first example, which is the current way of checking
> whether a (non null) collection has any elements, the negation and the
> method it negates are separated by the variable name.
> If the variable would instead be another method call or any other, longer,
> expression, this would make the example even less readable as the negation
> and final method call are further apart.
> 
> The second example has the advantage of more clearly expressing the intent
> of the user - which is to check if the collection has any elements.
> Moreover, it offers matching functionality for something which already
> exists in the JDK core libs, Optional's isPresent().

Apart what i've said above, Java inherits from C its weird compact syntax, "if 
(!myList.isEmpty())" instead of "if myList is not empty" like in HyperCard, i'm 
not sure that trying to solve that by adding one or more method is not trying 
to put lipstick to a pig. We are used to that pig since a long time to the 
point we don't think of it as a pig.

> 
> If this has already been discussed and dismissed, I'm sorry for bringing it
> up again and if anybody could point me to the conclusion and the reasoning
> behind it, I would be grateful.
> 
> Kind regards,

regards,

> 
> Dave Franken

Rémi

Why having the wrong InnerClasses attribute is an issue for the VM ?

[Remi Forax]

I've forgotten a cast in an invokedynamic, hence a call to wrongTargetType,
but in order to create the error message, MethodType.toString(), 
getSimpleName() is called and it fails because getDeclaringClass() verifies the 
InnerClasses attribute.

For me InnerClasses was just an attribute for javac not something the VM should 
take care of,
it seems that the VM strongly verifies this attribute and i wonder what is the 
reason behind that ?

regards,
Rémi

Exception in thread "main" java.lang.IncompatibleClassChangeError: 
fr.umlv.transmogrif.ImplMap and 
fr.umlv.transmogrif.ImplMap$Row/0x000801007400 disagree on InnerClasses 
attribute
at java.base/java.lang.Class.getDeclaringClass0(Native Method)
at java.base/java.lang.Class.isTopLevelClass(Class.java:1970)
at java.base/java.lang.Class.getSimpleBinaryName(Class.java:1955)
at java.base/java.lang.Class.getSimpleName0(Class.java:1835)
at java.base/java.lang.Class.getSimpleName(Class.java:1826)
at java.base/java.lang.Class.getSimpleName0(Class.java:1833)
at java.base/java.lang.Class.getSimpleName(Class.java:1826)
at java.base/java.lang.invoke.MethodType.toString(MethodType.java:895)
at java.base/java.lang.String.valueOf(String.java:3365)
at java.base/java.lang.StringBuilder.append(StringBuilder.java:169)
at 
java.base/java.lang.invoke.MethodHandle.standardString(MethodHandle.java:1611)
at 
java.base/java.lang.invoke.MethodHandle.toString(MethodHandle.java:1608)
at java.base/java.lang.String.valueOf(String.java:3365)
at 
java.base/java.lang.invoke.CallSite.wrongTargetType(CallSite.java:203)
at java.base/java.lang.invoke.CallSite.makeSite(CallSite.java:333)
at 
java.base/java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(MethodHandleNatives.java:280)
at 
java.base/java.lang.invoke.MethodHandleNatives.linkCallSite(MethodHandleNatives.java:270)
at 
fr.umlv.transmogrif.ImplMap/0x000801003c00.(ImplMap.java:21)
at fr.umlv.transmogrif.Main.main(Main.java:7)

Lookup.defineAnonymousClass() vs indy

[Remi Forax]

Hi Mandy, hi all,
it seems that when defineAnonymousClass rewrites the currentClass, it doesn't 
work if there is an invokedynamic in the classfile, so defineHiddenClass fails 
with a VerifyError when the hidden class is verified.

Here is an example showing the issue
---
import java.io.IOException;
import java.lang.invoke.MethodHandles;

public class HiddenClassWithIndy {
  public void test() {
var a = new HiddenClassWithIndy();
Runnable r = () -> System.out.println(a);
  }

  public static void main(String[] args) throws IOException, 
IllegalAccessException {
byte[] bytecode;
try(var input = 
HiddenClassWithIndy.class.getClassLoader().getResourceAsStream(HiddenClassWithIndy.class.getName().replace('.',
 '/') + ".class")) {
  if (input == null) {
throw new AssertionError();
  }
  bytecode = input.readAllBytes();
}
var hiddenLookup = MethodHandles.lookup().defineHiddenClass(bytecode, true);
  }
}

---
The error message:
Exception in thread "main" java.lang.VerifyError: Bad type on operand stack
Exception Details:
  Location:
fr/umlv/transmogrif/HiddenClassWithIndy+0x000801002400.test()V @9: 
invokedynamic
  Reason:
Type 'fr/umlv/transmogrif/HiddenClassWithIndy+0x000801002400' (current 
frame, stack[0]) is not assignable to 'fr/umlv/transmogrif/HiddenClassWithIndy'
  Current Frame:
bci: @9
flags: { }
locals: { 'fr/umlv/transmogrif/HiddenClassWithIndy+0x000801002400', 
'fr/umlv/transmogrif/HiddenClassWithIndy+0x000801002400' }
stack: { 'fr/umlv/transmogrif/HiddenClassWithIndy+0x000801002400' }
  Bytecode:
000: bb00 0759 b700 094c 2bba 000a  4db1
010:

at java.base/java.lang.ClassLoader.defineClass0(Native Method)
at java.base/java.lang.System$2.defineClass(System.java:2193)
at 
java.base/java.lang.invoke.MethodHandles$Lookup$ClassDefiner.defineClass(MethodHandles.java:2235)
at 
java.base/java.lang.invoke.MethodHandles$Lookup$ClassDefiner.defineClassAsLookup(MethodHandles.java:2216)
at 
java.base/java.lang.invoke.MethodHandles$Lookup.defineHiddenClass(MethodHandles.java:1952)
at 
fr.umlv.transmogrif.HiddenClassWithIndy.main(HiddenClassWithIndy.java:20)

regards,
Rémi

Re: RFR: 8180352: Add Stream.toList() method [v4]

[Remi Forax]

- Mail original -
> De: "Stuart Marks" 
> À: "core-libs-dev" 
> Envoyé: Mardi 24 Novembre 2020 08:10:14
> Objet: Re: RFR: 8180352: Add Stream.toList() method [v4]

>> This change introduces a new terminal operation on Stream. This looks like a
>> convenience method for Stream.collect(Collectors.toList()) or
>> Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this
>> method directly on Stream enables it to do what can't easily by done by a
>> Collector. In particular, it allows the stream to deposit results directly 
>> into
>> a destination array (even in parallel) and have this array be wrapped in an
>> unmodifiable List without copying.
>> 
>> In the past we've kept most things from the Collections Framework as
>> implementations of Collector, not directly on Stream, whereas only 
>> fundamental
>> things (like toArray) appear directly on Stream. This is true of most
>> Collections, but it does seem that List is special. It can be a thin wrapper
>> around an array; it can handle generics better than arrays; and unlike an
>> array, it can be made unmodifiable (shallowly immutable); and it can be
>> value-based. See John Rose's comments in the bug report:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8180352?focusedCommentId=14133065=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14133065
>> 
>> This operation is null-tolerant, which matches the rest of Streams. This 
>> isn't
>> specified, though; a general statement about null handling in Streams is
>> probably warranted at some point.
>> 
>> Finally, this method is indeed quite convenient (if the caller can deal with
>> what this operation returns), as collecting into a List is the most common
>> stream terminal operation.
> 
> Stuart Marks has updated the pull request incrementally with one additional
> commit since the last revision:
> 
>  Add comment and assert regarding array class; use switch expression.
> 
> -

Hi Stuart,
This version is Ok for me.

I still think that using a null friendly List is a mistake, but you, Brian and 
John all think it's not an issue, so let's go with it.

> 
> Changes:
>  - all: https://git.openjdk.java.net/jdk/pull/1026/files
>  - new: https://git.openjdk.java.net/jdk/pull/1026/files/15beacd2..bd890ae5
> 
> Webrevs:
> - full: https://webrevs.openjdk.java.net/?repo=jdk=1026=03
> - incr: https://webrevs.openjdk.java.net/?repo=jdk=1026=02-03
> 
>  Stats: 20 lines in 1 file changed: 4 ins; 4 del; 12 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/1026.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1026/head:pull/1026
> 
> PR: https://git.openjdk.java.net/jdk/pull/1026

Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second Preview)

[Remi Forax]

- Mail original -
> De: "Mandy Chung" 
> À: "compiler-dev" , "core-libs-dev" 
> , "hotspot-dev"
> 
> Envoyé: Mercredi 25 Novembre 2020 00:02:53
> Objet: Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second 
> Preview)

> On Tue, 17 Nov 2020 00:25:51 GMT, Mandy Chung  wrote:
> 
>>> src/java.base/share/classes/java/lang/Package.java line 227:
>>> 
 225:  * This method reports on a distinct concept of sealing from
 226:  * {@link Class#isSealed() Class::isSealed}.
 227:  *
>>> 
>>> This API note will be very confusing to readers. I think the javadoc will 
>>> need
>>> to be fleshed out and probably will need to link to a section the Package 
>>> class
>>> description that defines the legacy concept of sealing.
>>
>> I agree.  This @apiNote needs more clarification to help the readers to
>> understand the context here.One thing we could do in the Package class
>> description to add a "Package Sealing" section that can also explain that it
>> has no relationship to "sealed classes".
> 
> I added an API note in `Package::isSealed` [1] to clarify sealed package vs
> sealed class or interface.
> 
> The API note you added in `Class::isSealed` can be clarified in a similar
> fashion like: "Sealed class or interface has no relationship with {@linkplain
> Package#isSealed package sealing}".

Hi Mandy,
given that almost nobody knows about sealed packages, i'm not sure that adding 
a reference to Package::isSealed in Class::isSealed actually helps, it might be 
confusing.

> 
> [1] https://github.com/openjdk/jdk/commit/3c230b8a
> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/1227

Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second Preview)

[Remi Forax]

- Mail original -
> De: "David Holmes" 
> À: "Harold David Seigel" , "Vicente Romero" 
> , "compiler-dev"
> , "core-libs-dev" 
> , "hotspot-dev"
> 
> Envoyé: Mardi 24 Novembre 2020 02:04:55
> Objet: Re: RFR: 8246778: Compiler implementation for Sealed Classes (Second 
> Preview)

> Hi Harold,
> 
> On 24/11/2020 6:27 am, Harold Seigel wrote:
>> Hi David,
>> 
>> Thanks for looking at this.
>> 
>> The intent was for method Class.permittedSubclasses() to be implemented
>> similarly to Class.getNestMembers().  Are you suggesting that a security
>> manager check be added to permittedSubclasses() similar to the security
>> manager check in getNestMembers()?
> 
> No I'm suggesting the change to the API is plain wrong. :) Please see
> discussion in the CSR.

Given that the CSR is closed, i will answer here.
There are two issues with the previous implementation of permittedSubclasses, 
first it's the only method that using method desc which means that people has 
to be aware on another non trivial concept (object that describes constant pool 
constant) to understand how to use the method then i've tested this API with my 
students, all but one what able to correctly derives the Class from a 
ClassDesc, so instead of asking every users of permittedSubclasses to go 
through the oops of getting Class from a ClassDesc, i think we can agree that 
it's better to move the burden from the user to the JDK implementors.

> 
> Cheers,
> David

regards,
Rémi

> 
>> Thanks, Harold
>> 
>> On 11/18/2020 12:31 AM, David Holmes wrote:
>>> Hi Vincente,
>>>
>>> On 16/11/2020 11:36 pm, Vicente Romero wrote:
 Please review the code for the second iteration of sealed classes. In
 this iteration we are:

 - Enhancing narrowing reference conversion to allow for stricter
 checking of cast conversions with respect to sealed type hierarchies.
 - Also local classes are not considered when determining implicitly
 declared permitted direct subclasses of a sealed class or sealed
 interface
>>>
>>> The major change here seems to be that getPermittedSubclasses() now
>>> returns actual Class objects instead of ClassDesc. My recollection
>>> from earlier discussions here was that the use of ClassDesc was very
>>> deliberate as the permitted subclasses may not actually exist and
>>> there may be security concerns with returning them!
>>>
>>> Cheers,
>>> David
>>> -
>>>
 -

 Commit messages:
   - 8246778: Compiler implementation for Sealed Classes (Second Preview)

 Changes: https://git.openjdk.java.net/jdk/pull/1227/files
   Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1227=00
    Issue: https://bugs.openjdk.java.net/browse/JDK-8246778
    Stats: 589 lines in 12 files changed: 508 ins; 18 del; 63 mod
    Patch: https://git.openjdk.java.net/jdk/pull/1227.diff
    Fetch: git fetch https://git.openjdk.java.net/jdk
 pull/1227/head:pull/1227

 PR: https://git.openjdk.java.net/jdk/pull/1227

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators

[Remi Forax]

Ok, i've taking the time to read some literature about random generators 
because for me the Mersenne Twister was still the king.

The current API proposed as clearly two levels, you have the user level and the 
implementation level, at least the implementation level should seen as a SPI 

RandomGenerator is the user facing API, for me it should be the sole interface 
exposed by the API, the others (leap, arbitrary leap and split) should be 
optional methods.

In term of factory methods, we should have user driven methods:
- getDefaultGenerator() that currently returns a L64X128MixRandom and can be 
changed in the future
- getFastGenerator() that currently returns a Xoroshiro128PlusPlus and can be 
changed in the future
- getDefault[Splitable|Leapable|etc]Generator that returns a default generator 
with the methods splits|leaps|etc defined
- of / getByName that returns a specific generator by its name (but mov ed in a 
SPI class)

The example in the documentation should use getDefaultGenerator() and not of() 
to avoid the problem all the programming languages currently have by having 
over-specified that the default generator is a Mersenne Twister.

All methods that returns a stream of the available implementations should be 
moved in the SPI package.

Rémi 

---
An honest question,
why do we need so many interfaces for the different categories of 
RandomGenerator ?

My fear is that we are encoding the state of our knowledge of the different 
kinds of random generators now so it will not be pretty in the future when new 
categories of random generator are discovered/invented.
If we can take example of the past to predict the future, 20 years ago, what 
should have been the hierarchy at that time.
Is it not reasonable to think that we will need new kinds of random generator 
in the future ?

I wonder if it's not better to have one interface and several optional methods 
like we have with the collections, it means that we are loosing the 
possibilities to precisely type a method that only works with a precise type of 
generator but it will be more future proof.

Rémi

- Mail original -
> De: "Jim Laskey" 
> À: "core-libs-dev" , "security-dev" 
> 
> Envoyé: Mercredi 18 Novembre 2020 14:52:56
> Objet: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators

> This PR is to introduce a new random number API for the JDK. The primary API 
> is
> found in RandomGenerator and RandomGeneratorFactory. Further description can 
> be
> found in the JEP https://openjdk.java.net/jeps/356 .
> 
> javadoc can be found at
> http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html
> 
> old PR:  https://github.com/openjdk/jdk/pull/1273
> 
> -
> 
> Commit messages:
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862; Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - ... and 15 more: 
> https://git.openjdk.java.net/jdk/compare/f7517386...2b3e4ed7
> 
> Changes: https://git.openjdk.java.net/jdk/pull/1292/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1292=00
>  Issue: https://bugs.openjdk.java.net/browse/JDK-8248862
>  Stats: 13319 lines in 25 files changed: 0 ins; 2132 del; 77 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/1292.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1292/head:pull/1292
> 
> PR: https://git.openjdk.java.net/jdk/pull/1292

Re: RFR: 8180352: Add Stream.toList() method [v2]

[Remi Forax]

- Mail original -
> De: "Florian Weimer" 
> À: "Peter Levart" 
> Cc: "Stuart Marks" , "core-libs-dev" 
> 
> Envoyé: Mercredi 18 Novembre 2020 12:55:02
> Objet: Re: RFR: 8180352: Add Stream.toList() method [v2]

> * Peter Levart:
> 
>> But I see that the new  IMM_LIST_NULLS type is needed for one other
>> thing - the immutable list implementation of that type has different
>> behavior of indexOf and lastIndexOf methods (it doesn't throw NPE when
>> null is passed to those methods) so this behavior has to be preserved in
>> the deserialized instance and there is not way to achieve that on old
>> JDK with existing serialization format, so there has to be an
>> incompatible change in the serialization format for that matter.
> 
> I think it's also needed for an efficient null element check in
> List::copyOf.  I have a hunch that with the posted implementation, it
> would incorrectly produce an immutable list that contains null
> elements.

yes !

Rémi

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]

[Remi Forax]

An honest question,
why do we need so many interfaces for the different categories of 
RandomGenerator ?

My fear is that we are encoding the state of our knowledge of the different 
kinds of random generators now so it will not be pretty in the future when new 
categories of random generator are discovered/invented.
If we can take example of the past to predict the future, 20 years ago, what 
should have been the hierarchy at that time.
Is it not reasonable to think that we will need new kinds of random generator 
in the future ?

I wonder if it's not better to have one interface and several optional methods 
like we have with the collections, it means that we are loosing the 
possibilities to precisely type a method that only works with a precise type of 
generator but it will be more future proof.

Rémi

- Mail original -
> De: "Jim Laskey" 
> À: "build-dev" , "core-libs-dev" 
> ,
> security-...@openjdk.java.net
> Envoyé: Mardi 17 Novembre 2020 23:21:18
> Objet: Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators 
> [v3]

>> This PR is to introduce a new random number API for the JDK. The primary API 
>> is
>> found in RandomGenerator and RandomGeneratorFactory. Further description can 
>> be
>> found in the JEP https://openjdk.java.net/jeps/356 .
> 
> Jim Laskey has updated the pull request with a new target base due to a merge 
> or
> a rebase. The pull request now contains 40 commits:
> 
> - Merge branch 'master' into 8248862
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
>   
>   Update package-info.java
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
>   
>   Updated RandomGeneratorFactory javadoc.
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
>   
>   Updated documentation for RandomGeneratorFactory.
> - Merge branch 'master' into 8248862
> - Merge branch 'master' into 8248862
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
>   
>   Move RandomGeneratorProperty
> - Merge branch 'master' into 8248862
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
>   
>   Clear up javadoc
> - 8248862; Implement Enhanced Pseudo-Random Number Generators
>   
>   remove RandomGeneratorProperty from API
> - ... and 30 more: 
> https://git.openjdk.java.net/jdk/compare/f7517386...6fe94c68
> 
> -
> 
> Changes: https://git.openjdk.java.net/jdk/pull/1273/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1273=02
>  Stats: 14891 lines in 31 files changed: 0 ins; 3704 del; 77 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/1273.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1273/head:pull/1273
> 
> PR: https://git.openjdk.java.net/jdk/pull/1273

Re: 'Find' method for Iterable

[Remi Forax]

- Mail original -
> De: "Brian Goetz" 
> À: "Michael Kuhlmann" , "core-libs-dev" 
> 
> Envoyé: Mardi 17 Novembre 2020 20:44:17
> Objet: Re: 'Find' method for Iterable

> On 9/21/2020 4:08 AM, Michael Kuhlmann wrote:
>> But after thinking about it, I'm now convinced that it would be a bad
>> idea. Because it extends the scope of this small, tiny Iterable
>> interface to something bigger which it shouldn't be.
> 
> This response captures the essence of the problem.  You may think you
> are asking for "just one more method on Iterable", but really, what you
> are asking for is to turn Iterable into something it is not.  Iterable
> exists not to be "smallest collection", but as the interface to the
> foreach-loop.  Yes, we could have chosen a different design center for
> Iterable (Eclipse Collections did, see RichIterable), but we didn't.
> Adding this method merely sends the signal that we want to extend
> Iterable to be more than it is, which opens the floodgate to requests
> (and eventually demands) for more methods.
> 
>> I can ask about Iterable#forEach - is it there only because it was there to
>> begin with? Would it have been a bad idea to add one if we had streams
>> already?
> 
> While you can make an argument that forEach is excessive, the fact that
> you make this argument illustrates the essential problem with this
> proposal.  Your argument wrt forEach is "If that makes sense, then so
> does find."  If you pull on that string, then this method forms the
> basis of tomorrow's assumption: "You have forEach and find, it is
> unreasonable to not add ".

There is a good reason to have forEach,
Iterable in the interface of things that can be iterated, and in Java, there 
are two ways to do an iteration,
the internal iteration, where you ask the class to do the iteration for you 
using forEach and
the external iterator, where you ask for an Iterator and use it to iterate 
outside of the class.

In term of API design, it is push vs pull, using forEach, the element are is 
pushed to the Consumer while using an Iterator.next() allow you to pull the 
element.

Usually, forEach() is faster but it takes a Consumer, so you are limited by the 
limitation of the lambda (you can not mutate the local variables of the 
enclosing scope).
Using an Iterator is also more powerful because you can compose them.

> 
> So, Iterable should stay simple.  (The other arguments against it on
> this thread, are also valid, but this is the most important one.)

Yep.

Rémi

Re: RFR: 8230501: Class data support for hidden classes

[Remi Forax]

Hi Mandy,
maybe a stupid question but why this mechanism is limited to hidden classes ?

regards,
Rémi

- Mail original -
> De: "Mandy Chung" 
> À: "core-libs-dev" , "hotspot compiler" 
> 
> Envoyé: Mercredi 11 Novembre 2020 19:57:04
> Objet: RFR: 8230501: Class data support for hidden classes

> Provide the `Lookup::defineHiddenClassWithClassData` API that allows live
> objects
> be shared between a hidden class and other classes.  A hidden class can load
> these live objects as dynamically-computed constants via this API.
> 
> Specdiff
> http://cr.openjdk.java.net/~mchung/jdk16/webrevs/8230501/specdiff/overview-summary.html
> 
> With this class data support and hidden classes,
> `sun.misc.Unsafe::defineAnonymousClass`
> will be deprecated for removal.  Existing libraries should replace their
> calls to `sun.misc.Unsafe::defineAnonymousClass` with
> `Lookup::defineHiddenClass`
> or `Lookup::defineHiddenClassWithClassData`.
> 
> This patch also updates the implementation of lambda meta factory and
> `MemoryAccessVarHandleGenerator` to use class data.   No performance 
> difference
> observed in the jdk.incubator.foreign microbenchmarks.   A side note:
> `MemoryAccessVarHandleGenerator` is removed in the upcoming integration of
> JDK-8254162 but it helps validating the class data support.
> 
> Background
> --
> 
> This is an enhancement following up JEP 371: Hidden Classes w.r.t.
> "Constant-pool patching" in the "Risks and Assumption" section.
> 
> A VM-anonymous class can be defined with its constant-pool entries already
> resolved to concrete values. This allows critical constants to be shared
> between a VM-anonymous class and the language runtime that defines it, and
> between multiple VM-anonymous classes. For example, a language runtime will
> often have `MethodHandle` objects in its address space that would be useful
> to newly-defined VM-anonymous classes. Instead of the runtime serializing
> the objects to constant-pool entries in VM-anonymous classes and then
> generating bytecode in those classes to laboriously `ldc` the entries,
> the runtime can simply supply `Unsafe::defineAnonymousClass` with references
> to its live objects. The relevant constant-pool entries in the newly-defined
> VM-anonymous class are pre-linked to those objects, improving performance
> and reducing footprint. In addition, this allows VM-anonymous classes to
> refer to each other: Constant-pool entries in a class file are based on names.
> They thus cannot refer to nameless VM-anonymous classes. A language runtime 
> can,
> however, easily track the live Class objects for its VM-anonymous classes and
> supply them to `Unsafe::defineAnonymousClass`, thus pre-linking the new 
> class's
> constant pool entries to other VM-anonymous classes.
> 
> This extends the hidden classes to allow live objects to be injected
> in a hidden class and loaded them via condy.
> 
> Details
> ---
> 
> A new `Lookup::defineHiddenClassWithClassData` API takes additional
> `classData` argument compared to `Lookup::defineHiddenClass`.
> Class data can be method handles, lookup objects, arbitrary user objects
> or collections of all of the above.
> 
> This method behaves as if calling `Lookup::defineHiddenClass` to define
> a hidden class with a private static unnamed field that is initialized
> with `classData` at the first instruction of the class initializer.
> 
> `MethodHandles::classData(Lookup lookup, String name, Class type)`
> is a bootstrap method to load the class data of the given lookup's lookup 
> class.
> The hidden class will be initialized when `classData` method is called if
> the hidden class has not been initialized.
> 
> For a class data containing more than one single element, libraries can
> create their convenience method to load a single live object via condy.
> We can reconsider if such a convenience method is needed in the future.
> 
> Frameworks sometimes want to dynamically create a hidden class (HC) and add it
> it the lookup class nest and have HC to carry secrets hidden from that nest.
> In this case, frameworks should not to use private static finals (in the HCs
> they spin) to hold secrets because a nestmate of HC may obtain access to
> such a private static final and observe the framework's secret.  It should use
> condy.  In addition, we need to differentiate if a lookup object is created 
> from
> the original lookup class or created from teleporting e.g. `Lookup::in`
> and `MethodHandles::privateLookupIn`.
> 
> This proposes to add a new `ORIGINAL` bit that is only set if the lookup
> object is created by `MethodHandles::lookup` or by bootstrap method 
> invocation.
> The operations only apply to a Lookup object with original access are:
>   - create method handles for caller-sensitve methods
>   - obtain class data associated with the lookup class
> 
> No change to `Lookup::hasFullPrivilegeAccess` and `Lookup::toString` which
> ignores the ORIGINAL bit.
> 
> 
> Compatibility Risks
> 

Re: Class TreeMap | Lower and Upper Count Support

[Remi Forax]

Is it different from
 headMap(key, true).size() / tailMap(key, true).size() ?

https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/util/NavigableMap.html#headMap(K,boolean)
https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/util/NavigableMap.html#tailMap(K,boolean)

cheers,
Rémi

- Mail original -
> De: "mayank bansal" 
> À: "core-libs-dev" 
> Envoyé: Dimanche 8 Novembre 2020 11:22:01
> Objet: Class TreeMap | Lower and Upper Count Support

> Hi Everyone,
> 
> I would like to propose and work on a feature request of supporting the
> lower and higher count in java class TreeMap.
> "lower count" is the number of elements that are strictly less than the
> given value.
> "upper count" is the number of elements that are strictly greater than the
> given value.
> 
> *Method definitions-*
> int getLowerCount(K key);
> int getHigherCount(K key);
> 
> *Follow-up feature -*
> Class TreeSet constructor initializes the TreeMap in the TreeSet
> constructor.
> It puts the dummy value as *new Object()* whenever we add the entry in
> TreeSet.
> I would like to work on the feature to provide the *Duplicate count* in
> case of the same Key and the same Value.
> 
> I will be happy to work on both and raise a PR. I need some guidance if the
> proposed feature looks good and I can start working on it and also would
> like to know about the process whether I can directly raise the PR.
> 
> Thanks
> --
> Regards,
> Mayank Bansal

Re: RFR: 8180352: Add Stream.toList() method

[Remi Forax]

- Mail original -
> De: "Simon Roberts" 
> À: "core-libs-dev" 
> Envoyé: Jeudi 5 Novembre 2020 18:40:44
> Objet: Re: RFR: 8180352: Add Stream.toList() method

> At the risk of a can of worms, or at least of raising something that has
> long since been discussed and rejected...
> 
> This discussion of unmodifiable lists brings me back to the thought that
> there would be good client-side reasons for inserting an UnmodifiableList
> interface as a parent of LIst, not least because all our unmodifiable
> variants from the Collections.unmodifiableList proxy onward fail the Liskov
> substitution test for actually "being contract-fulfilling Lists".
> 
> Is this something that has been considered and rejected? (If so, is it easy
> to point me at the discussion, as I expect I'd find it fascinating). Is it
> worth considering, might it have some merit, or merely horrible
> unforeseen-by-me consequences to the implementation?

This question is asked at least every six months since 1998
https://docs.oracle.com/javase/8/docs/technotes/guides/collections/designfaq.html#a1

> 
> Cheers,
> Simon

regards,
Rémi Forax

> 
> 
> 
> On Thu, Nov 5, 2020 at 10:30 AM Stuart Marks 
> wrote:
> 
>> On Wed, 4 Nov 2020 23:03:02 GMT, Paŭlo Ebermann > 645859+ep...@openjdk.org> wrote:
>>
>> >> This change introduces a new terminal operation on Stream. This looks
>> like a convenience method for Stream.collect(Collectors.toList()) or
>> Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this
>> method directly on Stream enables it to do what can't easily by done by a
>> Collector. In particular, it allows the stream to deposit results directly
>> into a destination array (even in parallel) and have this array be wrapped
>> in an unmodifiable List without copying.
>> >>
>> >> In the past we've kept most things from the Collections Framework as
>> implementations of Collector, not directly on Stream, whereas only
>> fundamental things (like toArray) appear directly on Stream. This is true
>> of most Collections, but it does seem that List is special. It can be a
>> thin wrapper around an array; it can handle generics better than arrays;
>> and unlike an array, it can be made unmodifiable (shallowly immutable); and
>> it can be value-based. See John Rose's comments in the bug report:
>> >>
>> >>
>> https://bugs.openjdk.java.net/browse/JDK-8180352?focusedCommentId=14133065=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14133065
>> >>
>> >> This operation is null-tolerant, which matches the rest of Streams.
>> This isn't specified, though; a general statement about null handling in
>> Streams is probably warranted at some point.
>> >>
>> >> Finally, this method is indeed quite convenient (if the caller can deal
>> with what this operation returns), as collecting into a List is the most
>> common stream terminal operation.
>> >
>> > src/java.base/share/classes/java/util/ImmutableCollections.java line 199:
>> >
>> >> 197:  * safely reused as the List's internal storage, avoiding a
>> defensive copy. Declared
>> >> 198:  * with Object... instead of E... as the parameter type so
>> that varargs calls don't
>> >> 199:  * accidentally create an array of type other than Object[].
>> >
>> > Why would that be a problem? If the resulting list is immutable, then
>> the actual array type doesn't really matter, right?
>>
>> It's an implementation invariant that the internal array be Object[].
>> Having it be something other than Object[] can lead to subtle bugs. See
>> [JDK-6260652](https://bugs.openjdk.java.net/browse/JDK-6260652) for
>> example.
>>
>> -
>>
>> PR: https://git.openjdk.java.net/jdk/pull/1026
>>
> 
> 
> --
> Simon Roberts
> (303) 249 3613

Re: RFR: 8180352: Add Stream.toList() method

[Remi Forax]

- Mail original -
> De: "Stuart Marks" 
> À: "core-libs-dev" 
> Envoyé: Mercredi 4 Novembre 2020 01:14:54
> Objet: Re: RFR: 8180352: Add Stream.toList() method

> On Tue, 3 Nov 2020 18:53:23 GMT, Stuart Marks  wrote:
> 
>>> Should there be a test that tests the default implementation in 
>>> `j.u.s.Stream`?
>>> Or maybe there is and I missed that?
>>
>> @dfuch wrote:
>>> Should there be a test that tests the default implementation in 
>>> `j.u.s.Stream`?
>>> Or maybe there is and I missed that?
>> 
>> The test method `testDefaultOps` does that. The stream test framework has a
>> thing called `DefaultMethodStreams` that delegates everything except default
>> methods to another Stream instance, so this should test the default
>> implementation.
> 
> OK, there are rather too many forking threads here for me to try to reply to 
> any
> particular message, so I'll try to summarize things and say what I intend to
> do.
> 
> Null tolerance. While part of me wants to prohibit nulls, the fact is that
> Streams pass through nulls, and toList() would be much less useful if it were
> to reject nulls. The affinity here is closer to Stream.toArray(), which allows
> nulls, rather than Collectors.to[Unmodifiable]List.
> 
> Unmodifiability. Unlike with nulls, this is where we've been taking a strong
> stand recently, where new constructs are unmodifiable ("shallowly immutable").
> Consider the Java 9 unmodifiable collections, records, primitive classes, etc.
> -- they're all unmodifiable. They're data-race-free and are resistant to a
> whole class of bugs that arise from mutability.

again, the strong stance about null in collections was taken earlier that the 
one about unmodifiable collection,
all collections introduced since java 1.6 are null hostile.

I think we are bound to rediscover why :(


And BTW, an unmodifiable list is not data-race free, it depends if the content 
is immutable or not.


> 
> Unfortunately, the combination of the above means that the returned List is
> neither like an ArrayList nor like an unmodifiable list produced by List.of()
> or by Collectors.toUnmodifiableList(). [Heavy sigh.] While I've been somewhat
> reluctant to introduce this new thing, the alternatives of trying to reuse one
> of the existing things are worse.

This is the smell test here.
You want to introduce a 4th semantics, we already have ArrayList, 
Arrays.asList() and List.of().
Add a new semantics has not a linear cost, the cost is bigger than linear 
because of the all the interactions with iterators, streams, etc.
(BTW this remember me that List.of().stream() is not optimize well compared to 
ArrayList.stream())

This is a real danger, Java is already complex, if it becomes more and more, it 
will be irrelevant, because it becomes harder and harder to internalize all the 
semantics inside of your head.
There is already a C++, let us not become another one.

So i really like the fact that you have decided to go bold here and choose to 
return an unmodifiable list but we should go to full monty and be null hostile 
too, like toUnmodifiableList().

> 
> John and Rémi pointed out that the way I implemented this, using a subclass,
> reintroduces the possibility of problems with megamorphic dispatch which we 
> had
> so carefully avoided by reducing the number of implementation classes in this
> area. I agree this is a problem.
> 
> I also had an off-line discussion with John where we discussed the 
> serialization
> format, which unfortunately is somewhat coupled with this issue. (Fortunately 
> I
> think it can mostly be decoupled.) Given that we're introducing a new thing,
> which is an unmodifiable-list-that-allows-nulls, this needs to be manifested 
> in
> the serialized form of these new objects. (This corresponds to the new tag
> value of IMM_LIST_NULLS in the CollSer class.) The alternative is to reuse the
> existing serialized form, IMM_LIST, for both of these cases, relaxing it to
> allow nulls. This would be a serialization immutability problem. Suppose I had
> an application that created a data structure that used lists from List.of(),
> and I had a global assertion over that structure that it contained no nulls.
> Further suppose that I serialized and deserizalized this structure. I'd want
> that assertion to be preserved after deserialization. If another app (or a
> future version of this app) created the structure using Stream.to
> List(), this would allow nulls to leak into that structure and violate that
> assertion. Therefore, the result of Stream.toList() should not be
> serialization-compatible with List.of() et. al. That's why there's the new
> IMM_LIST_NULLS tag in the serial format.
> 
> However, the new representation in the serial format does NOT necessarily
> require the implementation to be a different class. I'm going to investigate
> collapsing ListN and ListNNullsAllowed back into a single class, while
> preserving the separate serialized forms. This should mitigate the concerns
> about 

Re: RFR: 8180352: Add Stream.toList() method

[Remi Forax]

- Mail original -
> De: "Martin Desruisseaux" 
> À: "core-libs-dev" 
> Envoyé: Mardi 3 Novembre 2020 23:16:49
> Objet: Re: RFR: 8180352: Add Stream.toList() method

> Hello
> 
> Le 03/11/2020 à 21:30, fo...@univ-mlv.fr a écrit :
> 
>> You know that you can not change the implementation of
>> Collectors.toList(), and you also know that if there is a method
>> Stream.toList(), people will replace the calls to
>> .collect(Collectors.toList()) by a call to Stream.toList() for the
>> very same reason (…snip…)
>>
> Would they would be so numerous to do this change without verifying if
> the specifications match? (on my side I do read the method javadoc). But
> even if we worry about developers not reading javadoc, the argument
> could go both ways: they could assume that all Stream methods accepts
> null and not read that Stream.toList() specifies otherwise.

yes, that why i said to Brian that he is right that stream.toList() should 
return a List that accept null.

> 
>      Martin

Rémi

Re: RFR: 8180352: Add Stream.toList() method

[Remi Forax]

- Mail original -
> De: "Stuart Marks" 
> À: "core-libs-dev" 
> Envoyé: Mardi 3 Novembre 2020 04:18:08
> Objet: RFR: 8180352: Add Stream.toList() method

> This change introduces a new terminal operation on Stream. This looks like a
> convenience method for Stream.collect(Collectors.toList()) or
> Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this
> method directly on Stream enables it to do what can't easily by done by a
> Collector. In particular, it allows the stream to deposit results directly 
> into
> a destination array (even in parallel) and have this array be wrapped in an
> unmodifiable List without copying.
> 
> In the past we've kept most things from the Collections Framework as
> implementations of Collector, not directly on Stream, whereas only fundamental
> things (like toArray) appear directly on Stream. This is true of most
> Collections, but it does seem that List is special. It can be a thin wrapper
> around an array; it can handle generics better than arrays; and unlike an
> array, it can be made unmodifiable (shallowly immutable); and it can be
> value-based. See John Rose's comments in the bug report:
> 
> https://bugs.openjdk.java.net/browse/JDK-8180352?focusedCommentId=14133065=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14133065
> 
> This operation is null-tolerant, which matches the rest of Streams. This isn't
> specified, though; a general statement about null handling in Streams is
> probably warranted at some point.
> 
> Finally, this method is indeed quite convenient (if the caller can deal with
> what this operation returns), as collecting into a List is the most common
> stream terminal operation.

Hi Stuart,
I'm Okay with the idea of having a method toList() on Stream but really dislike 
the proposed semantics because tit is neither stream.collect(toList()) nor 
stream.collect(toUnmodifiableList()) but something in between.

It's true that a Stream support nulls, we want people to be able map() with a 
method that returns null and then filter out the nulls (even if using flatMap 
for this case is usually a better idea),
but it doesn't mean that all methods of the Stream interface has to support 
nulls, the original idea was more to allow nulls to flow in the stream because 
at some point they will be removed before being stored in a collection.

For me allowing in 2020 to store null in a collection is very backward, all 
collections since 1.6 doesn't support nulls.

Also, adding a third immutable list creates a problem, it means that now when 
we get an immutable list it can be 3 different implementations but the VM only 
uses bimorphic inlining cache,
so more callsites will fail to inline because of that. I think we have already 
reduced the number of implementation of immutable map from 3 to 2 for the very 
same reasons.

I believe that instead of inventing a third semantics that allows to store null 
in a collection, we should use the semantics of 
stream.collect(Collectors.toUnmodifiableList()) even if it means that toList() 
will throw a NPE if one element is null.

Rémi

Re: RFR: 8255398: Add a dropReturn MethodHandle combinator

[Remi Forax]

- Mail original -
> De: "Jorn Vernee" 
> À: "core-libs-dev" 
> Envoyé: Lundi 26 Octobre 2020 16:37:20
> Objet: RFR: 8255398: Add a dropReturn MethodHandle combinator

> Hi,
> 
> This patch adds a `dropReturn` combinator to `MethodHandles` that can be used 
> to
> create a new method handle that drops the return value, from a given method
> handle. Similar to the following code:
> 
> MethodHandle target = ...;
> Class targetReturnType = target.type().returnType();
> if (targetReturnType != void.class)
>target = filterReturnValue(target, empty(methodType(void.class,
>targetReturnType)));
> // use target
> 
> But as a short-hand.

As i said in the JBS issue, it's not just a short hand,
it also the semantics of the POP/POP2 bytecode, remove the return value on top 
of the stack.

> 
> Thanks,
> Jorn

regards,
Rémi

> 
> -
> 
> Commit messages:
> - Add a dropReturn method handle combinator
> 
> Changes: https://git.openjdk.java.net/jdk/pull/866/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=866=00
>  Issue: https://bugs.openjdk.java.net/browse/JDK-8255398
>  Stats: 92 lines in 2 files changed: 92 ins; 0 del; 0 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/866.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/866/head:pull/866
> 
> PR: https://git.openjdk.java.net/jdk/pull/866

Re: 'Find' method for Iterable

[Remi Forax]

- Mail original -
> De: "Nir Lisker" 
> À: "core-libs-dev" 
> Envoyé: Lundi 14 Septembre 2020 20:56:27
> Objet: 'Find' method for Iterable

> Hi,
> 
> This has probably been brought up at some point. When we need to find an
> item in a collection based on its properties, we can either do it in a
> loop, testing each item, or in a stream with filter and findFirst/Any.
> 
> I would think that a method in Iterable be useful, along the lines of:
> 
> public  Optional find(Predicate condition) {
>Objects.requireNonNull(condition);
>for (T t : this) {
> if (condition.test(t)) {
> return Optional.of(t);
>}
>}
>return Optional.empty();
> }
> 
> With usage:
> 
> list.find(person -> person.id == 123456);
> 
> There are a few issues with the method here such as t being null in
> null-friendly collections and the lack of bound generic types, but this
> example is just used to explain the intention.
> 
> It will be an alternative to
> 
> list.stream().filter(person -> person.id == 123456).findAny/First()
> (depending on if the collection is ordered or not)
> 
> which doesn't create a stream, similar to Iterable#forEach vs
> Stream#forEach.
> 
> Maybe with pattern matching this would become more appetizing.

During the development of Java 8, we first tried to use Iterator/Iterable 
instead of using a novel interface Stream.
But a Stream cleanly separate the lazy side effect free API from the mutable 
one (Collection) and can be optimized better by the VM (it's a push API instead 
of being a pull API).

The other question is why there is no method find() on Collection, i believe 
it's because while find() is ok for any DB API, find() is dangerous on a 
Collection because the execution time is linear, so people may use it instead 
of using a Map.

> 
> - Nir

Rémi

Re: JDK 15 hidden classes & stack trace visibility

[Remi Forax]

Hi Uwe,

- Mail original -
> De: "Uwe Schindler" 
> À: "core-libs-dev" 
> Envoyé: Samedi 5 Septembre 2020 13:55:03
> Objet: JDK 15 hidden classes & stack trace visibility

> Hi,
> 
> I am doing a mockup for dynamically compiled scripts in Apache Lucene (later
> also painless scripting in elastcsearch), where I tried to use
> Lookup#defineHiddenClass (with default parameters, so weak and no nestmates
> - but: for painless scripts of Elasticsearch, nestmates will be great). It
> all looks great, as sometimes for every query a new class is generated.
> 
> The current approach (Java 8, Java 11) uses a Classloader per script, which
> is mostly OK, but hidden classes would be better, especially under high load
> with many new classes. The problem I see is: The hidden class and their
> methods are also hidden from stack traces, so people neither get the script
> source code reference, nor they get the method, which was called at all.
> 
> My question is: is it possible to mark such a hidden class in byte code or
> by a flag to defineHiddenClass() so it is "unhidden"? - I know you can do
> this by a command line option, but that's for developers and debugging only.
> In some cases, this is really wanted (like for scripting languages), but
> code still wants to use the better class unloading. If this is not possible,
> how about extending the Lookup.ClassOption enum (last parameter of
> defineHiddenClass), to also pass a flag to show them in stack traces?

If you don't want a hidden class, why not using Lookup.defineClass() for 11 and 
calling ClassLoader.defineClass() by reflection in 8 ?
With defineClass you get a resolvable real name and methods are visible in the 
stacktrace.

Obviously, it means that you are grouping several classes in the same 
classloader which may be not what you want.

> 
> Uwe

Rémi

> 
> -
> Uwe Schindler
> uschind...@apache.org
> ASF Member, Apache Lucene PMC / Committer
> Bremen, Germany
> https://lucene.apache.org/

Re: Review request for JDK-8251538: Modernize and lint Dynalink code

[Remi Forax]

- Mail original -
> De: "Attila Szegedi" 
> À: "core-libs-dev" 
> Envoyé: Jeudi 20 Août 2020 22:40:53
> Objet: Review request for JDK-8251538: Modernize and lint Dynalink code

> Following up on the previous e-mail, here’s the modernization and linting work
> on the existing Dynalink codebase:
> 
> Please review JDK-8251538 "Modernize and lint Dynalink code" at
>  for
> 
> 
> The Jira issue has a full enumeration of kinds of changes I did here; they’re
> mostly all source text removals :-)
> 
> Oh, BTW, I really got an inspiration for adding “public static ClassValue
> computingValue(Function, T> f)” method to ClassValue class, similar 
> to
> Comparator.comparing. It’d allow lambdifying class values. (If you’re
> listening, John :-) I suspect it being in java.lang would be a JCP-level 
> change
> so I kind-of don’t want to take it upon myself…)

this is also very similar to 
https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/ThreadLocal.html#withInitial(java.util.function.Supplier)

and the proposed signature doesn't have the right wildcards, it should be
  public static ClassValue computingValue(Function, ? 
extends T> f)
  

> 
> Thanks,
>   Attila.

regards,
Rémi

Re: RFR[8238286]: 'Add new flatMap stream operation that is more amenable to pushing’

[Remi Forax]

Hi Patrick,
I still don't like the fact that IntMapMultiConsumer, DoubleMapMultiConsumer 
and LongMapMultiConsumer are not in java.util.function unlike all other 
functional interfaces used by the Stream API.

Otherwise looks good.

regards,
Rémi

- Mail original -
> De: "Patrick Concannon" 
> À: "Julia Boes" 
> Cc: "Anthony Vanelverdinghe" , "core-libs-dev" 
> 
> Envoyé: Mardi 11 Août 2020 20:11:56
> Objet: Re: RFR[8238286]: 'Add new flatMap stream operation that is more 
> amenable to pushing’

> Hi,
> 
> Please find the next iteration of mapMulti in the new webrev below.
> 
> In this iteration the following changes have been made:
> The API note for mapMulti has been updated.
> Sub-interfaces for {Int, Double, Long}Stream have been refactored to be more
> specific to mapMulti.
> The examples have been changed, and now include a reference to how an explicit
> type parameter can be used in conjunction with mapMulti.
> 
> The CSR and specdiff have also been updated to reflect these changes.
> 
> webrev: http://cr.openjdk.java.net/~pconcannon/8238286/webrevs/webrev.05/
> specdiff: http://cr.openjdk.java.net/~pconcannon/8238286/specdiff/specout.02/
> CSR: https://bugs.openjdk.java.net/browse/JDK-8248166
> 
> Kind regards,
> Patrick

Re: Type inference: bug or feature?

[Remi Forax]

- Mail original -
> De: "Maurizio Cimadamore" 
> À: "Justin Dekeyser" , "core-libs-dev" 
> 
> Envoyé: Lundi 27 Juillet 2020 12:26:40
> Objet: Re: Type inference: bug or feature?

> CC'ing compiler-dev
> 
> Hi Justin,
> the behavior you are observing is normal. For a Java expression to be
> able to be influenced by type inference it has to be a poly expression
> (where poly stands for _many_ as in, the same expression can have many
> types).
> 
> At the time we did Java 8 we briefly considered making cast expressions
> part of the poly expression dance (like lambdas, method references,
> method calls, parens, new creation expression, conditionals and switch
> expression - see JLS 15.2), but the rule which dictate cast conversion
> (JLS 5.5) are so complex (as they have to take into account possibility
> for unchecked cast, etc.) that it felt like touching them was a very
> risky move, with no clear benefit.

There was another reason :)

The idea behind introducing generics is to get ride of casts because they are a 
hazard at runtime.
So allowing people to use casts as poly expression goes in the wrong direction, 
add more casts.

Instead of using a cast, it's better to use explicit type arguments,
  TheClass.emptyList()

[...]

> 
> Cheers
> Maurizio

regards,
Rémi

> 
> On 26/07/2020 18:22, Justin Dekeyser wrote:
>> Dear all,
>>
>> I'm not sure but I think I've found a bug in Java type inference mechanism.
>> It may also be a feature, so I'll expose the problem to you below; in terms
>> of Integer, Number and List, although you'll quickly realize it will work
>> wrong in any similar situation.
>>
>> Let's assume I have
>>
>> static  List emptyList(Class magnet) {
>> return Collections.emptyList();
>> }
>>
>> Then the following codes does not compile (for the same reason):
>>
>> var x = (List) emptyList(Number.class);
>> List x = (List) emptyList(Number.class);
>>
>> incompatible types: List cannot be converted to List
>>
>> however, the following do compile:
>>
>> var x = emptyList(Number.class); // inferred to List
>> List x = emptyList(Number.class); // no mistake here, it's Integer
>> on the left
>>
>> Is this the expected behavior? Why does casting operation here interfere
>> with type inference like this?
>>
>> Regards,
>>
>> Justin Dekeyser
>>
>>
>>
>>
>> 
>> Garanti
>> sans virus. www.avast.com
>> 
> > <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

Re: RFR[8238286]: 'Add new flatMap stream operation that is more amenable to pushing’

[Remi Forax]

Hi Patrick & Julia,
this version starts to look good.

I just don't understand why the new functional interfaces are not in 
java.util.function like the other ones ?
(BTW, in the javadoc, the link to the summary overview point to the wrong one, 
to java.util.stream and not java.util.function).

About the examples, i will try to think about that this evening :)

regards,
Rémi

- Mail original -
> De: "Patrick Concannon" 
> À: "Julia Boes" 
> Cc: "core-libs-dev" 
> Envoyé: Jeudi 2 Juillet 2020 15:30:45
> Objet: Re: RFR[8238286]: 'Add new flatMap stream operation that is more 
> amenable to pushing’

> Hi,
> 
> John: Thanks for your feedback. We've rearranged the ordering of the 
> parameters
> of the BiConsumer to follow the convention you suggested, and hopefully 
> improve
> readability going forward. Additional FIs (IntObjConsumer, etc.) have been
> added as sub-interfaces to the corresponding Stream classes i.e. {Int, Double,
> Long}Stream.
> 
> Remi: Your argument makes sense, and we have updated the BiConsumers generic
> type to `>` as you suggested. Thanks for pointing this 
> out.
> We have also removed the caching.
> WRT to the wrappers used in the examples: good examples are tough to nail 
> down.
> We think the examples in their current form do a good job of demonstrating how
> the method can be used, but we welcome any alternative suggestions.
> 
> 
> The changes discussed can be found in the updated webrev below.
> 
> http://cr.openjdk.java.net/~pconcannon/8238286/webrevs/webrev.02/
> 
> 
> 
> Kind regards,
> 
> Patrick
> 
>> On 26 Jun 2020, at 17:46, Julia Boes  wrote:
>> 
> > w

Re: Explicitly constructed NPE is showing (wrong) message

[Remi Forax]

- Mail original -
> De: "Christoph Dreis" 
> À: "hotspot-runtime-dev" 
> Envoyé: Samedi 27 Juin 2020 11:54:19
> Objet: Explicitly constructed NPE is showing (wrong) message

> Hi,

Hi Christoph,

> 
> I hope this is the correct mailing list.
> 
> The latest changes on JDK-8233014 to enable ShowCodeDetailsInExceptionMessages
> by default have uncovered a likely bug (I think).
> 
> To my understanding explicitly created NullPointerExceptions should not print
> any message.
> With the following example:
> 
> public class Main {
>   public static void main(String[] args) {
>   NullPointerException ex = new NullPointerException();
>   Throwable throwable = ex.fillInStackTrace();
>   System.out.println(throwable);
>   }
> }
> 
> I see the following output though:
> 
> java.lang.NullPointerException: Cannot invoke
> "java.lang.NullPointerException.fillInStackTrace()" (on slot 0) because "ex" 
> is
> null
> 
> Which obviously is not really true.
> I was debugging the thing (hence the additional "on slot 0") output, but
> couldn't find the error so far.
> 
> I'm suspecting the error to be somewhere around bytecodeUtils.cpp 1155:
> 
>if (name != vmSymbols::object_initializer_name()) {
>  int type_index = cp->signature_ref_index_at(name_and_type_index);
>  Symbol* signature  = cp->symbol_at(type_index);
>  // The 'this' parameter was null. Return the slot of it.
>  return ArgumentSizeComputer(signature).size();
>} else {
>  return NPE_EXPLICIT_CONSTRUCTED;
>}
> 
> I initially hoped to find a fix for it directly, but I would probably need a 
> bit
> more time for it, so I decided to report it.
> Is this a bug even or am I chasing ghosts here? In case it is, I would be 
> happy
> about a mentioning somewhere in an eventual commit.

I see the issue.
The idea of the algorithm is to use the backtrace stored in the exception to 
try to find where the NPE occurs, here by calling fillStackTrace() emplicitly, 
you are changing the backtrace so getExtendedNPEMessage now use the new 
backtrace reporting the error in main() :(

One way to fix the issue is to record if fillInStackTrace is called more than 
once (by having two sentinels by example) and to not call getExtendedNPEMessage 
if it's the case. Obviously the new added sentinel has to work with the 
serialization.

> 
> Cheers,
> Christoph

regards,
Rémi

Re: RFR[8238286]: 'Add new flatMap stream operation that is more amenable to pushing’

[Remi Forax]

- Mail original -
> De: "Remi Forax" 
> À: "Daniel Fuchs" 
> Cc: "Patrick Concannon" , "core-libs-dev" 
> 
> Envoyé: Jeudi 25 Juin 2020 23:46:08
> Objet: Re: RFR[8238286]: 'Add new flatMap stream operation that is more 
> amenable to pushing’

> ----- Mail original -
>> De: "Daniel Fuchs" 
>> À: "Remi Forax" , "Patrick Concannon"
>> 
>> Cc: "core-libs-dev" 
>> Envoyé: Jeudi 25 Juin 2020 11:28:00
>> Objet: Re: RFR[8238286]: 'Add new flatMap stream operation that is more 
>> amenable
>> to pushing’
> 
>> Hi Rémi,
> 
> Hi Daniel,
> 
>> 
>> On 25/06/2020 00:32, Remi Forax wrote:
>>> I get that you want to keep Consumer instead of Consumer 
>>> because
>>> Consumer is not a valid target type for a lambda, but the 
>>> BiConsumer
>>> should be able to take a ? super Consumer instead of just Consumer.
>> 
>> Though I don't dispute that a strict application of the rules of
>> covariance and contravariance would require to design a signature
>> that accepts a `? super Consumer` - how would you implement a
>> BiConsumer with a signature that doesn't take a Consumer?
>> 
>> Such an implementation would be unable to push anything downstream
>> without having to cast back the consumer to Consumer.
> 
> if i have already have a BiConsumer, Object>, i would like to
> be able to call Stream.mapMulti() with that bi-consumer as argument.

and obviously, i got it wrong, Consumer is not a super-type of 
Consumer, it should be a BiConsumer, Object> or a 
BiConsumer, Object>, etc.

> 
>> 
>> My personal preference would be to vote in favor of the simpler
>> signature - which IMO is more readable and easier to understand.
>> 
>> best regards,

regards,
Rémi

> 
>> 
> > -- daniel

Re: RFR[8238286]: 'Add new flatMap stream operation that is more amenable to pushing’

[Remi Forax]

Hi Patrick,

- Mail original -
> De: "Patrick Concannon" 
> À: "core-libs-dev" 
> Envoyé: Mercredi 24 Juin 2020 12:57:01
> Objet: RFR[8238286]:  'Add new flatMap stream operation that is more amenable 
> to pushing’

> Hi,

Hi,

> 
> Could someone please review myself and Julia's RFE and CSR for JDK-8238286 -
> 'Add new flatMap stream operation that is more amenable to pushing’?
> 
> This proposal is to add a new flatMap-like operation:
> 
> ` Stream mapMulti(BiConsumer, ? super T> mapper)`
> 
> to the java.util.Stream class. This operation is more receptive to the pushing
> or yielding of values than the current implementation that internally 
> assembles
> values (if any) into one or more streams. This addition includes the primitive
> variations of the operation i.e. mapMultiToInt, IntStream mapMulti, etc.

I don't really like the name "mapMulti", because flatMap can also be called 
mapMulti.
I'm sure someone will come with a better name, mapPush ?, mapYield ?

Why the BiConsumer takes a T as the second parameter and not as the first like 
in the bug description ?

I get that you want to keep Consumer instead of Consumer because 
Consumer is not a valid target type for a lambda, but the BiConsumer 
should be able to take a ? super Consumer instead of just Consumer.

In Stream.java, in the javadoc both the examples of mapMulti are using wrapper 
types instead of primitives, which is not the way we want people to use 
streams, stream of wrapper types cause the perf issue.

In *Stream.java, in the default method, the 'this' in 'this'.flatMap is useless.
 
In *Pipeline.java, i don't think you need the lambda downstream::accept given 
that a Sink is already a Consumer,
instead of
  mapper.accept(downstream::accept, u);
using the downstream directly should work
  mapper.accept(downstream, u);

also it seems to me that the implementation of cancellationRequested() is 
useless given that Sink.Chained*::cancellationRequested already delegates to 
the downstream sink.
 
I've also noticed that in ReferencePipelien.java, in  Stream 
mapMulti(BiConsumer, ? super P_OUT> mapper), the cache field is 
missing, but its should be necessary anymore.

> 
> issue: https://bugs.openjdk.java.net/browse/JDK-8238286
> 
> csr: https://bugs.openjdk.java.net/browse/JDK-8248166
> 
> 
> webrev: http://cr.openjdk.java.net/~pconcannon/8238286/webrevs/webrev.00/
> 
> specdiff:
> http://cr.openjdk.java.net/~pconcannon/8238286/specdiff/specout.00/overview-summary.html
>   
> 
> 
> 
> Kind regards,
> Patrick & Julia

regards,
Rémi

Re: [PATCH] remove redundant initialization of volatile fields with default values

[Remi Forax]

Hi Sergei,
the problem is that you are changing the semantics if there are several fields.

By example with the code below, you have the guarantee that the code will print 
4 (if it prints something),
if you remove the assignment field = false, the code can print 0 or 4.

  class A {
int i = 4;
volatile boolean field = false;
  }

thread 1:
  global = new A()

thread 2:
  var a = global;
  if (a != null) {
System.out.println(a.i);
  }

regards,
Rémi

- Mail original -
> De: "Сергей Цыпанов" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 19 Juin 2020 06:57:25
> Objet: [PATCH] remove redundant initialization of volatile fields with 
> default values

> Hello,
> 
> while investigating an issue I've found out that assignment of default value 
> to
> volatile fields slows down object instantiation.
> 
> Consider the benchmark:
> 
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @BenchmarkMode(value = Mode.AverageTime)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class VolatileFieldBenchmark {
>  @Benchmark
>  public Object explicitInit() {
>return new ExplicitInit();
>  }
> 
>  @Benchmark
>  public Object noInit() {
>return new NoInit();
>  }
> 
>  private static class ExplicitInit {
>private volatile boolean field = false;
>  }
>  private static class NoInit {
>private volatile boolean field;
>  }
> }
> 
> This gives the following results as of my machine:
> 
> BenchmarkMode  Cnt   Score   Error  Units
> VolatileFieldBenchmark.explicitInit  avgt   40  11.087 ± 0.140  ns/op
> VolatileFieldBenchmark.noInitavgt   40   3.367 ± 0.131  ns/op
> 
> 
> I've looked into source code of java.base and found out several cases where 
> the
> default value is assigned to volatile field.
> 
> Getting rid of such assignements demonstates improvement as of object
> instantiation, e.g. javax.security.auth.Subject:
> 
>   Mode  Cnt   Score   Error  Units
> before avgt   40  35.933 ± 2.647  ns/op
> after  avgt   40  30.817 ± 2.384  ns/op
> 
> As of testing tier1 and tier2 are both ok after the changes.
> 
> Best regards,
> Sergey Tsypanov

Re: RFR: 8247532: Records deserialization is slow

[Remi Forax]

Hi Peter,
is their a reason to not use a ClassValue using the record class 
as key instead of the more complex keys you are proposing ?

Rémi

- Mail original -
> De: "Chris Hegarty" 
> À: "Peter Levart" 
> Cc: "core-libs-dev" 
> Envoyé: Mardi 16 Juin 2020 17:15:03
> Objet: Re: RFR: 8247532: Records deserialization is slow

> Hi Peter,
> 
>> On 14 Jun 2020, at 17:28, Peter Levart  wrote:
>> 
>> ...
>> 
>> [2]
>> http://cr.openjdk.java.net/~plevart/jdk-dev/RecordsDeserialization/webrev.01/
>> 
> I think that this is very good. It’s clever to build a chain of method handles
> that can be invoked with the stream field values. This is a nice separation
> between the shape of the data and the actual stream data.
> 
> The caching is on a per-stream-field shape basis, which should be consistent 
> in
> the common case, but of course is not always guaranteed to be the case, hence
> the need for the cache. I think that this should be fine, since the
> ObjectStreamClass ( that holds the cache ) is already itself cached as a weak
> reference. But I did wonder if the size of this new cache should be limited.
> Probably not worth the complexity unless it is an obvious issue.
> 
> All the serailizable records tests pass successfully with your patch. Good. I
> did however notice that there is just a single test, DefaultValuesTest, that
> exercises different stream shapes for the same class in the stream. Would be
> good to expand coverage in this area, but of course some lower-level
> test-specific building blocks will be needed help build the specially crafted
> byte streams - I can help with this.
> 
> Overall I think that this change is good.
> 
> -Chris.

Re: (15) RFR: JDK-8247444: Trust final fields in records

[Remi Forax]

Hi Mndy,
Looks good,
I don't like too much the fact that there is a new boolean field in j.l.r.Field 
instead of using the modifiers like in the VM counterpart,
but it will require masking and unmasking the modifiers which is a far bigger 
change, too big to worth it in my opinion.

So +1

Rémi

- Mail original -
> De: "mandy chung" 
> À: "hotspot-dev" , "core-libs-dev" 
> , "amber-dev"
> 
> Envoyé: Lundi 15 Juin 2020 23:58:19
> Objet: (15) RFR: JDK-8247444: Trust final fields in records

> This patch is joint contribution from Christoph Dreis [1] and me.
> 
> Webrev: http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8247444/webrev.00/
> CSR: https://bugs.openjdk.java.net/browse/JDK-8247517
> 
> This proposes to make final fields in records notmodifiable via
> reflection.  Field::set throws IAE if a Field is not modifiable.
> Thecurrent specification specifies the following Fields not modifiable:
> - static final fields in any class
> - final fields in a hidden class
> 
> The spec of Field::set is changed to specify that records are not
> modifiable via reflection.
>  Noe that no change in Field::setAccessible(true), i.e. it will succeed
> to allow existing frameworks to have read access to final fields in
> records.  Just no write access.
> 
> VarHandle does not support write access if it's a static final field or
> an instance final field.
> 
> This patch also proposes `sun.misc.Unsafe::objectFieldOffset` and
> `sun.misc.Unsafe::staticField{Offset/Base}` not to support records.
> 
> No change is made in JNI.  JNI could be considered to disallow
> modification of final fields in records and hidden classes (static and
> instance fields).  But JNI has superpower and the current spec already
> allows to modify the value of a final static field even after it's
> constant folded (via JNI SetField and SetStaticField), then
> all bets are off.  This should be re-visited when we consider "final is
> truly final" for all classes.
> 
> Make final fields in records not modifiable via reflection enables JIT
> optimization as these final fields are effectively truly final.
> 
> This change impacts 3rd-party frameworks including 3rd-party
> serialization framework that rely on core reflection `setAccessible` or
> `sun.misc.Unsafe::allocateInstance` and `objectFieldOffset` etc to
> construct records but not using the canonical constructor.
> These frameworks would need to be updated to construct records via its
> canonical constructor as done by the Java serialization.
> 
> I see this change gives a good opportunity to engage the maintainers of
> the serialization frameworks and work together to support new features
> including records, inline classes and the new serialization mechanism
> and which I think it is worth the investment.
> 
> This is a low risk enhancement.  I'd like to request approval for a late
> enhancement in JDK 15.  It extends the pre-existing code path with
> refactoring the hidden classes to prepare for new kinds of classes with
> trusted final fields.  The change is straight-forward.
> 
> Can this wait to integrate in JDK 16?
> 
>   It's important to get this enhancement in when record is a preview
> feature that we can get feedback and give time to 3rd-party
> serialization frameworks to look into adding the support for records.
> If we delayed this change in 16 and records exit preview, it would be
> bad for frameworks if they verify that they support records in 15 but
> fail in 16.  OTOH the risk of this patch is low.
> 
> Performance Impact
> 
> I addressed the performance concern I raised earlier myself.  For
> reflection, VM creates the reflective Field objects and fills in
> MemberName when resolving a member.  VM will tag if this
> Field/MemberName is trusted final field.  I think this is a cleaner
> approach rather than in each place to check for final static and final
> fields in hidden or record class to determine if it has write access or
> not.
> 
> `sun.misc.Unsafe` does not use Field::isTrustedFinalField because (1)
> Unsafe has been allowing access of static final fields of any classes
> but isTrustedFinalField is not limited to instance fields (2) Unsafe
> disallows access to all fields in a hidden class (not limited to trusted
> final fields).  So it follows the precedence and simply checks if the
> declaring class is a record. `Class::isRecord` calls
> `Class::getSuperclass` to check if it's a subtype of `Record`.  As
> `Class::getSuperclass` is intrinsified, the call on isRecord on a normal
> class is fast. Christoph has contributed the microbenchmarks that
> confirm that no performance regression.
> 
> Thanks
> Mandy
> [1]
> https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-June/040096.html

Re: Comparator.reversed() type inference issue

[Remi Forax]

- Mail original -
> De: "Tagir Valeev" 
> À: "Attila Szegedi" 
> Cc: "core-libs-dev" 
> Envoyé: Mardi 9 Juin 2020 09:05:42
> Objet: Re: Comparator.reversed() type inference issue

> Hello!
> 
> This is more related to Java language specification and implementation
> than to core libraries, so compiler-dev is a more relevant mailing
> list. The current behavior perfectly follows the specification (JLS,
> 15.12):
> 
> A method invocation expression is a poly expression if all of the
> following are true:
> - The invocation appears in an assignment context or an invocation
> context (§5.2, §5.3). (1)
> - If the invocation is qualified (that is, any form of
> MethodInvocation except for the first), then the invocation elides
> TypeArguments to the left of the Identifier. (2)
> - The method to be invoked, as determined by the following
> subsections, is generic (§8.4.4) and has a return type that mentions
> at least one of the method's type parameters. (3)
> 
> If method invocation is a qualifier (in your case
> Map.Entry.comparingByValue() is a qualifier of reversed() call) then
> the context is neither assignment, nor invocation, hence (1) is not
> satisfied, hence it's a standalone expression, hence its type is
> determined solely by expression content. That's why to determine the
> type of Map.Entry.comparingByValue() we cannot look outside and must
> consider only the expression content. So its type is determined to be
> Comparator> (V#2 extends Comparable),
> as we don't have any other information. The reversed() call is also
> standalone, as the reversed() method is not generic, so (3) is not
> satisfied. Thus its type is just the same as its qualifier type.
> 
> People asked many times whether it's possible to make this code
> working without explicit type arguments. However, it's not just a
> trivial patch to the compiler and the spec. It's huge amount of work
> that may introduce tons of bugs, compiler crashes (I would expect
> StackOverflowError appear often during the compilation), compiler
> performance regression, and so on. Also, loads of inconsistencies
> between compiler and IDE behavior. I personally don't work on Java
> compiler, but I sometimes investigate bugs inside the IntelliJ IDEA
> type inference procedure. Believe me, it's already insanely complex,
> and the fact that at very least we can exclude qualifiers from the
> equation is really relieving. So while I cannot say for the whole
> OpenJDK team, I hardly believe this could be implemented any time
> soon.

I don't see how you can fix it even with a lot of dust of magic stars,
you have only the type information of the return value of
  Map.Entry.comparingByValue().reversed()

but they can no travel backward to Map.Entry.comparingByValue() because you 
have no idea of the return type of Map.Entry.comparingByValue() so you don't 
know the class that contains the method "reversed".

> 
> With best regards,
> Tagir Valeev.

Rémi

> 
> On Sun, Jun 7, 2020 at 7:34 PM Attila Szegedi  wrote:
>>
>> Hi folks,
>>
>> I'm teaching Java lately, and while teaching streams I use that good old
>> chestnut, the word count example. I'm baffled with some type inference woes,
>> though… trying to specify a reverse comparator using Comparator.reversed()
>> makes javac sad, e.g. this does not compile[1]:
>>
>>Map x = someMap();
>>
>>var rs1 = x.entrySet().stream().sorted(
>>Map.Entry.comparingByValue().reversed()
>>);
>>
>> On the other hand, using Collections.reverseOrder does compile:
>>
>>var rs2 = x.entrySet().stream().sorted(
>>Collections.reverseOrder(
>>Map.Entry.comparingByValue()
>>)
>>);
>>
>> Happens on both Java 11 and 14. It’s just baffling because based on type
>> signatures, it seems reasonable the first form should work as well.
>>
>> Thanks for any enlightenment!
>>
>> Attila.
>>
>> ---
>> [1] -Xdiags:verbose on Java 14 says:
>> error: no suitable method found for sorted(Comparator>)
>>var rs1 = x.entrySet().stream().sorted(
>>
>>method Stream.sorted() is not applicable
>>  (actual and formal argument lists differ in length)
>>method Stream.sorted(Comparator>) is not 
>> applicable
>>  (argument mismatch; Comparator> cannot be converted to
>>  Comparator>)
>>  where V#1,V#2 are type-variables:
>>V#1 extends Comparable
> >V#2 extends Comparable

Re: Comparator.reversed() type inference issue

[Remi Forax]

yep,
the inference is not smart enough.

So the way to fix the code is either to explicitly specify the type arguments 
  var rs1 = x.entrySet().stream().sorted(
  Map.Entry.comparingByValue().reversed()
);

or to store the result of comparingByValue() in a variable (so it's a poly 
expression)
  Comparator> comparator = Map.Entry.comparingByValue();
  var rs1 = x.entrySet().stream().sorted(comparator.reversed());

regards,
Rémi

- Mail original -
> De: "Tagir Valeev" 
> À: "Attila Szegedi" 
> Cc: "core-libs-dev" 
> Envoyé: Mardi 9 Juin 2020 09:05:42
> Objet: Re: Comparator.reversed() type inference issue

> Hello!
> 
> This is more related to Java language specification and implementation
> than to core libraries, so compiler-dev is a more relevant mailing
> list. The current behavior perfectly follows the specification (JLS,
> 15.12):
> 
> A method invocation expression is a poly expression if all of the
> following are true:
> - The invocation appears in an assignment context or an invocation
> context (§5.2, §5.3). (1)
> - If the invocation is qualified (that is, any form of
> MethodInvocation except for the first), then the invocation elides
> TypeArguments to the left of the Identifier. (2)
> - The method to be invoked, as determined by the following
> subsections, is generic (§8.4.4) and has a return type that mentions
> at least one of the method's type parameters. (3)
> 
> If method invocation is a qualifier (in your case
> Map.Entry.comparingByValue() is a qualifier of reversed() call) then
> the context is neither assignment, nor invocation, hence (1) is not
> satisfied, hence it's a standalone expression, hence its type is
> determined solely by expression content. That's why to determine the
> type of Map.Entry.comparingByValue() we cannot look outside and must
> consider only the expression content. So its type is determined to be
> Comparator> (V#2 extends Comparable),
> as we don't have any other information. The reversed() call is also
> standalone, as the reversed() method is not generic, so (3) is not
> satisfied. Thus its type is just the same as its qualifier type.
> 
> People asked many times whether it's possible to make this code
> working without explicit type arguments. However, it's not just a
> trivial patch to the compiler and the spec. It's huge amount of work
> that may introduce tons of bugs, compiler crashes (I would expect
> StackOverflowError appear often during the compilation), compiler
> performance regression, and so on. Also, loads of inconsistencies
> between compiler and IDE behavior. I personally don't work on Java
> compiler, but I sometimes investigate bugs inside the IntelliJ IDEA
> type inference procedure. Believe me, it's already insanely complex,
> and the fact that at very least we can exclude qualifiers from the
> equation is really relieving. So while I cannot say for the whole
> OpenJDK team, I hardly believe this could be implemented any time
> soon.
> 
> With best regards,
> Tagir Valeev.
> 
> On Sun, Jun 7, 2020 at 7:34 PM Attila Szegedi  wrote:
>>
>> Hi folks,
>>
>> I'm teaching Java lately, and while teaching streams I use that good old
>> chestnut, the word count example. I'm baffled with some type inference woes,
>> though… trying to specify a reverse comparator using Comparator.reversed()
>> makes javac sad, e.g. this does not compile[1]:
>>
>>Map x = someMap();
>>
>>var rs1 = x.entrySet().stream().sorted(
>>Map.Entry.comparingByValue().reversed()
>>);
>>
>> On the other hand, using Collections.reverseOrder does compile:
>>
>>var rs2 = x.entrySet().stream().sorted(
>>Collections.reverseOrder(
>>Map.Entry.comparingByValue()
>>)
>>);
>>
>> Happens on both Java 11 and 14. It’s just baffling because based on type
>> signatures, it seems reasonable the first form should work as well.
>>
>> Thanks for any enlightenment!
>>
>> Attila.
>>
>> ---
>> [1] -Xdiags:verbose on Java 14 says:
>> error: no suitable method found for sorted(Comparator>)
>>var rs1 = x.entrySet().stream().sorted(
>>
>>method Stream.sorted() is not applicable
>>  (actual and formal argument lists differ in length)
>>method Stream.sorted(Comparator>) is not 
>> applicable
>>  (argument mismatch; Comparator> cannot be converted to
>>  Comparator>)
>>  where V#1,V#2 are type-variables:
>>V#1 extends Comparable
> >V#2 extends Comparable