RFR: 8268222: javax/xml/jaxp/unittest/transform/Bug6216226Test.java failed, cannot delete file

[Joe Wang]

Revert changes in StreamResult.java made through 
https://github.com/openjdk/jdk/pull/4318 since it was creating a file stream on 
behalf of the Transformer, which resulted in a leaking file handle because the 
Transformer would only close files it opened.

This change instead replace the problematic file-uri-url-file conversion code 
with nio Paths. While we generally don't make such changes if it's not 
necessary as Apache still supports older versions of the JDK, we are okay with 
a code level 8.

-

Commit messages:
 - 8268222: javax/xml/jaxp/unittest/transform/Bug6216226Test.java failed, 
cannot delete file

Changes: https://git.openjdk.java.net/jdk/pull/4353/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4353=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8268222
  Stats: 37 lines in 2 files changed: 2 ins; 24 del; 11 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4353.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4353/head:pull/4353

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

Re: RFR: 8267630: Start of release updates for JDK 18 [v3]

[Joe Darcy]

> 8267630: Start of release updates for JDK 18

Joe Darcy has updated the pull request with a new target base due to a merge or 
a rebase. The incremental webrev excludes the unrelated changes brought in by 
the merge/rebase. The pull request contains 13 additional commits since the 
last revision:

 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - ... and 3 more: https://git.openjdk.java.net/jdk/compare/662de459...fc4d8725

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4175/files
  - new: https://git.openjdk.java.net/jdk/pull/4175/files/e584c467..fc4d8725

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4175=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4175=01-02

  Stats: 112 lines in 5 files changed: 10 ins; 50 del; 52 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4175.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4175/head:pull/4175

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

Re: RFR: 8268151: Vector API toShuffle optimization [v2]

[Xiaohong Gong]

On Thu, 3 Jun 2021 18:40:09 GMT, Sandhya Viswanathan  
wrote:

>> src/hotspot/share/opto/vectornode.cpp line 1246:
>> 
>>> 1244:   return new VectorLoadMaskNode(value, out_vt);
>>> 1245: } else if (is_vector_shuffle) {
>>> 1246:   if (!is_shuffle_to_vector()) {
>> 
>> Hi @sviswa7 , thanks for this change! I'm just curious whether 
>> `is_shuffle_to_vector()` is still needed for `VectorUnboxNode` with this 
>> change? It seems this flag can be removed, doesn't it?
>
> @XiaohongGong is_shuffle_to_vector is still needed as we shouldn't generate 
> VectorLoadShuffleNode for shuffle.toVector.

OK, got it. Thanks!

-

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

Re: RFR: 8268151: Vector API toShuffle optimization [v2]

[Xiaohong Gong]

On Thu, 3 Jun 2021 21:43:19 GMT, Sandhya Viswanathan  
wrote:

>> The Vector API toShuffle method can be optimized  using existing vector 
>> conversion intrinsic.
>> 
>> The following changes are made:
>> 1) vector.toShuffle java implementation is changed to call 
>> VectorSupport.convert.
>> 2) The conversion intrinsic (inline_vector_convert()) in 
>> vectorIntrinsics.cpp is changed to allow shuffle as a destination type.
>> 3) The shuffle.toVector intrinsic (inline_vector_shuffle_to_vector()) in 
>> vectorIntrinsics.cpp now explicitly generates conversion node instead of 
>> performing conversion during unbox. This is to remove unnecessary boxing 
>> during back to back vector.toShuffle and shuffle.toVector calls. 
>> 
>> Best Regards,
>> Sandhya
>
> Sandhya Viswanathan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Implement review comments

Looks good to me. Thanks!

-

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

Re: RFR: 8195129: System.load() fails to load from unicode paths [v3]

[David Holmes]

On Thu, 3 Jun 2021 17:48:59 GMT, Naoto Sato  wrote:

>> Right; I changed the code in NativeLibraries.c to pass down true UTF-8 
>> instead of "modified UTF-8". Please, take a look.
>
> I am not sure we can pass non `modified UTF-8` through `JVM_LoadLibrary()`. 
> Probably some VM folks can enlighten here?

Not an expert by my understanding is that the VM only deals with modified 
UTF-8, as does JNI. So the incoming string should be modified-UTF8 IMO and then 
converted to UTF16.

That said, this is shared code being modified on the JDK side so you can't just 
change the type of string being passed in without updating all the 
implementations of os::dll_load to support that!

-

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

Integrated: 8268224: Cleanup references to "strictfp" in core lib comments

[Joe Darcy]

On Fri, 4 Jun 2021 01:04:39 GMT, Joe Darcy  wrote:

> With the right baseline this time, as noticed by John Rose, after JEP 306 
> removing non-strict floating-point was integrated, a few stale references 
> were left to "strictfp" and "value sets" in core library comments. This 
> changeset removes those stray references.

This pull request has now been integrated.

Changeset: 05df1727
Author:Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/05df1727b529992aeac666b4984d0681d61ebb31
Stats: 97 lines in 2 files changed: 0 ins; 50 del; 47 mod

8268224: Cleanup references to "strictfp" in core lib comments

Reviewed-by: jrose

-

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

Re: RFR: 8268224: Cleanup references to "strictfp" in core lib comments [v2]

[Joe Darcy]

> With the right baseline this time, as noticed by John Rose, after JEP 306 
> removing non-strict floating-point was integrated, a few stale references 
> were left to "strictfp" and "value sets" in core library comments. This 
> changeset removes those stray references.

Joe Darcy has updated the pull request incrementally with one additional commit 
since the last revision:

  Reflow paragraphs.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4349/files
  - new: https://git.openjdk.java.net/jdk/pull/4349/files/7ab8f9f2..596971e0

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4349=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4349=00-01

  Stats: 50 lines in 2 files changed: 0 ins; 5 del; 45 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4349.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4349/head:pull/4349

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

Re: RFR: 8268224: Cleanup references to "strictfp" in core lib comments

[John R Rose]

On Fri, 4 Jun 2021 01:04:39 GMT, Joe Darcy  wrote:

> With the right baseline this time, as noticed by John Rose, after JEP 306 
> removing non-strict floating-point was integrated, a few stale references 
> were left to "strictfp" and "value sets" in core library comments. This 
> changeset removes those stray references.

Good.  I note that `strictfp` occurs in a few other places, notably two files 
in `java.lang.reflect`.  But (as you explain) those occurrences are necessary 
to support the contents of old-format class files.

-

Marked as reviewed by jrose (Reviewer).

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

Re: RFR: 8174222: LambdaMetafactory: validate inputs and improve documentation [v2]

[liach]

On Thu, 3 Jun 2021 20:40:23 GMT, Dan Smith  wrote:

>> Standardizes and better specifies the errors thrown by LambdaMetafactory, 
>> including for inputs that would not be generated by javac.
>> 
>> Does some renaming of core parameters/fields to better reflect their purpose.
>> 
>> In the implementation, stops using ACC_BRIDGE for any methods, which is not 
>> a good fit for what these methods do (they don't delegate to each other, but 
>> all invoke the same implementation method).
>
> Dan Smith has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix accidentally commented-out parts of test

src/java.base/share/classes/java/lang/invoke/LambdaMetafactory.java line 547:

> 545: throw new IllegalArgumentException("argument has wrong 
> type");
> 546: }
> 547: return type.cast(result);

Can we just use a `return (T) result` as there will always be a checked cast on 
the caller's end, and this call seems redundant? The only shortcoming is that 
the call will raise an unchecked warning that needs to be suppressed. Or is 
this negligible after hotspot optimization?

-

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

Re: RFR: 8268224: Cleanup references to "strictfp" in core lib comments

[Joe Darcy]

On Fri, 4 Jun 2021 01:04:39 GMT, Joe Darcy  wrote:

> With the right baseline this time, as noticed by John Rose, after JEP 306 
> removing non-strict floating-point was integrated, a few stale references 
> were left to "strictfp" and "value sets" in core library comments. This 
> changeset removes those stray references.

PS I'll re-flow the paragraph before pushing. I left them un-re-flowed to ease 
review.

-

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

RFR: 8268224: Cleanup references to "strictfp" in core lib comments

[Joe Darcy]

With the right baseline this time, as noticed by John Rose, after JEP 306 
removing non-strict floating-point was integrated, a few stale references were 
left to "strictfp" and "value sets" in core library comments. This changeset 
removes those stray references.

-

Commit messages:
 - 8268224: Cleanup references to "strictfp" in core lib comments

Changes: https://git.openjdk.java.net/jdk/pull/4349/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4349=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8268224
  Stats: 56 lines in 2 files changed: 0 ins; 45 del; 11 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4349.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4349/head:pull/4349

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

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

[forax]

> De: "John Rose" 
> À: "Remi Forax" 
> Cc: "Peter Levart" , "Rémi Forax"
> , "core-libs-dev" 
> Envoyé: Vendredi 4 Juin 2021 02:05:41
> Objet: Re: [External] : Re: RFR: 8199318: add idempotent copy operation for
> Map.Entry

> On Jun 3, 2021, at 3:17 PM, [ mailto:fo...@univ-mlv.fr | fo...@univ-mlv.fr ]
> wrote:

>>> De: "John Rose" mailto:r.r...@oracle.com | r.r...@oracle.com ] >
>>> À: "Remi Forax" < [ mailto:fo...@univ-mlv.fr | fo...@univ-mlv.fr ] >
>>> Cc: "Peter Levart" < [ mailto:peter.lev...@gmail.com | 
>>> peter.lev...@gmail.com ]
>>> >, "Rémi Forax" < [ mailto:fo...@openjdk.java.net | fo...@openjdk.java.net 
>>> >] >,
>>> "core-libs-dev" < [ mailto:core-libs-dev@openjdk.java.net |
>>> core-libs-dev@openjdk.java.net ] >
>>> Envoyé: Jeudi 3 Juin 2021 22:51:28
>>> Objet: Re: RFR: 8199318: add idempotent copy operation for Map.Entry

>>> ...

>>> interface ComparableRecord>
>>> extends Comparable { … }

>>> record Foo(int x, String y) implements ComparableRecord { … }

