Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Stuart Marks
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

Reviewed java.io, java.lang, java.lang.ref.

-

Marked as reviewed by smarks (Reviewer).

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


Re: RFR: 8285745: Re-examine PushbackInputStream mark/reset

2022-04-27 Thread Alan Bateman
On Wed, 27 Apr 2022 20:10:03 GMT, Brian Burkhalter  wrote:

> Please review this request to remove the `synchronized` keyword from the 
> `mark(int)` and `reset()` methods of `java.io.PushbackInputStream`.

Marked as reviewed by alanb (Reviewer).

-

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


Re: RFR: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java

2022-04-27 Thread Joe Wang
On Wed, 20 Apr 2022 15:37:13 GMT, Shruthi  wrote:

> Removing the Duplicate keys present in XSLTErrorResources.java and 
> XPATHErrorResources.java
> 
> The bug report for the same: https://bugs.openjdk.java.net/browse/JDK-8285097

src/java.xml/share/classes/com/sun/org/apache/xpath/internal/res/XPATHErrorResources.java
 line 598:

> 596:"rtf() not supported by XRTreeFragSelectWrapper"},
> 597: 
> 598:   { ER_ASNODEITERATOR_NOT_SUPPORTED_XRTREEFRAGSELECTWRAPPER,

Please replace the usage in XRTreeFragSelectWrapper, and also update the 
LastModified tag.

-

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


Re: RFR: 8285745: Re-examine PushbackInputStream mark/reset

2022-04-27 Thread Jaikiran Pai
On Wed, 27 Apr 2022 20:10:03 GMT, Brian Burkhalter  wrote:

> Please review this request to remove the `synchronized` keyword from the 
> `mark(int)` and `reset()` methods of `java.io.PushbackInputStream`.

Looks fine to me.
Given that `mark()` currently is a no-op and `reset()` throws an exception, the 
`synchronized` wouldn't be doing anything when it comes to calls on an exact 
instance of `PushbackInputStream`.
Calls on subclasses of `PushbackInputStream` which did support mark/reset, 
would be overriding this method already and handling any synchronization 
themselves.

-

Marked as reviewed by jpai (Committer).

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Serguei Spitsyn
On Wed, 27 Apr 2022 19:00:52 GMT, Coleen Phillimore  wrote:

>> Alan Bateman has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 2388:
> 
>> 2386: }
>> 2387: 
>> 2388: void
> 
> @sspitsyn We should clean up this aberrant style of putting the return type 
> on the line before the rest of the function declaration after this 
> integration.  It looks so strange.  I noticed that it's pre-existing for 
> other functions in this file.

I'm okay to follow any style. We can do it after the integration.

-

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


Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]

2022-04-27 Thread Joe Darcy
On Wed, 27 Apr 2022 23:24:57 GMT, Stuart Marks  wrote:

>> I said "keys maintained", omitting "by this map" to finesse the question of 
>> if the SimpleEntry class *is* a map, or is used to implement a map, etc. I 
>> can change it to include "by this map" if the map/entry distinction is okay 
>> to be blurred.
>
> Whoops, sorry, this is `SimpleEntry`, which is _not_ a `Map`. In this case 
> I'd follow `Map.Entry` which says "the type of the key" and "the type of the 
> map".

Will change to the Map.Entry wording "the type of key" and "the type of the 
value", respectively.

-

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


Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]

2022-04-27 Thread Joe Darcy
> To enable more complete doclint checking (courtesy @jonathan-gibbons), please 
> review this PR to add type-level @param tags where they are missing.
> 
> To the maintainers of java.util.concurrent, those changes could be separated 
> out in another bug if that would ease maintenance of that code.
> 
> Making these library fixes is a blocker for correcting and expanding the 
> doclint checks (JDK-8285496).
> 
> I'll update copyright years before pushing.

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

  Respond to more review feedback.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8410/files
  - new: https://git.openjdk.java.net/jdk/pull/8410/files/e0ac5dcb..db4919a9

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

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

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


Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]

2022-04-27 Thread Phil Race
On Thu, 28 Apr 2022 01:31:13 GMT, Joe Darcy  wrote:

>> To enable more complete doclint checking (courtesy @jonathan-gibbons), 
>> please review this PR to add type-level @param tags where they are missing.
>> 
>> To the maintainers of java.util.concurrent, those changes could be separated 
>> out in another bug if that would ease maintenance of that code.
>> 
>> Making these library fixes is a blocker for correcting and expanding the 
>> doclint checks (JDK-8285496).
>> 
>> I'll update copyright years before pushing.
>
> Joe Darcy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Respond to more review feedback.

Marked as reviewed by prr (Reviewer).

-

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


Re: RFR: 8284992: Fix misleading Vector API doc for LSHR operator [v3]

2022-04-27 Thread Jie Fu
On Thu, 28 Apr 2022 00:08:41 GMT, Paul Sandoz  wrote:

> I created one, filled it in, and assigned it to you (for other examples you 
> can search in the issue tracker, this one quite is simple so i thought it was 
> quicker to do myself to show you). For any specification change we need to 
> review and track that change (independent of any implementation changes, if 
> any).
> 
> If you are ok with it I can add myself as reviewer, then you can "Finalize" 
> it (see button on same line as "Edit"), triggering a review request, from 
> which we may receive comments to address, and once addressed and final, it 
> will unblock the PR for integration.

Thanks @PaulSandoz for your help.

Yes, I think it's good enough.

I made a small change which just adding a `(`.
https://user-images.githubusercontent.com/19923746/165657177-f44f7f7d-44da-4921-a98c-5f6b9a3d9e36.png;>

Thanks.

-

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


Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v2]

2022-04-27 Thread Sandhya Viswanathan
On Fri, 22 Apr 2022 07:08:24 GMT, Xiaohong Gong  wrote:

>> Currently the vector load with mask when the given index happens out of the 
>> array boundary is implemented with pure java scalar code to avoid the IOOBE 
>> (IndexOutOfBoundaryException). This is necessary for architectures that do 
>> not support the predicate feature. Because the masked load is implemented 
>> with a full vector load and a vector blend applied on it. And a full vector 
>> load will definitely cause the IOOBE which is not valid. However, for 
>> architectures that support the predicate feature like SVE/AVX-512/RVV, it 
>> can be vectorized with the predicated load instruction as long as the 
>> indexes of the masked lanes are within the bounds of the array. For these 
>> architectures, loading with unmasked lanes does not raise exception.
>> 
>> This patch adds the vectorization support for the masked load with IOOBE 
>> part. Please see the original java implementation (FIXME: optimize):
>> 
>> 
>>   @ForceInline
>>   public static
>>   ByteVector fromArray(VectorSpecies species,
>>byte[] a, int offset,
>>VectorMask m) {
>>   ByteSpecies vsp = (ByteSpecies) species;
>>   if (offset >= 0 && offset <= (a.length - species.length())) {
>>   return vsp.dummyVector().fromArray0(a, offset, m);
>>   }
>> 
>>   // FIXME: optimize
>>   checkMaskFromIndexSize(offset, vsp, m, 1, a.length);
>>   return vsp.vOp(m, i -> a[offset + i]);
>>   }
>> 
>> Since it can only be vectorized with the predicate load, the hotspot must 
>> check whether the current backend supports it and falls back to the java 
>> scalar version if not. This is different from the normal masked vector load 
>> that the compiler will generate a full vector load and a vector blend if the 
>> predicate load is not supported. So to let the compiler make the expected 
>> action, an additional flag (i.e. `usePred`) is added to the existing 
>> "loadMasked" intrinsic, with the value "true" for the IOOBE part while 
>> "false" for the normal load. And the compiler will fail to intrinsify if the 
>> flag is "true" and the predicate load is not supported by the backend, which 
>> means that normal java path will be executed.
>> 
>> Also adds the same vectorization support for masked:
>>  - fromByteArray/fromByteBuffer
>>  - fromBooleanArray
>>  - fromCharArray
>> 
>> The performance for the new added benchmarks improve about `1.88x ~ 30.26x` 
>> on the x86 AVX-512 system:
>> 
>> Benchmark  before   After  Units
>> LoadMaskedIOOBEBenchmark.byteLoadArrayMaskIOOBE   737.542 1387.069 ops/ms
>> LoadMaskedIOOBEBenchmark.doubleLoadArrayMaskIOOBE 118.366  330.776 ops/ms
>> LoadMaskedIOOBEBenchmark.floatLoadArrayMaskIOOBE  233.832 6125.026 ops/ms
>> LoadMaskedIOOBEBenchmark.intLoadArrayMaskIOOBE233.816 7075.923 ops/ms
>> LoadMaskedIOOBEBenchmark.longLoadArrayMaskIOOBE   119.771  330.587 ops/ms
>> LoadMaskedIOOBEBenchmark.shortLoadArrayMaskIOOBE  431.961  939.301 ops/ms
>> 
>> Similar performance gain can also be observed on 512-bit SVE system.
>
> Xiaohong Gong has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Rename the "usePred" to "offsetInRange"

Rest of the patch looks good to me.

src/hotspot/share/opto/vectorIntrinsics.cpp line 1232:

> 1230:   // out when current case uses the predicate feature.
> 1231:   if (!supports_predicate) {
> 1232: bool use_predicate = false;

If we rename this to needs_predicate it will be easier to understand.

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Coleen Phillimore
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

I've reviewed the new files in Hotspot runtime and the cpu directories, as well 
as the changes to existing files in runtime, oops, utilities, interpreter and 
classfile.
I'm happy to approve this PR. Thank you to everyone for their hard work in 
reviewing this code, and well done to @pron and @AlanBateman and all the loom 
team for bringing us this significant and exciting new feature for Java!

src/hotspot/share/prims/jvmtiEnvBase.cpp line 2388:

> 2386: }
> 2387: 
> 2388: void

@sspitsyn We should clean up this aberrant style of putting the return type on 
the line before the rest of the function declaration after this integration.  
It looks so strange.  I noticed that it's pre-existing for other functions in 
this file.

-

Marked as reviewed by coleenp (Reviewer).

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


Re: RFR: 8284992: Fix misleading Vector API doc for LSHR operator [v3]

2022-04-27 Thread Paul Sandoz
On Wed, 27 Apr 2022 09:06:12 GMT, Jie Fu  wrote:

>> Hi all,
>> 
>> The Current Vector API doc for `LSHR` is
>> 
>> Produce a>>>(n&(ESIZE*8-1)). Integral only.
>> 
>> 
>> This is misleading which may lead to bugs for Java developers.
>> This is because for negative byte/short elements, the results computed by 
>> `LSHR` will be different from that of `>>>`.
>> For more details, please see 
>> https://github.com/openjdk/jdk/pull/8276#issue-1206391831 .
>> 
>> After the patch, the doc for `LSHR` is
>> 
>> Produce zero-extended right shift of a by (n&(ESIZE*8-1)) bits. Integral 
>> only.
>> 
>> 
>> Thanks.
>> Best regards,
>> Jie
>
> Jie Fu 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:
> 
>  - Address review comments
>  - Merge branch 'master' into JDK-8284992
>  - Merge branch 'master' into JDK-8284992
>  - Address review comments
>  - Merge branch 'master' into JDK-8284992
>  - 8284992: Fix misleading Vector API doc for LSHR operator

