Re: RFR: 8240903: Add a regression test for JDK-8240734 [v3]

2022-03-24 Thread Dongbo He 
> This creates a regression test for 
> [JDK-8240734](https://bugs.openjdk.java.net/browse/JDK-8240734). This was 
> once blocked 
> by a time stamp issue which has been resolved by: [JDK-8276766 
> ](https://bugs.openjdk.java.net/browse/JDK-8276766)
> 
> We found the issue can be produced stably with at least 64 modules.
> Note that we need to add the --date jmod option to avoid the timestamp issue.
> 
> Testing:
> Added test case fails without fix for JDK-8240734, and passes otherwise.
> Tested with tier2 on linux x86.

Dongbo He has updated the pull request incrementally with one additional commit 
since the last revision:

  Get date by 'date +%Y-%m-%dT%H:%M:%S%:z'

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7948/files
  - new: https://git.openjdk.java.net/jdk/pull/7948/files/b2a1c843..aea79a40

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

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

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

Re: RFR: JDK-8283668: Update IllegalFormatException to use sealed classes

2022-03-24 Thread Iris Clark 
On Fri, 25 Mar 2022 04:17:57 GMT, Joe Darcy  wrote:

> Working down the list of candidates to be sealed, this time 
> IllegalFormatException.
> 
> Please also review the companion CSR: 
> https://bugs.openjdk.java.net/browse/JDK-8283669

Marked as reviewed by iris (Reviewer).

-

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

RFR: JDK-8283668: Update IllegalFormatException to use sealed classes

2022-03-24 Thread Joe Darcy 
Working down the list of candidates to be sealed, this time 
IllegalFormatException.

Please also review the companion CSR: 
https://bugs.openjdk.java.net/browse/JDK-8283669

-

Commit messages:
 - JDK-8283668: Update IllegalFormatException to use sealed classes

Changes: https://git.openjdk.java.net/jdk/pull/7950/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7950=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8283668
  Stats: 38 lines in 13 files changed: 12 ins; 0 del; 26 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7950.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7950/head:pull/7950

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

Re: RFR: 8240903: Add a regression test for JDK-8240734 [v2]

2022-03-24 Thread Dongbo He 
> This creates a regression test for JDK-8240734. This was once blocked 
> by a time stamp issue which has been resolved by: [JDK-8276766 
> ](https://bugs.openjdk.java.net/browse/JDK-8276766)
> 
> We found the issue can always be produced with at least 64 modules.
> Note that we need to add the --date jmod option to avoid the timestamp issue.
> 
> Testing:
> Added test case fails before JDK-8240734, and passes after JDK-8240734.
> Tested with tier2 on linux x86.

Dongbo He has updated the pull request incrementally with one additional commit 
since the last revision:

  Update Copyright

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7948/files
  - new: https://git.openjdk.java.net/jdk/pull/7948/files/12c35178..b2a1c843

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

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

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

Re: RFR: 8282819: Deprecate Locale class constructors

2022-03-24 Thread Naoto Sato 
On Fri, 25 Mar 2022 00:18:54 GMT, Stuart Marks  wrote:

>> Proposing to deprecate the constructors in the `java.util.Locale` class. 
>> There is already a factory method and a builder to return singletons, so 
>> there is no need to have constructors anymore unless one purposefully wants 
>> to create `ill-formed` Locale objects, which is discouraged. We cannot 
>> terminally deprecate those constructors for the compatibility to serialized 
>> objects created with older JDKs. Please see the draft CSR for more detail.
>
> src/java.base/share/classes/sun/util/locale/provider/LocaleServiceProviderPool.java
>  line 375:
> 
>> 373: (locale.getLanguage().isEmpty() ? "und" : 
>> locale.getLanguage()) +
>> 374: (locale.getCountry().isEmpty() ? "" : "-" + 
>> locale.getCountry()) +
>> 375: (locale.getVariant().isEmpty() ? "" : 
>> "-x-lvariant-" + locale.getVariant()));
> 
> It seems like this snippet (and ones very similar to it) are repeated several 
> times throughout the JDK code as replacements for the two- and three-arg 
> constructors. This seems like a fair increase in complexity, and the use of 
> "und" and "-x-lvariant-" are quite non-obvious. Would we recommend that third 
> party code that uses the Locale constructors replace them with this snippet? 
> Is there something better that we can provide?

True. One solution could be to expose `Locale.getInstance()`, which is 
currently a package-private static method, for this purpose. Though that means 
promoting `ill-formed` locale objects which defy some part of the deprecation 
cause.

-

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

RFR: 8240903: Add a regression test for JDK-8240734

2022-03-24 Thread Dongbo He 
This creates a regression test for JDK-8240734. This was once blocked 
by a time stamp issue which has been resolved by: [JDK-8276766 
](https://bugs.openjdk.java.net/browse/JDK-8276766)

We found the issue can always be produced with at least 64 modules.
Note that we need to add the --date jmod option to avoid the timestamp issue.

Testing:
Added test case fails before JDK-8240734, and passes after JDK-8240734.
Tested with tier2 on linux x86.

-

Commit messages:
 - 8240903: Add a regression test for JDK-8240734

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

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

Re: RFR: 8282819: Deprecate Locale class constructors

2022-03-24 Thread Stuart Marks 
On Thu, 24 Mar 2022 22:01:30 GMT, Naoto Sato  wrote:

> Proposing to deprecate the constructors in the `java.util.Locale` class. 
> There is already a factory method and a builder to return singletons, so 
> there is no need to have constructors anymore unless one purposefully wants 
> to create `ill-formed` Locale objects, which is discouraged. We cannot 
> terminally deprecate those constructors for the compatibility to serialized 
> objects created with older JDKs. Please see the draft CSR for more detail.

Specs looks good, with minor modifications. I'm not so sure about replacement 
of the constructors with string concatenation with ternary operators; see my 
other comment.

src/java.base/share/classes/java/util/Locale.java line 252:

> 250:  * The {@code Locale} class provides a number of convenient constants
> 251:  * that you can use to obtain {@code Locale} objects for commonly used
> 252:  * locales. For example, the following obtains a {@code Locale} object

I'd replace this part (and the example below) with something like,

For example, {@code Locale.US} is the {@code Locale} object for the United 
States.

src/java.base/share/classes/java/util/Locale.java line 2511:

> 2509:  * constructors, the {@code Builder} checks if a value configured 
> by a
> 2510:  * setter satisfies the syntax requirements defined by the {@code 
> Locale}
> 2511:  * class.  A {@code Locale} object obtained by a {@code Builder} is

...obtained from a Builder...

src/java.base/share/classes/java/util/Locale.java line 2526:

> 2524:  *
> 2525:  * The following example shows how to obtain a {@code Locale} 
> object
> 2526:  * with the {@code Builder}.

...shows how to obtain a Locale object using a Builder.

src/java.base/share/classes/java/util/Locale.java line 2660:

> 2658:  *
> 2659:  * The country value in the {@code Locale} obtained by the
> 2660:  * {@code Builder} is always normalized to upper case.

...obtained from a Builder...

src/java.base/share/classes/sun/util/locale/provider/LocaleServiceProviderPool.java
 line 375:

> 373: (locale.getLanguage().isEmpty() ? "und" : 
> locale.getLanguage()) +
> 374: (locale.getCountry().isEmpty() ? "" : "-" + 
> locale.getCountry()) +
> 375: (locale.getVariant().isEmpty() ? "" : 
> "-x-lvariant-" + locale.getVariant()));

It seems like this snippet (and ones very similar to it) are repeated several 
times throughout the JDK code as replacements for the two- and three-arg 
constructors. This seems like a fair increase in complexity, and the use of 
"und" and "-x-lvariant-" are quite non-obvious. Would we recommend that third 
party code that uses the Locale constructors replace them with this snippet? Is 
there something better that we can provide?

-

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

Integrated: 8267319: Use larger default key sizes and algorithms based on CNSA

2022-03-24 Thread Valerie Peng 
On Wed, 2 Mar 2022 00:13:41 GMT, Valerie Peng  wrote:

> It's been several years since we increased the default key sizes. Before 
> shifting to PQC, NSA replaced its Suite B cryptography recommendations with 
> the Commercial National Security Algorithm Suite which suggests:
> 
> - SHA-384 for secure hashing
> - AES-256 for symmetric encryption
> - RSA with 3072 bit keys for digital signatures and for key exchange
> - Diffie Hellman (DH) with 3072 bit keys for key exchange
> - Elliptic curve [P-384] for key exchange (ECDH) and for digital signatures 
> (ECDSA)
> 
> So, this proposed changes made the suggested key size and algorithm changes. 
> The changes are mostly in keytool, jarsigner and their regression tests, so 
> @wangweij Could you please take a look?
> 
> Thanks!

This pull request has now been integrated.

Changeset: 313bc7f6
Author:Valerie Peng 
URL:   
https://git.openjdk.java.net/jdk/commit/313bc7f64f69d8f352d495d2c35bea62aca910e4
Stats: 504 lines in 29 files changed: 326 ins; 8 del; 170 mod

8267319: Use larger default key sizes and algorithms based on CNSA

Reviewed-by: weijun, xuelei

-

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

Re: RFR: 8273370: Preferences.exportSubtree() generates invalid XML if value contains control char

2022-03-24 Thread Naoto Sato 
On Thu, 24 Mar 2022 18:21:05 GMT, Joe Wang  wrote:

> The issue was caused by the difference on handling control characters between 
> the parser and serializer. The parser rejected control characters while the 
> serializer converted them to NCRs. The fix is for the later to be aligned 
> with the parser, reporting error upon encountering control characters. 
> 
> Mach5 tier2 passed.

Looks good. Thanks for the explanation, Joe.

-

Marked as reviewed by naoto (Reviewer).

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

Re: RFR: 8273370: Preferences.exportSubtree() generates invalid XML if value contains control char

2022-03-24 Thread Joe Wang 
On Thu, 24 Mar 2022 18:21:05 GMT, Joe Wang  wrote:

> The issue was caused by the difference on handling control characters between 
> the parser and serializer. The parser rejected control characters while the 
> serializer converted them to NCRs. The fix is for the later to be aligned 
> with the parser, reporting error upon encountering control characters. 
> 
> Mach5 tier2 passed.

For XML 1.0, it's known that they were invalid. For prefs, it would be nice to 
add InvalidPreferencesFormatException to exportSubtree to match with that of 
importPreferences. But it's such an edge case, for this patch, I opted for no 
signature change.

-

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

RFR: 8282819: Deprecate Locale class constructors

2022-03-24 Thread Naoto Sato 
Proposing to deprecate the constructors in the `java.util.Locale` class. There 
is already a factory method and a builder to return singletons, so there is no 
need to have constructors anymore unless one purposefully wants to create 
`ill-formed` Locale objects, which is discouraged. We cannot terminally 
deprecate those constructors for the compatibility to serialized objects 
created with older JDKs. Please see the draft CSR for more detail.

-

Commit messages:
 - 8282819: Deprecate Locale class constructors

Changes: https://git.openjdk.java.net/jdk/pull/7947/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7947=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8282819
  Stats: 904 lines in 196 files changed: 74 ins; 22 del; 808 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7947.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7947/head:pull/7947

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

Re: RFR: 8283349 : Robustness improvements to java/util/prefs/AddNodeChangeListener.jar [v2]

2022-03-24 Thread Brent Christian 
On Wed, 23 Mar 2022 17:29:04 GMT, Naoto Sato  wrote:

>> Brent Christian has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Add test failure message to 'couldn't remove' RuntimeException
>
> test/jdk/java/util/prefs/AddNodeChangeListener.java line 63:
> 
>> 61: } finally {
>> 62: // Make sure test node is not present after the test
>> 63: clearPrefs();
> 
> If this call throws a `RuntimeException("Unable to clear...")`, it will 
> override the possible `RuntimeException("Failed")` in the `try` block, but I 
> think it is OK.

That's true - I hadn't considered that. I think it's OK, too. Even so, I've 
added a message to the exception thrown from clearPrefs() to indicate if the 
test failed.

-

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

Re: RFR: 8283349 : Robustness improvements to java/util/prefs/AddNodeChangeListener.jar [v2]

2022-03-24 Thread Brent Christian 
> Please review this change to the java/util/prefs/AddNodeChangeListener.jar 
> test.
> 
> Although the test specifies "-Djava.util.prefs.userRoot=." to ensure a fresh 
> Preferences on each test run, MacOS does not seem to honor this, and still 
> stores prefs in Library/.
> 
> This test has long suffered intermittent failures. 8255031 added some 
> debugging code; as of that change, the test fails fast as soon as an expected 
> change event is not received. In particular, if the expected event is not 
> received for adding the node, the test exits without removing the node. In 
> this way, on Mac, the node can get "stuck" in the preferences of the machine. 
> Future runs on the machine will already have the node, no node added change 
> event will be generated (because the node was already there), the test will 
> continue to fail without removing the node, etc. This matches observations on 
> some CI machines.  
> 
> This change ensures that:
> * the test node is not present before the test
> * the test runs to completion, including removing the test node
> * the test node is not left behind after the test
> 
> Thanks.

Brent Christian has updated the pull request incrementally with one additional 
commit since the last revision:

  Add test failure message to 'couldn't remove' RuntimeException

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7908/files
  - new: https://git.openjdk.java.net/jdk/pull/7908/files/71ab1f17..139b6a94

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

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

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

Re: RFR: 8273370: Preferences.exportSubtree() generates invalid XML if value contains control char

2022-03-24 Thread Naoto Sato 
On Thu, 24 Mar 2022 18:21:05 GMT, Joe Wang  wrote:

> The issue was caused by the difference on handling control characters between 
> the parser and serializer. The parser rejected control characters while the 
> serializer converted them to NCRs. The fix is for the later to be aligned 
> with the parser, reporting error upon encountering control characters. 
> 
> Mach5 tier2 passed.

So the fix is now to throw an exception on serialization. Would there be any 
need to let the users know as it becomes a stricter behavior?

-

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

Integrated: 8283426: Fix 'exeption' typo

2022-03-24 Thread Andrey Turbanov 
On Sun, 20 Mar 2022 13:30:01 GMT, Andrey Turbanov  wrote:

> Fix repeated typo `exeption`

This pull request has now been integrated.

Changeset: dc5a65ab
Author:Andrey Turbanov 
URL:   
https://git.openjdk.java.net/jdk/commit/dc5a65ab378f0780f7760965f2b52cbbd7c62aad
Stats: 38 lines in 17 files changed: 0 ins; 2 del; 36 mod

8283426: Fix 'exeption' typo

Reviewed-by: xuelei, iris, dholmes, wetmore, aivanov

-

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v5]

2022-03-24 Thread Jorn Vernee 
On Thu, 24 Mar 2022 18:35:12 GMT, Jorn Vernee  wrote:

>> Sure, this is problematic - but at the same time I don't think there's a 
>> better way to deal with this? I'd prefer to defer this to a separate issue 
>> (and I think the build team is in a much better position to suggest a better 
>> fix). IIRC we had this problem in the past as well.
>
> I'd suggest at least adding `--enable-preview` as an argument when running 
> benchmarks through the build system in that case. I think this should do the 
> trick:
> 
> 
> diff --git a/make/RunTests.gmk b/make/RunTests.gmk
> index 81540266ec0..9ed45fb02a8 100644
> --- a/make/RunTests.gmk
> +++ b/make/RunTests.gmk
> @@ -583,7 +583,7 @@ define SetupRunMicroTestBody
>$$(eval $$(call SetMicroValue,$1,MICRO_JAVA_OPTIONS))
> 
># Current tests needs to open java.io
> -  $1_MICRO_JAVA_OPTIONS += --add-opens=java.base/java.io=ALL-UNNAMED
> +  $1_MICRO_JAVA_OPTIONS += --add-opens=java.base/java.io=ALL-UNNAMED 
> --enable-preview
> 
># Save output as JSON or CSV file
>ifneq ($$(MICRO_RESULTS_FORMAT), )
> 
> 
> People manually running the benchmarks.jar will have to pass 
> `--enable-preview` still though.

After discussing this offline, it seems that javac no longer poisons the minor 
class file version of every class file, but only of those that use preview 
features. So, my concern is not warranted.

-

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v5]