>>> [ http://cr.openjdk.java.net/~jrose/draft/ComparableRecord.java |
>>> http://cr.openjdk.java.net/~jrose/draft/ComparableRecord.java ]

>> [repost with a link]

>> The main issue with this kind of code is that the JIT does not see through 
>> the
>> ClassValue.

> Wow, it’s almost as if we’ve discussed this already! ;-)
> So, [ https://bugs.openjdk.java.net/browse/JDK-8238260 |
> https://bugs.openjdk.java.net/browse/JDK-8238260 ]
> Part of my agenda here is finding more reasons why
> JDK-8238260 deserves some love.

> Currently, the translation strategy for Records
> requires a lot of boilerplate generated into each
> subclass for toString, equals, and hashCode.
> It is partially declarative, because it uses indy.
> So, yay for that. But it is also a bad smell (IMO)
> that the trick needs to be played in each subclass.

> If ClassValue were performant, we could have
> just one method in Record for each of toString,
> equals, and hashCode, and have them DTRT.
> The user code would then be able to call
> super.toString() to get the standard behavior
> in a refining wrapper method in a subclass.

> Looking further, it would be nice to have methods
> which (a) inherit from supers as reusable code
> ought to, but (b) re-specialize themselves once
> per subclass indy-style. This is a VM trick I hope
> to look into after we do specialization. For now,
> ClassValue is the way to simulate that.

yes, it's a specialization problem. 
I wonder if it an be resolved using a combination of a species-static variable 
and magic shibboleth to ask the type parameter to be always reified 

interface ComparableRecord  > extends Comparable< T > { 
species-static Comparator COMPARATOR; // a parameteric condy ? 
species-static { 
COMPARATOR = ... 
} 

default int compareTo(T other) { 
return COMPARATOR.compare(this, other); 
} 
} 

>> Tweaking a little bit your code, I get
>> [
>> https://urldefense.com/v3/__https://gist.github.com/forax/e76367e1a90bf011692ee9bec65ff0f8__;!!GqivPVa7Brio!MheW9rHDHXlXYNKUju7v5G0vXlpj1YOoDWFjG9AcpnXnVz2TxlMYDM7i86yFtT_B$
>> | https://gist.github.com/forax/e76367e1a90bf011692ee9bec65ff0f8 ]

>> (It's a PITA that we have to use a raw type to workaround circularly defined
>> parameter type)

> I tried to DTRT and got into Inference Hell, surrounded
> by a swarms of unfriendly wildcards with pitchforks.
> Glad it wasn’t just me.

> Inspired by your more whole-hearted use of streams
> to build the code, I updated my example. Thanks.

> — John

Rémi 

Re: RFR: 8174222: LambdaMetafactory: validate inputs and improve documentation [v2]

[Mandy Chung]

On Thu, 3 Jun 2021 20:40:23 GMT, Dan Smith  wrote:

>> Standardizes and better specifies the errors thrown by LambdaMetafactory, 
>> including for inputs that would not be generated by javac.
>> 
>> Does some renaming of core parameters/fields to better reflect their purpose.
>> 
>> In the implementation, stops using ACC_BRIDGE for any methods, which is not 
>> a good fit for what these methods do (they don't delegate to each other, but 
>> all invoke the same implementation method).
>
> Dan Smith has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix accidentally commented-out parts of test

src/java.base/share/classes/java/lang/invoke/LambdaMetafactory.java line 457:

> 455:  * @return a CallSite whose target can be used to perform capture, 
> generating
> 456:  * instances of the interface named by {@code factoryType}
> 457:  * @throws LambdaConversionException If {@code caller} does not have 
> private

One additional comment:

The lookup access has been extended since 14 to include `MODULE` and `ORIGINAL` 
access.  
`Lookup::hasFullPrivilegeAccess` returns true if the lookup has `PRIVATE` and 
`MODULE` which means that this lookup is not teleported from another module via 
`Lookup::in` and `MethodHandles::privateLookupIn`.

What privilege do you expect the `caller` lookup should have?  I believe full 
privilege access is the appropriate privilege.  The `ORIGINAL` access is 
stricter as the lookup must be created from the original lookup class.

[1] shows what access a `Lookup` has when being produced via different APIs.

[1] 
https://download.java.net/java/early_access/jdk17/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#access-modes

-

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

Re: RFR: 8267630: Start of release updates for JDK 18 [v2]

[Joe Darcy]

> 8267630: Start of release updates for JDK 18

Joe Darcy has updated the pull request with a new target base due to a merge or 
a rebase. The incremental webrev excludes the unrelated changes brought in by 
the merge/rebase. The pull request contains 12 additional commits since the 
last revision:

 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - Merge branch 'master' into 8267630
 - ... and 2 more: https://git.openjdk.java.net/jdk/compare/388f8dae...e584c467

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4175/files
  - new: https://git.openjdk.java.net/jdk/pull/4175/files/9c7c88bf..e584c467

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4175=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4175=00-01

  Stats: 428395 lines in 342 files changed: 426299 ins; 917 del; 1179 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4175.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4175/head:pull/4175

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

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

[John Rose]

On Jun 3, 2021, at 3:17 PM, fo...@univ-mlv.fr wrote:




De: "John Rose" mailto:r.r...@oracle.com>>
À: "Remi Forax" mailto:fo...@univ-mlv.fr>>
Cc: "Peter Levart" mailto:peter.lev...@gmail.com>>, 
"Rémi Forax" mailto:fo...@openjdk.java.net>>, 
"core-libs-dev" 
mailto:core-libs-dev@openjdk.java.net>>
Envoyé: Jeudi 3 Juin 2021 22:51:28
Objet: Re: RFR: 8199318: add idempotent copy operation for Map.Entry
...

interface ComparableRecord>
extends Comparable { … }

record Foo(int x, String y) implements ComparableRecord { … }

http://cr.openjdk.java.net/~jrose/draft/ComparableRecord.java

[repost with a link]

The main issue with this kind of code is that the JIT does not see through the 
ClassValue.

Wow, it’s almost as if we’ve discussed this already! ;-)
So, https://bugs.openjdk.java.net/browse/JDK-8238260
Part of my agenda here is finding more reasons why
JDK-8238260 deserves some love.

Currently, the translation strategy for Records
requires a lot of boilerplate generated into each
subclass for toString, equals, and hashCode.
It is partially declarative, because it uses indy.
So, yay for that.  But it is also a bad smell (IMO)
that the trick needs to be played in each subclass.

If ClassValue were performant, we could have
just one method in Record for each of toString,
equals, and hashCode, and have them DTRT.
The user code would then be able to call
super.toString() to get the standard behavior
in a refining wrapper method in a subclass.

Looking further, it would be nice to have methods
which (a) inherit from supers as reusable code
ought to, but (b) re-specialize themselves once
per subclass indy-style.  This is a VM trick I hope
to look into after we do specialization.  For now,
ClassValue is the way to simulate that.


Tweaking a little bit your code, I get
https://gist.github.com/forax/e76367e1a90bf011692ee9bec65ff0f8

(It's a PITA that we have to use a raw type to workaround circularly defined 
parameter type)

I tried to DTRT and got into Inference Hell, surrounded
by a swarms of unfriendly wildcards with pitchforks.
Glad it wasn’t just me.

Inspired by your more whole-hearted use of streams
to build the code, I updated my example.  Thanks.

— John

Re: RFR: 8174222: LambdaMetafactory: validate inputs and improve documentation [v2]

[Joe Darcy]

Yes, the CSR should be re-reviewed before the change goes in.

I've changed the CSR status to indicate it needs another review.

-Joe


On 6/3/2021 4:07 PM, Mandy Chung wrote:

On Thu, 3 Jun 2021 20:40:23 GMT, Dan Smith  wrote:


Standardizes and better specifies the errors thrown by LambdaMetafactory, 
including for inputs that would not be generated by javac.

Does some renaming of core parameters/fields to better reflect their purpose.

In the implementation, stops using ACC_BRIDGE for any methods, which is not a 
good fit for what these methods do (they don't delegate to each other, but all 
invoke the same implementation method).

Dan Smith has updated the pull request incrementally with one additional commit 
since the last revision:

   Fix accidentally commented-out parts of test

Looks fine to me.   The CSR was approved for 13 and its fixVersion should be 
updated to 17.

test/jdk/java/lang/invoke/lambda/MetafactoryArgValidationTest.java line 2:


1: /*
2:  * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.

copyright year needs update.

-

Marked as reviewed by mchung (Reviewer).

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

Re: RFR: 8174222: LambdaMetafactory: validate inputs and improve documentation [v2]

[Paul Sandoz]

On Thu, 3 Jun 2021 20:40:23 GMT, Dan Smith  wrote:

>> Standardizes and better specifies the errors thrown by LambdaMetafactory, 
>> including for inputs that would not be generated by javac.
>> 
>> Does some renaming of core parameters/fields to better reflect their purpose.
>> 
>> In the implementation, stops using ACC_BRIDGE for any methods, which is not 
>> a good fit for what these methods do (they don't delegate to each other, but 
>> all invoke the same implementation method).
>
> Dan Smith has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix accidentally commented-out parts of test

Code looks good, nice test.

-

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

Re: RFR: 8174222: LambdaMetafactory: validate inputs and improve documentation [v2]

[Mandy Chung]

On Thu, 3 Jun 2021 20:40:23 GMT, Dan Smith  wrote:

>> Standardizes and better specifies the errors thrown by LambdaMetafactory, 
>> including for inputs that would not be generated by javac.
>> 
>> Does some renaming of core parameters/fields to better reflect their purpose.
>> 
>> In the implementation, stops using ACC_BRIDGE for any methods, which is not 
>> a good fit for what these methods do (they don't delegate to each other, but 
>> all invoke the same implementation method).
>
> Dan Smith has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix accidentally commented-out parts of test

Looks fine to me.   The CSR was approved for 13 and its fixVersion should be 
updated to 17.

test/jdk/java/lang/invoke/lambda/MetafactoryArgValidationTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.

copyright year needs update.

-

Marked as reviewed by mchung (Reviewer).

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

Re: RFR: 8268151: Vector API toShuffle optimization [v2]

[Sandhya Viswanathan]

On Thu, 3 Jun 2021 22:01:12 GMT, Paul Sandoz  wrote:

>> Sandhya Viswanathan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Implement review comments
>
> Java changes look good.

@PaulSandoz Thanks a lot for the review.

-

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

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

[forax]

> De: "John Rose" 
> À: "Remi Forax" < fo...@univ-mlv.fr >
> Cc: "Peter Levart" < peter.lev...@gmail.com >, "Rémi Forax" <
> fo...@openjdk.java.net >, "core-libs-dev" < core-libs-dev@openjdk.java.net >
> Envoyé: Jeudi 3 Juin 2021 22:51:28
> Objet: Re: RFR: 8199318: add idempotent copy operation for Map.Entry

> On Jun 3, 2021, at 12:46 PM, Remi Forax < [ mailto:fo...@univ-mlv.fr |
> fo...@univ-mlv.fr ] > wrote:

>> 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.

> That’s a slippery slope. IIRC we consciously stopped
> before that step.

> That said, there are other ways to fix this. We should
> have utilities (maybe in the JDK but not the JLS) which
> build such methods and make it easy for users to grab onto
> them. Maybe something like this:

> interface ComparableRecord>
> extends Comparable { … }

> record Foo(int x, String y) implements ComparableRecord { … }

> [ http://cr.openjdk.java.net/~jrose/draft/ComparableRecord.java |
> http://cr.openjdk.java.net/~jrose/draft/ComparableRecord.java ]

[repost with a link] 

The main issue with this kind of code is that the JIT does not see through the 
ClassValue. 

Tweaking a little bit your code, I get 
https://gist.github.com/forax/e76367e1a90bf011692ee9bec65ff0f8 

(It's a PITA that we have to use a raw type to workaround circularly defined 
parameter type) 

> — John

Rémi 

Re: RFR: 8268151: Vector API toShuffle optimization [v2]

[Paul Sandoz]

On Thu, 3 Jun 2021 21:43:19 GMT, Sandhya Viswanathan  
wrote:

>> The Vector API toShuffle method can be optimized  using existing vector 
>> conversion intrinsic.
>> 
>> The following changes are made:
>> 1) vector.toShuffle java implementation is changed to call 
>> VectorSupport.convert.
>> 2) The conversion intrinsic (inline_vector_convert()) in 
>> vectorIntrinsics.cpp is changed to allow shuffle as a destination type.
>> 3) The shuffle.toVector intrinsic (inline_vector_shuffle_to_vector()) in 
>> vectorIntrinsics.cpp now explicitly generates conversion node instead of 
>> performing conversion during unbox. This is to remove unnecessary boxing 
>> during back to back vector.toShuffle and shuffle.toVector calls. 
>> 
>> Best Regards,
>> Sandhya
>
> Sandhya Viswanathan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Implement review comments

Java changes look good.

-

Marked as reviewed by psandoz (Reviewer).

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

Integrated: 8267939: Clarify the specification of iterator and spliterator forEachRemaining

[Paul Sandoz]

On Tue, 1 Jun 2021 19:54:59 GMT, Paul Sandoz  wrote:

> The specification of `forEachRemaining`, accepting a primitive functional 
> interface, on the primitive iterators is updated to be the same as for 
> `Iterator.forEachRemaining`, specifically the following is added:
> 
> 
>  * 
>  * Subsequent behavior of an iterator is unspecified if the action throws 
> an
>  * exception.
> 
> 
> In addition the specification of `tryAdvance` and `forEachRemaining` on 
> `Spliterator` and the primitive specializations are also updated to include a 
> similar statement:
> 
> 
>  * Subsequent behavior of a spliterator is unspecified if the action 
> throws
>  * an exception.

This pull request has now been integrated.

Changeset: c1f3094f
Author:Paul Sandoz 
URL:   
https://git.openjdk.java.net/jdk/commit/c1f3094f814a4f3586222aad50ed314906b5bc9c
Stats: 48 lines in 2 files changed: 20 ins; 21 del; 7 mod

8267939: Clarify the specification of iterator and spliterator forEachRemaining

Reviewed-by: smarks

-

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

Withdrawn: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS

[Henry Jen]

On Fri, 28 May 2021 21:55:24 GMT, Henry Jen  wrote:

> …d on macOS
> 
> This patch simply round up the specified stack size to multiple of the system 
> page size. 
> 
> Test is trivial, simply run java with -Xss option against following code. On 
> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get 
> `7183` and `649` respectively. After fix, both would output `649`, while 
> `-Xss161k` would be same as `-Xss164k` and see 691 as the output.
> 
> ```code:java
> public class StackLeak {
> public int depth = 0;
> public void stackLeak() {
> depth++;
> stackLeak();
> }
> 
> public static void main(String[] args) {
> var test = new StackLeak();
> try {
> test.stackLeak();
> } catch (Throwable e) {
> System.out.println(test.depth);
> }
> }
> }

This pull request has been closed without being integrated.

-

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

Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS

[Henry Jen]

On Fri, 28 May 2021 21:55:24 GMT, Henry Jen  wrote:

> …d on macOS
> 
> This patch simply round up the specified stack size to multiple of the system 
> page size. 
> 
> Test is trivial, simply run java with -Xss option against following code. On 
> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get 
> `7183` and `649` respectively. After fix, both would output `649`, while 
> `-Xss161k` would be same as `-Xss164k` and see 691 as the output.
> 
> ```code:java
> public class StackLeak {
> public int depth = 0;
> public void stackLeak() {
> depth++;
> stackLeak();
> }
> 
> public static void main(String[] args) {
> var test = new StackLeak();
> try {
> test.stackLeak();
> } catch (Throwable e) {
> System.out.println(test.depth);
> }
> }
> }

Withdraw for keep current behavior for compatibility. It would be preferred for 
user to specify proper value than we change the value on user's behalf.

-

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

Re: RFR: 8266846: Add java.time.InstantSource [v6]

[Naoto Sato]

On Thu, 3 Jun 2021 20:59:24 GMT, Stephen Colebourne  
wrote:

>> 8266846: Add java.time.InstantSource
>
> Stephen Colebourne has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8266846: Add java.time.InstantSource

Marked as reviewed by naoto (Reviewer).

-

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

Re: RFR: 8266846: Add java.time.InstantSource [v6]

[Joe Darcy]

On Thu, 3 Jun 2021 20:59:24 GMT, Stephen Colebourne  
wrote:

>> 8266846: Add java.time.InstantSource
>
> Stephen Colebourne has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8266846: Add java.time.InstantSource

Marked as reviewed by darcy (Reviewer).

-

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

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

[forax]

> De: "John Rose" 
> À: "Remi Forax" 
> Cc: "Peter Levart" , "Rémi Forax"
> , "core-libs-dev" 
> Envoyé: Jeudi 3 Juin 2021 22:51:28
> Objet: Re: RFR: 8199318: add idempotent copy operation for Map.Entry

> On Jun 3, 2021, at 12:46 PM, Remi Forax < [ mailto:fo...@univ-mlv.fr |
> fo...@univ-mlv.fr ] > wrote:

>> 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.

> That’s a slippery slope. IIRC we consciously stopped
> before that step.

> That said, there are other ways to fix this. We should
> have utilities (maybe in the JDK but not the JLS) which
> build such methods and make it easy for users to grab onto
> them. Maybe something like this:

> interface ComparableRecord>
> extends Comparable { … }

> record Foo(int x, String y) implements ComparableRecord { … }

> [ http://cr.openjdk.java.net/~jrose/draft/ComparableRecord.java |
> http://cr.openjdk.java.net/~jrose/draft/ComparableRecord.java ]

The main issue with this kind of code is that the JIT does not see through the 
ClassValue. 

Tweaking a little bit your code, I get 
(It's a PITA that we have to use a raw type to workaround circularly defined 
parameter type) 

import java.util.ArrayList; 
import java.util.Comparator; 
import java.util.List; 
import java.util.stream.Stream; 

@SuppressWarnings({"rawtypes","unchecked"}) 
interface ComparableRecord> extends 
Comparable { 
@Override default int compareTo(T that) { 
if (this.getClass() != that.getClass()) { 
throw new IllegalArgumentException("not same class"); 
} 
return COMPARE_TO_METHODS.get(this.getClass()).compare(this, that); 
} 
static final ClassValue> COMPARE_TO_METHODS = new 
ClassValue<>() { 
@Override 
protected Comparator computeValue(Class type) { 
return Stream.of(type.getRecordComponents()) 
.map(component -> { 
var accessor = component.getAccessor(); 
return Comparator.comparing(r -> { 
try { 
return (Comparable) accessor.invoke(r); 
} catch (ReflectiveOperationException ex) { 
throw new IllegalArgumentException(ex); 
} 
}); 
}) 
.reduce((r1, r2) -> 0, Comparator::thenComparing, (_1, _2) -> { throw null; }); 
} 
}; 

static void main(String[] args) { 
record Foo(int x, String y) implements ComparableRecord { } 

var list = Stream.of(new Foo(2, "foo"), new Foo(2, "bar")) .sorted().toList(); 
System.out.println(list); 
} 
} 

> — John

Re: RFR: 8268151: Vector API toShuffle optimization [v2]

[Sandhya Viswanathan]

On Thu, 3 Jun 2021 02:31:51 GMT, Xiaohong Gong  wrote:

>> Sandhya Viswanathan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Implement review comments
>
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java
>  line 335:
> 
>> 333: @ForceInline
>> 334: private final
>> 335: VectorShuffle toShuffleTemplate(AbstractSpecies dsp) {
> 
> Is it better to move this template method to the super class like other APIs?

Yes, can be moved to super class. Done in the updated commit.

> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java
>  line 350:
> 
>> 348:  Byte128Shuffle.class, byte.class, 
>> VLENGTH,
>> 349:  this, VSPECIES,
>> 350:  Byte128Vector::toShuffleTemplate);
> 
> ditto

Yes, can be moved to super class. Done in the updated commit.

-

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

Re: RFR: 8268151: Vector API toShuffle optimization [v2]

[Sandhya Viswanathan]

> The Vector API toShuffle method can be optimized  using existing vector 
> conversion intrinsic.
> 
> The following changes are made:
> 1) vector.toShuffle java implementation is changed to call 
> VectorSupport.convert.
> 2) The conversion intrinsic (inline_vector_convert()) in vectorIntrinsics.cpp 
> is changed to allow shuffle as a destination type.
> 3) The shuffle.toVector intrinsic (inline_vector_shuffle_to_vector()) in 
> vectorIntrinsics.cpp now explicitly generates conversion node instead of 
> performing conversion during unbox. This is to remove unnecessary boxing 
> during back to back vector.toShuffle and shuffle.toVector calls. 
> 
> Best Regards,
> Sandhya

Sandhya Viswanathan has updated the pull request incrementally with one 
additional commit since the last revision:

  Implement review comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4326/files
  - new: https://git.openjdk.java.net/jdk/pull/4326/files/d5652051..ab582a1c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4326=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4326=00-01

  Stats: 657 lines in 38 files changed: 161 ins; 465 del; 31 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4326.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4326/head:pull/4326

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

Re: RFR: 8266846: Add java.time.InstantSource [v6]

[Roger Riggs]

On Thu, 3 Jun 2021 20:59:24 GMT, Stephen Colebourne  
wrote:

>> 8266846: Add java.time.InstantSource
>
> Stephen Colebourne has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8266846: Add java.time.InstantSource

Marked as reviewed by rriggs (Reviewer).

-

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

Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS

[Henry Jen]

On Mon, 31 May 2021 20:23:53 GMT, Vladimir Kempik  wrote:

>> src/java.base/macosx/native/libjli/java_md_macosx.m line 727:
>> 
>>> 725: 
>>> 726: static size_t alignUp(size_t stack_size) {
>>> 727: long page_size = sysconf(_SC_PAGESIZE);
>> 
>> In hotspot we use `getpagesize()`. There is also a guard for a very large 
>> stack (within a page of SIZE_MAX) so that rounding up does not produce zero.
>
> sounds like that (getpagesize) should work with m1 mac as well, as they have 
> 16k pages. will it ?

sysconf is the portable way based on POSIX, we can use getpagesize give this is 
macOS specific code, which is BSD based.

-

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

Integrated: 8266019: StreamResult(File) writes to incorrect file path if # is part of the file path

[Joe Wang]

On Wed, 2 Jun 2021 18:17:43 GMT, Joe Wang  wrote:

> Special characters are different in File and URI. Treat File input as File 
> using FileInputStream instead of converting to an URI, but fall back to URI 
> in case of error for compatibility (in error handling).

This pull request has now been integrated.

Changeset: 460ce555
Author:Joe Wang 
URL:   
https://git.openjdk.java.net/jdk/commit/460ce5553c587715ef8244bee7e363b3587d2d0f
Stats: 39 lines in 2 files changed: 31 ins; 1 del; 7 mod

8266019: StreamResult(File) writes to incorrect file path if # is part of the 
file path

Reviewed-by: dfuchs

-

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

Re: RFR: 8266846: Add java.time.InstantSource [v6]

[Stephen Colebourne]

> 8266846: Add java.time.InstantSource

Stephen Colebourne has updated the pull request incrementally with one 
additional commit since the last revision:

  8266846: Add java.time.InstantSource

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4016/files
  - new: https://git.openjdk.java.net/jdk/pull/4016/files/f01c5cdd..d564956c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4016=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4016=04-05

  Stats: 7 lines in 1 file changed: 6 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4016.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4016/head:pull/4016

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

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

[John Rose]

On Jun 3, 2021, at 12:46 PM, Remi Forax 
mailto:fo...@univ-mlv.fr>> wrote:

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.

That’s a slippery slope.  IIRC we consciously stopped
before that step.

That said, there are other ways to fix this.  We should
have utilities (maybe in the JDK but not the JLS) which
build such methods and make it easy for users to grab onto
them.  Maybe something like this:

interface ComparableRecord>
extends Comparable { … }

record Foo(int x, String y) implements ComparableRecord { … }

http://cr.openjdk.java.net/~jrose/draft/ComparableRecord.java

— John

Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries [v7]

[Maurizio Cimadamore]

> This patch overhauls the library loading mechanism used by the Foreign Linker 
> API. We realized that, while handy, the *default* lookup abstraction 
> (`LibraryLookup::ofDefault`) was behaving inconsistentlt across platforms.
> 
> This patch replaces `LibraryLookup` with a simpler `SymbolLookup` 
> abstraction, a functional interface. Crucially, `SymbolLookup` does not 
> concern with library loading, only symbol lookup. For this reason, two 
> factories are added:
> 
> * `SymbolLookup::loaderLookup` - which obtains a lookup that can be used to 
> lookup symbols in libraries loaded by current loader
> * `CLinker::systemLookup` - a more stable replacement for the *default* 
> lookup, which looks for symbols in libc.so (or its equivalent in other 
> platforms). The contents of this lookup are unspecified.
> 
> Both factories are *restricted*, so they can only be called when 
> `--enable-native-access` is set.

Maurizio Cimadamore has updated the pull request incrementally with one 
additional commit since the last revision:

  Address review comments on shim lib makefile

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4316/files
  - new: https://git.openjdk.java.net/jdk/pull/4316/files/2545e2b6..23d66faf

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4316=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4316=05-06

  Stats: 15 lines in 1 file changed: 0 ins; 0 del; 15 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4316.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4316/head:pull/4316

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

Integrated: 8267995: Add reference to JVMS class file format in Lookup::defineHiddenClass

[Mandy Chung]

On Thu, 3 Jun 2021 18:21:31 GMT, Mandy Chung  wrote:

> Trivial javadoc change to add a reference to JVMS 4.1 in 
> `Lookup::defineHiddenClass` such that the reference like `this_class` would 
> be referred to JVMS for details.

This pull request has now been integrated.

Changeset: b9558655
Author:Mandy Chung 
URL:   
https://git.openjdk.java.net/jdk/commit/b95586559ca44b040261168cbe5ba90689cab17e
Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod

8267995: Add reference to JVMS class file format in Lookup::defineHiddenClass

Reviewed-by: darcy

-

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

Re: RFR: 8174222: LambdaMetafactory: validate inputs and improve documentation [v2]

[Dan Smith]

> Standardizes and better specifies the errors thrown by LambdaMetafactory, 
> including for inputs that would not be generated by javac.
> 
> Does some renaming of core parameters/fields to better reflect their purpose.
> 
> In the implementation, stops using ACC_BRIDGE for any methods, which is not a 
> good fit for what these methods do (they don't delegate to each other, but 
> all invoke the same implementation method).

Dan Smith has updated the pull request incrementally with one additional commit 
since the last revision:

  Fix accidentally commented-out parts of test

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4346/files
  - new: https://git.openjdk.java.net/jdk/pull/4346/files/81db8e5e..4b8d0dab

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4346=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4346=00-01

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4346.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4346/head:pull/4346

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

Re: RFR: 8174222: LambdaMetafactory: validate inputs and improve documentation

[Dan Smith]

On Thu, 3 Jun 2021 20:03:53 GMT, Dan Smith  wrote:

> Standardizes and better specifies the errors thrown by LambdaMetafactory, 
> including for inputs that would not be generated by javac.
> 
> Does some renaming of core parameters/fields to better reflect their purpose.
> 
> In the implementation, stops using ACC_BRIDGE for any methods, which is not a 
> good fit for what these methods do (they don't delegate to each other, but 
> all invoke the same implementation method).

CSR was completed months ago: JDK-8221255. Just never got around to pushing the 
changes. Since then, InnerClassLambdaMetafactory was changed significantly, but 
the the public API is unchanged.

In testing, I discovered one issue leading to an internal error: JDK-8268192. I 
commented out my test case and will follow up on that issue separately.

-

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

RFR: 8174222: LambdaMetafactory: validate inputs and improve documentation

[Dan Smith]

Standardizes and better specifies the errors thrown by LambdaMetafactory, 
including for inputs that would not be generated by javac.

Does some renaming of core parameters/fields to better reflect their purpose.

In the implementation, stops using ACC_BRIDGE for any methods, which is not a 
good fit for what these methods do (they don't delegate to each other, but all 
invoke the same implementation method).

-

Commit messages:
 - Remove use of ACC_BRIDGE
 - Comment out failing test cases, will fix separately
 - Fixes from merge
 - Merge branch 'master' into 8174222
 - 8174222: LambdaMetafactory: validate inputs and improve documentation

Changes: https://git.openjdk.java.net/jdk/pull/4346/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4346=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8174222
  Stats: 726 lines in 4 files changed: 397 ins; 33 del; 296 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4346.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4346/head:pull/4346

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

Integrated: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics

[Sandhya Viswanathan]

On Thu, 22 Apr 2021 19:07:28 GMT, Sandhya Viswanathan 
 wrote:

> This PR contains Short Vector Math Library support related changes for 
> [JEP-414 Vector API (Second Incubator)](https://openjdk.java.net/jeps/414), 
> in preparation for when targeted.
> 
> Intel Short Vector Math Library (SVML) based intrinsics in native x86 
> assembly provide optimized implementation for Vector API transcendental and 
> trigonometric methods.
> These methods are built into a separate library instead of being part of 
> libjvm.so or jvm.dll.
> 
> The following changes are made:
>The source for these methods is placed in the jdk.incubator.vector module 
> under src/jdk.incubator.vector/linux/native/libsvml and 
> src/jdk.incubator.vector/windows/native/libsvml.
>The assembly source files are named as “*.S” and include files are named 
> as “*.S.inc”.
>The corresponding build script is placed at 
> make/modules/jdk.incubator.vector/Lib.gmk.
>Changes are made to build system to support dependency tracking for 
> assembly files with includes.
>The built native libraries (libsvml.so/svml.dll) are placed in bin 
> directory of JDK on Windows and lib directory of JDK on Linux.
>The C2 JIT uses the dll_load and dll_lookup to get the addresses of 
> optimized methods from this library.
> 
> Build system changes and module library build scripts are contributed by 
> Magnus (magnus.ihse.bur...@oracle.com).
> 
> Looking forward to your review and feedback.
> 
> Performance:
> Micro benchmark Base Optimized Unit Gain(Optimized/Base)
> Double128Vector.ACOS 45.91 87.34 ops/ms 1.90
> Double128Vector.ASIN 45.06 92.36 ops/ms 2.05
> Double128Vector.ATAN 19.92 118.36 ops/ms 5.94
> Double128Vector.ATAN2 15.24 88.17 ops/ms 5.79
> Double128Vector.CBRT 45.77 208.36 ops/ms 4.55
> Double128Vector.COS 49.94 245.89 ops/ms 4.92
> Double128Vector.COSH 26.91 126.00 ops/ms 4.68
> Double128Vector.EXP 71.64 379.65 ops/ms 5.30
> Double128Vector.EXPM1 35.95 150.37 ops/ms 4.18
> Double128Vector.HYPOT 50.67 174.10 ops/ms 3.44
> Double128Vector.LOG 61.95 279.84 ops/ms 4.52
> Double128Vector.LOG10 59.34 239.05 ops/ms 4.03
> Double128Vector.LOG1P 18.56 200.32 ops/ms 10.79
> Double128Vector.SIN 49.36 240.79 ops/ms 4.88
> Double128Vector.SINH 26.59 103.75 ops/ms 3.90
> Double128Vector.TAN 41.05 152.39 ops/ms 3.71
> Double128Vector.TANH 45.29 169.53 ops/ms 3.74
> Double256Vector.ACOS 54.21 106.39 ops/ms 1.96
> Double256Vector.ASIN 53.60 107.99 ops/ms 2.01
> Double256Vector.ATAN 21.53 189.11 ops/ms 8.78
> Double256Vector.ATAN2 16.67 140.76 ops/ms 8.44
> Double256Vector.CBRT 56.45 397.13 ops/ms 7.04
> Double256Vector.COS 58.26 389.77 ops/ms 6.69
> Double256Vector.COSH 29.44 151.11 ops/ms 5.13
> Double256Vector.EXP 86.67 564.68 ops/ms 6.52
> Double256Vector.EXPM1 41.96 201.28 ops/ms 4.80
> Double256Vector.HYPOT 66.18 305.74 ops/ms 4.62
> Double256Vector.LOG 71.52 394.90 ops/ms 5.52
> Double256Vector.LOG10 65.43 362.32 ops/ms 5.54
> Double256Vector.LOG1P 19.99 300.88 ops/ms 15.05
> Double256Vector.SIN 57.06 380.98 ops/ms 6.68
> Double256Vector.SINH 29.40 117.37 ops/ms 3.99
> Double256Vector.TAN 44.90 279.90 ops/ms 6.23
> Double256Vector.TANH 54.08 274.71 ops/ms 5.08
> Double512Vector.ACOS 55.65 687.54 ops/ms 12.35
> Double512Vector.ASIN 57.31 777.72 ops/ms 13.57
> Double512Vector.ATAN 21.42 729.21 ops/ms 34.04
> Double512Vector.ATAN2 16.37 414.33 ops/ms 25.32
> Double512Vector.CBRT 56.78 834.38 ops/ms 14.69
> Double512Vector.COS 59.88 837.04 ops/ms 13.98
> Double512Vector.COSH 30.34 172.76 ops/ms 5.70
> Double512Vector.EXP 99.66 1608.12 ops/ms 16.14
> Double512Vector.EXPM1 43.39 318.61 ops/ms 7.34
> Double512Vector.HYPOT 73.87 1502.72 ops/ms 20.34
> Double512Vector.LOG 74.84 996.00 ops/ms 13.31
> Double512Vector.LOG10 71.12 1046.52 ops/ms 14.72
> Double512Vector.LOG1P 19.75 776.87 ops/ms 39.34
> Double512Vector.POW 37.42 384.13 ops/ms 10.26
> Double512Vector.SIN 59.74 728.45 ops/ms 12.19
> Double512Vector.SINH 29.47 143.38 ops/ms 4.87
> Double512Vector.TAN 46.20 587.21 ops/ms 12.71
> Double512Vector.TANH 57.36 495.42 ops/ms 8.64
> Double64Vector.ACOS 24.04 73.67 ops/ms 3.06
> Double64Vector.ASIN 23.78 75.11 ops/ms 3.16
> Double64Vector.ATAN 14.14 62.81 ops/ms 4.44
> Double64Vector.ATAN2 10.38 44.43 ops/ms 4.28
> Double64Vector.CBRT 16.47 107.50 ops/ms 6.53
> Double64Vector.COS 23.42 152.01 ops/ms 6.49
> Double64Vector.COSH 17.34 113.34 ops/ms 6.54
> Double64Vector.EXP 27.08 203.53 ops/ms 7.52
> Double64Vector.EXPM1 18.77 96.73 ops/ms 5.15
> Double64Vector.HYPOT 18.54 103.62 ops/ms 5.59
> Double64Vector.LOG 26.75 142.63 ops/ms 5.33
> Double64Vector.LOG10 25.85 139.71 ops/ms 5.40
> Double64Vector.LOG1P 13.26 97.94 ops/ms 7.38
> Double64Vector.SIN 23.28 146.91 ops/ms 6.31
> Double64Vector.SINH 17.62 88.59 ops/ms 5.03
> Double64Vector.TAN 21.00 86.43 ops/ms 4.12
> Double64Vector.TANH 23.75 111.35 ops/ms 4.69
> Float128Vector.ACOS 57.52 110.65 ops/ms 1.92
> Float128Vector.ASIN 57.15 117.95 ops/ms 2.06
> Float128Vector.ATAN 22.52 318.74 ops/ms 14.15

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

Integrated: JDK-8267598: jpackage removes system libraries from java.library.path

[Andy Herrick]

On Wed, 26 May 2021 11:26:24 GMT, Andy Herrick  wrote:

> JDK-8267598: jpackage removes system libraries from java.library.path

This pull request has now been integrated.

Changeset: af3df630
Author:Andy Herrick 
URL:   
https://git.openjdk.java.net/jdk/commit/af3df6300efddc8ba12f095b87205cc2fea1f1e8
Stats: 38 lines in 9 files changed: 26 ins; 5 del; 7 mod

8267598: jpackage removes system libraries from java.library.path

Reviewed-by: almatvee, asemenyuk

-

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

Comment on JDK-826722: SoftReference not cleared on OutOfMemoryError: Requested array size exceeds VM limit

[Raffaello Giulietti]

Hi,

upon reading [1] I tried a similar scenario, but where OOME are caused 
by "Java heap space" exhaustion rather than by VM limits.



import java.lang.ref.SoftReference;
import java.text.DecimalFormat;
import java.util.ArrayList;

public class Softly {

public static void main(String[] args) {
var size = Integer.parseInt(args[0]);
var format = new DecimalFormat("#,###");
var news = 0;
var ref = new SoftReference<>(new ArrayList<>());
for (;;) {
byte[] b = null;
try {
b = new byte[size];
++news;
ref.get().add(b);
} catch (NullPointerException __) {
System.out.format("totSize = %20s, allocations = %d\n", 
format.format((long) news * size), news);

ref = new SoftReference<>(new ArrayList<>());
ref.get().add(b);
} catch (OutOfMemoryError e) {
if (ref.refersTo(null)) {
throw new AssertionError("allocations = 
%d".formatted((news)), e);

}
throw new AssertionError("non-null referent", e);
}
}
}

}


E.g.,
java -XX:+UseG1GC -Xms1g -Xmx1g -cp ... Softly 8


Depending on the collector and how tight the heap is, I sometimes 
observe a "Java heap space" OOME but then the referent of ref is null. I 
never observed a OOME with a non-null referent for ref. Hence, in 
scenarios where OOME are caused by heap exhaustion, soft refs seem to 
work as advertised.


Tried on AdoptOpenJDK-16.0.1+9 with SerialGC, ParallelGC, G1GC, ZGC and 
ShenandoahGC with either -Xms1g/-Xmx1g or -Xms2g/-Xmx2g (small heaps) 
and various byte[] sizes.


Thus, the current wording in SoftReference's javadoc:

"All soft references to softly-reachable objects are guaranteed to have 
been cleared before the virtual machine throws an OutOfMemoryError."


could be amended to read:

"All soft references to softly-reachable objects are guaranteed to have 
been cleared before the virtual machine throws an OutOfMemoryError 
caused by Java heap space exhaustion."



Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8267222

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

[Peter Levart]

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?



Peter

Re: RFR: 8268151: Vector API toShuffle optimization

[Sandhya Viswanathan]

On Thu, 3 Jun 2021 02:27:35 GMT, Xiaohong Gong  wrote:

>> The Vector API toShuffle method can be optimized  using existing vector 
>> conversion intrinsic.
>> 
>> The following changes are made:
>> 1) vector.toShuffle java implementation is changed to call 
>> VectorSupport.convert.
>> 2) The conversion intrinsic (inline_vector_convert()) in 
>> vectorIntrinsics.cpp is changed to allow shuffle as a destination type.
>> 3) The shuffle.toVector intrinsic (inline_vector_shuffle_to_vector()) in 
>> vectorIntrinsics.cpp now explicitly generates conversion node instead of 
>> performing conversion during unbox. This is to remove unnecessary boxing 
>> during back to back vector.toShuffle and shuffle.toVector calls. 
>> 
>> Best Regards,
>> Sandhya
>
> src/hotspot/share/opto/vectornode.cpp line 1246:
> 
>> 1244:   return new VectorLoadMaskNode(value, out_vt);
>> 1245: } else if (is_vector_shuffle) {
>> 1246:   if (!is_shuffle_to_vector()) {
> 
> Hi @sviswa7 , thanks for this change! I'm just curious whether 
> `is_shuffle_to_vector()` is still needed for `VectorUnboxNode` with this 
> change? It seems this flag can be removed, doesn't it?

@XiaohongGong is_shuffle_to_vector is still needed as we shouldn't generate 
VectorLoadShuffleNode for shuffle.toVector.

-

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

Re: RFR: 8267995: Add reference to JVMS class file format in Lookup::defineHiddenClass

[Joe Darcy]

On Thu, 3 Jun 2021 18:21:31 GMT, Mandy Chung  wrote:

> Trivial javadoc change to add a reference to JVMS 4.1 in 
> `Lookup::defineHiddenClass` such that the reference like `this_class` would 
> be referred to JVMS for details.

Marked as reviewed by darcy (Reviewer).

-

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

RFR: 8267995: Add reference to JVMS class file format in Lookup::defineHiddenClass

[Mandy Chung]

Trivial javadoc change to add a reference to JVMS 4.1 in 
`Lookup::defineHiddenClass` such that the reference like `this_class` would be 
referred to JVMS for details.

-

Commit messages:
 - 8267995: Add reference to JVMS class file format in Lookup::defineHiddenClass

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

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

Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries [v6]

[Erik Joelsson]

On Thu, 3 Jun 2021 16:43:51 GMT, Maurizio Cimadamore  
wrote:

>> This patch overhauls the library loading mechanism used by the Foreign 
>> Linker API. We realized that, while handy, the *default* lookup abstraction 
>> (`LibraryLookup::ofDefault`) was behaving inconsistentlt across platforms.
>> 
>> This patch replaces `LibraryLookup` with a simpler `SymbolLookup` 
>> abstraction, a functional interface. Crucially, `SymbolLookup` does not 
>> concern with library loading, only symbol lookup. For this reason, two 
>> factories are added:
>> 
>> * `SymbolLookup::loaderLookup` - which obtains a lookup that can be used to 
>> lookup symbols in libraries loaded by current loader
>> * `CLinker::systemLookup` - a more stable replacement for the *default* 
>> lookup, which looks for symbols in libc.so (or its equivalent in other 
>> platforms). The contents of this lookup are unspecified.
>> 
>> Both factories are *restricted*, so they can only be called when 
>> `--enable-native-access` is set.
>
> Maurizio Cimadamore has updated the pull request with a new target base due 
> to a merge or a rebase. The pull request now contains 16 commits:
> 
>  - Merge branch 'master' into symbolLookup
>  - Forgot to add makefile for building shim library
>  - Address review comments
>  - Update test/jdk/java/foreign/TestIntrinsics.java
>
>Co-authored-by: Paul Sandoz 
>  - Update test/jdk/java/foreign/valist/VaListTest.java
>
>Co-authored-by: Paul Sandoz 
>  - Update test/jdk/java/foreign/TestVarArgs.java
>
>Co-authored-by: Paul Sandoz 
>  - Update test/jdk/java/foreign/TestUpcall.java
>
>Co-authored-by: Paul Sandoz 
>  - Update test/jdk/java/foreign/TestIllegalLink.java
>
>Co-authored-by: Paul Sandoz 
>  - Update test/jdk/java/foreign/TestSymbolLookup.java
>
>Co-authored-by: Paul Sandoz 
>  - Update test/jdk/java/foreign/TestDowncall.java
>
>Co-authored-by: Paul Sandoz 
>  - ... and 6 more: 
> https://git.openjdk.java.net/jdk/compare/52d8215a...2545e2b6

Looks pretty good, just a few comments.

make/modules/jdk.incubator.foreign/Lib.gmk line 28:

> 26: include LibCommon.gmk
> 27: 
> 28: ifeq ($(call isTargetOs, linux), true)

Please indent everything inside the ifeq-block 2 spaces. (See 
http://openjdk.java.net/groups/build/doc/code-conventions.html)

make/modules/jdk.incubator.foreign/Lib.gmk line 34:

> 32: CFLAGS := $(CFLAGS_JDKLIB), \
> 33: CXXFLAGS := $(CXXFLAGS_JDKLIB), \
> 34: LDFLAGS := -Wl$(COMMA)--no-as-needed -lc -lm -ldl $(LDFLAGS_JDKLIB) 
> $(call SET_SHARED_LIBRARY_ORIGIN), \

Unless you link with any other library in the JDK (typically libjava and/or 
libjvm), I don't think there is a need for adding SET_SHARED_LIBRARY_ORIGIN.

Please put all the -l* flags in LIBS rather than LDFLAGS.

I also recommend putting any additional flags after the general LDFLAGS_JDKLIB. 
That way you are guaranteed that your flag takes precedence over anything that 
may be added to LDFLAGS_JDKLIB in the future.

-

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

Re: RFR: JDK-8267598: jpackage removes system libraries from java.library.path [v2]

[Alexey Semenyuk]

On Thu, 3 Jun 2021 16:07:13 GMT, Andy Herrick  wrote:

>> JDK-8267598: jpackage removes system libraries from java.library.path
>
> Andy Herrick has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains eight additional 
> commits since the last revision:
> 
>  - JDK-8267598: jpackage removes system libraries from java.library.path
>  - Merge branch 'master' into JDK-8267598
>  - JDK-8267598: jpackage removes system libraries from java.library.path
>  - JDK-8267598: jpackage removes system libraries from java.library.path
>  - JDK-8267598: jpackage removes system libraries from java.library.path
>  - JDK-8267598: jpackage removes system libraries from java.library.path
>  - JDK-8267598: jpackage removes system libraries from java.library.path
>  - JDK-8267598: jpackage removes system libraries from java.library.path

Marked as reviewed by asemenyuk (Reviewer).

-

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

Re: RFR: 8263512: [macos_aarch64] issues with calling va_args functions from invoke_native [v3]

[Jorn Vernee]

On Thu, 3 Jun 2021 03:28:56 GMT, Nick Gasson  wrote:

>> macOS on Apple silicon uses slightly different ABI conventions to the
>> standard AArch64 ABI.  The differences are outlined in [1].  In
>> particular in the standard (AAPCS) ABI, variadic arguments may be passed
>> in either registers or on the stack following the normal calling
>> convention.  To handle this, va_list is a struct containing separate
>> pointers for arguments located in integer registers, floating point
>> registers, and on the stack.  Apple's ABI simplifies this by passing all
>> variadic arguments on the stack and the va_list type becomes a simple
>> char* pointer.
>> 
>> This patch adds a new MacOsAArch64 CABI type and MacOsAArch64Linker to
>> represent the new ABI variant on macOS.  StackVaList is based on
>> WinVaList lightly modified to handle the different TypeClasses on
>> AArch64.  The original AArch64Linker is renamed to AapcsLinker and is
>> currently used for all non-Mac platforms.  I think we also need to add a
>> WinAArch64 CABI but I haven't yet been able to test on a Windows system
>> so will do that later.
>> 
>> The macOS ABI also uses a different method of spilling arguments to the
>> stack (the standard ABI pads each argument to a multiple of 8 byte stack
>> slots, but the Mac ABI packs arguments according to their natural
>> alignment).  None of the existing tests exercise this so I'll open a new
>> JBS issue and work on that separately.
>> 
>> Tested jdk_foreign on macOS AArch64, Linux AArch64, and Linux X86_64.
>> 
>> [1] 
>> https://developer.apple.com/documentation/xcode/writing_arm64_code_for_apple_platforms
>
> Nick Gasson has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   No variadic upcalls
>   
>   Change-Id: Ibf91c570c88be2587e9e23240477c4a5cc56b4c5
>   CustomizedGitHooks: yes

Looks very good, thanks!

-

Marked as reviewed by jvernee (Reviewer).

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

Re: RFR: 8195129: System.load() fails to load from unicode paths [v3]

[Naoto Sato]

On Thu, 3 Jun 2021 06:59:01 GMT, Maxim Kartashev 
 wrote:

>> test/hotspot/jtreg/runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java
>>  line 42:
>> 
>>> 40: String nativePathSetting = "-Dtest.nativepath=" + 
>>> getSystemProperty("test.nativepath");
>>> 41: ProcessBuilder pb = 
>>> ProcessTools.createTestJvm(nativePathSetting, 
>>> LoadLibraryUnicode.class.getName());
>>> 42: pb.environment().put("LC_ALL", "en_US.UTF-8");
>> 
>> Some environments/user configs may not have `UTF-8` codeset on the platform. 
>> May need to gracefully exit in such a case.
>
> I added `java.nio.charset.Charset.isSupported("UTF-8")` check to the test. 
> Hope that's enough for the environments without `UTF-8`.

`Charset.isSupported()` returns whether Java encoder/decoder supports it or 
not, not the platform has the codeset. I think we can simply limit the test 
platform only to Windows in `@requires` tag in the test. Also, I would see the 
test case using some supplementary characters.

-

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

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v17]

[Sandhya Viswanathan]

> This PR contains Short Vector Math Library support related changes for 
> [JEP-414 Vector API (Second Incubator)](https://openjdk.java.net/jeps/414), 
> in preparation for when targeted.
> 
> Intel Short Vector Math Library (SVML) based intrinsics in native x86 
> assembly provide optimized implementation for Vector API transcendental and 
> trigonometric methods.
> These methods are built into a separate library instead of being part of 
> libjvm.so or jvm.dll.
> 
> The following changes are made:
>The source for these methods is placed in the jdk.incubator.vector module 
> under src/jdk.incubator.vector/linux/native/libsvml and 
> src/jdk.incubator.vector/windows/native/libsvml.
>The assembly source files are named as “*.S” and include files are named 
> as “*.S.inc”.
>The corresponding build script is placed at 
> make/modules/jdk.incubator.vector/Lib.gmk.
>Changes are made to build system to support dependency tracking for 
> assembly files with includes.
>The built native libraries (libsvml.so/svml.dll) are placed in bin 
> directory of JDK on Windows and lib directory of JDK on Linux.
>The C2 JIT uses the dll_load and dll_lookup to get the addresses of 
> optimized methods from this library.
> 
> Build system changes and module library build scripts are contributed by 
> Magnus (magnus.ihse.bur...@oracle.com).
> 
> Looking forward to your review and feedback.
> 
> Performance:
> Micro benchmark Base Optimized Unit Gain(Optimized/Base)
> Double128Vector.ACOS 45.91 87.34 ops/ms 1.90
> Double128Vector.ASIN 45.06 92.36 ops/ms 2.05
> Double128Vector.ATAN 19.92 118.36 ops/ms 5.94
> Double128Vector.ATAN2 15.24 88.17 ops/ms 5.79
> Double128Vector.CBRT 45.77 208.36 ops/ms 4.55
> Double128Vector.COS 49.94 245.89 ops/ms 4.92
> Double128Vector.COSH 26.91 126.00 ops/ms 4.68
> Double128Vector.EXP 71.64 379.65 ops/ms 5.30
> Double128Vector.EXPM1 35.95 150.37 ops/ms 4.18
> Double128Vector.HYPOT 50.67 174.10 ops/ms 3.44
> Double128Vector.LOG 61.95 279.84 ops/ms 4.52
> Double128Vector.LOG10 59.34 239.05 ops/ms 4.03
> Double128Vector.LOG1P 18.56 200.32 ops/ms 10.79
> Double128Vector.SIN 49.36 240.79 ops/ms 4.88
> Double128Vector.SINH 26.59 103.75 ops/ms 3.90
> Double128Vector.TAN 41.05 152.39 ops/ms 3.71
> Double128Vector.TANH 45.29 169.53 ops/ms 3.74
> Double256Vector.ACOS 54.21 106.39 ops/ms 1.96
> Double256Vector.ASIN 53.60 107.99 ops/ms 2.01
> Double256Vector.ATAN 21.53 189.11 ops/ms 8.78
> Double256Vector.ATAN2 16.67 140.76 ops/ms 8.44
> Double256Vector.CBRT 56.45 397.13 ops/ms 7.04
> Double256Vector.COS 58.26 389.77 ops/ms 6.69
> Double256Vector.COSH 29.44 151.11 ops/ms 5.13
> Double256Vector.EXP 86.67 564.68 ops/ms 6.52
> Double256Vector.EXPM1 41.96 201.28 ops/ms 4.80
> Double256Vector.HYPOT 66.18 305.74 ops/ms 4.62
> Double256Vector.LOG 71.52 394.90 ops/ms 5.52
> Double256Vector.LOG10 65.43 362.32 ops/ms 5.54
> Double256Vector.LOG1P 19.99 300.88 ops/ms 15.05
> Double256Vector.SIN 57.06 380.98 ops/ms 6.68
> Double256Vector.SINH 29.40 117.37 ops/ms 3.99
> Double256Vector.TAN 44.90 279.90 ops/ms 6.23
> Double256Vector.TANH 54.08 274.71 ops/ms 5.08
> Double512Vector.ACOS 55.65 687.54 ops/ms 12.35
> Double512Vector.ASIN 57.31 777.72 ops/ms 13.57
> Double512Vector.ATAN 21.42 729.21 ops/ms 34.04
> Double512Vector.ATAN2 16.37 414.33 ops/ms 25.32
> Double512Vector.CBRT 56.78 834.38 ops/ms 14.69
> Double512Vector.COS 59.88 837.04 ops/ms 13.98
> Double512Vector.COSH 30.34 172.76 ops/ms 5.70
> Double512Vector.EXP 99.66 1608.12 ops/ms 16.14
> Double512Vector.EXPM1 43.39 318.61 ops/ms 7.34
> Double512Vector.HYPOT 73.87 1502.72 ops/ms 20.34
> Double512Vector.LOG 74.84 996.00 ops/ms 13.31
> Double512Vector.LOG10 71.12 1046.52 ops/ms 14.72
> Double512Vector.LOG1P 19.75 776.87 ops/ms 39.34
> Double512Vector.POW 37.42 384.13 ops/ms 10.26
> Double512Vector.SIN 59.74 728.45 ops/ms 12.19
> Double512Vector.SINH 29.47 143.38 ops/ms 4.87
> Double512Vector.TAN 46.20 587.21 ops/ms 12.71
> Double512Vector.TANH 57.36 495.42 ops/ms 8.64
> Double64Vector.ACOS 24.04 73.67 ops/ms 3.06
> Double64Vector.ASIN 23.78 75.11 ops/ms 3.16
> Double64Vector.ATAN 14.14 62.81 ops/ms 4.44
> Double64Vector.ATAN2 10.38 44.43 ops/ms 4.28
> Double64Vector.CBRT 16.47 107.50 ops/ms 6.53
> Double64Vector.COS 23.42 152.01 ops/ms 6.49
> Double64Vector.COSH 17.34 113.34 ops/ms 6.54
> Double64Vector.EXP 27.08 203.53 ops/ms 7.52
> Double64Vector.EXPM1 18.77 96.73 ops/ms 5.15
> Double64Vector.HYPOT 18.54 103.62 ops/ms 5.59
> Double64Vector.LOG 26.75 142.63 ops/ms 5.33
> Double64Vector.LOG10 25.85 139.71 ops/ms 5.40
> Double64Vector.LOG1P 13.26 97.94 ops/ms 7.38
> Double64Vector.SIN 23.28 146.91 ops/ms 6.31
> Double64Vector.SINH 17.62 88.59 ops/ms 5.03
> Double64Vector.TAN 21.00 86.43 ops/ms 4.12
> Double64Vector.TANH 23.75 111.35 ops/ms 4.69
> Float128Vector.ACOS 57.52 110.65 ops/ms 1.92
> Float128Vector.ASIN 57.15 117.95 ops/ms 2.06
> Float128Vector.ATAN 22.52 318.74 ops/ms 14.15
> Float128Vector.ATAN2 17.06 246.07 ops/ms 14.42
> 

Re: RFR: 8195129: System.load() fails to load from unicode paths [v3]

[Naoto Sato]

On Thu, 3 Jun 2021 06:55:26 GMT, Maxim Kartashev 
 wrote:

>> src/hotspot/os/windows/os_windows.cpp line 1491:
>> 
>>> 1489: static errno_t convert_UTF8_to_UTF16(char const* utf8_str, LPWSTR* 
>>> utf16_str) {
>>> 1490:   return convert_to_UTF16(utf8_str, CP_UTF8, utf16_str);
>>> 1491: }
>> 
>> IIUC, `utf8_str` is the "modified" UTF-8 string in JNI. Using it as the 
>> standard UTF-8 (I believe Windows' encoding `CP_UTF8` is the one) may end up 
>> in incorrect conversions in some corner cases, e.g., for supplementary 
>> characters.
>
> Right; I changed the code in NativeLibraries.c to pass down true UTF-8 
> instead of "modified UTF-8". Please, take a look.

I am not sure we can pass non `modified UTF-8` through `JVM_LoadLibrary()`. 
Probably some VM folks can enlighten here?

-

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

Re: RFR: 8268124: Update java.lang to use switch expressions [v3]

[Iris Clark]

On Thu, 3 Jun 2021 11:01:02 GMT, Patrick Concannon  
wrote:

>> Hi,
>> 
>> Could someone please review my code for updating the code in the `java.lang` 
>> packages to make use of the switch expressions?
>> 
>> Kind regards,
>> Patrick
>
> Patrick Concannon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8268124: small refactoring; fixed misplaced comment and added missing 
> lambda operator

Marked as reviewed by iris (Reviewer).

-

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

Re: RFR: 8268124: Update java.lang to use switch expressions [v3]

[Mandy Chung]

On Thu, 3 Jun 2021 11:01:02 GMT, Patrick Concannon  
wrote:

>> Hi,
>> 
>> Could someone please review my code for updating the code in the `java.lang` 
>> packages to make use of the switch expressions?
>> 
>> Kind regards,
>> Patrick
>
> Patrick Concannon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8268124: small refactoring; fixed misplaced comment and added missing 
> lambda operator

Looks good with one minor comment in `DirectMethodHandleDescImpl.java`.

src/java.base/share/classes/java/lang/constant/DirectMethodHandleDescImpl.java 
line 138:

> 136: public String lookupDescriptor() {
> 137: return switch (kind) {
> 138: case VIRTUAL, SPECIAL,

Nit: I prefer to have each case in a separate line (in this switch and also the 
switch in `resolveConstantDesc`.

-

Marked as reviewed by mchung (Reviewer).

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

Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries [v6]

[Maurizio Cimadamore]

> This patch overhauls the library loading mechanism used by the Foreign Linker 
> API. We realized that, while handy, the *default* lookup abstraction 
> (`LibraryLookup::ofDefault`) was behaving inconsistentlt across platforms.
> 
> This patch replaces `LibraryLookup` with a simpler `SymbolLookup` 
> abstraction, a functional interface. Crucially, `SymbolLookup` does not 
> concern with library loading, only symbol lookup. For this reason, two 
> factories are added:
> 
> * `SymbolLookup::loaderLookup` - which obtains a lookup that can be used to 
> lookup symbols in libraries loaded by current loader
> * `CLinker::systemLookup` - a more stable replacement for the *default* 
> lookup, which looks for symbols in libc.so (or its equivalent in other 
> platforms). The contents of this lookup are unspecified.
> 
> Both factories are *restricted*, so they can only be called when 
> `--enable-native-access` is set.

Maurizio Cimadamore has updated the pull request with a new target base due to 
a merge or a rebase. The pull request now contains 16 commits:

 - Merge branch 'master' into symbolLookup
 - Forgot to add makefile for building shim library
 - Address review comments
 - Update test/jdk/java/foreign/TestIntrinsics.java
   
   Co-authored-by: Paul Sandoz 
 - Update test/jdk/java/foreign/valist/VaListTest.java
   
   Co-authored-by: Paul Sandoz 
 - Update test/jdk/java/foreign/TestVarArgs.java
   
   Co-authored-by: Paul Sandoz 
 - Update test/jdk/java/foreign/TestUpcall.java
   
   Co-authored-by: Paul Sandoz 
 - Update test/jdk/java/foreign/TestIllegalLink.java
   
   Co-authored-by: Paul Sandoz 
 - Update test/jdk/java/foreign/TestSymbolLookup.java
   
   Co-authored-by: Paul Sandoz 
 - Update test/jdk/java/foreign/TestDowncall.java
   
   Co-authored-by: Paul Sandoz 
 - ... and 6 more: https://git.openjdk.java.net/jdk/compare/52d8215a...2545e2b6

-

Changes: https://git.openjdk.java.net/jdk/pull/4316/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4316=05
  Stats: 1351 lines in 47 files changed: 626 ins; 621 del; 104 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4316.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4316/head:pull/4316

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

Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries [v5]

[Maurizio Cimadamore]

> This patch overhauls the library loading mechanism used by the Foreign Linker 
> API. We realized that, while handy, the *default* lookup abstraction 
> (`LibraryLookup::ofDefault`) was behaving inconsistentlt across platforms.
> 
> This patch replaces `LibraryLookup` with a simpler `SymbolLookup` 
> abstraction, a functional interface. Crucially, `SymbolLookup` does not 
> concern with library loading, only symbol lookup. For this reason, two 
> factories are added:
> 
> * `SymbolLookup::loaderLookup` - which obtains a lookup that can be used to 
> lookup symbols in libraries loaded by current loader
> * `CLinker::systemLookup` - a more stable replacement for the *default* 
> lookup, which looks for symbols in libc.so (or its equivalent in other 
> platforms). The contents of this lookup are unspecified.
> 
> Both factories are *restricted*, so they can only be called when 
> `--enable-native-access` is set.

Maurizio Cimadamore has updated the pull request incrementally with one 
additional commit since the last revision:

  Forgot to add makefile for building shim library

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4316/files
  - new: https://git.openjdk.java.net/jdk/pull/4316/files/7be87eae..6480332c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4316=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4316=03-04

  Stats: 53 lines in 1 file changed: 53 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4316.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4316/head:pull/4316

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

Withdrawn: 8264896: Remove redundant '& 0xFF' from int-to-byte cast

[duke]

On Thu, 4 Mar 2021 12:16:29 GMT, Сергей Цыпанов 
 wrote:

> When we do
> 
> byte b1 = (byte) (value & 0xFF);
> 
> we keep from int only 1 lower byte and exactly the same can be achieved with 
> plain cast. See the test below:
> 
> public class Main {
>   public static void main(String[] args) throws Exception {
> IntStream.range(Integer.MIN_VALUE, Integer.MAX_VALUE).forEach(value -> {
>   byte b1 = (byte) (value & 0xFF);
>   byte b2 = (byte) value;
>   if (b1 != b2) {
> throw new RuntimeException("" + value);
>   }
> });
> }
> 
> 
> Also tier1 and tier2 are both OK.

This pull request has been closed without being integrated.

-

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

Integrated: 8268131: 2 java/foreign tests timed out

[Maurizio Cimadamore]

On Wed, 2 Jun 2021 21:20:53 GMT, Maurizio Cimadamore  
wrote:

> This patch increases time out for both TestUpcall and TestDowncall. These 
> tests were already long-running, but with JEP-412, they were beefed up even 
> more, so now they time out on some debug builds.
> 
> This patch also address a minor issue in TestResourceScope which can 
> sometimes lead to intermittent failure, depending on how threads are 
> scheduled.

This pull request has now been integrated.

Changeset: 52d8215a
Author:Maurizio Cimadamore 
URL:   
https://git.openjdk.java.net/jdk/commit/52d8215a1ec42d67217505fe3167c70460f5a639
Stats: 10 lines in 3 files changed: 7 ins; 0 del; 3 mod

8268131: 2 java/foreign tests timed out

Reviewed-by: dcubed

-

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

Re: RFR: 8268131: 2 java/foreign tests timed out

[Maurizio Cimadamore]

On Wed, 2 Jun 2021 21:20:53 GMT, Maurizio Cimadamore  
wrote:

> This patch increases time out for both TestUpcall and TestDowncall. These 
> tests were already long-running, but with JEP-412, they were beefed up even 
> more, so now they time out on some debug builds.
> 
> This patch also address a minor issue in TestResourceScope which can 
> sometimes lead to intermittent failure, depending on how threads are 
> scheduled.

I've adjusted timeout to 240, following suggestions - let's see what happens.

-

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

Re: RFR: 8268131: 2 java/foreign tests timed out [v2]

[Maurizio Cimadamore]

> This patch increases time out for both TestUpcall and TestDowncall. These 
> tests were already long-running, but with JEP-412, they were beefed up even 
> more, so now they time out on some debug builds.
> 
> This patch also address a minor issue in TestResourceScope which can 
> sometimes lead to intermittent failure, depending on how threads are 
> scheduled.

Maurizio Cimadamore has updated the pull request incrementally with one 
additional commit since the last revision:

  Adjust timeout, according to review comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4321/files
  - new: https://git.openjdk.java.net/jdk/pull/4321/files/b4329076..1ce9ff64

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4321=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4321=00-01

  Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4321.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4321/head:pull/4321

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

Integrated: 8268077: java.util.List missing from Collections Framework Overview

[Raffaello Giulietti]

On Tue, 1 Jun 2021 19:51:39 GMT, Raffaello Giulietti 
 wrote:

> Trivial changes to this non-normative document.

This pull request has now been integrated.

Changeset: 5405f983
Author:Raffaello Giulietti 
Committer: Stuart Marks 
URL:   
https://git.openjdk.java.net/jdk/commit/5405f983db7d359bb65c42366541104c5e9ef7c3
Stats: 7 lines in 1 file changed: 2 ins; 0 del; 5 mod

8268077: java.util.List missing from Collections Framework Overview

Reviewed-by: smarks

-

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

Re: RFR: JDK-8267598: jpackage removes system libraries from java.library.path [v2]

[Andy Herrick]

> JDK-8267598: jpackage removes system libraries from java.library.path

Andy Herrick has updated the pull request with a new target base due to a merge 
or a rebase. The incremental webrev excludes the unrelated changes brought in 
by the merge/rebase. The pull request contains eight additional commits since 
the last revision:

 - JDK-8267598: jpackage removes system libraries from java.library.path
 - Merge branch 'master' into JDK-8267598
 - JDK-8267598: jpackage removes system libraries from java.library.path
 - JDK-8267598: jpackage removes system libraries from java.library.path
 - JDK-8267598: jpackage removes system libraries from java.library.path
 - JDK-8267598: jpackage removes system libraries from java.library.path
 - JDK-8267598: jpackage removes system libraries from java.library.path
 - JDK-8267598: jpackage removes system libraries from java.library.path

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4203/files
  - new: https://git.openjdk.java.net/jdk/pull/4203/files/a0b5c0e8..3639dde4

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4203=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4203=00-01

  Stats: 60464 lines in 2313 files changed: 10695 ins; 45162 del; 4607 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4203.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4203/head:pull/4203

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

Re: RFR: 8268131: 2 java/foreign tests timed out

[Daniel D . Daugherty]

On Wed, 2 Jun 2021 21:20:53 GMT, Maurizio Cimadamore  
wrote:

> This patch increases time out for both TestUpcall and TestDowncall. These 
> tests were already long-running, but with JEP-412, they were beefed up even 
> more, so now they time out on some debug builds.
> 
> This patch also address a minor issue in TestResourceScope which can 
> sometimes lead to intermittent failure, depending on how threads are 
> scheduled.

Thumbs up.

You might starting with 240 instead of 720 for the initial timeout bump.

I would definitely prefer to have Tier4 quiet down for the upcoming code fork
so increasing the timeout is good from my GK POV.

These two tests were using the default timeout of 120 seconds/2 minutes and
with a default timeoutFactor of 4 (set by the test task), the total timeout was
480 seconds/8 minutes.

I recommend change the timeout from 720 to 240. That will give you a total
timeout of 960 seconds/16 minutes. For the TestDowncall.java failure, that
should cover that case. For the TestUpcall.java failure, we don't know if
doubling the total timeout will cover that case or not because that test
didn't pass while the timeout handler is running.

However, doubling is a good start for a bump. Also, using a higher timeout
value doesn't hurt anything if the test doesn't timeout.

-

Marked as reviewed by dcubed (Reviewer).

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

Integrated: 8266317: Vector API enhancements

[Paul Sandoz]

On Thu, 29 Apr 2021 21:13:38 GMT, Paul Sandoz  wrote:

> This PR contains API and implementation changes for [JEP-414 Vector API 
> (Second Incubator)](https://openjdk.java.net/jeps/414), in preparation for 
> when targeted.
> 
> Enhancements are made to the API for the support of operations on characters, 
> such as for UTF-8 character decoding. Specifically, methods for 
> loading/storing a `short` vector from/to a `char[]` array, and new vector 
> comparison operators for unsigned comparisons with integral vectors. The x64 
> implementation is enhanced to supported unsigned comparisons.
> 
> Enhancements are made to the API for loading/storing a `byte` vector from/to 
> a `boolean[]` array.
> 
> The testing of loads/stores can be expanded for scatter/gather, but before 
> doing that i think some refactoring of the tests is required to reposition 
> tests in the right classes. I would like to do that work after integration of 
> the PR.

This pull request has now been integrated.

Changeset: 5982cfc8
Author:Paul Sandoz 
URL:   
https://git.openjdk.java.net/jdk/commit/5982cfc85602862608fae56adb6041794e8c0d59
Stats: 10017 lines in 121 files changed: 9085 ins; 190 del; 742 mod

8266317: Vector API enhancements

Co-authored-by: Paul Sandoz 
Co-authored-by: Sandhya Viswanathan 
Reviewed-by: jbhateja, vlivanov

-

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

Re: RFR: 8268124: Update java.lang to use switch expressions [v3]

[Naoto Sato]

On Thu, 3 Jun 2021 11:01:02 GMT, Patrick Concannon  
wrote:

>> Hi,
>> 
>> Could someone please review my code for updating the code in the `java.lang` 
>> packages to make use of the switch expressions?
>> 
>> Kind regards,
>> Patrick
>
> Patrick Concannon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8268124: small refactoring; fixed misplaced comment and added missing 
> lambda operator

LGTM

-

Marked as reviewed by naoto (Reviewer).

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

Re: Some questions about String interning for primitives

[Roger Riggs]

Hi Dave,

Have you seen the related thread about Integer.toString()?

https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-April/076454.html
Allocation is cheap and short lived object storage is quick to be reclaimed.

What's missing is data about the cost vs savings in real applications.
And it extends all the way down to memory timing changes for recently
accessed memory vs not recently accessed memory, multi-level memory 
caches, etc.


Regards, Roger


On 6/3/21 3:47 AM, dfranken@gmail.com wrote:

Dear readers,

Apologies in advance if these questions have been asked and discussed
before or if they are on the wrong mailing list, maybe they are more
suited for project Valhalla, I'm not sure.

I was wondering if it would be possible / feasible to intern primitive
values and byte arrays and use these interned values instead of
creating a new object for each conversion.

Currently, the following code prints 'false' as s1 and s2 are
references to different objects:

   String s1 = Integer.toString(1);
   s1 = s1.intern(); // Makes no difference whether intern is called
   String s2 = Integer.toString(1);
   System.out.println(s1 == s2);

I know that there is an integer cache for boxing / unboxing commonly
used integers (for numbers ranging from -128 to 127), how about a
String cache for commonly converted primitives? We could use the same
ranges initially.

I.e. when I use Integer.toString(1) I don't really care if I get a
newly allocated String, I only care that I get back a String which
equals "1" and it's okay if that is a reference to an interned value.
Likewise with the mirrored variants such as String.valueOf(..).

I was also wondering how byte conversions could work with such a cache.
Currently, the only way for me to go from bytes to a String is with
new String(bytes, charset) which guarantees creation of a new String
object each time. Well, what if the bytes often contain the same value?

Would it be useful to be able to do:
   
   String s1 = String.valueOf(bytes, UTF_8);

   String s2 = String.valueOf(bytes, UTF_8); // <-- returns the same
refrence as s1

Kind regards,

Dave Franken
   

Re: RFR: JDK-8268150: tier2: test/jdk/tools/jpackage/junit/junit.java needs updating for jtreg 6

[Andy Herrick]

On Thu, 3 Jun 2021 00:47:45 GMT, Jonathan Gibbons  wrote:

> Please review a small test fix, to include hamcrest.jar, as required by the 
> latest version of JUnit  in jtreg 6.

looks fine

-

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

Re: RFR: JDK-8267936: PreserveAllAnnotations option isn't tested

[David Holmes]

Hi Peter,

On 3/06/2021 6:52 pm, Peter Levart wrote:

On Thu, 3 Jun 2021 07:31:14 GMT, David Holmes  wrote:


The code is confusing because it gives no indication that it is aware
that runtime invisible annotations could be present:
/**
* Parses the annotations described by the specified byte array.
* resolving constant references in the specified constant pool.
* The array must contain an array of annotations as described
* in the RuntimeVisibleAnnotations_attribute:
but at the same time it checks for the RUNTIME retention, which means it
must have recognised the possibility there could be something else
there.



Yes, there could be a CLASS retention annotation there (even though 
`-XX+PreserveAllAnnotations` was not used at runtime, so rawAnnotations 
contains the content of `RuntimeVisibleAnnotations` only). Again, such 
annotation was RUNTIME retention when its use was compiled into a class, but at 
runtime such annotation may be updated to have CLASS or even SOURCE retention. 
Such annotation use is filtered out.


Sorry Peter I'm not following you. I am only talking about runtime here.
The VM loads a class at runtime and examines the annotation attributes
that exist in that classfile.


Right, but into which annotation attribute (`RuntimeVisibleAnnotations` vs. 
`RuntimeInvisibleAnnotations`) of that class the annotation use was encoded 
depends on what retention the annotation had at compile time, because those 
attributes are created by `javac` compiler.


Okay


Some will be visible annotations (which
implies RUNTIME retention), others may be invisible (which implies
!RUNTIME which I assume means CLASS). It provides access to these via
the "raw annotations" and the reflection code then processes that
through the AnnotationProcessor.


Right, so reflection code at runtime accesses the annotations that were 
compiled into the class annotation attributes by parsing them and filtering out 
all but RUNTIME annotations. But this filtering is done at runtime and uses 
annotation's current set of meta-annotations values (i.e. `@Retention`) which 
can differ from what this same annotation had at class compile time. So this is 
how current RUNTIME annotations can end up in the `RuntimeInvisibleAnnotations` 
class attribute and how current CLASS annotations can end up in 
`RuntimeVisibleAnnotations` class attribute. It's the consequence of separate 
compilation.


Okay I see what you are talking about in regards to separate compilation.



I don't know exactly what you mean by an annotation use being compiled
into a class, but I assume it is something like the way compile-time
constants are compiled in. But I don't see how that relates to the
current discussion.


An annotation use is the use of annotation in the source code to annotate 
something (Class, Method, Field, ...). For example:


@AnnA
public class Use { ... }


`javac` decides into which class attribute (`RuntimeVisibleAnnotations` vs. 
`RuntimeInvisibleAnnotations`) of Use.class file it will encode the `@AnnA` use 
by examining `AnnA`'s `@Retention` meta-annotation at that time. Say that at 
that time the annotation was this:


@Retention(CLASS)
public @interface AnnA {}


`javac` would encode the `Use`'s use of that annotation into the 
`RuntimeInvisibleAnnotations` attribute of `Use.class`.


Now comes runtime. Annotation maintainer decides to lift the retention of 
`AnnA` annotation to RUNTIME and now it looks like this:


@Retention(RUNTIME)
public @interface AnnA {}


He tries to run some new code to extract the annotation value from `Use` class 
at runtime via reflection, because now at runtime the annotation is updated to 
have RUNTIME retention. But he also doesn't have access to `Use.java` to 
recompile it with updated annotation. He just has access to `Use.class` that 
was compiled with an old version of the same annotation. As we said, 
`Use.class` has encoded the annotation use in its `RuntimeInvisibleAnnotations` 
attribute. Voila, here comes `-XX+PreserveAllAnnotations` option to enable 
passing `RuntimeInvisibleAnnotations` attribute with encoded annotations to the 
reflection runtime and annotation parser which would return such annotation use.


I see the picture you are painting and that things could work that way, 
but I don't know if I agree that this was an intention or that they 
should work that way.


The separate compilation story with annotations is somewhat different to 
other separate compilation issues as the JVMS does not really provide 
any guidance here. The RuntimeInvisibleAnnotations are not even parsed 
by the VM so it know nothing about which annotation types are referred 
to, never loads those types or does any checking to see if the 
properties of that type (like Retention) remain the same. Even 
RuntimeVisibleAnnotations are only structurally parsed to see of there 
are annotations the VM has to be aware of (ie @Contended) but those can 
never be out-of-sync through separate compilation.


So in essence 

Re: RFR: 8267630: Start of release updates for JDK 18

[Erik Joelsson]

On Mon, 24 May 2021 22:35:04 GMT, Joe Darcy  wrote:

> 8267630: Start of release updates for JDK 18

Build change looks good.

Please be aware of the incoming change 
https://bugs.openjdk.java.net/browse/JDK-8263468 which will add another version 
field to version-numbers.conf, which may need to be updated too. The exact 
policy around that is still unclear though.

-

Marked as reviewed by erikj (Reviewer).

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

Re: RFR: JDK-8266254: Update to use jtreg 6 [v2]

[Erik Joelsson]

On Wed, 2 Jun 2021 20:15:55 GMT, Jonathan Gibbons  wrote:

>> Please review the change to update to using jtreg 6. 
>> 
>> The primary change is to the jib-profiles.js file, which specifies the 
>> version of jtreg to use, for those systems that rely on this file. In 
>> addition, the `requiredVersion` has been updated in the various `TEST.ROOT` 
>> files.
>> 
>> All the tests that could be updated ahead of time have been updated. There 
>> are a few tests remaining that need to be done at this time, because of the 
>> change in the module name for TestNG 7.3. It changed from a default of 
>> `testng` to an explicit `org.testng`.
>
> Jonathan Gibbons has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update copyright years

This PR wasn't handled properly when integrated. The change looks like it was 
pushed correctly and the bug was updated, so we are only missing some book 
keeping in the PR. I've filed https://bugs.openjdk.java.net/browse/SKARA-1069.

-

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

Re: RFR: 8268124: Update java.lang to use switch expressions [v3]

[Rémi Forax]

On Thu, 3 Jun 2021 11:01:02 GMT, Patrick Concannon  
wrote:

>> Hi,
>> 
>> Could someone please review my code for updating the code in the `java.lang` 
>> packages to make use of the switch expressions?
>> 
>> Kind regards,
>> Patrick
>
> Patrick Concannon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8268124: small refactoring; fixed misplaced comment and added missing 
> lambda operator

Looks good to me

-

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

Re: RFR: 8268124: Update java.lang to use switch expressions [v3]

[Rémi Forax]

On Thu, 3 Jun 2021 10:57:16 GMT, Patrick Concannon  
wrote:

> My mistake. I've replaced the colon now with the lambda operator.

Drive by comment, in term of name, `->` is the arrow operator not the lambda 
operator.
- lambda = parameters + arrow + code
- arrow case =  case + arrow + code

The difference is important because a lambda is a function while an arrow case 
is a block.

-

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

Re: RFR: 8268124: Update java.lang to use switch expressions [v3]

[Patrick Concannon]

On Wed, 2 Jun 2021 16:06:42 GMT, Rémi Forax  wrote:

>> Patrick Concannon has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8268124: small refactoring; fixed misplaced comment and added missing 
>> lambda operator
>
> src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java 
> line 1663:
> 
>> 1661: case D_TYPE -> mv.visitInsn(Opcodes.DCONST_0);
>> 1662: case L_TYPE -> mv.visitInsn(Opcodes.ACONST_NULL);
>> 1663: default -> throw new InternalError("unknown type: " + 
>> type);
> 
> perhaps
> 
>   mv.visitInsn(switch(type) { ...

Hi Remi, thanks for the suggestion. I've added that in now. See commit a8706b0

> src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line 
> 278:
> 
>> 276: private static boolean isObjectMethod(Method m) {
>> 277: return switch (m.getName()) {
>> 278: case "toString" -> (m.getReturnType() == String.class
> 
> the extra parenthesis are not needed

Parenthesis removed, as suggested. See a8706b0

> src/java.base/share/classes/java/lang/runtime/ObjectMethods.java line 366:
> 
>> 364: }
>> 365: default -> throw new IllegalArgumentException(methodName);
>> 366: };
> 
> I thinki it's simpler to have something like that
> 
>   var handle = switch(methodName) {
> ...
>   };
>   return methodType != null ? new ConstantCallSite(handle) : handle;

I think that looks better. I've made that change now. See a8706b0

-

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

Re: RFR: 8268124: Update java.lang to use switch expressions [v3]

[Patrick Concannon]

> Hi,
> 
> Could someone please review my code for updating the code in the `java.lang` 
> packages to make use of the switch expressions?
> 
> Kind regards,
> Patrick

Patrick Concannon has updated the pull request incrementally with one 
additional commit since the last revision:

  8268124: small refactoring; fixed misplaced comment and added missing lambda 
operator

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4312/files
  - new: https://git.openjdk.java.net/jdk/pull/4312/files/d705efbd..a8706b02

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4312=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4312=01-02

  Stats: 52 lines in 6 files changed: 9 ins; 12 del; 31 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4312.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4312/head:pull/4312

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

Re: RFR: 8268124: Update java.lang to use switch expressions [v3]

[Patrick Concannon]

On Wed, 2 Jun 2021 16:43:25 GMT, Naoto Sato  wrote:

>> Patrick Concannon has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8268124: small refactoring; fixed misplaced comment and added missing 
>> lambda operator
>
> src/java.base/share/classes/java/lang/CharacterData.java line 80:
> 
>> 78: case 2 -> CharacterData02.instance;
>> 79: case 3 -> CharacterData03.instance;
>> 80: case 14 -> CharacterData0E.instance; // Private Use
> 
> Plane 14 is not `private use`

Hi Naoto. Well spotted. I've corrected that now. See a8706b0

> src/java.base/share/classes/java/lang/invoke/DirectMethodHandle.java line 90:
> 
>> 88: // findVirtual etc.
>> 89: return switch (refKind) {
>> 90: case REF_invokeSpecial: {
> 
> Is ':' preferred here (and other cases too) instead of "->"?

My mistake. I've replaced the colon now with the lambda operator. See a8706b0

-

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

Re: RFR: 8268124: Update java.lang to use switch expressions [v2]

[Patrick Concannon]

> Hi,
> 
> Could someone please review my code for updating the code in the `java.lang` 
> packages to make use of the switch expressions?
> 
> Kind regards,
> Patrick

Patrick Concannon has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains two additional 
commits since the last revision:

 - Merge remote-tracking branch 'origin/master' into JDK-8268124
 - 8268124: Update java.lang to use switch expressions

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4312/files
  - new: https://git.openjdk.java.net/jdk/pull/4312/files/72c93053..d705efbd

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4312=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4312=00-01

  Stats: 6221 lines in 174 files changed: 4475 ins; 1291 del; 455 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4312.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4312/head:pull/4312

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

Re: RFR: 8268131: 2 java/foreign tests timed out

[Maurizio Cimadamore]

On Wed, 2 Jun 2021 21:20:53 GMT, Maurizio Cimadamore  
wrote:

> This patch increases time out for both TestUpcall and TestDowncall. These 
> tests were already long-running, but with JEP-412, they were beefed up even 
> more, so now they time out on some debug builds.
> 
> This patch also address a minor issue in TestResourceScope which can 
> sometimes lead to intermittent failure, depending on how threads are 
> scheduled.

> _Mailing list message from [Joe Darcy](mailto:joe.da...@oracle.com) on 
> [core-libs-dev](mailto:core-libs-...@mail.openjdk.java.net):_
> 
> Can the test cases be broken into pieces or otherwise decomposed into
> several shorter-running tests?
> 
> -Joe
> 
> On 6/2/2021 2:35 PM, Maurizio Cimadamore wrote:

I believe they can - but it would require more work - for now the goal was to 
bring tier4 testing back to stable.

-

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

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

[Jan Lahoda]

> This is a preview of a patch implementing JEP 406: Pattern Matching for 
> switch (Preview):
> https://bugs.openjdk.java.net/browse/JDK-8213076
> 
> The current draft of the specification is here:
> http://cr.openjdk.java.net/~gbierman/jep406/jep406-20210430/specs/patterns-switch-jls.html
> 
> A summary of notable parts of the patch:
> -to support cases expressions and patterns in cases, there is a new common 
> superinterface for expressions and patterns, `CaseLabelTree`, which 
> expressions and patterns implement, and a list of case labels is returned 
> from `CaseTree.getLabels()`.
> -to support `case default`, there is an implementation of `CaseLabelTree` 
> that represents it (`DefaultCaseLabelTree`). It is used also to represent the 
> conventional `default` internally and in the newly added methods.
> -in the parser, parenthesized patterns and expressions need to be 
> disambiguated when parsing case labels.
> -Lower has been enhanced to handle `case null` for ordinary (boxed-primitive, 
> String, enum) switches. This is a bit tricky for boxed primitives, as there 
> is no value that is not part of the input domain so that could be used to 
> represent `case null`. Requires a bit shuffling with values.
> -TransPatterns has been enhanced to handle the pattern matching switch. It 
> produces code that delegates to a new bootstrap method, that will classify 
> the input value to the switch and return the case number, to which the switch 
> then jumps. To support guards, the switches (and the bootstrap method) are 
> restartable. The bootstrap method as such is written very simply so far, but 
> could be much more optimized later.
> -nullable type patterns are `case String s, null`/`case null, String s`/`case 
> null: case String s:`/`case String s: case null:`, handling of these required 
> a few tricks in `Attr`, `Flow` and `TransPatterns`.
> 
> The specdiff for the change is here (to be updated):
> http://cr.openjdk.java.net/~jlahoda/8265981/specdiff.preview.01/overview-summary.html

Jan Lahoda has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains 19 commits:

 - Merging master into JDK-8262891
 - Enhancing tests as suggested.
 - Fixing enum switch with patterns with guards.
 - Fixing tests.
 - Total pattern dominates the null pattern.
 - Properly report errors for pattern+default clash.
 - Avoiding unnecessary StackMap point.
 - Post-merge fix - need to include jdk.internal.javac in the list of packages 
used by jdk.compiler again, as we now (again) have a preview feature in javac.
 - Correcting LineNumberTable for rule switches.
 - Merging master into JDK-8262891
 - ... and 9 more: https://git.openjdk.java.net/jdk/compare/bdeaeb47...fa50b5fb

-

Changes: https://git.openjdk.java.net/jdk/pull/3863/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3863=09
  Stats: 4828 lines in 79 files changed: 4509 ins; 118 del; 201 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3863.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3863/head:pull/3863

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

Re: RFR: JDK-8267936: PreserveAllAnnotations option isn't tested

[Peter Levart]

On Thu, 3 Jun 2021 07:31:14 GMT, David Holmes  wrote:

> > > The code is confusing because it gives no indication that it is aware
> > > that runtime invisible annotations could be present:
> > > /**
> > > * Parses the annotations described by the specified byte array.
> > > * resolving constant references in the specified constant pool.
> > > * The array must contain an array of annotations as described
> > > * in the RuntimeVisibleAnnotations_attribute:
> > > but at the same time it checks for the RUNTIME retention, which means it
> > > must have recognised the possibility there could be something else
> > > there.
> > 
> > 
> > Yes, there could be a CLASS retention annotation there (even though 
> > `-XX+PreserveAllAnnotations` was not used at runtime, so rawAnnotations 
> > contains the content of `RuntimeVisibleAnnotations` only). Again, such 
> > annotation was RUNTIME retention when its use was compiled into a class, 
> > but at runtime such annotation may be updated to have CLASS or even SOURCE 
> > retention. Such annotation use is filtered out.
> 
> Sorry Peter I'm not following you. I am only talking about runtime here.
> The VM loads a class at runtime and examines the annotation attributes
> that exist in that classfile.

Right, but into which annotation attribute (`RuntimeVisibleAnnotations` vs. 
`RuntimeInvisibleAnnotations`) of that class the annotation use was encoded 
depends on what retention the annotation had at compile time, because those 
attributes are created by `javac` compiler.

> Some will be visible annotations (which
> implies RUNTIME retention), others may be invisible (which implies
> !RUNTIME which I assume means CLASS). It provides access to these via
> the "raw annotations" and the reflection code then processes that
> through the AnnotationProcessor.

Right, so reflection code at runtime accesses the annotations that were 
compiled into the class annotation attributes by parsing them and filtering out 
all but RUNTIME annotations. But this filtering is done at runtime and uses 
annotation's current set of meta-annotations values (i.e. `@Retention`) which 
can differ from what this same annotation had at class compile time. So this is 
how current RUNTIME annotations can end up in the `RuntimeInvisibleAnnotations` 
class attribute and how current CLASS annotations can end up in 
`RuntimeVisibleAnnotations` class attribute. It's the consequence of separate 
compilation. 

> 
> I don't know exactly what you mean by an annotation use being compiled
> into a class, but I assume it is something like the way compile-time
> constants are compiled in. But I don't see how that relates to the
> current discussion.

An annotation use is the use of annotation in the source code to annotate 
something (Class, Method, Field, ...). For example:


@AnnA
public class Use { ... }


`javac` decides into which class attribute (`RuntimeVisibleAnnotations` vs. 
`RuntimeInvisibleAnnotations`) of Use.class file it will encode the `@AnnA` use 
by examining `AnnA`'s `@Retention` meta-annotation at that time. Say that at 
that time the annotation was this:


@Retention(CLASS)
public @interface AnnA {}


`javac` would encode the `Use`'s use of that annotation into the 
`RuntimeInvisibleAnnotations` attribute of `Use.class`.


Now comes runtime. Annotation maintainer decides to lift the retention of 
`AnnA` annotation to RUNTIME and now it looks like this:


@Retention(RUNTIME)
public @interface AnnA {}


He tries to run some new code to extract the annotation value from `Use` class 
at runtime via reflection, because now at runtime the annotation is updated to 
have RUNTIME retention. But he also doesn't have access to `Use.java` to 
recompile it with updated annotation. He just has access to `Use.class` that 
was compiled with an old version of the same annotation. As we said, 
`Use.class` has encoded the annotation use in its `RuntimeInvisibleAnnotations` 
attribute. Voila, here comes `-XX+PreserveAllAnnotations` option to enable 
passing `RuntimeInvisibleAnnotations` attribute with encoded annotations to the 
reflection runtime and annotation parser which would return such annotation use.

Now imagine whole libraries with classes that use an annotation which began its 
life as CLASS annotation because it was paired with AnnotationProcessor(s) that 
processed those annotation uses at compile time, but now some runtime tool 
emerges that would like to see those annotations too. The maintainer accepts 
the proposal to promote annotation's retention from CLASS to RUNTIME in new 
version of annotation. But who is going to re-compile all those libraries?
> 
> David

Regards, Peter

-

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

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

[Daniel Fuchs]

On Wed, 2 Jun 2021 17:54:06 GMT, Stuart Marks  wrote:

>> I'm fixing this along with a couple intertwined issues.
>> 
>> 1. Add Map.Entry::copyOf as an idempotent copy operation.
>> 
>> 2. Clarify that AbstractMap.SimpleImmutableEntry is itself unmodifiable (not 
>> really immutable) but that subclasses can be modifiable.
>> 
>> 3. Clarify some confusing, historical wording about Map.Entry instances 
>> being obtainable only via iteration of a Map's entry-set view. This was 
>> never really true, since anyone could implement the Map.Entry interface, and 
>> it certainly was not true since JDK 1.6 when SimpleEntry and 
>> SimpleImmutableEntry were added.
>
> Stuart Marks has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Tiny editorial tweaks.

Marked as reviewed by dfuchs (Reviewer).

-

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

Re: RFR: JDK-8267936: PreserveAllAnnotations option isn't tested

[Peter Levart]

On Tue, 1 Jun 2021 09:30:40 GMT, Jaroslav Tulach 
 wrote:

> There doesn't seem to be much support for the complete changes in #4245. To 
> get at least something useful from that endeavor I have extracted the test 
> for existing behavior of `-XX:+PreserveAllAnnotations` and I am offering it 
> in this pull request without any changes to the JVM behavior.

> _Mailing list message from [Brian Goetz](mailto:brian.go...@oracle.com) on 
> [core-libs-dev](mailto:core-libs-...@mail.openjdk.java.net):_
> 
> It seems pretty clear that this "feature" is a leftover from an early
> implementation, doesn't clearly say what it is supposed to do, is more
> complicated than it looks, and is buggily implemented.?

I agree that this "feature" is poorly documented and buggily implemented, but I 
can't agree that it was not created for a reason. At least I can't so easily 
classify it as a "leftover". In particular because it makes sense.

> While I
> understand the temptation to "fix" it, at this point we'd realistically
> be adding a basically entirely new reflection feature that hasn't gone
> through any sort of design analysis.

That "feature" is there already from JDK 5 days. We would not be adding it. The 
reason why we are dealing with it here is very simple. It was/is useful for 
solving a real world problem and a user (looking a Jaroslav) tried to use it, 
but because it was not documented, the user miss-interpreted its meaning and 
tried to propose a "patch" that would solve his problem in the wrong way. 
Through the discussion we established a more correct way to use the "feature" 
(he just has to modify the annotation). Jaroslav can use it. Nothing has to be 
done to it.
Now because the feature wasn't documented and didn't even have a unit test, 
Jaroslav volunteered to create a test for it. Reviewing that test, I found an 
inconsistency and discovered a corner case where the "feature" behaves 
inconsistently. Now at this point we can either fix this inconsistency or not. 
I guess not many were affected by it since nobody officially complained and 
from that we can predict that it is highly unlikely that anybody will.

> -- we'd just be guessing about what
> the original intent of this vestigial feature is.?

The feature makes sense to me. But would it help if we invited original authors 
(Joshua Bloch and/or Neal Gafter) to explain? Of course, it they're willing to 
participate and if they remember what the heck they were doing :-) ...

> It seems the
> reasonable options are to either leave it alone, deprecate it, or engage
> in a much more deliberate redesign.? (And given the fact that I can
> think of at least 1,000 things that are higher priority, I'd not be
> inclined to pursue the third option.)

I see deprecating a feature so easily on the terms of "it is not documented and 
it is buggy" and without even trying to understand the feature and the 
consequences of its deprecation as a more "ruthless" act than trying to fix its 
corner-case inconsistency by carefully paying attention to compatibility. The 
feature is clearly useful (a user is using it to solve the problem). 
Deprecating it and later removing it would leave such user(s) without a means 
to solve such problem(s). So I'm at least for leaving it alone at this point.

Regards, Peter

-

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

Some questions about String interning for primitives

[dfranken . jdk]

Dear readers,

Apologies in advance if these questions have been asked and discussed
before or if they are on the wrong mailing list, maybe they are more
suited for project Valhalla, I'm not sure.

I was wondering if it would be possible / feasible to intern primitive
values and byte arrays and use these interned values instead of
creating a new object for each conversion.

Currently, the following code prints 'false' as s1 and s2 are
references to different objects:

  String s1 = Integer.toString(1);
  s1 = s1.intern(); // Makes no difference whether intern is called
  String s2 = Integer.toString(1);
  System.out.println(s1 == s2);

I know that there is an integer cache for boxing / unboxing commonly
used integers (for numbers ranging from -128 to 127), how about a
String cache for commonly converted primitives? We could use the same
ranges initially.

I.e. when I use Integer.toString(1) I don't really care if I get a
newly allocated String, I only care that I get back a String which
equals "1" and it's okay if that is a reference to an interned value.
Likewise with the mirrored variants such as String.valueOf(..).

I was also wondering how byte conversions could work with such a cache.
Currently, the only way for me to go from bytes to a String is with 
new String(bytes, charset) which guarantees creation of a new String
object each time. Well, what if the bytes often contain the same value?

Would it be useful to be able to do:
  
  String s1 = String.valueOf(bytes, UTF_8);
  String s2 = String.valueOf(bytes, UTF_8); // <-- returns the same
refrence as s1

Kind regards,

Dave Franken
  

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

[Jan Lahoda]

> This is a preview of a patch implementing JEP 406: Pattern Matching for 
> switch (Preview):
> https://bugs.openjdk.java.net/browse/JDK-8213076
> 
> The current draft of the specification is here:
> http://cr.openjdk.java.net/~gbierman/jep406/jep406-20210430/specs/patterns-switch-jls.html
> 
> A summary of notable parts of the patch:
> -to support cases expressions and patterns in cases, there is a new common 
> superinterface for expressions and patterns, `CaseLabelTree`, which 
> expressions and patterns implement, and a list of case labels is returned 
> from `CaseTree.getLabels()`.
> -to support `case default`, there is an implementation of `CaseLabelTree` 
> that represents it (`DefaultCaseLabelTree`). It is used also to represent the 
> conventional `default` internally and in the newly added methods.
> -in the parser, parenthesized patterns and expressions need to be 
> disambiguated when parsing case labels.
> -Lower has been enhanced to handle `case null` for ordinary (boxed-primitive, 
> String, enum) switches. This is a bit tricky for boxed primitives, as there 
> is no value that is not part of the input domain so that could be used to 
> represent `case null`. Requires a bit shuffling with values.
> -TransPatterns has been enhanced to handle the pattern matching switch. It 
> produces code that delegates to a new bootstrap method, that will classify 
> the input value to the switch and return the case number, to which the switch 
> then jumps. To support guards, the switches (and the bootstrap method) are 
> restartable. The bootstrap method as such is written very simply so far, but 
> could be much more optimized later.
> -nullable type patterns are `case String s, null`/`case null, String s`/`case 
> null: case String s:`/`case String s: case null:`, handling of these required 
> a few tricks in `Attr`, `Flow` and `TransPatterns`.
> 
> The specdiff for the change is here (to be updated):
> http://cr.openjdk.java.net/~jlahoda/8265981/specdiff.preview.01/overview-summary.html

Jan Lahoda has updated the pull request incrementally with one additional 
commit since the last revision:

  Enhancing tests as suggested.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3863/files
  - new: https://git.openjdk.java.net/jdk/pull/3863/files/80b1392b..79e3621b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3863=08
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3863=07-08

  Stats: 44 lines in 1 file changed: 44 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3863.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3863/head:pull/3863

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

Re: RFR: JDK-8267936: PreserveAllAnnotations option isn't tested

[David Holmes]

On 3/06/2021 4:49 pm, Peter Levart wrote:

On Wed, 2 Jun 2021 22:44:10 GMT, David Holmes  wrote:


I think this is not deliberate. Since `rawAnnotations` may end up having any of 
the following:
- `RuntimeVisibleAnnotations` (when there were just `RUNTIME` annotation usages 
compiled into the class or `-XX+PreserveAllAnnotations` was not used at runtime)
- `RuntimeInvisibleAnnotations` (when there were just `CLASS` annotation usages 
compiled into the class and `-XX+PreserveAllAnnotations` was used at runtime)
- `RuntimeVisibleAnnotations + RuntimeInvisibleAnnotations` (when there were 
`RUNTIME` and `CLASS` annotation usages compiled into the class and 
`-XX+PreserveAllAnnotations` was used at runtime)
So why would `RuntimeInvisibleAnnotations` be skipped in 3rd case but not in 
2nd case?


Well in the second case there is no way to know it is looking only at
invisible annotations, so it has to read them and then discard them as
they were in fact CLASS retention. The skipping could be seen as an
optimization.


Not all annotations in the 2nd case need to be CLASS retention. They were CLASS 
retention when the class that uses them was compiled, but at runtime, the 
retention may be different (separate compilation). So they are actually 
returned by the reflection in that case.



The code is confusing because it gives no indication that it is aware
that runtime invisible annotations could be present:

/**
* Parses the annotations described by the specified byte array.
* resolving constant references in the specified constant pool.
* The array must contain an array of annotations as described
* in the RuntimeVisibleAnnotations_attribute:

but at the same time it checks for the RUNTIME retention, which means it
must have recognised the possibility there could be something else
there.


Yes, there could be a CLASS retention annotation there (even though 
`-XX+PreserveAllAnnotations` was not used at runtime, so rawAnnotations 
contains the content of `RuntimeVisibleAnnotations` only). Again, such 
annotation was RUNTIME retention when its use was compiled into a class, but at 
runtime such annotation may be updated to have CLASS or even SOURCE retention. 
Such annotation use is filtered out.


Sorry Peter I'm not following you. I am only talking about runtime here. 
The VM loads a class at runtime and examines the annotation attributes 
that exist in that classfile. Some will be visible annotations (which 
implies RUNTIME retention), others may be invisible (which implies 
!RUNTIME which I assume means CLASS). It provides access to these via 
the "raw annotations" and the reflection code then processes that 
through the AnnotationProcessor.


I don't know exactly what you mean by an annotation use being compiled 
into a class, but I assume it is something like the way compile-time 
constants are compiled in. But I don't see how that relates to the 
current discussion.


David
-




That check is day 2 code though, on day 1 there was this comment:

/* XXX Uncomment when this meta-attribute on meta-attributes (Neal's
putback)
if (type.retention() == VisibilityLevel.RUNTIME)
XXX */


I guess this was just code in creation before release. At some point, the 
`RetentionPolicy` enum was added, but above comment refers to it by the name 
`VisibilityLevel`



But the end result is that the code in AnnotationParser correctly
filters out all non-RUNTIME annotations, either directly, or by skipping
them in the stream in the first place. So in that sense there is no bug,
but the code could do with some additional comments.


The problem is that it sometimes skips RUNTIME annotations too. I consider this 
a bug.


Cheers,
David


Regard, Peter

-

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

Re: RFR: JDK-8267936: PreserveAllAnnotations option isn't tested

[Peter Levart]

On Wed, 2 Jun 2021 22:47:13 GMT, David Holmes  wrote:

> On 3/06/2021 2:54 am, Joe Darcy wrote:
> 
> > If the reflection runtime doesn't implement the semantics of
> > -XX+PreserveAllAnnotations, I suggest deprecating/obsoleting/etc. that
> > option now.
> 
> I have to agree with Joe now. I mistakenly thought the raw annotation
> stream was exposed to some parts of reflection, but now I see that it
> all goes through AnnotationParser, which strips out all non-Runtime
> retention annotations. So the flag is useless as the data it causes to
> be passed to the JDK is thrown away anyway.
> 
> Cheers,
> David

We are constantly dancing around the same problem which is a false assumption 
that `RuntimeVisibleAnnotations` attribute contains only RUNTIME retention 
annotations and that `RuntimeInvisibleAnnotations` attribute contains only 
CLASS retention annotations. This is simply not true in a real world. It is 
only true in a world where either nothing changes from the day 1 when it is 1st 
created or in a world where all the sources that compose the application are 
always recompiled. Those two worlds are ideal worlds that don't exist.
In other words, the flag is useless in the dream world(s) but not in the real 
world.

-

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

Re: RFR: 8195129: System.load() fails to load from unicode paths [v3]

[Maxim Kartashev]

On Tue, 1 Jun 2021 18:42:34 GMT, Naoto Sato  wrote:

>> Maxim Kartashev has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - Coding style-related corrections.
>>  - Corrected the test to use Platform.sharedLibraryExt()
>
> test/hotspot/jtreg/runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java 
> line 42:
> 
>> 40: String nativePathSetting = "-Dtest.nativepath=" + 
>> getSystemProperty("test.nativepath");
>> 41: ProcessBuilder pb = 
>> ProcessTools.createTestJvm(nativePathSetting, 
>> LoadLibraryUnicode.class.getName());
>> 42: pb.environment().put("LC_ALL", "en_US.UTF-8");
> 
> Some environments/user configs may not have `UTF-8` codeset on the platform. 
> May need to gracefully exit in such a case.

I added `java.nio.charset.Charset.isSupported("UTF-8")` check to the test. Hope 
that's enough for the environments without `UTF-8`.

-

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

Re: RFR: 8195129: System.load() fails to load from unicode paths [v3]

[Maxim Kartashev]

On Tue, 1 Jun 2021 18:24:05 GMT, Naoto Sato  wrote:

>> Maxim Kartashev has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - Coding style-related corrections.
>>  - Corrected the test to use Platform.sharedLibraryExt()
>
> src/hotspot/os/windows/os_windows.cpp line 1491:
> 
>> 1489: static errno_t convert_UTF8_to_UTF16(char const* utf8_str, LPWSTR* 
>> utf16_str) {
>> 1490:   return convert_to_UTF16(utf8_str, CP_UTF8, utf16_str);
>> 1491: }
> 
> IIUC, `utf8_str` is the "modified" UTF-8 string in JNI. Using it as the 
> standard UTF-8 (I believe Windows' encoding `CP_UTF8` is the one) may end up 
> in incorrect conversions in some corner cases, e.g., for supplementary 
> characters.

Right; I changed the code in NativeLibraries.c to pass down true UTF-8 instead 
of "modified UTF-8". Please, take a look.

-

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

Re: RFR: 8195129: System.load() fails to load from unicode paths [v4]

[Maxim Kartashev]

> Character strings within JVM are produced and consumed in several formats. 
> Strings come from/to Java in the UTF8 format and POSIX APIs (like fprintf() 
> or dlopen()) consume strings also in UTF8. On Windows, however, the situation 
> is far less simple: some new(er) APIs expect UTF16 (wide-character strings), 
> some older APIs can only work with strings in a "platform" format, where not 
> all UTF8 characters can be represented; which ones can depends on the current 
> "code page".
> 
> This commit switches the Windows version of native library loading code to 
> using the new UTF16 API `LoadLibraryW()` and attempts to streamline the use 
> of various string formats in the surrounding code. 
> 
> Namely, exception messages are made to consume strings explicitly in the UTF8 
> format, while logging functions (that end up using legacy Windows API) are 
> made to consume "platform" strings in most cases. One exception is 
> `JVM_LoadLibrary()` logging where the UTF8 name of the library is logged, 
> which can, of course, be fixed, but was considered not worth the additional 
> code (NB: this isn't a new bug).
> 
> The test runs in a separate JVM in order to make NIO happy about non-ASCII 
> characters in the file name; tests are executed with LC_ALL=C and that 
> doesn't let NIO work with non-ASCII file names even on Linux or MacOS.
> 
> Tested by running `test/hotspot/jtreg:tier1` on Linux and 
> `jtreg:test/hotspot/jtreg/runtime` on Windows 10. The new test (`   
> jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode`) was explicitly ran 
> on those platforms as well.
> 
> Results from Linux:
> 
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>jtreg:test/hotspot/jtreg:tier1 1784  1784 0 0  
>  
> ==
> TEST SUCCESS
> 
> 
> Building target 'run-test-only' in configuration 'linux-x86_64-server-release'
> Test selection 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode', 
> will run:
> * jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode
> 
> Running test 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode'
> Passed: runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java
> Test results: passed: 1
> 
> 
> Results from Windows 10:
> 
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR
>jtreg:test/hotspot/jtreg/runtime746   746 0 0
> ==
> TEST SUCCESS
> Finished building target 'run-test-only' in configuration 
> 'windows-x86_64-server-fastdebug'
> 
> 
> Building target 'run-test-only' in configuration 
> 'windows-x86_64-server-fastdebug'
> Test selection 'test/hotspot/jtreg/runtime/jni/loadLibraryUnicode', will run:
> * jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode
> 
> Running test 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode'
> Passed: runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java
> Test results: passed: 1

Maxim Kartashev has updated the pull request incrementally with one additional 
commit since the last revision:

  Updated Java_jdk_internal_loader_NativeLibraries_load() to use a new helper 
function
  to get the library name encoded as true UTF-8 (not in "modified UTF-8" form).
  Also updated the test to automatically pass if "UTF-8" is not supported by NIO
  on the platform.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4169/files
  - new: https://git.openjdk.java.net/jdk/pull/4169/files/c8ef8b64..97c918ca

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4169=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4169=02-03

  Stats: 71 lines in 4 files changed: 57 ins; 6 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4169.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4169/head:pull/4169

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

Re: RFR: JDK-8267936: PreserveAllAnnotations option isn't tested

[Peter Levart]

On Wed, 2 Jun 2021 22:44:10 GMT, David Holmes  wrote:

> > I think this is not deliberate. Since `rawAnnotations` may end up having 
> > any of the following:
> > - `RuntimeVisibleAnnotations` (when there were just `RUNTIME` annotation 
> > usages compiled into the class or `-XX+PreserveAllAnnotations` was not used 
> > at runtime)
> > - `RuntimeInvisibleAnnotations` (when there were just `CLASS` annotation 
> > usages compiled into the class and `-XX+PreserveAllAnnotations` was used at 
> > runtime)
> > - `RuntimeVisibleAnnotations + RuntimeInvisibleAnnotations` (when there 
> > were `RUNTIME` and `CLASS` annotation usages compiled into the class and 
> > `-XX+PreserveAllAnnotations` was used at runtime)
> > So why would `RuntimeInvisibleAnnotations` be skipped in 3rd case but not 
> > in 2nd case?
> 
> Well in the second case there is no way to know it is looking only at
> invisible annotations, so it has to read them and then discard them as
> they were in fact CLASS retention. The skipping could be seen as an
> optimization.

Not all annotations in the 2nd case need to be CLASS retention. They were CLASS 
retention when the class that uses them was compiled, but at runtime, the 
retention may be different (separate compilation). So they are actually 
returned by the reflection in that case.

> 
> The code is confusing because it gives no indication that it is aware
> that runtime invisible annotations could be present:
> 
> /**
> * Parses the annotations described by the specified byte array.
> * resolving constant references in the specified constant pool.
> * The array must contain an array of annotations as described
> * in the RuntimeVisibleAnnotations_attribute:
> 
> but at the same time it checks for the RUNTIME retention, which means it
> must have recognised the possibility there could be something else
> there.

Yes, there could be a CLASS retention annotation there (even though 
`-XX+PreserveAllAnnotations` was not used at runtime, so rawAnnotations 
contains the content of `RuntimeVisibleAnnotations` only). Again, such 
annotation was RUNTIME retention when its use was compiled into a class, but at 
runtime such annotation may be updated to have CLASS or even SOURCE retention. 
Such annotation use is filtered out.

> That check is day 2 code though, on day 1 there was this comment:
> 
> /* XXX Uncomment when this meta-attribute on meta-attributes (Neal's
> putback)
> if (type.retention() == VisibilityLevel.RUNTIME)
> XXX */

I guess this was just code in creation before release. At some point, the 
`RetentionPolicy` enum was added, but above comment refers to it by the name 
`VisibilityLevel`

> 
> But the end result is that the code in AnnotationParser correctly
> filters out all non-RUNTIME annotations, either directly, or by skipping
> them in the stream in the first place. So in that sense there is no bug,
> but the code could do with some additional comments.

The problem is that it sometimes skips RUNTIME annotations too. I consider this 
a bug.

> 
> Cheers,
> David

Regard, Peter

-

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

Re: RFR: 8267630: Start of release updates for JDK 18

[Iris Clark]

On Mon, 24 May 2021 22:35:04 GMT, Joe Darcy  wrote:

> 8267630: Start of release updates for JDK 18

Marked as reviewed by iris (Reviewer).

-

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