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: 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: 8284161: Implementation of Virtual Threads (Preview) [v7]

2022-04-27 Thread Lance Andersen
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

I have been through all of the tests and they look good.

-

Marked as reviewed by lancea (Reviewer).

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


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

2022-04-27 Thread Thomas Schatzl
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

Reviewed changes in gc/, oops/, memory/, partially in runtime/.

-

Marked as reviewed by tschatzl (Reviewer).

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


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

2022-04-27 Thread Stefan Karlsson
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

Similar comments to @fisk.

This is what I've been focusing on:
* In-depth:  gc/, oops/, memory/, utilities/
* Partially: runtime/
* Cleanups: hotspot/

-

Marked as reviewed by stefank (Reviewer).

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


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

2022-04-27 Thread Serguei Spitsyn
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

I've reviewed JDI, JDB and JDWP changes.
All look good.
Thanks,
Serguei

-

Marked as reviewed by sspitsyn (Reviewer).

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


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

2022-04-27 Thread Robbin Ehn
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

I have looked mostly on runtime, and to much lesser extent the others parts.
There are some loose ends, but nothing major that can't wait, so I think it's 
time.
Looks fine, thanks.

-

Marked as reviewed by rehn (Reviewer).

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


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

2022-04-26 Thread Alan Bateman
On Tue, 26 Apr 2022 18:58:23 GMT, Daniel Fuchs  wrote:

>> Alan Bateman has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69
>
> src/java.base/share/classes/sun/nio/ch/DatagramChannelImpl.java line 770:
> 
>> 768: synchronized (p) {
>> 769: DatagramPackets.setLength(p, n);
>> 770: p.setSocketAddress(sender);
> 
> Hmmm... For integrity it might be better to call the public  
> `DatagramPacket::setData(byte[] buf, int offset, int length)` method here 
> now. The additional advantage is that the private access to  
> `DatagramPackets.setLength(p, n);` will no longer be needed.

DatagramPackets.setLength is not really new, it's just moved. There's a flaw in 
DatagramPacket that forces its usage -  the details are in JDK-8232817.

-

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


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

2022-04-26 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/java.base/share/classes/sun/nio/ch/DatagramChannelImpl.java line 770:

> 768: synchronized (p) {
> 769: DatagramPackets.setLength(p, n);
> 770: p.setSocketAddress(sender);

Hmmm... For integrity it might be better to call the public  
`DatagramPacket::setData(byte[] buf, int offset, int length)` method here now. 
The additional advantage is that the private access to  
`DatagramPackets.setLength(p, n);` will no longer be needed.

-

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


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

2022-04-26 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 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69

-

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

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8166&range=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8166&range=05-06

  Stats: 582 lines in 34 files changed: 285 ins; 128 del; 169 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