2022-03-24 Thread Maurizio Cimadamore 
On Thu, 24 Mar 2022 19:17:40 GMT, Jorn Vernee  wrote:

>> I'd suggest at least adding `--enable-preview` as an argument when running 
>> benchmarks through the build system in that case. I think this should do the 
>> trick:
>> 
>> 
>> diff --git a/make/RunTests.gmk b/make/RunTests.gmk
>> index 81540266ec0..9ed45fb02a8 100644
>> --- a/make/RunTests.gmk
>> +++ b/make/RunTests.gmk
>> @@ -583,7 +583,7 @@ define SetupRunMicroTestBody
>>$$(eval $$(call SetMicroValue,$1,MICRO_JAVA_OPTIONS))
>> 
>># Current tests needs to open java.io
>> -  $1_MICRO_JAVA_OPTIONS += --add-opens=java.base/java.io=ALL-UNNAMED
>> +  $1_MICRO_JAVA_OPTIONS += --add-opens=java.base/java.io=ALL-UNNAMED 
>> --enable-preview
>> 
>># Save output as JSON or CSV file
>>ifneq ($$(MICRO_RESULTS_FORMAT), )
>> 
>> 
>> People manually running the benchmarks.jar will have to pass 
>> `--enable-preview` still though.
>
> After discussing this offline, it seems that javac no longer poisons the 
> minor class file version of every class file, but only of those that use 
> preview features. So, my concern is not warranted.

Turns out this is no longer necessary. As part of the support for preview API, 
javac now only pollutes the classfile if a source file is using preview 
features, as described in this PR:
https://github.com/openjdk/jdk/pull/703

-

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v11]

2022-03-24 Thread Jorn Vernee 
On Thu, 24 Mar 2022 19:19:34 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-424 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/424
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Revert changes to RunTests.gmk

Looks Good!

-

Marked as reviewed by jvernee (Reviewer).

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v11]