I created one, filled it in, and assigned it to you (for other examples you can 
search in the issue tracker, this one quite is simple so i thought it was 
quicker to do myself to show you). For any specification change we need to 
review and track that change (independent of any implementation changes, if 
any).

If you are ok with it I can add myself as reviewer, then you can "Finalize" it 
(see button on same line as "Edit"), triggering a review request, from which we 
may receive comments to address, and once addressed and final, it will unblock 
the PR for integration.

-

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


Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v2]

2022-04-27 Thread Stuart Marks
On Wed, 27 Apr 2022 23:04:47 GMT, Joe Darcy  wrote:

>> src/java.base/share/classes/java/util/AbstractMap.java line 601:
>> 
>>> 599:  * {@code Map.entrySet().toArray}.
>>> 600:  *
>>> 601:  * @param  the type of keys maintained
>> 
>> Please update to match java.util.Map, which says "the type of keys 
>> maintained by this map"
>
> I said "keys maintained", omitting "by this map" to finesse the question of 
> if the SimpleEntry class *is* a map, or is used to implement a map, etc. I 
> can change it to include "by this map" if the map/entry distinction is okay 
> to be blurred.

Whoops, sorry, this is `SimpleEntry`, which is _not_ a `Map`. In this case I'd 
follow `Map.Entry` which says "the type of the key" and "the type of the map".

>> src/java.base/share/classes/java/util/Dictionary.java line 44:
>> 
>>> 42:  * @param  the type of keys
>>> 43:  * @param  the type of mapped values
>>> 44:  *
>> 
>> Urgh. This class is obsolete, but it was retrofitted to implement Map and 
>> was subsequently generified, so I'd update these to match java.util.Map.
>
> The javadoc of Dictionary states "The Dictionary class [...] maps keys to 
> values." which was my guide for the wording, but I don't mind changing it.

My bad, `Dictionary` was not retrofitted to implement `Map` but it gained `K` 
and `V` type parameters to align with `Map`. No need to change this; it doesn't 
really matter.

-

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


Re: RFR: 8272352: Java launcher can not parse Chinese character when system locale is set to UTF-8

2022-04-27 Thread Naoto Sato
On Wed, 27 Apr 2022 23:12:59 GMT, Jie Fu  wrote:

> Is it possible to write a jtreg test for this fix? Thanks.

I am happy to write a test if possible, but AFAIK it requires configuring 
Windows settings (and reboot), so I don't think jtreg is capable of doing so. 
Thus I added `noreg-hard` to the JBS issue.

-

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


Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v2]

2022-04-27 Thread Joe Darcy
On Wed, 27 Apr 2022 10:55:22 GMT, Daniel Fuchs  wrote:

>> 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-8285676
>>  - JDK-8285676: Add missing @param tags for type parameters on classes and 
>> interfaces
>
> src/java.management/share/classes/javax/management/openmbean/SimpleType.java 
> line 60:
> 
>> (failed to retrieve contents of file, check the PR for context)
> I would suggest to say "that values described by this type"...

Will change to "the Java type that values described by this SimpleType must 
have."

-

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


Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v2]

2022-04-27 Thread Joe Darcy
> To enable more complete doclint checking (courtesy @jonathan-gibbons), please 
> review this PR to add type-level @param tags where they are missing.
> 
> To the maintainers of java.util.concurrent, those changes could be separated 
> out in another bug if that would ease maintenance of that code.
> 
> Making these library fixes is a blocker for correcting and expanding the 
> doclint checks (JDK-8285496).
> 
> I'll update copyright years before pushing.

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-8285676
 - JDK-8285676: Add missing @param tags for type parameters on classes and 
interfaces

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8410/files
  - new: https://git.openjdk.java.net/jdk/pull/8410/files/fe47dd2f..e0ac5dcb

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

  Stats: 5958 lines in 128 files changed: 4827 ins; 485 del; 646 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8410.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8410/head:pull/8410

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


Re: RFR: 8272352: Java launcher can not parse Chinese character when system locale is set to UTF-8

2022-04-27 Thread Jie Fu
On Wed, 27 Apr 2022 20:23:32 GMT, Naoto Sato  wrote:

> Java runtime has been detecting the Windows system locale encoding using 
> `GetLocaleInfo(GetSystemDefaultLCID(), LOCALE_IDEFAULTANSICODEPAGE, ...)`, 
> but it returns the *legacy* ANSI code page value, e.g, 1252 for US-English. 
> In order to detect whether the user has selected `UTF-8` as the default, the 
> code page has to be queried with `GetACP()`.
> Also, the case if the call to `GetLocaleInfo` fails changed to fall back to 
> `UTF-8` instead of `Cp1252`.

Is it possible to write a jtreg test for this fix?
Thanks.

-

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


Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces

2022-04-27 Thread Joe Darcy
On Wed, 27 Apr 2022 10:54:00 GMT, Daniel Fuchs  wrote:

>> To enable more complete doclint checking (courtesy @jonathan-gibbons), 
>> please review this PR to add type-level @param tags where they are missing.
>> 
>> To the maintainers of java.util.concurrent, those changes could be separated 
>> out in another bug if that would ease maintenance of that code.
>> 
>> Making these library fixes is a blocker for correcting and expanding the 
>> doclint checks (JDK-8285496).
>> 
>> I'll update copyright years before pushing.
>
> src/java.management/share/classes/javax/management/openmbean/ArrayType.java 
> line 96:
> 
>> 94:  *
>> 95:  * @param  the Java type that instances described by this type must
>> 96:  * have.
> 
> Instead of "instances" - would it be more correct to say "array elements"?

Will change to "the Java component type that arrays described by ArrayType must 
have"

-

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


Re: RFR: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java

2022-04-27 Thread Shruthi
On Wed, 20 Apr 2022 15:37:13 GMT, Shruthi  wrote:

> Removing the Duplicate keys present in XSLTErrorResources.java and 
> XPATHErrorResources.java
> 
> The bug report for the same: https://bugs.openjdk.java.net/browse/JDK-8285097

This contribution is on behalf of my employer, IBM, which is a corporate OCA 
signatory.

`/covered`

-

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


Re: RFR: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java

2022-04-27 Thread Tyler Steele
On Wed, 20 Apr 2022 15:37:13 GMT, Shruthi  wrote:

> Removing the Duplicate keys present in XSLTErrorResources.java and 
> XPATHErrorResources.java
> 
> The bug report for the same: https://bugs.openjdk.java.net/browse/JDK-8285097

@shruacha1234, a few notes specific to openJDK PRs:
- Please change the issue name to: "8285097: Duplicate XML keys in 
XPATHErrorResources.java and XSLTErrorResources.java" this will correctly link 
the issue in bugs.openjdk.net
- Please enable github actions. This allows the pre-tests to run.

-

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


RFR: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java

2022-04-27 Thread Shruthi
Removing the Duplicate keys present in XSLTErrorResources.java and 
XPATHErrorResources.java

The bug report for the same: https://bugs.openjdk.java.net/browse/JDK-8285097

-

Commit messages:
 - Removing Duplicate keys present in XSLTErrorResources.java and 
XPATHErrorResources.java

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

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


Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces

2022-04-27 Thread Joe Darcy
On Wed, 27 Apr 2022 01:39:27 GMT, Stuart Marks  wrote:

>> To enable more complete doclint checking (courtesy @jonathan-gibbons), 
>> please review this PR to add type-level @param tags where they are missing.
>> 
>> To the maintainers of java.util.concurrent, those changes could be separated 
>> out in another bug if that would ease maintenance of that code.
>> 
>> Making these library fixes is a blocker for correcting and expanding the 
>> doclint checks (JDK-8285496).
>> 
>> I'll update copyright years before pushing.
>
> src/java.base/share/classes/java/util/AbstractMap.java line 601:
> 
>> 599:  * {@code Map.entrySet().toArray}.
>> 600:  *
>> 601:  * @param  the type of keys maintained
> 
> Please update to match java.util.Map, which says "the type of keys maintained 
> by this map"

I said "keys maintained", omitting "by this map" to finesse the question of if 
the SimpleEntry class *is* a map, or is used to implement a map, etc. I can 
change it to include "by this map" if the map/entry distinction is okay to be 
blurred.

> src/java.base/share/classes/java/util/Dictionary.java line 44:
> 
>> 42:  * @param  the type of keys
>> 43:  * @param  the type of mapped values
>> 44:  *
> 
> Urgh. This class is obsolete, but it was retrofitted to implement Map and was 
> subsequently generified, so I'd update these to match java.util.Map.

The javadoc of Dictionary states "The Dictionary class [...] maps keys to 
values." which was my guide for the wording, but I don't mind changing it.

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Brian Burkhalter
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

The changes to the `java.io` package and related files in `libjava`, and the 
changes to the non-networking parts of the `java.nio.channels`, `sun.nio.ch`, 
and `sun.nio.fs` packages and related files in `libnio` all look fine to me.

-

Marked as reviewed by bpb (Reviewer).

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


Re: RFR: 8284992: Fix misleading Vector API doc for LSHR operator [v3]

2022-04-27 Thread Jie Fu
On Wed, 27 Apr 2022 17:17:55 GMT, Paul Sandoz  wrote:

> Thanks, looks good, we will need to create a CSR. Have you done that before?

No, and I don't know much about a CSR.
Is there any example for a doc fix CSR to follow?
Thanks.

-

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


RFR: JDK-8285764: Add system property for Java SE specification maintenance version

2022-04-27 Thread Joe Darcy
Add a new system property, java.specification.maintenance.version, to return 
the maintenance release number of the Java SE specification being implemented. 
The property is unset, optional in the terminology of System.getProperties, for 
an initial release of a specification.

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

I'll update copyright years before an integration.

-

Commit messages:
 - Update comment in template.
 - JDK-8285497: Add system property for Java SE specification maintenance 
version

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

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Paul Sandoz
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

It's great to see end game in play for this multi-year effort, impressive work 
by all involved.

I looked through code in the base module. Generally looks well structured and 
documented.

The fork join code is naturally hard to review. I did try! (it takes the prize 
for the highest Java code density in the JDK). I mostly looked for regular and 
repeated structure rather than trying to fully understand/review the intricate 
concurrency details.

IMO there is some post integration work to do around the architecture of thread 
containers, i presume the structured concurrency implementation (use of thread 
flock) will help with that.

-

Marked as reviewed by psandoz (Reviewer).

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Mandy Chung
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

I reviewed most of the source files in `java.base` except 
`java.util.concurrent`.  I also reviewed `java.logging`, `java.management` and 
`jdk.management` modules.   The changes are easy to follow and clean.  Looks 
good to me.