2022-03-24 Thread Maurizio Cimadamore 
> This PR contains the API and implementation changes for JEP-424 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/424

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

  Revert changes to RunTests.gmk

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7888/files
  - new: https://git.openjdk.java.net/jdk/pull/7888/files/6e7189b4..504b564a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7888=10
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7888=09-10

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

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

Re: RFR: 8283426: Fix 'exeption' typo [v4]

2022-03-24 Thread Alexey Ivanov 
On Wed, 23 Mar 2022 19:41:58 GMT, Andrey Turbanov  wrote:

>> Fix repeated typo `exeption`
>
> Andrey Turbanov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8283426: Fix 'exeption' typo
>   
>   Co-authored-by: Alexey Ivanov 
> <70774172+aivanov-...@users.noreply.github.com>

Marked as reviewed by aivanov (Reviewer).

-

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

Re: [External] : Re: jpackage Windows support for 4 digits version

2022-03-24 Thread Alexey Semenyuk 

Hi Sverre,

Oh, I misunderstood the problem. I though the issue was with parsing WiX 
version string, but the problem is that jpackage doesn't like the value 
of --version cli parameter.
On windows this value should satisfy constrains of MSI ProductVersion 
property [1]:

The format of the string is as follows:
major.minor.build
The first field is the major version and has a maximum value of 255. The 
second field is the minor version and has a maximum value of 255. The 
third field is called the build version or the update version and has a 
maximum value of 65,535.


If not doing validation early, wix candle will fail later. This is 
platform limitation, not the limitation of jpackage.


[1] https://msdn.microsoft.com/en-us/library/aa370859%28v=VS.85%29.aspx

- Alexey

On 3/24/2022 2:04 PM, Sverre Moe wrote:

Hi,

This has been attempted on Java 15, Java 16 and the latest Java 17.

This is the output from Java 17

14:46:49 
 C:\cygwin64\home\build\jenkins-test\workspace\our-awesome-project_sverre_SF-99>gradlew.bat 
   --no-daemon --version

14:46:51
14:46:51  
14:46:51  Gradle 7.3.3
14:46:51  
14:46:51
14:46:51  Build time:   2021-12-22 12:37:54 UTC
14:46:51  Revision: 6f556c80f945dc54b50e0be633da6c62dbe8dc71
14:46:51
14:46:51  Kotlin:       1.5.31
14:46:51  Groovy:       3.0.9
14:46:51  Ant:          Apache Ant(TM) version 1.10.11 compiled on 
July 10 2021

14:46:51  JVM:          17.0.2 (Eclipse Adoptium 17.0.2+8)
14:46:51  OS:           Windows 10 10.0 amd64


14:46:53 
 C:\cygwin64\home\build\jenkins-test\workspace\our-awesome-project_sverre_SF-99>gradlew.bat 
   --no-daemon --stacktrace --refresh-dependencies -Pheadless 
createPackage

14:48:51  > Task :jpackage
14:48:51  [14:49:12.348] Running candle.exe
14:48:51  [14:49:12.386] Running C:\Program Files (x86)\WiX Toolset 
v3.11\bin\candle.exe

14:48:51  [14:49:12.762] Running light.exe
14:48:51  [14:49:12.773] Running C:\Program Files (x86)\WiX Toolset 
v3.11\bin\light.exe

14:48:51  [14:49:13.207] Detected [candle.exe] version [3.11.2.4516].
14:48:51  [14:49:13.208] Detected [light.exe] version [3.11.2.4516].
14:48:51  [14:49:13.210] WiX 3.11.2.4516 detected. Enabling advanced 
cleanup action.
14:48:51  [14:49:13.211] jdk.jpackage.internal.ConfigException: 
Version sting may have between 1 and 3 components: 1, 1.2, 1.2.3.
14:48:51   at 
jdk.jpackage/jdk.jpackage.internal.WinMsiBundler.validate(WinMsiBundler.java:319)
14:48:51   at 
jdk.jpackage/jdk.jpackage.internal.WinExeBundler.validate(WinExeBundler.java:93)
14:48:51   at 
jdk.jpackage/jdk.jpackage.internal.Arguments.generateBundle(Arguments.java:675)
14:48:51   at 
jdk.jpackage/jdk.jpackage.internal.Arguments.processArguments(Arguments.java:550)

14:48:51   at jdk.jpackage/jdk.jpackage.main.Main.execute(Main.java:91)
14:48:51   at jdk.jpackage/jdk.jpackage.main.Main.main(Main.java:52)
14:48:51  Caused by: java.lang.IllegalArgumentException: Version sting 
may have between 1 and 3 components: 1, 1.2, 1.2.3.
14:48:51   at 
jdk.jpackage/jdk.jpackage.internal.MsiVersion.of(MsiVersion.java:46)
14:48:51   at 
jdk.jpackage/jdk.jpackage.internal.WinMsiBundler.validate(WinMsiBundler.java:317)

14:48:51   ... 5 more
14:48:51  [14:49:13.218] jdk.jpackage.internal.PackagerException: 
Bundler EXE Installer Package skipped because of a configuration 
problem: Version sting may have between 1 and 3 components: 1, 1.2, 
1.2.3.
14:48:51  Advice to fix: Set value of --app-version parameter 
according to these rules: 
https://msdn.microsoft.com/en-us/library/aa370859%28v=VS.85%29.aspx 

14:48:51   at 
jdk.jpackage/jdk.jpackage.internal.Arguments.generateBundle(Arguments.java:688)
14:48:51   at 
jdk.jpackage/jdk.jpackage.internal.Arguments.processArguments(Arguments.java:550)

14:48:51   at jdk.jpackage/jdk.jpackage.main.Main.execute(Main.java:91)
14:48:51   at jdk.jpackage/jdk.jpackage.main.Main.main(Main.java:52)
14:48:51  Caused by: jdk.jpackage.internal.ConfigException: Version 
sting may have between 1 and 3 components: 1, 1.2, 1.2.3.
14:48:51   at 
jdk.jpackage/jdk.jpackage.internal.WinMsiBundler.validate(WinMsiBundler.java:319)
14:48:51   at 
jdk.jpackage/jdk.jpackage.internal.WinExeBundler.validate(WinExeBundler.java:93)
14:48:51   at 
jdk.jpackage/jdk.jpackage.internal.Arguments.generateBundle(Arguments.java:675)

14:48:51   ... 3 more
14:48:51  Caused by: java.lang.IllegalArgumentException: Version sting 
may have between 1 and 3 components: 1, 1.2, 1.2.3.
14:48:51   at 
jdk.jpackage/jdk.jpackage.internal.MsiVersion.of(MsiVersion.java:46)
14:48:51   at 
jdk.jpackage/jdk.jpackage.internal.WinMsiBundler.validate(WinMsiBundler.java:317)

14:48:51   ... 5 more
14:48:51
14:48:51
14:48:51  > Task

Integrated: JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes

2022-03-24 Thread Joe Darcy 
On Sun, 20 Mar 2022 21:31:15 GMT, Joe Darcy  wrote:

> Small refactoring to use sealed classes in the MethodHandle implementation 
> hierarchy.
> 
> DelegatingMethodHandle is non-sealed rather than sealed since it has two 
> subclasses, one defined as a nested class and only a separate type in the 
> same package. The permits clause does not allow listed those two kinds of 
> subclasses.
> 
> Please also review the corresponding CSR 
> https://bugs.openjdk.java.net/browse/JDK-8283434

This pull request has now been integrated.

Changeset: f1624450
Author:Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/f16244509df7659572bfd23e55a62bb9a891000a
Stats: 23 lines in 6 files changed: 6 ins; 0 del; 17 mod

8283416: Update java.lang.invoke.MethodHandle to use sealed classes

Reviewed-by: sundar, mchung

-

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v10]

2022-03-24 Thread Maurizio Cimadamore 
> This PR contains the API and implementation changes for JEP-424 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/424

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

  Add --enable-preview to micro benchmark java options

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7888/files
  - new: https://git.openjdk.java.net/jdk/pull/7888/files/d95c6d0f..6e7189b4

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

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

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

Re: RFR: 8273370: Preferences.exportSubtree() generates invalid XML if value contains control char

2022-03-24 Thread Lance Andersen 
On Thu, 24 Mar 2022 18:21:05 GMT, Joe Wang  wrote:

> The issue was caused by the difference on handling control characters between 
> the parser and serializer. The parser rejected control characters while the 
> serializer converted them to NCRs. The fix is for the later to be aligned 
> with the parser, reporting error upon encountering control characters. 
> 
> Mach5 tier2 passed.

Marked as reviewed by lancea (Reviewer).

-

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v9]

2022-03-24 Thread Maurizio Cimadamore 
> This PR contains the API and implementation changes for JEP-424 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/424

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

  Address more review comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7888/files
  - new: https://git.openjdk.java.net/jdk/pull/7888/files/6881b6dc..d95c6d0f

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

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

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

Re: RFR: JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes [v2]

2022-03-24 Thread Mandy Chung 
On Thu, 24 Mar 2022 18:58:43 GMT, Joe Darcy  wrote:

>> Small refactoring to use sealed classes in the MethodHandle implementation 
>> hierarchy.
>> 
>> DelegatingMethodHandle is non-sealed rather than sealed since it has two 
>> subclasses, one defined as a nested class and only a separate type in the 
>> same package. The permits clause does not allow listed those two kinds of 
>> subclasses.
>> 
>> Please also review the corresponding CSR 
>> https://bugs.openjdk.java.net/browse/JDK-8283434
>
> 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 three additional commits since 
> the last revision:
> 
>  - Respond to review feedback.
>  - Merge branch 'master' into JDK-8283416
>  - JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes

Looks good.

-

Marked as reviewed by mchung (Reviewer).

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v8]

2022-03-24 Thread Maurizio Cimadamore 
> This PR contains the API and implementation changes for JEP-424 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/424

Maurizio Cimadamore has updated the pull request incrementally with two 
additional commits since the last revision:

 - Update src/java.base/share/classes/java/lang/foreign/SymbolLookup.java
   
   Co-authored-by: Jorn Vernee 
 - Update src/java.base/share/classes/java/lang/foreign/SymbolLookup.java
   
   Co-authored-by: Jorn Vernee 

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7888/files
  - new: https://git.openjdk.java.net/jdk/pull/7888/files/3e8cfd74..6881b6dc

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

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

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

Re: RFR: JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes [v2]

2022-03-24 Thread Joe Darcy 
> Small refactoring to use sealed classes in the MethodHandle implementation 
> hierarchy.
> 
> DelegatingMethodHandle is non-sealed rather than sealed since it has two 
> subclasses, one defined as a nested class and only a separate type in the 
> same package. The permits clause does not allow listed those two kinds of 
> subclasses.
> 
> Please also review the corresponding CSR 
> https://bugs.openjdk.java.net/browse/JDK-8283434

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 three additional commits since the 
last revision:

 - Respond to review feedback.
 - Merge branch 'master' into JDK-8283416
 - JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7881/files
  - new: https://git.openjdk.java.net/jdk/pull/7881/files/f4f79411..e7c6495f

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

  Stats: 65233 lines in 1034 files changed: 62326 ins; 1345 del; 1562 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7881.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7881/head:pull/7881

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

RFR: 8273370: Preferences.exportSubtree() generates invalid XML if value contains control char

2022-03-24 Thread Joe Wang 
The issue was caused by the difference on handling control characters between 
the parser and serializer. The parser rejected control characters while the 
serializer converted them to NCRs. The fix is for the later to be aligned with 
the parser, reporting error upon encountering control characters. 

Mach5 tier2 passed.

-

Commit messages:
 - 8273370: Preferences.exportSubtree() generates invalid XML if value contains 
control char

Changes: https://git.openjdk.java.net/jdk/pull/7945/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7945=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8273370
  Stats: 197 lines in 4 files changed: 182 ins; 3 del; 12 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7945.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7945/head:pull/7945

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v5]

2022-03-24 Thread Jorn Vernee 
On Thu, 24 Mar 2022 17:48:23 GMT, Maurizio Cimadamore  
wrote:

>> make/test/BuildMicrobenchmark.gmk line 97:
>> 
>>> 95: SRC := $(MICROBENCHMARK_SRC), \
>>> 96: BIN := $(MICROBENCHMARK_CLASSES), \
>>> 97: JAVAC_FLAGS := --add-exports 
>>> java.base/sun.security.util=ALL-UNNAMED --enable-preview, \
>> 
>> It still seems like this would lead to potential issues. i.e. requiring all 
>> benchmarks to be run with `--enable-preview`? We ended up adding 
>> `--enable-preview` to our benchmarks, but do other benchmarks still work 
>> without it? AFAIK the entire benchmarks.jar will have the altered class file 
>> version.
>
> Sure, this is problematic - but at the same time I don't think there's a 
> better way to deal with this? I'd prefer to defer this to a separate issue 
> (and I think the build team is in a much better position to suggest a better 
> fix). IIRC we had this problem in the past as well.

I'd suggest at least adding `--enable-preview` as an argument when running 
benchmarks through the build system in that case. I think this should do the 
trick:


diff --git a/make/RunTests.gmk b/make/RunTests.gmk
index 81540266ec0..9ed45fb02a8 100644
--- a/make/RunTests.gmk
+++ b/make/RunTests.gmk
@@ -583,7 +583,7 @@ define SetupRunMicroTestBody
   $$(eval $$(call SetMicroValue,$1,MICRO_JAVA_OPTIONS))

   # Current tests needs to open java.io
-  $1_MICRO_JAVA_OPTIONS += --add-opens=java.base/java.io=ALL-UNNAMED
+  $1_MICRO_JAVA_OPTIONS += --add-opens=java.base/java.io=ALL-UNNAMED 
--enable-preview

   # Save output as JSON or CSV file
   ifneq ($$(MICRO_RESULTS_FORMAT), )


People manually running the benchmarks.jar will have to pass `--enable-preview` 
still though.

-

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v5]