-

Marked as reviewed by mchung (Reviewer).

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


Re: RFR: JDK-8285730: unify _WIN32_WINNT settings

2022-04-27 Thread David Holmes
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken  wrote:

> Currently we set _WIN32_WINNT at various places in the codebase; this is used 
> to target a minimum Windows version we want to support. See also for more 
> detailled information :
> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt
> Macros for Conditional Declarations
> "Certain functions that depend on a particular version of Windows are 
> declared using conditional code. This enables you to use the compiler to 
> detect whether your application uses functions that are not supported on its 
> target version(s) of Windows."
> 
> However currently we have quite a lot of differing settings of _WIN32_WINNT 
> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible 
> would make sense because we have this setting already at   java_props_md.c  
> (so targeting older Windows versions at other places is most likely not 
> useful).

There is obviously a lot of history here and different parts of the codebase 
had hit different minimum OS version requirements at different times. While at 
one time we would have had to cater for building on systems with and without a 
given API those days are long gone for the code being touched here (AFAICS). As 
Erik states we should be able to set a minimum _WIN32_WINNT_ value needed to 
build all the codebase and simply have that checked and set at configure time. 
The code itself would not need to define _WIN32_WINNT.

-

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


Re: RFR: 8284640: CollectorImpl class could be a record class

2022-04-27 Thread liach
On Mon, 11 Apr 2022 13:08:43 GMT, altrisi  wrote:

> Changes the definition of `CollectorImpl` to be a record.

src/java.base/share/classes/java/util/stream/Collectors.java line 197:

> 195:  * @param  the type of the result
> 196:  */
> 197: static record CollectorImpl(Supplier supplier,

Suggestion:

record CollectorImpl(Supplier supplier,

[Nested records are implicitly 
static.](https://docs.oracle.com/javase/specs/jls/se18/html/jls-8.html#jls-8.10-220)

-

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


RFR: 8284640: CollectorImpl class could be a record class

2022-04-27 Thread altrisi
Changes the definition of `CollectorImpl` to be a record.

-

Commit messages:
 - Remove explicit `static` modifier
 - 8284640: CollectorImpl class could be a record class

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

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


RFR: 8272352: Java launcher can not parse Chinese character when system locale is set to UTF-8

2022-04-27 Thread Naoto Sato
Java runtime has been detecting the Windows system locale encoding using 
`GetLocaleInfo(GetSystemDefaultLCID(), LOCALE_IDEFAULTANSICODEPAGE, ...)`, but 
it returns the *legacy* ANSI code page value, e.g, 1252 for US-English. In 
order to detect whether the user has selected `UTF-8` as the default, the code 
page has to be queried with `GetACP()`.
Also, the case if the call to `GetLocaleInfo` fails changed to fall back to 
`UTF-8` instead of `Cp1252`.

-

Commit messages:
 - 8272352: Java launcher can not parse Chinese character when system locale is 
set to UTF-8

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

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


Re: RFR: 8285745: Re-examine PushbackInputStream mark/reset

2022-04-27 Thread Brian Burkhalter
On Wed, 27 Apr 2022 20:10:03 GMT, Brian Burkhalter  wrote:

> Please review this request to remove the `synchronized` keyword from the 
> `mark(int)` and `reset()` methods of `java.io.PushbackInputStream`.

Please see also https://github.com/openjdk/jdk/pull/8309.

-

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


RFR: 8285745: Re-examine PushbackInputStream mark/reset

2022-04-27 Thread Brian Burkhalter
Please review this request to remove the `synchronized` keyword from the 
`mark(int)` and `reset()` methods of `java.io.PushbackInputStream`.

-

Commit messages:
 - 8285745: Re-examine PushbackInputStream mark/reset

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

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Paul Sandoz
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

src/java.base/share/classes/java/lang/Thread.java line 116:

> 114:  * carrier threads. Locking and I/O operations are examples of 
> operations
> 115:  * where a carrier thread may be re-scheduled from one virtual thread to 
> another.
> 116:  * Code executing in a virtual thread is not aware of underlying carrier 
> thread.

Suggestion:

 * Code executing in a virtual thread is not aware of the underlying carrier 
thread.

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Paul Sandoz
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

src/java.base/share/classes/java/util/concurrent/ThreadPerTaskExecutor.java 
line 45:

> 43:  * threads is unbounded.
> 44:  */
> 45: class ThreadPerTaskExecutor implements ExecutorService {

This class manages the set of per-task threads which arguably should be the job 
of the thread container, and it awkwardly overrides the container's set of 
threads by setting a field on `SharedThreadContainer.threadsSupplier`.

`SharedThreadContainer` supports a number of different shared container 
policies that could be made clearer.

Architecturally i think this could be better layered but it can be iterated on 
later.

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Alan Bateman
On Wed, 27 Apr 2022 18:16:54 GMT, Mandy Chung  wrote:

>> I was wondering that too, but held off commenting since it's not super 
>> performance critical given `classToHashes` is synchronized on. However, it 
>> does make the code a little clearer.
>
> It's not critical and no problem if this is cleaned up as a follow-up.

Good idea, this could be improved. As Paul says, it's not performance critical 
as this is a diagnostic option for printing the stack trace when pinned.

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Dean Long
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

The compiler changes look good to me.

Marked as reviewed by dlong (Reviewer).

-

Marked as reviewed by dlong (Reviewer).

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


Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v20]

2022-04-27 Thread Stuart Marks
On Thu, 21 Apr 2022 03:48:21 GMT, Yasser Bazzi  wrote:

>> Hi, could i get a review on this implementation proposed by Stuart Marks, i 
>> decided to use the 
>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/random/RandomGenerator.html
>>  interface to create the default method `asRandom()` that wraps around the 
>> newer algorithms to be used on classes that do not accept the new interface.
>> 
>> Some things to note as proposed by the bug report, the protected method 
>> next(int bits) is not overrided and setSeed() method if left blank up to 
>> discussion on what to do with it.
>> 
>> Small test done on 
>> https://gist.github.com/YShow/da678561419cda8e32fccf3a27a649d4
>
> Yasser Bazzi has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Update setSeed docs on Random class
>  - Add nicer toString method to random wrapper

Hi, thanks for the updates.

I've made some modifications to the specs of `setSeed` and `next(bits)` for 
good measure, since the latter also needed to be updated to accommodate 
overrides made by subclasses. If these changes are satisfactory, please pull 
them into this PR, and I'll update the CSR correspondingly. Changes are in this 
branch:

https://github.com/stuart-marks/jdk/tree/JDK-8279598-RandomGenerator-adapter

-

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


Re: Floating Point Repair?

2022-04-27 Thread Jochen Theodorou

On 27.04.22 12:47, sminervini.prism wrote:

To the Java OpenJDK Java Community Process,

[...]

You could propose a new data type that follows BCDs from IEE754-2008.
You would have to define conversions of course.

You would have to of course define type conversions, find somebody doing
whatever a JSR is today, extend hotspot and the java compiler, or
prototype with a software emulation.

bye Jochen


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Alan Bateman
On Wed, 27 Apr 2022 17:53:01 GMT, Mandy Chung  wrote:

>> Alan Bateman has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/java.base/share/classes/jdk/internal/misc/UnsafeConstants.java line 122:
> 
>> 120:  */
>> 121: 
>> 122: public static final int SCOPED_CACHE_SHIFT;
> 
> I can't find this constant being used.   If added for future, maybe keep 
> `UnsafeConstants` class and this static field package-private for now.

It originally configured the number of lines in extent local cache but the 
implementation has changed. @theRealAph would be best to comment on this, it 
may be possible to delete it.

> src/java.management/share/classes/sun/management/ThreadImpl.java line 447:
> 
>> 445: if (threads != null) {
>> 446: long[] tids = Stream.of(threads)
>> 447: .filter(t -> !(t instanceof 
>> jdk.internal.misc.CarrierThread))
> 
> Returning an array of thread IDs of just the platform threads is good.   The 
> javadoc needs to be updated to say "Returns an array of thread identifiers 
> for the platform threads"

I think you mean the comment on the private method. Yes, that could be clearer 
that it just returns platform threads.

-

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


Integrated: 8285658: Fix two typos in the spec of j.u.random.RandomGenerator

2022-04-27 Thread Raffaello Giulietti
On Tue, 26 Apr 2022 16:38:37 GMT, Raffaello Giulietti  
wrote:

> The spec of the interface `java.util.random.RandomGenerator` is slightly 
> incorrect when it discusses `float` and `double` random values.

This pull request has now been integrated.

Changeset: 1f868f1d
Author:Raffaello Giulietti 
Committer: Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/1f868f1d091602cc462ee0fe5fa613a3638a5f1c
Stats: 9 lines in 1 file changed: 1 ins; 0 del; 8 mod

8285658: Fix two typos in the spec of j.u.random.RandomGenerator

Reviewed-by: bpb, darcy

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Dean Long
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

Marked as reviewed by dlong (Reviewer).

-

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


Re: RFR: JDK-8285730: unify _WIN32_WINNT settings

2022-04-27 Thread Erik Joelsson
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken  wrote:

> Currently we set _WIN32_WINNT at various places in the codebase; this is used 
> to target a minimum Windows version we want to support. See also for more 
> detailled information :
> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt
> Macros for Conditional Declarations
> "Certain functions that depend on a particular version of Windows are 
> declared using conditional code. This enables you to use the compiler to 
> detect whether your application uses functions that are not supported on its 
> target version(s) of Windows."
> 
> However currently we have quite a lot of differing settings of _WIN32_WINNT 
> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible 
> would make sense because we have this setting already at   java_props_md.c  
> (so targeting older Windows versions at other places is most likely not 
> useful).

Requiring different windows versions in different parts of the product doesn't 
make much sense, but I think it's even worse trying to keep all these different 
locations in sync. At least most of these have a comment explaining why that 
particular Windows version is required there. Changing these values invalidates 
those comments. 

If we are to do something about this, then we need to define this macro in a 
central location, and verify the value in configure. Then we would provide it 
either on compile command lines, or in a globally included config.h.

-

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


Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]

2022-04-27 Thread Kim Barrett
On Wed, 27 Apr 2022 18:25:27 GMT, Mandy Chung  wrote:

>> That was my initial thought, but it doesn't work - CNSE is a checked 
>> exception so must be handled.
>
>> test() and main will need to declare this checked exception.
> 
> 
> diff --git a/test/jdk/java/lang/ref/ReferenceClone.java 
> b/test/jdk/java/lang/ref/ReferenceClone.java
> index bd1ead81bec..2f9386b81e4 100644
> --- a/test/jdk/java/lang/ref/ReferenceClone.java
> +++ b/test/jdk/java/lang/ref/ReferenceClone.java
> @@ -31,12 +31,12 @@ import java.lang.ref.*;
>  
>  public class ReferenceClone {
>  private static final ReferenceQueue QUEUE = new 
> ReferenceQueue<>();
> -public static void main(String... args) {
> +public static void main(String... args) throws Exception {
>  ReferenceClone refClone = new ReferenceClone();
>  refClone.test();
>  }
>  
> -public void test() {
> +public void test() throws CloneNotSupportedException {
>  // test Reference::clone that throws CNSE
>  Object o = new Object();
>  assertCloneNotSupported(new SoftRef(o));
> @@ -45,9 +45,7 @@ public class ReferenceClone {
>  
>  // Reference subclass may override the clone method
>  CloneableReference ref = new CloneableReference(o);
> -try {
>  ref.clone();
> -} catch (CloneNotSupportedException e) {}
>  }
>  
>  private void assertCloneNotSupported(CloneableRef ref) {
>  ```

Yes, that would work.  But I'd rather keep the code for that subtest close 
together, i.e. as currently written.

-

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


Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]

2022-04-27 Thread Mandy Chung
On Wed, 27 Apr 2022 18:19:57 GMT, Kim Barrett  wrote:

>> test/jdk/java/lang/ref/ReferenceClone.java line 52:
>> 
>>> 50: } catch (CloneNotSupportedException e) {
>>> 51: throw new RuntimeException("CloneableReference::clone 
>>> should not throw CloneNotSupportedException");
>>> 52: }
>> 
>> Alternatively, it could simply let CNSE propagate.
>> 
>> 
>> CloneableReference ref = new CloneableReference(o);
>> ref.clone();
>> 
>> 
>> `test()` and `main` will need to declare this checked exception.
>
> That was my initial thought, but it doesn't work - CNSE is a checked 
> exception so must be handled.

> test() and main will need to declare this checked exception.


diff --git a/test/jdk/java/lang/ref/ReferenceClone.java 
b/test/jdk/java/lang/ref/ReferenceClone.java
index bd1ead81bec..2f9386b81e4 100644
--- a/test/jdk/java/lang/ref/ReferenceClone.java
+++ b/test/jdk/java/lang/ref/ReferenceClone.java
@@ -31,12 +31,12 @@ import java.lang.ref.*;
 
 public class ReferenceClone {
 private static final ReferenceQueue QUEUE = new ReferenceQueue<>();
-public static void main(String... args) {
+public static void main(String... args) throws Exception {
 ReferenceClone refClone = new ReferenceClone();
 refClone.test();
 }
 
-public void test() {
+public void test() throws CloneNotSupportedException {
 // test Reference::clone that throws CNSE
 Object o = new Object();
 assertCloneNotSupported(new SoftRef(o));
@@ -45,9 +45,7 @@ public class ReferenceClone {
 
 // Reference subclass may override the clone method
 CloneableReference ref = new CloneableReference(o);
-try {
 ref.clone();
-} catch (CloneNotSupportedException e) {}
 }
 
 private void assertCloneNotSupported(CloneableRef ref) {
 ```

-

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


Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]

2022-04-27 Thread Kim Barrett
On Wed, 27 Apr 2022 15:57:34 GMT, Mandy Chung  wrote:

>> Kim Barrett has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   update copyright, @bug list
>
> test/jdk/java/lang/ref/ReferenceClone.java line 52:
> 
>> 50: } catch (CloneNotSupportedException e) {
>> 51: throw new RuntimeException("CloneableReference::clone should 
>> not throw CloneNotSupportedException");
>> 52: }
> 
> Alternatively, it could simply let CNSE propagate.
> 
> 
> CloneableReference ref = new CloneableReference(o);
> ref.clone();
> 
> 
> `test()` and `main` will need to declare this checked exception.

That was my initial thought, but it doesn't work - CNSE is a checked exception 
so must be handled.

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Mandy Chung
On Wed, 27 Apr 2022 18:13:15 GMT, Paul Sandoz  wrote:

>> src/java.base/share/classes/java/lang/PinnedThreadPrinter.java line 58:
>> 
>>> 56: 
>>> 57: // maps a class to the hashes of stack traces pinned by that code 
>>> in that class
>>> 58: private static final Map, Hashes> classToHashes = new 
>>> WeakHashMap<>();
>> 
>> Can you use `ClassValue` in this case?
>
> I was wondering that too, but held off commenting since it's not super 
> performance critical given `classToHashes` is synchronized on. However, it 
> does make the code a little clearer.

It's not critical and no problem if this is cleaned up as a follow-up.

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Paul Sandoz
On Wed, 27 Apr 2022 17:27:56 GMT, Mandy Chung  wrote:

>> Alan Bateman has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/java.base/share/classes/java/lang/PinnedThreadPrinter.java line 58:
> 
>> 56: 
>> 57: // maps a class to the hashes of stack traces pinned by that code in 
>> that class
>> 58: private static final Map, Hashes> classToHashes = new 
>> WeakHashMap<>();
> 
> Can you use `ClassValue` in this case?

I was wondering that too, but held off commenting since it's not super 
performance critical given `classToHashes` is synchronized on. However, it does 
make the code a little clearer.

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Mandy Chung
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

src/java.management/share/classes/sun/management/ThreadImpl.java line 447:

> 445: if (threads != null) {
> 446: long[] tids = Stream.of(threads)
> 447: .filter(t -> !(t instanceof 
> jdk.internal.misc.CarrierThread))

Returning an array of thread IDs of just the platform threads is good.   The 
javadoc needs to be updated to say "Returns an array of thread identifiers for 
the platform threads"

-

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


Re: RFR: 8285633: Take better advantage of generic MethodType cache [v2]

2022-04-27 Thread John R Rose
On Tue, 26 Apr 2022 20:47:39 GMT, Claes Redestad  wrote:

>> The `MethodType.genericMethodType` methods provide convenience methods for 
>> certain common method types and also provide `@Stable` cache that allows for 
>> constant folding. This patch enhances the more generic `methodType` methods 
>> to take advantage of this cache, when possible. This allows calls like 
>> `MethodType.methodType(Object.class, Object.class, ...)` to constant fold, 
>> making them 50x+ faster and allocation-free.
>> 
>> Baseline:
>> 
>> BenchmarkMode  Cnt   Score   Error  Units
>> MethodTypeAcquire.baselineRawavgt   15   0.673 ? 0.017  ns/op
>> MethodTypeAcquire.testGenericObject  avgt   15   0.579 ? 0.125  ns/op
>> MethodTypeAcquire.testMultiPType avgt   15  46.671 ? 1.134  ns/op
>> MethodTypeAcquire.testMultiPType_Arg avgt   15  27.019 ? 0.437  ns/op
>> MethodTypeAcquire.testMultiPType_ObjectAndA  avgt   15  57.217 ? 0.505  ns/op
>> MethodTypeAcquire.testMultiPType_ObjectOnly  avgt   15  57.114 ? 1.214  ns/op
>> MethodTypeAcquire.testObjectObject   avgt   15  33.380 ? 1.810  ns/op
>> MethodTypeAcquire.testReturnInt  avgt   15  28.043 ? 0.187  ns/op
>> MethodTypeAcquire.testReturnObject   avgt   15  24.313 ? 0.074  ns/op
>> MethodTypeAcquire.testReturnVoid avgt   15  24.360 ? 0.155  ns/op
>> MethodTypeAcquire.testSinglePTypeavgt   15  33.572 ? 1.101  ns/op
>> 
>> 
>> Patched:
>> 
>> BenchmarkMode  Cnt   Score   Error  Units
>> MethodTypeAcquire.baselineRawavgt   15   0.683 ? 0.024  ns/op
>> MethodTypeAcquire.testGenericObject  avgt   15   0.547 ? 0.047  ns/op
>> MethodTypeAcquire.testMultiPType avgt   15  48.532 ? 0.982  ns/op
>> MethodTypeAcquire.testMultiPType_Arg avgt   15  31.390 ? 5.269  ns/op
>> MethodTypeAcquire.testMultiPType_ObjectAndA  avgt   15  60.165 ? 2.756  ns/op
>> MethodTypeAcquire.testMultiPType_ObjectOnly  avgt   15   0.599 ? 0.166  ns/op
>> MethodTypeAcquire.testObjectObject   avgt   15   0.541 ? 0.045  ns/op
>> MethodTypeAcquire.testReturnInt  avgt   15  28.174 ? 0.257  ns/op
>> MethodTypeAcquire.testReturnObject   avgt   15   0.542 ? 0.045  ns/op
>> MethodTypeAcquire.testReturnVoid avgt   15  24.621 ? 0.202  ns/op
>> MethodTypeAcquire.testSinglePTypeavgt   15  33.614 ? 1.109  ns/op
>> 
>> 
>> The cost on method types that don't match are in the noise (0-2ns/op). Even 
>> on a test I constructed to act as a worst case (`testMultiPType_ObjectAndA`) 
>> there's barely any appreciable cost (~3ns/op, with overlapping error).
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add micros using non-constant arguments to assess performance in absense of 
> constant folding

Good work.

-

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


Integrated: 8285633: Take better advantage of generic MethodType cache

2022-04-27 Thread Claes Redestad
On Tue, 26 Apr 2022 10:57:04 GMT, Claes Redestad  wrote:

> The `MethodType.genericMethodType` methods provide convenience methods for 
> certain common method types and also provide `@Stable` cache that allows for 
> constant folding. This patch enhances the more generic `methodType` methods 
> to take advantage of this cache, when possible. This allows calls like 
> `MethodType.methodType(Object.class, Object.class, ...)` to constant fold, 
> making them 50x+ faster and allocation-free.
> 
> Baseline:
> 
> BenchmarkMode  Cnt   Score   Error  Units
> MethodTypeAcquire.baselineRawavgt   15   0.673 ? 0.017  ns/op
> MethodTypeAcquire.testGenericObject  avgt   15   0.579 ? 0.125  ns/op
> MethodTypeAcquire.testMultiPType avgt   15  46.671 ? 1.134  ns/op
> MethodTypeAcquire.testMultiPType_Arg avgt   15  27.019 ? 0.437  ns/op
> MethodTypeAcquire.testMultiPType_ObjectAndA  avgt   15  57.217 ? 0.505  ns/op
> MethodTypeAcquire.testMultiPType_ObjectOnly  avgt   15  57.114 ? 1.214  ns/op
> MethodTypeAcquire.testObjectObject   avgt   15  33.380 ? 1.810  ns/op
> MethodTypeAcquire.testReturnInt  avgt   15  28.043 ? 0.187  ns/op
> MethodTypeAcquire.testReturnObject   avgt   15  24.313 ? 0.074  ns/op
> MethodTypeAcquire.testReturnVoid avgt   15  24.360 ? 0.155  ns/op
> MethodTypeAcquire.testSinglePTypeavgt   15  33.572 ? 1.101  ns/op
> 
> 
> Patched:
> 
> BenchmarkMode  Cnt   Score   Error  Units
> MethodTypeAcquire.baselineRawavgt   15   0.683 ? 0.024  ns/op
> MethodTypeAcquire.testGenericObject  avgt   15   0.547 ? 0.047  ns/op
> MethodTypeAcquire.testMultiPType avgt   15  48.532 ? 0.982  ns/op
> MethodTypeAcquire.testMultiPType_Arg avgt   15  31.390 ? 5.269  ns/op
> MethodTypeAcquire.testMultiPType_ObjectAndA  avgt   15  60.165 ? 2.756  ns/op
> MethodTypeAcquire.testMultiPType_ObjectOnly  avgt   15   0.599 ? 0.166  ns/op
> MethodTypeAcquire.testObjectObject   avgt   15   0.541 ? 0.045  ns/op
> MethodTypeAcquire.testReturnInt  avgt   15  28.174 ? 0.257  ns/op
> MethodTypeAcquire.testReturnObject   avgt   15   0.542 ? 0.045  ns/op
> MethodTypeAcquire.testReturnVoid avgt   15  24.621 ? 0.202  ns/op
> MethodTypeAcquire.testSinglePTypeavgt   15  33.614 ? 1.109  ns/op
> 
> 
> The cost on method types that don't match are in the noise (0-2ns/op). Even 
> on a test I constructed to act as a worst case (`testMultiPType_ObjectAndA`) 
> there's barely any appreciable cost (~3ns/op, with overlapping error).

This pull request has now been integrated.

Changeset: 6c79671e
Author:Claes Redestad 
URL:   
https://git.openjdk.java.net/jdk/commit/6c79671e50d572f3da3a286d34a98dcb83b8d906
Stats: 119 lines in 8 files changed: 96 ins; 1 del; 22 mod

8285633: Take better advantage of generic MethodType cache

Reviewed-by: jvernee

-

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


Re: RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character

2022-04-27 Thread Roger Riggs
On Sun, 24 Apr 2022 09:18:54 GMT, Ichiroh Takiguchi  
wrote:

> On JDK19 with Linux ja_JP.eucjp locale,
> System.getenv() returns unexpected value if environment variable has Japanese 
> EUC characters.
> It seems this issue happens because of JEP 400.
> Arguments for ProcessBuilder have same kind of issue.

src/java.base/unix/classes/java/lang/ProcessEnvironment.java line 68:

> 66: private static final Map theUnmodifiableEnvironment;
> 67: static final int MIN_NAME_LENGTH = 0;
> 68: static final Charset cs = 
> Charset.forName(StaticProperty.nativeEncoding(),

cs should have an all-CAPS name to make it clear its a static value throughout 
the implementation.
And `private` too.

test/jdk/java/lang/System/i18nEnvArg.java line 41:

> 39: 
> 40: public class i18nEnvArg {
> 41: final static String text = "\u6F22\u5B57";

Can this be renamed to indicate it is the string under test?  like KANJI_TEXT 
or EUC_JP_TEXT or similar.

test/jdk/java/lang/System/i18nEnvArg.java line 49:

> 47: final static int maxSize = 4096;
> 48: 
> 49: public static void main(String[] args) throws Exception {

There are 3 main()'s in this test; it would be useful to add a comment 
indicating the purpose of each.

test/jdk/java/lang/System/i18nEnvArg.java line 60:

> 58: Process p = pb.start();
> 59: InputStream is = p.getInputStream();
> 60: byte[] ba = new byte[maxSize];

You can use `InputStream.readAllBytes()` here to return a byte buffer.

Its not clear why all the bytes are read?  I assume its only for a possible 
error from the child.

test/jdk/java/lang/System/i18nEnvArg.java line 128:

> 126: Method enviorn_mid = cls.getDeclaredMethod("environ");
> 127: enviorn_mid.setAccessible(true);
> 128: byte[][] environ = (byte[][]) enviorn_mid.invoke(null,

typo: "enviorn_mid" -> "environ_mid".

test/jdk/java/lang/System/i18nEnvArg.java line 138:

> 136: bb.put(environ[i+1]);
> 137: byte[] envb = Arrays.copyOf(ba, bb.position());
> 138: if (Arrays.equals(kanji, envb)) return;

It seems odd to exit the loop on the first match.

I think AIX always has environment variables not set by the caller.

test/jdk/java/lang/System/i18nEnvArg.java line 146:

> 144: sb.append((char)b);
> 145: } else {
> 146: sb.append(String.format("\\x%02X", (int)b & 
> 0xFF));

The methods of java.util.HexFormat may be useful here.
It seems that the "5C" would fit into this "else" clause and not need a 
separate statement.

-

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


Re: RFR: JDK-8285730: unify _WIN32_WINNT settings

2022-04-27 Thread Phil Race
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken  wrote:

> Currently we set _WIN32_WINNT at various places in the codebase; this is used 
> to target a minimum Windows version we want to support. See also for more 
> detailled information :
> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt
> Macros for Conditional Declarations
> "Certain functions that depend on a particular version of Windows are 
> declared using conditional code. This enables you to use the compiler to 
> detect whether your application uses functions that are not supported on its 
> target version(s) of Windows."
> 
> However currently we have quite a lot of differing settings of _WIN32_WINNT 
> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible 
> would make sense because we have this setting already at   java_props_md.c  
> (so targeting older Windows versions at other places is most likely not 
> useful).

This is OK by me, but I wonder if the build folks might want to think about 
this and whether it should be centralised somehow
since it seems odd to mandate different versions of windows in different places.

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Mandy Chung
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

src/java.base/share/classes/jdk/internal/misc/UnsafeConstants.java line 122:

> 120:  */
> 121: 
> 122: public static final int SCOPED_CACHE_SHIFT;

I can't find this constant being used.   If added for future, maybe keep 
`UnsafeConstants` class and this static field package-private for now.

-

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


Re: RFR: 8285485: Fix typos in corelibs

2022-04-27 Thread Lance Andersen
On Wed, 27 Apr 2022 17:02:40 GMT, Pavel Rappo  wrote:

>> I ran `codespell` on modules owned by core-libs, and accepted those changes 
>> where it indeed discovered real typos.
>> 
>> I will update copyright years using a script before pushing (otherwise like 
>> every second change would be a copyright update, making reviewing much 
>> harder).
>> 
>> The long term goal here is to make tooling support for running `codespell`. 
>> The trouble with automating this is of course all false positives. But 
>> before even trying to solve that issue, all true positives must be fixed. 
>> Hence this PR.
>
> src/java.sql.rowset/share/classes/com/sun/rowset/providers/RIXMLProvider.java 
> line 239:
> 
>> 237: 
>> 238: /**
>> 239:  * Returns the vendor name of the Reference Implementation 
>> Optimistic
> 
> L240: an optimistic _what_ provider? :-)

Yep typo there.  The original authors needed to run spell check ;-)

-

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


Re: RFR: 8285485: Fix typos in corelibs

2022-04-27 Thread Lance Andersen
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie  wrote:

> I ran `codespell` on modules owned by core-libs, and accepted those changes 
> where it indeed discovered real typos.
> 
> I will update copyright years using a script before pushing (otherwise like 
> every second change would be a copyright update, making reviewing much 
> harder).
> 
> The long term goal here is to make tooling support for running `codespell`. 
> The trouble with automating this is of course all false positives. But before 
> even trying to solve that issue, all true positives must be fixed. Hence this 
> PR.

Changes look OK with the exception of the comment pointed out below

-

Marked as reviewed by lancea (Reviewer).

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Alex Menkov
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

Reviewed debugger-related code (JDI/JDWP/JDB/tests) and partially JVMTI.

-

Marked as reviewed by amenkov (Reviewer).

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


Re: RFR: 8285485: Fix typos in corelibs

2022-04-27 Thread Pavel Rappo
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie  wrote:

> I ran `codespell` on modules owned by core-libs, and accepted those changes 
> where it indeed discovered real typos.
> 
> I will update copyright years using a script before pushing (otherwise like 
> every second change would be a copyright update, making reviewing much 
> harder).
> 
> The long term goal here is to make tooling support for running `codespell`. 
> The trouble with automating this is of course all false positives. But before 
> even trying to solve that issue, all true positives must be fixed. Hence this 
> PR.

Thanks for doing this so carefully. If reviewers decide that parts of this PR 
need to be addressed upstream, we should probably consider contributing those 
parts to respective projects.

src/java.sql.rowset/share/classes/com/sun/rowset/CachedRowSetImpl.java line 970:

> 968:  * and sends a rowSetChanged event to all registered
> 969:  * listeners.
> 970:  * @throws SQLException if an error is occurs rolling back the RowSet

L976, same as above: "is occurs".

src/java.sql.rowset/share/classes/com/sun/rowset/JoinRowSetImpl.java line 636:

> 634: // to be INNER JOIN'ED to form a new rowset
> 635: // Compare table1.MatchColumn1.value1 == { 
> table2.MatchColumn2.value1
> 636: //  ... up to 
> table2.MatchColumn2.valueN }

Curious: it is not some established string representation, is it?

src/java.sql.rowset/share/classes/com/sun/rowset/internal/SyncResolverImpl.java 
line 664:

> 662:  * and sends a {@code rowSetChanged} event to all registered
> 663:  * listeners.
> 664:  * @throws SQLException if an error is occurs rolling back the RowSet

L664: delete "is" in "is occurs".

src/java.sql.rowset/share/classes/com/sun/rowset/providers/RIXMLProvider.java 
line 239:

> 237: 
> 238: /**
> 239:  * Returns the vendor name of the Reference Implementation Optimistic

L240: an optimistic _what_ provider? :-)

src/java.xml/share/classes/com/sun/xml/internal/stream/writers/XMLDOMWriterImpl.java
 line 238:

> 236: 
> 237: /**
> 238:  * Creates a DOM Attribute @see org.w3c.dom.Node and associates it 
> with the current DOM element @see org.w3c.dom.Node.

Not that it matters in this PR, but I think I should mention that `@see` tags 
do not work like that. (No action needed from you.)

src/java.xml/share/classes/javax/xml/transform/Transformer.java line 127:

> 125:  * namespace URI in curly braces ({}).
> 126:  * @param value The value object.  This can be any valid Java 
> object. It is
> 127:  * up to the processor to provide the proper object coersion or to 
> simply

That made me pause: some systems have the notion of _type coercion_; but your 
change looks right.

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Mandy Chung
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

src/java.base/share/classes/java/lang/PinnedThreadPrinter.java line 58:

> 56: 
> 57: // maps a class to the hashes of stack traces pinned by that code in 
> that class
> 58: private static final Map, Hashes> classToHashes = new 
> WeakHashMap<>();

Can you use `ClassValue` in this case?

-

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


Re: RFR: 8284992: Fix misleading Vector API doc for LSHR operator [v3]

2022-04-27 Thread Paul Sandoz
On Wed, 27 Apr 2022 09:06:12 GMT, Jie Fu  wrote:

>> Hi all,
>> 
>> The Current Vector API doc for `LSHR` is
>> 
>> Produce a>>>(n&(ESIZE*8-1)). Integral only.
>> 
>> 
>> This is misleading which may lead to bugs for Java developers.
>> This is because for negative byte/short elements, the results computed by 
>> `LSHR` will be different from that of `>>>`.
>> For more details, please see 
>> https://github.com/openjdk/jdk/pull/8276#issue-1206391831 .
>> 
>> After the patch, the doc for `LSHR` is
>> 
>> Produce zero-extended right shift of a by (n&(ESIZE*8-1)) bits. Integral 
>> only.
>> 
>> 
>> Thanks.
>> Best regards,
>> Jie
>
> Jie Fu 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:
> 
>  - Address review comments
>  - Merge branch 'master' into JDK-8284992
>  - Merge branch 'master' into JDK-8284992
>  - Address review comments
>  - Merge branch 'master' into JDK-8284992
>  - 8284992: Fix misleading Vector API doc for LSHR operator

Thanks, looks good, we will need to create a CSR. Have you done that before?

-

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


Integrated: 8285736: JDK-8236128 causes validate-source failures

2022-04-27 Thread Daniel D . Daugherty
On Wed, 27 Apr 2022 16:53:44 GMT, Daniel D. Daugherty  
wrote:

> A trivial fix for JDK-8236128 causes validate-source failures.

This pull request has now been integrated.

Changeset: 5b42747b
Author:Daniel D. Daugherty 
URL:   
https://git.openjdk.java.net/jdk/commit/5b42747ba1606b34b05449518fa601d2451c5c66
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8285736: JDK-8236128 causes validate-source failures

Reviewed-by: mikael, asemenyuk

-

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


Re: Integrated: 8285736: JDK-8236128 causes validate-source failures

2022-04-27 Thread Alexey Semenyuk
On Wed, 27 Apr 2022 16:53:44 GMT, Daniel D. Daugherty  
wrote:

> A trivial fix for JDK-8236128 causes validate-source failures.

Marked as reviewed by asemenyuk (Reviewer).

-

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


Re: Integrated: 8285736: JDK-8236128 causes validate-source failures

2022-04-27 Thread Daniel D . Daugherty
On Wed, 27 Apr 2022 16:53:44 GMT, Daniel D. Daugherty  
wrote:

> A trivial fix for JDK-8236128 causes validate-source failures.

The bot is being very, very, slow to integrate this...

-

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


Re: Integrated: 8285736: JDK-8236128 causes validate-source failures

2022-04-27 Thread Daniel D . Daugherty
On Wed, 27 Apr 2022 16:55:11 GMT, Mikael Vidstedt  wrote:

>> A trivial fix for JDK-8236128 causes validate-source failures.
>
> Marked as reviewed by mikael (Reviewer).

@vidmik and @alexeysemenyukoracle - Thanks for the fast reviews.

-

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


Re: Integrated: 8285736: JDK-8236128 causes validate-source failures

2022-04-27 Thread Mikael Vidstedt
On Wed, 27 Apr 2022 16:53:44 GMT, Daniel D. Daugherty  
wrote:

> A trivial fix for JDK-8236128 causes validate-source failures.

Marked as reviewed by mikael (Reviewer).

-

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


Integrated: 8285736: JDK-8236128 causes validate-source failures

2022-04-27 Thread Daniel D . Daugherty
A trivial fix for JDK-8236128 causes validate-source failures.

-

Commit messages:
 - 8285736: JDK-8236128 causes validate-source failures

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

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Daniel D . Daugherty
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

I've reviewed the JVM/TI, JDWP and JDI changes and I'm good with those.

I haven't reviewed the JDB changes (forgot about those) and I have not
(yet) reviewed the Runtime changes.

-

Marked as reviewed by dcubed (Reviewer).

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Kevin Walls
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

Been through the JDI changes and jdb/example code.
Built and manually tested some operations with jdb observing virtual threads.

Been through jdk.management/share/classes/com/sun/management and also manually 
tested jconsole attaching (e.g. dumpThreads operation in both TEXT_PLAIN and 
JSON).

-

Marked as reviewed by kevinw (Committer).

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


Re: RFR: 8285485: Fix typos in corelibs

2022-04-27 Thread Pavel Rappo
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie  wrote:

> I ran `codespell` on modules owned by core-libs, and accepted those changes 
> where it indeed discovered real typos.
> 
> I will update copyright years using a script before pushing (otherwise like 
> every second change would be a copyright update, making reviewing much 
> harder).
> 
> The long term goal here is to make tooling support for running `codespell`. 
> The trouble with automating this is of course all false positives. But before 
> even trying to solve that issue, all true positives must be fixed. Hence this 
> PR.

src/java.se/share/data/jdwp/jdwp.spec line 47:

> 45: "Returns reference types for all the classes loaded by the target 
> VM "
> 46: "which match the given signature. "
> 47: "Multiple reference types will be returned if two or more class "

This file's name scares me. The fact that there's no "serviceability" label on 
this PR adds to this feeling. I would ask around if I were you.

-

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


Re: RFR: 8238373: Punctuation should be same in jlink help usage on Japanese language

2022-04-27 Thread Naoto Sato
On Wed, 27 Apr 2022 08:59:20 GMT, KIRIYAMA Takuya  wrote:

> When showing help for the jlink command in a Japanese locale, delimiters of 
> option's aliases are a mixture of "," and \u3001. Delimiter should be unified 
> to ",".
> As the same, there is a inconsistency of delimiters in the jar command help 
> in a Japanese locale, and "," should be used.
> Similarly, the javap command help uses space as delimiters other than the 
> module option in all locales. This inconsistency should also be corrected.
> 
> Would you please review this fix?

Looks fine to me. Nit: please modify the copyright years for `javap` properties 
files, as you modified the base (`javap.properties`) file.

-

Marked as reviewed by naoto (Reviewer).

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


Integrated: JDK-8236128: Allow jpackage create installers for services

2022-04-27 Thread Alexey Semenyuk
On Fri, 11 Mar 2022 23:37:06 GMT, Alexey Semenyuk  wrote:

> Implementation of [JDK-8275062: "Allow jpackage create installers for 
> services"](https://bugs.openjdk.java.net/browse/JDK-8275062)
>  CSR

This pull request has now been integrated.

Changeset: b675c597
Author:Alexey Semenyuk 
URL:   
https://git.openjdk.java.net/jdk/commit/b675c597e3f22af9e75992dab27001b9875af32e
Stats: 3116 lines in 64 files changed: 2848 ins; 121 del; 147 mod

8236128: Allow jpackage create installers for services

Reviewed-by: almatvee

-

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


Re: RFR: 8285485: Fix typos in corelibs

2022-04-27 Thread Naoto Sato
On Wed, 27 Apr 2022 06:39:30 GMT, Athijegannathan Sundararajan 
 wrote:

>> I ran `codespell` on modules owned by core-libs, and accepted those changes 
>> where it indeed discovered real typos.
>> 
>> I will update copyright years using a script before pushing (otherwise like 
>> every second change would be a copyright update, making reviewing much 
>> harder).
>> 
>> The long term goal here is to make tooling support for running `codespell`. 
>> The trouble with automating this is of course all false positives. But 
>> before even trying to solve that issue, all true positives must be fixed. 
>> Hence this PR.
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins_zh_CN.properties
>  line 188:
> 
>> 186: main.plugin.module=\u63D2\u4EF6\u6A21\u5757
>> 187: 
>> 188: main.plugin.category=\u7C7B\u522B
> 
> what's this?

Removing the trailing space? A similar one is in the `ja` resource bundle.

-

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


Re: RFR: 8285485: Fix typos in corelibs

2022-04-27 Thread Naoto Sato
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie  wrote:

> I ran `codespell` on modules owned by core-libs, and accepted those changes 
> where it indeed discovered real typos.
> 
> I will update copyright years using a script before pushing (otherwise like 
> every second change would be a copyright update, making reviewing much 
> harder).
> 
> The long term goal here is to make tooling support for running `codespell`. 
> The trouble with automating this is of course all false positives. But before 
> even trying to solve that issue, all true positives must be fixed. Hence this 
> PR.

Marked as reviewed by naoto (Reviewer).

-

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


Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]

2022-04-27 Thread Mandy Chung
On Wed, 27 Apr 2022 10:11:15 GMT, Kim Barrett  wrote:

>> Please review this fix to test/jdk/java/lang/ref/ReferenceClone.java.  It was
>> passing if CloneableReference::clone were to throw 
>> CloneNotSupportedException.
>> That should be a failure.
>> 
>> Testing:
>> Locally (linux-x64) verified test still passes.  Temporarily changed
>> CloneableReference::clone to throw and verified the expected failure gets
>> reported, unlike before.
>
> Kim Barrett has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   update copyright, @bug list

Marked as reviewed by mchung (Reviewer).

test/jdk/java/lang/ref/ReferenceClone.java line 52:

> 50: } catch (CloneNotSupportedException e) {
> 51: throw new RuntimeException("CloneableReference::clone should 
> not throw CloneNotSupportedException");
> 52: }

Alternatively, it could simply let CNSE propagate.


CloneableReference ref = new CloneableReference(o);
ref.clone();


`test()` and `main` will need to declare this checked exception.

-

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


Re: RFR: 8285658: Fix two typos in the spec of j.u.random.RandomGenerator [v3]

2022-04-27 Thread Brian Burkhalter
On Wed, 27 Apr 2022 07:34:29 GMT, Raffaello Giulietti  
wrote:

>> The spec of the interface `java.util.random.RandomGenerator` is slightly 
>> incorrect when it discusses `float` and `double` random values.
>
> Raffaello Giulietti has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8285658: Fix two typos in the spec of j.u.random.RandomGenerator

Marked as reviewed by bpb (Reviewer).

-

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


Re: RFR: 8285485: Fix typos in corelibs

2022-04-27 Thread Roger Riggs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie  wrote:

> I ran `codespell` on modules owned by core-libs, and accepted those changes 
> where it indeed discovered real typos.
> 
> I will update copyright years using a script before pushing (otherwise like 
> every second change would be a copyright update, making reviewing much 
> harder).
> 
> The long term goal here is to make tooling support for running `codespell`. 
> The trouble with automating this is of course all false positives. But before 
> even trying to solve that issue, all true positives must be fixed. Hence this 
> PR.

Since you've changed some code; you need to run tests for tiers 1-3.

(Note that even for trivial changes, hundreds of OpenJDK developers are 
notified and distracted; be considerate of other developers).

src/java.xml/share/classes/com/sun/xml/internal/stream/XMLEventReaderImpl.java 
line 140:

> 138: } else if(type == XMLEvent.START_ELEMENT) {
> 139: throw new XMLStreamException(
> 140: "elementGetText() function expects text only element but 
> START_ELEMENT was encountered.", event.getLocation());

Should we be fixing typos in third party software; it can easily get wiped out 
in an update.

src/java.xml/share/classes/com/sun/xml/internal/stream/writers/WriterUtility.java
 line 97:

> 95: }
> 96: else{
> 97: //attempt to retrieve default fEncoderoder

"fEncoderoder" looks out of place, please recheck.

-

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


Re: RFR: 8285690: CloneableReference subtest should not throw CloneNotSupportedException [v2]

2022-04-27 Thread Roger Riggs
On Wed, 27 Apr 2022 10:11:15 GMT, Kim Barrett  wrote:

>> Please review this fix to test/jdk/java/lang/ref/ReferenceClone.java.  It was
>> passing if CloneableReference::clone were to throw 
>> CloneNotSupportedException.
>> That should be a failure.
>> 
>> Testing:
>> Locally (linux-x64) verified test still passes.  Temporarily changed
>> CloneableReference::clone to throw and verified the expected failure gets
>> reported, unlike before.
>
> Kim Barrett has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   update copyright, @bug list

LGTM

-

Marked as reviewed by rriggs (Reviewer).

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


Re: RFR: JDK-8285730: unify _WIN32_WINNT settings

2022-04-27 Thread Alan Bateman
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken  wrote:

> Currently we set _WIN32_WINNT at various places in the codebase; this is used 
> to target a minimum Windows version we want to support. See also for more 
> detailled information :
> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt
> Macros for Conditional Declarations
> "Certain functions that depend on a particular version of Windows are 
> declared using conditional code. This enables you to use the compiler to 
> detect whether your application uses functions that are not supported on its 
> target version(s) of Windows."
> 
> However currently we have quite a lot of differing settings of _WIN32_WINNT 
> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible 
> would make sense because we have this setting already at   java_props_md.c  
> (so targeting older Windows versions at other places is most likely not 
> useful).

src/java.base/windows/native/libnio/ch/wepoll.c line 159:

> 157: 
> 158: #undef _WIN32_WINNT
> 159: #define _WIN32_WINNT 0x0601

This is 3rd party code and would prefer not change it if possible.

-

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


RFR: JDK-8285730: unify _WIN32_WINNT settings

2022-04-27 Thread Matthias Baesken
Currently we set _WIN32_WINNT at various places in the codebase; this is used 
to target a minimum Windows version we want to support. See also for more 
detailled information :
https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt
Macros for Conditional Declarations
"Certain functions that depend on a particular version of Windows are declared 
using conditional code. This enables you to use the compiler to detect whether 
your application uses functions that are not supported on its target version(s) 
of Windows."

However currently we have quite a lot of differing settings of _WIN32_WINNT in 
the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible would 
make sense because we have this setting already at   java_props_md.c  (so 
targeting older Windows versions at other places is most likely not useful).

-

Commit messages:
 - JDK-8285730

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

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Leonid Mesnik
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

Reviewed jvmti changes, hotspot tests (excluding tests modified by lmesnik), 
jdk svc tests changes.

-

Marked as reviewed by lmesnik (Reviewer).

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


Re: RFR: 8284950: CgroupV1 detection code should consider memory.swappiness [v6]

2022-04-27 Thread Severin Gehwolf
On Wed, 27 Apr 2022 10:34:33 GMT, xpbob  wrote:

>> set memory.swappiness to 0,swap space will not be used 
>> determine the value of memory.swappiness
>> https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt
>> 
>> 
>> Memory Limit: 50.00M
>> Memory Soft Limit: Unlimited
>> Memory & Swap Limit: 100.00M
>> Maximum Processes Limit: 4194305 
>> 
>> =>
>> 
>> Memory Limit: 50.00M
>> Memory Soft Limit: Unlimited
>> Memory & Swap Limit: 50.00M
>> Maximum Processes Limit: 4194305
>
> xpbob has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   revert file

Changes requested by sgehwolf (Reviewer).

test/hotspot/jtreg/containers/docker/TestMisc.java line 60:

> 58: testPrintContainerInfo();
> 59: testPrintContainerInfoActiveProcessorCount();
> 60: testPrintContainerMemoryInfo("100M", "150M");

Again. This test runs unconditionally. `--memory-swappiness` is not supported 
in cgroups v2. Thus, the test will fail on a cgroups v2 system. You need to 
only run this test on a cgroups v1 system. Have a look at 
`test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java` how could could 
detect this and only run on `cgroupv1` providers.

-

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


Re: RFR: 8284950: CgroupV1 detection code should consider memory.swappiness [v6]

2022-04-27 Thread Severin Gehwolf
On Wed, 27 Apr 2022 14:24:50 GMT, Severin Gehwolf  wrote:

>> xpbob has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   revert file
>
> test/hotspot/jtreg/containers/docker/TestMisc.java line 60:
> 
>> 58: testPrintContainerInfo();
>> 59: testPrintContainerInfoActiveProcessorCount();
>> 60: testPrintContainerMemoryInfo("100M", "150M");
> 
> Again. This test runs unconditionally. `--memory-swappiness` is not supported 
> in cgroups v2. Thus, the test will fail on a cgroups v2 system. You need to 
> only run this test on a cgroups v1 system. Have a look at 
> `test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java` how could could 
> detect this and only run on `cgroupv1` providers.

On my cgroups v2 system (i.e. it's using 200m memory + 200m swap):

$ sudo docker run --rm -ti --memory-swappiness=0 --memory=200m fedora:35
WARNING: Your kernel does not support memory swappiness capabilities or the 
cgroup is not mounted. Memory swappiness discarded.
[root@fb3182c8e23c /]# cat /sys/fs/cgroup/memory.max 
209715200
[root@fb3182c8e23c /]# cat /sys/fs/cgroup/memory.swap.max 
209715200

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Daniel Fuchs
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

Thanks for the refresh Alan! Things look good to me now. 
I have gone over java.io, the networking parts of java.nio, java.net, and the 
JMX related changes.
For what I have reviewed, I believe this is good to go.

-

Marked as reviewed by dfuchs (Reviewer).

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

2022-04-27 Thread Alan Bateman
> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
> JDK version to target.
> 
> We will refresh this PR periodically to pick up changes and fixes from the 
> loom repo.
> 
> Most of the new mechanisms in the HotSpot VM are disabled by default and 
> require running with `--enable-preview` to enable.
> 
> The patch has support for x64 and aarch64 on the usual operating systems 
> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for zero 
> and some of the other ports. Additional ports can be contributed via PRs 
> against the fibers branch in the loom repo.
> 
> There are changes in many areas. To reduce notifications/mails, the labels 
> have been trimmed down for now to hotspot, serviceability and core-libs. 
> We'll add the complete set of labels when the PR is further along.
> 
> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
> Doug Lea's CVS. These changes will probably be proposed and integrated in 
> advance of this PR.
> 
> The changes include some non-exposed and low-level infrastructure to support 
> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
> make life a bit easier and avoid having to separate VM changes and juggle 
> branches at this time.

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

  Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8166/files
  - new: https://git.openjdk.java.net/jdk/pull/8166/files/0864cb09..f2ed4f9c

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

  Stats: 505 lines in 15 files changed: 139 ins; 189 del; 177 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8166.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8166/head:pull/8166

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


Integrated: 8285440: Typo in Collections.addAll method javadoc

2022-04-27 Thread Johnny Lim
On Fri, 31 Dec 2021 18:58:43 GMT, Johnny Lim  wrote:

> This PR fixes a typo.

This pull request has now been integrated.

Changeset: 4919525d
Author:Johnny Lim 
Committer: Jaikiran Pai 
URL:   
https://git.openjdk.java.net/jdk/commit/4919525ddb55ba52d199a37c3b0e14e4a0c7c738
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8285440: Typo in Collections.addAll method javadoc

Reviewed-by: jpai, rriggs

-

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


Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]

2022-04-27 Thread Weijun Wang
On Fri, 22 Apr 2022 14:35:14 GMT, Sibabrata Sahoo  wrote:

>> A new API to support replacing selective lines with desired content.
>
> Sibabrata Sahoo has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update FileUtils.java

I've updated the title of the bug. Siba can update the PR title.

You're right that it's not easy to discover such APIs. We put it in `FileUtils` 
hoping people who does not want to write their own method will search from 
there first.

As for the API itself, we've imagined something like

FileUtils.patch(inputFile)
.with(1, 10, List.of(lines), List.of(newLines))
.with(100, 100, linesAsAString, newLinesAsAString)
.writeTo(outputFile)

but the current form is the simplest case and could be kept even if we have a 
verbose one.

-

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


Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]

2022-04-27 Thread Roger Riggs
On Fri, 22 Apr 2022 14:35:14 GMT, Sibabrata Sahoo  wrote:

>> A new API to support replacing selective lines with desired content.
>
> Sibabrata Sahoo has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update FileUtils.java

Please change the issue/pr title to indicate it is a new API in the **test** 
library.
The problem with single purpose APIs with not enough forethought is that they 
are not discoverable
in the library and fall into disuse or are not appropriate for someone else to 
find and use.

-

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


Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]

2022-04-27 Thread Weijun Wang
On Fri, 22 Apr 2022 14:35:14 GMT, Sibabrata Sahoo  wrote:

>> A new API to support replacing selective lines with desired content.
>
> Sibabrata Sahoo has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update FileUtils.java

Currently it's just for one test, but it's the exact kind of method that should 
go into the test library, i.e. a general purpose file manipulation utility that 
can be useful by everyone.

`patch` overwrites the input file (by default) too. We can always enhance it to 
support `-o`.

-

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


Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]

2022-04-27 Thread Roger Riggs
On Fri, 22 Apr 2022 14:35:14 GMT, Sibabrata Sahoo  wrote:

>> A new API to support replacing selective lines with desired content.
>
> Sibabrata Sahoo has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update FileUtils.java

If it is just for one/few tests, it would fit better in the directory with the 
test or in the test itself.
This doesn't seem like enough of a general purpose function to be in the test 
library.

Overwriting the input file seems like a bad choice, it will be a trap for 
someone.
It prevents the use of the function in a case where the output is written to a 
different file.

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v7]

2022-04-27 Thread Daniel Fuchs
On Tue, 26 Apr 2022 17:27:35 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69

test/jdk/java/net/vthread/HttpALot.java line 76:

> 74: var address = server.getAddress();
> 75: URL url = new URL("http://; + address.getHostName() + ":" + 
> address.getPort() + "/hello");
> 76: 

Nit: Ideally we should use the URIBuilder from the test library here, and the 
IP literal address to improve stability of the test on machines that may have 
strange /etc/hosts configuration. This can be taken care of after this PR has 
been integrated.

test/jdk/java/net/vthread/InterruptHttp.java line 88:

> 86: InetAddress lb = InetAddress.getLoopbackAddress();
> 87: listener = new ServerSocket(0, -1, lb);
> 88: address = "http://; + lb.getHostAddress() + ":" + 
> listener.getLocalPort();

Same remark about using URIBuilder here. It would take care of properly 
formatting the address in case of IPv6 literals. This can be taken care of 
after this PR has been integrated.

-

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


Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v7]

2022-04-27 Thread Alan Bateman
On Wed, 27 Apr 2022 11:34:51 GMT, Daniel Fuchs  wrote:

>> Alan Bateman has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69
>
> src/jdk.management/share/classes/com/sun/management/HotSpotDiagnosticMXBean.java
>  line 149:
> 
>> 147:  * access to the file or {@link 
>> java.lang.management.ManagementPermission
>> 148:  * ManagementPermission("control")} is denied
>> 149:  * @since 19
> 
> Maybe there ought to be an `@throws UnsupportedOperationException` here since 
> that is what the default implementation of the method is supposed to do.

Well spotted. The implSpec further up documents it but it should be in the 
throws list too.

> test/jdk/java/net/vthread/HttpALot.java line 76:
> 
>> 74: var address = server.getAddress();
>> 75: URL url = new URL("http://; + address.getHostName() + ":" + 
>> address.getPort() + "/hello");
>> 76: 
> 
> Nit: Ideally we should use the URIBuilder from the test library here, and the 
> IP literal address to improve stability of the test on machines that may have 
> strange /etc/hosts configuration. This can be taken care of after this PR has 
> been integrated.

A few of these tests started out as standalone classes that could be run 
without test infrastructure. This one, and InterruptHttp, can be easily changed 
to use the URIBuilder infra library, so we can do that.

-

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


Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]

2022-04-27 Thread Weijun Wang
On Fri, 22 Apr 2022 14:35:14 GMT, Sibabrata Sahoo  wrote:

>> A new API to support replacing selective lines with desired content.
>
> Sibabrata Sahoo has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update FileUtils.java

Or we can provide an example in the method spec.

-

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


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

2022-04-27 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:

  Simplify libraryLookup impl

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7888/files
  - new: https://git.openjdk.java.net/jdk/pull/7888/files/6990183f..5866bbb5

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

  Stats: 21 lines in 1 file changed: 6 ins; 11 del; 4 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: 8282191: Implementation of Foreign Function & Memory API (Preview) [v32]

2022-04-27 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 CSR comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7888/files
  - new: https://git.openjdk.java.net/jdk/pull/7888/files/746de5ce..6990183f

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

  Stats: 114 lines in 9 files changed: 36 ins; 0 del; 78 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: 8284161: Implementation of Virtual Threads (Preview) [v7]

2022-04-27 Thread Daniel Fuchs
On Tue, 26 Apr 2022 17:27:35 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69

src/jdk.management/share/classes/com/sun/management/HotSpotDiagnosticMXBean.java
 line 149:

> 147:  * access to the file or {@link 
> java.lang.management.ManagementPermission
> 148:  * ManagementPermission("control")} is denied
> 149:  * @since 19

Maybe there ought to be an `@throws UnsupportedOperationException` here since 
that is what the default implementation of the method is supposed to do.

-

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


Re: RFR: 8284950: CgroupV1 detection code should consider memory.swappiness [v6]

2022-04-27 Thread xpbob
On Wed, 27 Apr 2022 10:34:33 GMT, xpbob  wrote:

>> set memory.swappiness to 0,swap space will not be used 
>> determine the value of memory.swappiness
>> https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt
>> 
>> 
>> Memory Limit: 50.00M
>> Memory Soft Limit: Unlimited
>> Memory & Swap Limit: 100.00M
>> Maximum Processes Limit: 4194305 
>> 
>> =>
>> 
>> Memory Limit: 50.00M
>> Memory Soft Limit: Unlimited
>> Memory & Swap Limit: 50.00M
>> Maximum Processes Limit: 4194305
>
> xpbob has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   revert file

I refer to the content of this test
https://github.com/openjdk/jdk/blob/0f81d8fcc3fb703760b1cddb01861ea5031023fb/test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java#L119

-

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


Re: RFR: 8284950: CgroupV1 detection code should consider memory.swappiness [v6]

2022-04-27 Thread xpbob
On Wed, 27 Apr 2022 10:34:33 GMT, xpbob  wrote:

>> set memory.swappiness to 0,swap space will not be used 
>> determine the value of memory.swappiness
>> https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt
>> 
>> 
>> Memory Limit: 50.00M
>> Memory Soft Limit: Unlimited
>> Memory & Swap Limit: 100.00M
>> Maximum Processes Limit: 4194305 
>> 
>> =>
>> 
>> Memory Limit: 50.00M
>> Memory Soft Limit: Unlimited
>> Memory & Swap Limit: 50.00M
>> Maximum Processes Limit: 4194305
>
> xpbob has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   revert file

Thanks for review.
@iklam @jerboaa 
I added test to TestMisc with print_container_info and -XshowSettings:system

-

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


Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces

2022-04-27 Thread Pavel Rappo
On Tue, 26 Apr 2022 22:24:26 GMT, Joe Darcy  wrote:

> To enable more complete doclint checking (courtesy @jonathan-gibbons), please 
> review this PR to add type-level @param tags where they are missing.
> 
> To the maintainers of java.util.concurrent, those changes could be separated 
> out in another bug if that would ease maintenance of that code.
> 
> Making these library fixes is a blocker for correcting and expanding the 
> doclint checks (JDK-8285496).
> 
> I'll update copyright years before pushing.

I note that many years ago you filed JDK-6327933, which I'm currently looking 
at. If implemented, many of the issues from this PR will be addressed 
automatically, including those in `java.util.concurrent`.

-

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


Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces

2022-04-27 Thread Daniel Fuchs
On Tue, 26 Apr 2022 22:24:26 GMT, Joe Darcy  wrote:

> To enable more complete doclint checking (courtesy @jonathan-gibbons), please 
> review this PR to add type-level @param tags where they are missing.
> 
> To the maintainers of java.util.concurrent, those changes could be separated 
> out in another bug if that would ease maintenance of that code.
> 
> Making these library fixes is a blocker for correcting and expanding the 
> doclint checks (JDK-8285496).
> 
> I'll update copyright years before pushing.

Hi Joe, just two suggestions about the javax.management changes. Otherwise 
looks good!

src/java.management/share/classes/javax/management/openmbean/ArrayType.java 
line 96:

> 94:  *
> 95:  * @param  the Java type that instances described by this type must
> 96:  * have.

Instead of "instances" - would it be more correct to say "array elements"?

src/java.management/share/classes/javax/management/openmbean/SimpleType.java 
line 60:

> 58:  * @param  the Java type that instances described by this type must
> 59:  * have.
> 60:  *

I would suggest to say "that values described by this type"...

-

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


RE: System.exit deadlock if called within a hook

2022-04-27 Thread remi.catherinot
Ok,

Yes that would be hard to track down hooks "blocked" either directly or 
indirectly that way. Indirect blocking case are the hardest one, like a hook 
joining another thread that is blocked calling System.exit. This hook might 
really be blocked if joining again and again regardless of any join 
timeout/join interruption, and maybe just delayed because it joined with a 
timeout without retrying after a timeout: in both case, at some point, both 
logic (the blocked and not-so-blocked ones) have the same kind of stack and 
inter-thread join sequence. So scanning on join-chains is not reliable. So 
detecting blocked hooks is not either. I might have overlooked other solutions, 
but I think keeping the blocked threads alive and try to be smart at having the 
hook detected/handle them is a deadend.

Having a thread calling System.exit while System.exit is already in progress 
being able to "exit" itself (like pthread_exit for posix threads for exemple) 
would also solve those hook joining blocked thread scenario.

Having a thread being able to cease execution change what could be expected 
from the documented behavior. In term of execution flow, having the thread 
blocked indefinitely on a monitor enter or have it cease execution is the same 
: it won't run any other piece of code. Calling Thread.stop while on threads 
trying to monitor enter the synchronized block has no effect: they need to 
"enter" the monitor before being able to see the ThreadDeath error to throw. 
The thread running inside the synchronized block will never do a monitor exit 
since it calls halt within the synchronized boundaries. The difference between 
blocked versus cease is the garbage collector behavior: the blocked thread is 
still alive, still holds refs that cannot be GCed. the thread that cease 
execution allow objects to be GCed. Even if I don't think that would really be 
an issue, I still have to mention it.

By the way, the hook calling System.exit might be tricky to see/avoid. My demo 
code is simple but I got this more or less in this kind of execution flow
public static void main(String[] args) {
Thread.setDefaultUncaughtExceptionHandler((th, trw)->{
/* if trw is considered a fatal throwable then ask for a graceful JVM 
shutdown */ System.exit(1);
/* OOM errors often screw too many things, leaving not-daemon thread 
hanging around, preventing JVM standard shutdown without either a System.exit 
or a Runtime.halt, so I think OOM errors are good candidates for being fatal 
throwables */
});
Runtime.getRuntime().addShutdownHook(new Thread("hook") {
public void run() {
// the hook does not need to do a hard work to throw an OOM, if 
running inside a saturated JVM, just a new String to log some message will do
throw new OutOfMemoryError("error in hook");
}
});
}

Regards,
Remi


Orange Restricted

-Message d'origine-
De : David Holmes  
Envoyé : mardi 26 avril 2022 01:24
À : CATHERINOT Rémi DTSI/PFS ; 
core-libs-dev@openjdk.java.net
Objet : Re: System.exit deadlock if called within a hook

On 25/04/2022 11:08 pm, remi.catheri...@orange.com wrote:
> Hi,
> 
> Exemple code to deadlock the JVM. Only Runtime.halt from within the JVM or a 
> SIGKILL from outside stops it. Normal kills and Ctrl-C (which is a SIGTERM) 
> fail to do so.
> 
> public class ExitInHookDemo {
>  public static void main(String...args) {
>  Runtime.getRuntime().addShutdownHook(new 
> Thread("hook") {
>  public void run() { 
> System.exit(1); } });
>  }
> }

This is a documented usage limitation of exit().

" All registered shutdown hooks, if any, are started in some unspecified order 
and allowed to run concurrently until they finish. Once this is done the 
virtual machine halts.

If this method is invoked after all shutdown hooks have already been run and 
the status is nonzero then this method halts the virtual machine with the given 
status code. Otherwise, this method blocks indefinitely. "

Here you have a hook that calls exit() before all hooks have been run, hence it 
blocks indefinitely. As the hook blocks indefinitely the running of hooks 
cannot finish as required and so the virtual machine does not halt.

Conceptually you are asking for the indefinitely blocked hook to be treated 
as-if "finished", but there is no actual mechanism to implement that. You don't 
need to "destroy" the thread to force it to be "finished", but you do need a 
way for exit() to check if the current thread is a hook and to adapt the logic 
so that thread.join() is not used to wait for true termination. It would be 
possible to construct something I think, but I don't know if such an 
enhancement would be considered worthwhile versus don't call exit() from a hook.

Cheers,
David
-

> I've tried to more or less do a pure java patch in java.lang.Shutdown 
> but the only way to have a real patch would be to 

  1   2   >