2022-03-24 Thread Maurizio Cimadamore 
On Thu, 24 Mar 2022 13:00:12 GMT, Jorn Vernee  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Drop redundant javadoc statements re. handling of nulls
>>   (handling of nulls is specified once and for all in the package javadoc)
>
> src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java line 1071:
> 
>> 1069: sessionImpl.checkValidStateSlow();
>> 1070: if (offset < 0) throw new IllegalArgumentException("Requested 
>> bytes offset must be >= 0.");
>> 1071: if (size < 0) throw new IllegalArgumentException("Requested 
>> bytes size must be >= 0.");
> 
> The javadoc also says that IAE will be thrown if `offset + size < 0` I think 
> to guard against overflow, but I don't see that checked here. Is it missing?

`mapInternal` in FileChannelImpl takes care of that for both flavors of `map`

-

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v7]

2022-03-24 Thread Maurizio Cimadamore 
> This PR contains the API and implementation changes for JEP-424 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/424

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

  Address review comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7888/files
  - new: https://git.openjdk.java.net/jdk/pull/7888/files/95f65eea..3e8cfd74

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

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

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

Re: RFR: 8267319: Use larger default key sizes and algorithms based on CNSA [v9]

2022-03-24 Thread Valerie Peng 
> It's been several years since we increased the default key sizes. Before 
> shifting to PQC, NSA replaced its Suite B cryptography recommendations with 
> the Commercial National Security Algorithm Suite which suggests:
> 
> - SHA-384 for secure hashing
> - AES-256 for symmetric encryption
> - RSA with 3072 bit keys for digital signatures and for key exchange
> - Diffie Hellman (DH) with 3072 bit keys for key exchange
> - Elliptic curve [P-384] for key exchange (ECDH) and for digital signatures 
> (ECDSA)
> 
> So, this proposed changes made the suggested key size and algorithm changes. 
> The changes are mostly in keytool, jarsigner and their regression tests, so 
> @wangweij Could you please take a look?
> 
> Thanks!

Valerie Peng has updated the pull request incrementally with one additional 
commit since the last revision:

  Use uppercase for 1st letter in some comments.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7652/files
  - new: https://git.openjdk.java.net/jdk/pull/7652/files/a48ccef5..8b7bc16a

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

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

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

Re: RFR: 8267319: Use larger default key sizes and algorithms based on CNSA [v8]

2022-03-24 Thread Valerie Peng 
On Thu, 24 Mar 2022 06:41:11 GMT, Xue-Lei Andrew Fan  wrote:

>> Valerie Peng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Added comment regarding possible deadlocks.
>
> src/java.base/share/classes/sun/security/util/SecurityProviderConstants.java 
> line 137:
> 
>> 135: public static final int DEF_ED_KEY_SIZE;
>> 136: public static final int DEF_XEC_KEY_SIZE;
>> 137: // the logic for finding the max allowable value in 
>> getDefAESKeySize()
> 
> Capital the 1st letter?

Ok~

-

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

Re: jpackage Windows support for 4 digits version

2022-03-24 Thread Sverre Moe 
Hi,

This has been attempted on Java 15, Java 16 and the latest Java 17.

This is the output from Java 17

14:46:49
 
C:\cygwin64\home\build\jenkins-test\workspace\our-awesome-project_sverre_SF-99>gradlew.bat
   --no-daemon --version
14:46:51
14:46:51  
14:46:51  Gradle 7.3.3
14:46:51  
14:46:51
14:46:51  Build time:   2021-12-22 12:37:54 UTC
14:46:51  Revision: 6f556c80f945dc54b50e0be633da6c62dbe8dc71
14:46:51
14:46:51  Kotlin:   1.5.31
14:46:51  Groovy:   3.0.9
14:46:51  Ant:  Apache Ant(TM) version 1.10.11 compiled on July 10
2021
14:46:51  JVM:  17.0.2 (Eclipse Adoptium 17.0.2+8)
14:46:51  OS:   Windows 10 10.0 amd64


14:46:53
 
C:\cygwin64\home\build\jenkins-test\workspace\our-awesome-project_sverre_SF-99>gradlew.bat
   --no-daemon --stacktrace --refresh-dependencies -Pheadless createPackage
14:48:51  > Task :jpackage
14:48:51  [14:49:12.348] Running candle.exe
14:48:51  [14:49:12.386] Running C:\Program Files (x86)\WiX Toolset
v3.11\bin\candle.exe
14:48:51  [14:49:12.762] Running light.exe
14:48:51  [14:49:12.773] Running C:\Program Files (x86)\WiX Toolset
v3.11\bin\light.exe
14:48:51  [14:49:13.207] Detected [candle.exe] version [3.11.2.4516].
14:48:51  [14:49:13.208] Detected [light.exe] version [3.11.2.4516].
14:48:51  [14:49:13.210] WiX 3.11.2.4516 detected. Enabling advanced
cleanup action.
14:48:51  [14:49:13.211] jdk.jpackage.internal.ConfigException: Version
sting may have between 1 and 3 components: 1, 1.2, 1.2.3.
14:48:51   at
jdk.jpackage/jdk.jpackage.internal.WinMsiBundler.validate(WinMsiBundler.java:319)
14:48:51   at
jdk.jpackage/jdk.jpackage.internal.WinExeBundler.validate(WinExeBundler.java:93)
14:48:51   at
jdk.jpackage/jdk.jpackage.internal.Arguments.generateBundle(Arguments.java:675)
14:48:51   at
jdk.jpackage/jdk.jpackage.internal.Arguments.processArguments(Arguments.java:550)
14:48:51   at jdk.jpackage/jdk.jpackage.main.Main.execute(Main.java:91)
14:48:51   at jdk.jpackage/jdk.jpackage.main.Main.main(Main.java:52)
14:48:51  Caused by: java.lang.IllegalArgumentException: Version sting may
have between 1 and 3 components: 1, 1.2, 1.2.3.
14:48:51   at
jdk.jpackage/jdk.jpackage.internal.MsiVersion.of(MsiVersion.java:46)
14:48:51   at
jdk.jpackage/jdk.jpackage.internal.WinMsiBundler.validate(WinMsiBundler.java:317)
14:48:51   ... 5 more
14:48:51  [14:49:13.218] jdk.jpackage.internal.PackagerException: Bundler
EXE Installer Package skipped because of a configuration problem: Version
sting may have between 1 and 3 components: 1, 1.2, 1.2.3.
14:48:51  Advice to fix: Set value of --app-version parameter according to
these rules:
https://msdn.microsoft.com/en-us/library/aa370859%28v=VS.85%29.aspx
14:48:51   at
jdk.jpackage/jdk.jpackage.internal.Arguments.generateBundle(Arguments.java:688)
14:48:51   at
jdk.jpackage/jdk.jpackage.internal.Arguments.processArguments(Arguments.java:550)
14:48:51   at jdk.jpackage/jdk.jpackage.main.Main.execute(Main.java:91)
14:48:51   at jdk.jpackage/jdk.jpackage.main.Main.main(Main.java:52)
14:48:51  Caused by: jdk.jpackage.internal.ConfigException: Version sting
may have between 1 and 3 components: 1, 1.2, 1.2.3.
14:48:51   at
jdk.jpackage/jdk.jpackage.internal.WinMsiBundler.validate(WinMsiBundler.java:319)
14:48:51   at
jdk.jpackage/jdk.jpackage.internal.WinExeBundler.validate(WinExeBundler.java:93)
14:48:51   at
jdk.jpackage/jdk.jpackage.internal.Arguments.generateBundle(Arguments.java:675)
14:48:51   ... 3 more
14:48:51  Caused by: java.lang.IllegalArgumentException: Version sting may
have between 1 and 3 components: 1, 1.2, 1.2.3.
14:48:51   at
jdk.jpackage/jdk.jpackage.internal.MsiVersion.of(MsiVersion.java:46)
14:48:51   at
jdk.jpackage/jdk.jpackage.internal.WinMsiBundler.validate(WinMsiBundler.java:317)
14:48:51   ... 5 more
14:48:51
14:48:51
14:48:51  > Task :jpackage FAILED
14:48:51
14:48:51  FAILURE: Build failed with an exception.
14:48:51
14:48:51  * What went wrong:
14:48:51  Execution failed for task ':jpackage'.
14:48:51  > Process 'command 'C:\Program Files\Eclipse
Adoptium\jdk-17.0.2.8-hotspot/bin/jpackage.exe'' finished with non-zero
exit



tor. 24. mar. 2022 kl. 17:39 skrev Alexey Semenyuk <
alexey.semen...@oracle.com>:

> Hi Sverre,
>
> The output comes from quite old jpackage (jdk15, I guess). Please try
> jpackage from the newer jdk (the latest one would be the best option).
> They don't have this issue.
>
> - Alexey
>
> On 3/23/2022 10:01 AM, Sverre Moe wrote:
> > Could jpackage instruct WiX when building a native application on
> Windows,
> > to support 4 digits in the version?
> >
> > 14:41:18  Detected [light.exe] version [3.11.2.4516].
> > 14:41:18  Detected [candle.exe] version [3.11.2.4516].
> > 14:41:18  WiX 3.11.2.4516 detected. Enabling advanced cleanup action.
> > 14:41:18  Version sting may have up to 3 components - major.minor.build .
> > 14:41:18

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v5]

2022-03-24 Thread Maurizio Cimadamore 
On Thu, 24 Mar 2022 13:10:20 GMT, Jorn Vernee  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Drop redundant javadoc statements re. handling of nulls
>>   (handling of nulls is specified once and for all in the package javadoc)
>
> make/test/BuildMicrobenchmark.gmk line 97:
> 
>> 95: SRC := $(MICROBENCHMARK_SRC), \
>> 96: BIN := $(MICROBENCHMARK_CLASSES), \
>> 97: JAVAC_FLAGS := --add-exports java.base/sun.security.util=ALL-UNNAMED 
>> --enable-preview, \
> 
> It still seems like this would lead to potential issues. i.e. requiring all 
> benchmarks to be run with `--enable-preview`? We ended up adding 
> `--enable-preview` to our benchmarks, but do other benchmarks still work 
> without it? AFAIK the entire benchmarks.jar will have the altered class file 
> version.

Sure, this is problematic - but at the same time I don't think there's a better 
way to deal with this? I'd prefer to defer this to a separate issue (and I 
think the build team is in a much better position to suggest a better fix). 
IIRC we had this problem in the past as well.

-

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v6]

2022-03-24 Thread Maurizio Cimadamore 
> This PR contains the API and implementation changes for JEP-424 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/424

Maurizio Cimadamore has updated the pull request incrementally with three 
additional commits since the last revision:

 - Update src/java.base/share/classes/java/lang/foreign/MemorySegment.java
   
   Co-authored-by: Jorn Vernee 
 - Update src/java.base/share/classes/java/lang/foreign/MemorySegment.java
   
   Co-authored-by: Jorn Vernee 
 - Update src/java.base/share/classes/java/lang/foreign/MemorySegment.java
   
   Co-authored-by: Jorn Vernee 

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7888/files
  - new: https://git.openjdk.java.net/jdk/pull/7888/files/c9bc9a70..95f65eea

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

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

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

Re: RFR: 8282429: StringBuilder/StringBuffer.toString() skip compressing for UTF16 strings [v4]

2022-03-24 Thread Xin Liu 
On Tue, 22 Mar 2022 10:09:40 GMT, Claes Redestad  wrote:

>> Xin Liu 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 six additional commits 
>> since the last revision:
>> 
>>  - Merge branch 'master' into JDK-8282429
>>  - split StringBuilder.toString() performance test out of StringBuilders.java
>>  - Change growOnly to maybeLatin.
>>
>>This patch also copys over the attribute from the other 
>> AbstractStringBuilder.
>>Add a unit test to cover methods which cause maybeLatin1 becomes true.
>>  - make sure String(StringBuffer) is still synchronized.
>>  - Add a microbenchmark.
>>  - 8282429:  StringBuilder/StringBuffer.toString() skip compressing for 
>> UTF16 strings
>
> Thanks for refactoring the micro to avoid the redundant run overheads, and 
> checking that the we're size neutral on all configurations. 
> 
> I have added a few minor comments inline that you can choose to address or 
> ignore. 
> 
> Good work!

hi, @cl4es  @djelinski , 
Could you take another look at the latest revision?  

thanks, 
--lx

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v7]

2022-03-24 Thread XenoAmess 
> 8186958: Need method to create pre-sized HashMap

XenoAmess has updated the pull request incrementally with two additional 
commits since the last revision:

 - update jmh
 - refine javadoc; refine implement when expectedSize < 0

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7928/files
  - new: https://git.openjdk.java.net/jdk/pull/7928/files/5b7eec6c..293b949c

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

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

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v5]

2022-03-24 Thread XenoAmess 
On Thu, 24 Mar 2022 17:09:21 GMT, Roger Riggs  wrote:

> I'm not too picky about how it is documented, as long as it is clear how the 
> number 1610612733 is computed and why its the correct number. And I think 
> that's working backward from Integer.MAX_VALUE. As for its precision, its 
> almost irrelevant since the actual size must be a power of two; and precision 
> is only important near the boundary of a power of two. A simple and obvious 
> explanation is better than one that is overly precise (when it matters).

@RogerRiggs
Got it. I''ve changed it and added some docs about the calculation, please have 
a look again. thanks.

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v5]

2022-03-24 Thread Roger Riggs 
On Thu, 24 Mar 2022 16:46:11 GMT, XenoAmess  wrote:

>>> It would be clearer with a comment on the constant or use the expression 
>>> (the compiler will evaluate it). `Integer.MAX_VALUE / 4 * 3`.
>> 
>> @RogerRiggs 
>> 
>> No, as `Integer.MAX_VALUE / 4 * 3` is actually 1610612733,
>> 
>> and `(int)Math.ceil(1610612733/0.75)` is actually 2147483644, 
>> 
>> while `Integer.MAX_VALUE` is 2147483647
>> 
>> Although in this situation 2147483644 and 2147483647 is same (both would be 
>> trimed down to 1073741824 when enter the HashMap constructor), but I think 
>> it be better to be more precise.
>> 
>> However if you want me to change it to `expectedSize >= Integer.MAX_VALUE / 
>> 4 * 3 + 3`, then I think it acceptable, but looks a little weird.
>> 
>> So should we change it to `expectedSize >= Integer.MAX_VALUE / 4 * 3 + 3`?
>
>> There also might be a valid point to make about a fail-fast exception if the 
>> requested capacity cannot be achieved. If defaulted to MAX_VALUE, the 
>> application would start populating the map and then at some later point get 
>> an exception when it cannot be resized to accommodate the n+1'th item.
> 
> @RogerRiggs 
> 
> No, as I want 100% same reaction to use these functions like using new 
> HashMap(int)
> 
> So as the HashMap constructor itself doesn't throw exception when oversize, 
> this function shall neither.

I'm not too picky about how it is documented, as long as it is clear how the 
number 1610612733 is computed and why its the correct number. And I think 
that's working backward from Integer.MAX_VALUE.
As for its precision, its almost irrelevant since the actual size must be a 
power of two; and precision is only important near the boundary of a power of 
two. A simple and obvious explanation is better than one that is overly precise 
(when it matters).

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v5]

2022-03-24 Thread XenoAmess 
On Thu, 24 Mar 2022 16:44:50 GMT, XenoAmess  wrote:

>> src/java.base/share/classes/java/util/HashMap.java line 2556:
>> 
>>> 2554:  */
>>> 2555: static int calculateHashMapCapacity(int expectedSize) {
>>> 2556: if (expectedSize >= 1610612736) {
>> 
>> It would be clearer with a comment on the constant or use the expression 
>> (the compiler will evaluate it).
>> `Integer.MAX_VALUE / 4 * 3`.
>> 
>> There also might be a valid point to make about a fail-fast exception if the 
>> requested capacity cannot be achieved.   If defaulted to MAX_VALUE, the 
>> application would start populating the map and then at some later point get 
>> an exception when it cannot be resized to accommodate the n+1'th item.
>
>> It would be clearer with a comment on the constant or use the expression 
>> (the compiler will evaluate it). `Integer.MAX_VALUE / 4 * 3`.
> 
> @RogerRiggs 
> 
> No, as `Integer.MAX_VALUE / 4 * 3` is actually 1610612733,
> 
> and `(int)Math.ceil(1610612733/0.75)` is actually 2147483644, 
> 
> while `Integer.MAX_VALUE` is 2147483647
> 
> Although in this situation 2147483644 and 2147483647 is same (both would be 
> trimed down to 1073741824 when enter the HashMap constructor), but I think it 
> be better to be more precise.
> 
> However if you want me to change it to `expectedSize >= Integer.MAX_VALUE / 4 
> * 3 + 3`, then I think it acceptable, but looks a little weird.
> 
> So should we change it to `expectedSize >= Integer.MAX_VALUE / 4 * 3 + 3`?

> There also might be a valid point to make about a fail-fast exception if the 
> requested capacity cannot be achieved. If defaulted to MAX_VALUE, the 
> application would start populating the map and then at some later point get 
> an exception when it cannot be resized to accommodate the n+1'th item.

@RogerRiggs 

No, as I want 100% same reaction to use these functions like using new 
HashMap(int)

So as the HashMap constructor itself doesn't throw exception when oversize, 
this function shall neither.

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v5]

2022-03-24 Thread XenoAmess 
On Thu, 24 Mar 2022 14:54:39 GMT, Roger Riggs  wrote:

> It would be clearer with a comment on the constant or use the expression (the 
> compiler will evaluate it). `Integer.MAX_VALUE / 4 * 3`.

@RogerRiggs 

No, as `Integer.MAX_VALUE / 4 * 3` is actually 1610612733,

and `(int)Math.ceil(1610612733/0.75)` is actually 2147483644, 

while `Integer.MAX_VALUE` is 2147483647

Although in this situation 2147483644 and 2147483647 is same (both would be 
trimed down to 1073741824 when enter the HashMap constructor), but I think it 
be better to be more precise.

However if you want me to change it to `expectedSize >= Integer.MAX_VALUE / 4 * 
3 + 3`, then I think it acceptable, but looks a little weird.

So should we change it to `expectedSize >= Integer.MAX_VALUE / 4 * 3 + 3`?

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v5]

2022-03-24 Thread XenoAmess 
On Thu, 24 Mar 2022 14:36:36 GMT, Roger Riggs  wrote:

> This method should say it is using a default load factor (0.75).

@RogerRiggs done.

> The default load factor should be mentioned.

@RogerRiggs done.

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v6]

2022-03-24 Thread XenoAmess 
> 8186958: Need method to create pre-sized HashMap

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

  refine javadoc about default load factor

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7928/files
  - new: https://git.openjdk.java.net/jdk/pull/7928/files/8f4ecf63..5b7eec6c

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

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

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

Re: jpackage Windows support for 4 digits version

2022-03-24 Thread Alexey Semenyuk 

Hi Sverre,

The output comes from quite old jpackage (jdk15, I guess). Please try 
jpackage from the newer jdk (the latest one would be the best option). 
They don't have this issue.


- Alexey

On 3/23/2022 10:01 AM, Sverre Moe wrote:

Could jpackage instruct WiX when building a native application on Windows,
to support 4 digits in the version?

14:41:18  Detected [light.exe] version [3.11.2.4516].
14:41:18  Detected [candle.exe] version [3.11.2.4516].
14:41:18  WiX 3.11.2.4516 detected. Enabling advanced cleanup action.
14:41:18  Version sting may have up to 3 components - major.minor.build .
14:41:18  jdk.incubator.jpackage.internal.ConfigException: Version string
is not compatible with MSI rules [1.10.3.1]
14:41:18   at
jdk.incubator.jpackage/jdk.incubator.jpackage.internal.WinMsiBundler.validate(WinMsiBundler.java:256)
14:41:18   at
jdk.incubator.jpackage/jdk.incubator.jpackage.internal.WinExeBundler.validate(WinExeBundler.java:99)
14:41:18   at
jdk.incubator.jpackage/jdk.incubator.jpackage.internal.Arguments.generateBundle(Arguments.java:640)
14:41:18   at
jdk.incubator.jpackage/jdk.incubator.jpackage.internal.Arguments.processArguments(Arguments.java:514)
14:41:18   at
jdk.incubator.jpackage/jdk.incubator.jpackage.main.Main.execute(Main.java:97)
14:41:18   at
jdk.incubator.jpackage/jdk.incubator.jpackage.main.Main.main(Main.java:51)
14:41:18  jdk.incubator.jpackage.internal.PackagerException: Bundler EXE
Installer Package skipped because of a configuration problem: Version
string is not compatible with MSI rules [1.10.3.1]
14:41:18  Advice to fix: Set the bundler argument "win.msi.productVersion"
according to these rules:
https://msdn.microsoft.com/en-us/library/aa370859%28v=VS.85%29.aspx .
14:41:18   at
jdk.incubator.jpackage/jdk.incubator.jpackage.internal.Arguments.generateBundle(Arguments.java:653)
14:41:18   at
jdk.incubator.jpackage/jdk.incubator.jpackage.internal.Arguments.processArguments(Arguments.java:514)
14:41:18   at
jdk.incubator.jpackage/jdk.incubator.jpackage.main.Main.execute(Main.java:97)
14:41:18   at
jdk.incubator.jpackage/jdk.incubator.jpackage.main.Main.main(Main.java:51)
14:41:18  Caused by: jdk.incubator.jpackage.internal.ConfigException:
Version string is not compatible with MSI rules [1.10.3.1]
14:41:18   at
jdk.incubator.jpackage/jdk.incubator.jpackage.internal.WinMsiBundler.validate(WinMsiBundler.java:256)
14:41:18   at
jdk.incubator.jpackage/jdk.incubator.jpackage.internal.WinExeBundler.validate(WinExeBundler.java:99)
14:41:18   at
jdk.incubator.jpackage/jdk.incubator.jpackage.internal.Arguments.generateBundle(Arguments.java:640)
14:41:18   ... 3 more

Perhaps something like this solution could be incorporated with the
JPackage tool.
https://github.com/hvandrie/msiCAFindRelatedProductsEx
jpackage --windows-version-components=4

Re: RFR: 8279508: Auto-vectorize Math.round API [v18]

2022-03-24 Thread Jatin Bhateja 
On Wed, 23 Mar 2022 06:55:50 GMT, Tobias Hartmann  wrote:

>> Jatin Bhateja has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 22 commits:
>> 
>>  - 8279508: Using an explicit scratch register since rscratch1 is bound to 
>> r10 and its usage is transparent to compiler.
>>  - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8279508
>>  - 8279508: Windows build failure fix.
>>  - 8279508: Styling comments resolved.
>>  - 8279508: Creating separate test for round double under feature check.
>>  - 8279508: Reducing the invocation count and compile thresholds for 
>> RoundTests.java.
>>  - 8279508: Review comments resolution.
>>  - 8279508: Preventing domain switch-over penalty for Math.round(float) and 
>> constraining unrolling to prevent code bloating.
>>  - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8279508
>>  - 8279508: Removing +LogCompilation flag.
>>  - ... and 12 more: 
>> https://git.openjdk.java.net/jdk/compare/ff0b0927...c17440cf
>
> All tests passed.

Hi @TobiHartmann , thanks for confirming.
Hi @jddarcy , @theRealAph , kindly let me know if its good to integrate this.

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v5]

2022-03-24 Thread Roger Riggs 
On Thu, 24 Mar 2022 12:10:28 GMT, XenoAmess  wrote:

>> 8186958: Need method to create pre-sized HashMap
>
> XenoAmess has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   delete a space.

src/java.base/share/classes/java/util/HashMap.java line 2549:

> 2547: 
> 2548: /**
> 2549:  * Calculate initial capacity for HashMap based classes, from 
> expected size.

This method should say it is using a default load factor  (0.75).
I'm not sure how often anyone creates a map with a non-default load factor
but that might suggest a more general API pattern either in the supporting 
calculation
or the public factories.

src/java.base/share/classes/java/util/HashMap.java line 2556:

> 2554:  */
> 2555: static int calculateHashMapCapacity(int expectedSize) {
> 2556: if (expectedSize >= 1610612736) {

It would be clearer with a comment on the constant or use the expression (the 
compiler will evaluate it).
`Integer.MAX_VALUE / 4 * 3`.

There also might be a valid point to make about a fail-fast exception if the 
requested capacity cannot be achieved.   If defaulted to MAX_VALUE, the 
application would start populating the map and then at some later point get an 
exception when it cannot be resized to accommodate the n+1'th item.

src/java.base/share/classes/java/util/HashMap.java line 2565:

> 2563:  * Creates a new, empty HashMap with an initial table size
> 2564:  * accommodating the specified number of elements without the need
> 2565:  * to dynamically resize.

The default load factor should be mentioned.

-

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

Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v5]

2022-03-24 Thread Jorn Vernee 
On Wed, 23 Mar 2022 14:06:56 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-424 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/424
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Drop redundant javadoc statements re. handling of nulls
>   (handling of nulls is specified once and for all in the package javadoc)

Some more nits.

One potential issue with adding --enable-preview when building benchmarks (last 
comment of the bunch). 

Other than that, I think this looks good.

make/test/BuildMicrobenchmark.gmk line 97:

> 95: SRC := $(MICROBENCHMARK_SRC), \
> 96: BIN := $(MICROBENCHMARK_CLASSES), \
> 97: JAVAC_FLAGS := --add-exports java.base/sun.security.util=ALL-UNNAMED 
> --enable-preview, \

It still seems like this would lead to potential issues. i.e. requiring all 
benchmarks to be run with `--enable-preview`? We ended up adding 
`--enable-preview` to our benchmarks, but do other benchmarks still work 
without it? AFAIK the entire benchmarks.jar will have the altered class file 
version.

src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 61:

> 59:  * {@linkplain MemorySegment#allocateNative(long, long, 
> MemorySession) native memory segments}, backed by off-heap memory;
> 60:  * {@linkplain FileChannel#map(FileChannel.MapMode, long, long, 
> MemorySession) mapped memory segments}, obtained by mapping
> 61:  * a file into main memory ({@code mmap}); tha contents of a mapped 
> memory segments can be {@linkplain #force() persisted} and

Suggestion:

 * a file into main memory ({@code mmap}); the contents of a mapped memory 
segments can be {@linkplain #force() persisted} and

src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 298:

> 296: 
> 297: /**
> 298:  * Returns a slice of this memory segment, at given offset. The 
> returned segment's base address is the base address

Saw a similar change in other places, so I'll suggest this here as well.
Suggestion:

 * Returns a slice of this memory segment, at the given offset. The 
returned segment's base address is the base address

src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 311:

> 309: 
> 310: /**
> 311:  * Returns a slice of this memory segment, at given offset. The 
> returned segment's base address is the base address

Suggestion:

 * Returns a slice of this memory segment, at the given offset. The 
returned segment's base address is the base address

src/java.base/share/classes/java/lang/foreign/MemorySession.java line 143:

> 141: 
> 142: /**
> 143:  * {@return the owner thread associated with this memory session (if 
> any)}

Maybe the "if any" here could be more specific. e.g. saying that `null` is 
returned if the session doesn't have an owner thread.

src/java.base/share/classes/java/lang/foreign/MemorySession.java line 165:

> 163: 
> 164: /**
> 165:  * Closes this memory session. As a side effect, if this operation 
> completes without exceptions, this session

I'd suggest change this to "As a result of this", since the effects listed are 
the main reason for closing a session. (it strikes me as strange. If the things 
listed are side-effects, then what is the main effect of closing a segment?)
Suggestion:

 * Closes this memory session. As a result of this, if this operation 
completes without exceptions, this session

src/java.base/share/classes/java/lang/foreign/SymbolLookup.java line 51:

> 49:  * 
> 50:  * Clients can obtain a {@linkplain #loaderLookup() loader lookup},
> 51:  * which can be used to search symbols in libraries loaded by the current 
> classloader (e.g. using {@link System#load(String)},

"search symbols" sounds a bit unnatural to me... I like the wording in the 
libraryLookup doc more
Suggestion:

 * which can be used to find symbols in libraries loaded by the current 
classloader (e.g. using {@link System#load(String)},

src/java.base/share/classes/java/lang/foreign/SymbolLookup.java line 59:

> 57:  * 
> 58:  * Finally, clients can load a library and obtain a {@linkplain 
> #libraryLookup(Path, MemorySession) library lookup} which can be used
> 59:  * to search symbols in that library. A library lookup is associated with 
> a {@linkplain  MemorySession memory session},

Suggestion:

 * to find symbols in that library. A library lookup is associated with a 
{@linkplain  MemorySession memory session},

src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 7895:

> 7893:  * VarHandle handle = 
> MethodHandles.memorySegmentViewVarHandle(ValueLayout.JAVA_INT.withOrder(ByteOrder.BIG_ENDIAN));
>  //(MemorySegment, long) -> int
> 7894:  * handle = MethodHandles.insertCoordinates(handle, 1, 4); 
>

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

2022-03-24 Thread liach 
On Wed, 23 Feb 2022 22:33:42 GMT, liach  wrote:

> Replaces simple `for (Map.Entry entry : map.entrySet())` with 
> `map.forEach((k, v) ->)` calls. This change is better for thread-safety and 
> reduces allocation for some map implementations.
> 
> A more in-depth description of benefits is available at 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2022-February/086201.html
>  and at the JBS issue itself.
> 
> A [jmh 
> comparison](https://jmh.morethan.io/?sources=https://gist.githubusercontent.com/liach/0c0f79f0c0b9b78f474d65cda2c5f7b5/raw/4f2a160c51164aefdfac6ab5a19bdbc8c65f5fcf/base-results.json,https://gist.githubusercontent.com/liach/0c0f79f0c0b9b78f474d65cda2c5f7b5/raw/4f2a160c51164aefdfac6ab5a19bdbc8c65f5fcf/head-results.json)
>  on the performance of the existing `HashMapBench` shows that the performance 
> of `putAll` for `HashMap` has not significantly changed.

In a short summary, on mailing list, stuart says this doesn't benefit 
concurrent maps; Remi proposed to deprecate the synchronized collections for 
the peculiarities of iteration; kevin said that this change improves code 
clarity.

Hence, I still believe changing to foreach calls in putAll can potentially 
benefit adding from maps that have expensive iterators and entry wrappers.

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v5]

2022-03-24 Thread XenoAmess 
> 8186958: Need method to create pre-sized HashMap

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

  delete a space.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7928/files
  - new: https://git.openjdk.java.net/jdk/pull/7928/files/435793c9..8f4ecf63

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

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

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v4]

2022-03-24 Thread XenoAmess 
> 8186958: Need method to create pre-sized HashMap

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

  use jmh Blackhole

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7928/files
  - new: https://git.openjdk.java.net/jdk/pull/7928/files/45f4aae9..435793c9

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

  Stats: 13 lines in 1 file changed: 1 ins; 4 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7928.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7928/head:pull/7928

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v4]

2022-03-24 Thread XenoAmess 
On Wed, 23 Mar 2022 21:04:52 GMT, liach  wrote:

>> XenoAmess has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   use jmh Blackhole
>
> test/jdk/java/util/Collections/CalculateHashMapCapacityTestJMH.java line 42:
> 
>> 40: public class CalculateHashMapCapacityTestJMH {
>> 41: 
>> 42: private void consume(int tmp) {
> 
> You should pass a jmh Blackhole object in the parameter of test methods and 
> call its consume method. jmh will automatically pass the blackhole. This one 
> will be eliminated as dead code.

@liach done.

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v3]

2022-03-24 Thread XenoAmess 
> 8186958: Need method to create pre-sized HashMap

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

  move the static functions to map classes themselves.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7928/files
  - new: https://git.openjdk.java.net/jdk/pull/7928/files/78419b28..45f4aae9

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

  Stats: 125 lines in 4 files changed: 61 ins; 64 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7928.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7928/head:pull/7928

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v3]

2022-03-24 Thread XenoAmess 
On Thu, 24 Mar 2022 00:44:02 GMT, liach  wrote:

> Note that the methods may have to go to somewhere else in `java.util` if 
> `Collections` cannot be loaded too early during startup, as HashMap is loaded 
> very early

I'm afraid you are correct... moved the calculation function into HashMap seems 
better.

@liach done.

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v2]

2022-03-24 Thread XenoAmess 
On Thu, 24 Mar 2022 11:15:12 GMT, David Holmes  wrote:

>> XenoAmess has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   fix javadoc's @return
>
> src/java.base/share/classes/java/util/Collections.java line 5836:
> 
>> 5834:  * @param   the type of keys maintained by this map
>> 5835:  * @param   the type of mapped values
>> 5836:  * @return initial capacity for HashMap based classes.
> 
> Again this method returns a Map

@dholmes-ora done.

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v2]

2022-03-24 Thread XenoAmess 
On Thu, 24 Mar 2022 11:14:54 GMT, David Holmes  wrote:

>>> That isn't what is returned.
>> 
>> @dholmes-ora Yes it isn't actually. but I cannot find a better description 
>> string for it.
>> 
>> "the pre-processed raw initial capacity for HashMap based classes." is more 
>> exact but sounds weird.
>> 
>> Have you any suggestions to replace this? Thanks!
>
> This method doesn't return the capacity it returns the Map!

@dholmes-ora done.

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap [v2]

2022-03-24 Thread XenoAmess 
> 8186958: Need method to create pre-sized HashMap

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

  fix javadoc's @return

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7928/files
  - new: https://git.openjdk.java.net/jdk/pull/7928/files/2c02ea8f..78419b28

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

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

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

Re: RFR: 8186958: Need method to create pre-sized HashMap

2022-03-24 Thread XenoAmess 
On Thu, 24 Mar 2022 11:14:54 GMT, David Holmes  wrote:

>>> That isn't what is returned.
>> 
>> @dholmes-ora Yes it isn't actually. but I cannot find a better description 
>> string for it.
>> 
>> "the pre-processed raw initial capacity for HashMap based classes." is more 
>> exact but sounds weird.
>> 
>> Have you any suggestions to replace this? Thanks!
>
> This method doesn't return the capacity it returns the Map!

my mistake. will fix it in hours.

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap

2022-03-24 Thread David Holmes 
On Wed, 23 Mar 2022 18:41:59 GMT, XenoAmess  wrote:

> 8186958: Need method to create pre-sized HashMap

src/java.base/share/classes/java/util/Collections.java line 5836:

> 5834:  * @param   the type of keys maintained by this map
> 5835:  * @param   the type of mapped values
> 5836:  * @return initial capacity for HashMap based classes.

Again this method returns a Map

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap

2022-03-24 Thread David Holmes 
On Thu, 24 Mar 2022 07:01:43 GMT, XenoAmess  wrote:

>> src/java.base/share/classes/java/util/Collections.java line 5822:
>> 
>>> 5820:  * @param   the type of keys maintained by this map
>>> 5821:  * @param   the type of mapped values
>>> 5822:  * @return initial capacity for HashMap based classes.
>> 
>> That isn't what is returned.
>
>> That isn't what is returned.
> 
> @dholmes-ora Yes it isn't actually. but I cannot find a better description 
> string for it.
> 
> "the pre-processed raw initial capacity for HashMap based classes." is more 
> exact but sounds weird.
> 
> Have you any suggestions to replace this? Thanks!

This method doesn't return the capacity it returns the Map!

-

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

Integrated: 8276799: Implementation of JEP 422: Linux/RISC-V Port

2022-03-24 Thread Fei Yang 
On Mon, 8 Nov 2021 11:17:47 GMT, Fei Yang  wrote:

> This PR implements JEP 422: Linux/RISC-V Port [1].
> The PR starts as a squashed merge of the 
> https://openjdk.java.net/projects/riscv-port branch.
> 
> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive 
> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are also 
> carried out regularly. So it should be good enough to run most Java programs.
> 
> [1] https://openjdk.java.net/jeps/422

This pull request has now been integrated.

Changeset: 5905b02c
Author:Fei Yang 
URL:   
https://git.openjdk.java.net/jdk/commit/5905b02c0e2643ae8d097562f181953f6c88fc89
Stats: 59141 lines in 188 files changed: 58964 ins; 54 del; 123 mod

8276799: Implementation of JEP 422: Linux/RISC-V Port

Co-authored-by: Yadong Wang 
Co-authored-by: Yanhong Zhu 
Co-authored-by: Feilong Jiang 
Co-authored-by: Kun Wang 
Co-authored-by: Zhuxuan Ni 
Co-authored-by: Taiping Guo 
Co-authored-by: Kang He 
Co-authored-by: Aleksey Shipilev 
Co-authored-by: Xiaolin Zheng 
Co-authored-by: Kuai Wei 
Co-authored-by: Magnus Ihse Bursie 
Reviewed-by: ihse, dholmes, rriggs, kvn, shade

-

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

Re: RFR: 8186958: Need method to create pre-sized HashMap

2022-03-24 Thread XenoAmess 
On Thu, 24 Mar 2022 04:52:16 GMT, David Holmes  wrote:

> That isn't what is returned.

@dholmes-ora Yes it isn't actually. but I cannot find a better description 
string for it.

"the pre-processed raw initial capacity for HashMap based classes." is more 
exact but sounds weird.

Have you any suggestions to replace this? Thanks!

-

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

Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v4]

2022-03-24 Thread Fei Yang 
On Wed, 23 Mar 2022 02:17:22 GMT, Vladimir Kozlov  wrote:

>> Fei Yang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Fix copyright header
>
> Update looks good.
> Testing results are also good.

@vnkozlov @shipilev : Thanks for reviewing this :-)

-

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

Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v5]

2022-03-24 Thread Fei Yang 
> This PR implements JEP 422: Linux/RISC-V Port [1].
> The PR starts as a squashed merge of the 
> https://openjdk.java.net/projects/riscv-port branch.
> 
> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive 
> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are also 
> carried out regularly. So it should be good enough to run most Java programs.
> 
> [1] https://openjdk.java.net/jeps/422

Fei Yang 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 five additional commits since the 
last revision:

 - Merge remote-tracking branch 'upstream/master' into JDK-8276799
 - Fix copyright header
 - Address review comments
 - Merge remote-tracking branch 'upstream/master' into JDK-8276799
 - 8276799: Implementation of JEP 422: Linux/RISC-V Port

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6294/files
  - new: https://git.openjdk.java.net/jdk/pull/6294/files/d8bef7fa..90db70eb

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

  Stats: 3082 lines in 147 files changed: 1635 ins; 374 del; 1073 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6294.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6294/head:pull/6294

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

Re: RFR: 8267319: Use larger default key sizes and algorithms based on CNSA [v7]

2022-03-24 Thread Xue-Lei Andrew Fan 
On Wed, 23 Mar 2022 21:59:02 GMT, Valerie Peng  wrote:

>> I see.
>> 
>> Would you mind add a comment about the provider loading impact, just in case 
>> someone else have similar questions in the future?
>
> Sure, I can do that. Will add a comment about this.

Thank you.  I have no more comment except a minor nit.

-

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

Re: RFR: 8267319: Use larger default key sizes and algorithms based on CNSA [v8]

2022-03-24 Thread Xue-Lei Andrew Fan 
On Wed, 23 Mar 2022 22:48:41 GMT, Valerie Peng  wrote:

>> It's been several years since we increased the default key sizes. Before 
>> shifting to PQC, NSA replaced its Suite B cryptography recommendations with 
>> the Commercial National Security Algorithm Suite which suggests:
>> 
>> - SHA-384 for secure hashing
>> - AES-256 for symmetric encryption
>> - RSA with 3072 bit keys for digital signatures and for key exchange
>> - Diffie Hellman (DH) with 3072 bit keys for key exchange
>> - Elliptic curve [P-384] for key exchange (ECDH) and for digital signatures 
>> (ECDSA)
>> 
>> So, this proposed changes made the suggested key size and algorithm changes. 
>> The changes are mostly in keytool, jarsigner and their regression tests, so 
>> @wangweij Could you please take a look?
>> 
>> Thanks!
>
> Valerie Peng has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Added comment regarding possible deadlocks.

Looks good to me.  Thanks!

src/java.base/share/classes/sun/security/util/SecurityProviderConstants.java 
line 137:

> 135: public static final int DEF_ED_KEY_SIZE;
> 136: public static final int DEF_XEC_KEY_SIZE;
> 137: // the logic for finding the max allowable value in 
> getDefAESKeySize()

Capital the 1st letter?

-

Marked as reviewed by xuelei (Reviewer).

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