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

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

test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp
 line 66:

> 64:   for (i = 0; i < METH_NUM; i++)
> 65: bpEvents[i] = 0;
> 66: }

As I understand all new tests are C++ based.
So, I'd suggest to always use C++ style of loops:
  for (int i = 0; i < METH_NUM; i++)

-

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


Re: RFR: JDK-8285730: unify _WIN32_WINNT settings

2022-04-28 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).

That only seems to be half of the issue though. If we are defining 
_WIN32_WINNT=0x0601 because the minimum required OS API support level is 
Windows 7, then don't we need a check that the build platform is also at least 
Windows 7?

-

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


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

2022-04-28 Thread Will
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 will-mol...@github.com (no known OpenJDK username).

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v5]

2022-04-28 Thread Joe Darcy
On Fri, 29 Apr 2022 02:54:06 GMT, Joe Darcy  wrote:

> > Is there intent to backport this to the RI for JSR 337 MR 4?
> 
> I think that would be helpful, if not strictly necessary.

PS And if backported, the spec could be updated for earlier release trains to 
start at a different value like "4".

-

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


Re: RFR: 8285295: Need better testing for IdentityHashMap [v2]

2022-04-28 Thread Stuart Marks
On Fri, 29 Apr 2022 03:00:40 GMT, Stuart Marks  wrote:

>> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch 
>> in the bug report that breaks `IdentityHashMap` now causes several cases in 
>> this new test to fail. There's more that could be done, but the new tests 
>> cover most of the core functions of `IdentityHashMap`. Unfortunately it 
>> seems difficult to merge this with the existing, comprehensive Collections 
>> tests (e.g., MOAT.java) because those tests implicity rely on 
>> `equals()`-based contract instead of the special-purpose `==`-based contract 
>> used by `IdentityHashMap`.
>
> Stuart Marks has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Assertions over return values. Some refinement of equals() testing.
>  - Add comment about Map.Entry identity not guaranteed.

I've added checking of return values for I think everything that has a 
significant return value. I've elected to store the return value in a local 
variable and add a separate assert line. For example, instead of

assertNull(map.put(newKey, newVal));

I've done

Box r = map.put(newKey, newVal);
assertNull(r);

The reason is that I think it separates the test setup/action from the test 
assertions. I tried it the first way, but it felt like the lack of this 
separation made things messy.

I've also added a couple more tests over `equals` and added more asserts over 
`equals` to the `keySet` and `entrySet` view sets. (Note, the `values` 
collection is just a `Collection` and thus doesn't have a defined notion of 
`equals`.) The testing of the view collections could probably be made more 
comprehensive, but I think what's here is a good start.

Please take a look.

-

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


Re: RFR: 8285295: Need better testing for IdentityHashMap [v2]

2022-04-28 Thread Stuart Marks
> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch 
> in the bug report that breaks `IdentityHashMap` now causes several cases in 
> this new test to fail. There's more that could be done, but the new tests 
> cover most of the core functions of `IdentityHashMap`. Unfortunately it seems 
> difficult to merge this with the existing, comprehensive Collections tests 
> (e.g., MOAT.java) because those tests implicity rely on `equals()`-based 
> contract instead of the special-purpose `==`-based contract used by 
> `IdentityHashMap`.

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

 - Assertions over return values. Some refinement of equals() testing.
 - Add comment about Map.Entry identity not guaranteed.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8354/files
  - new: https://git.openjdk.java.net/jdk/pull/8354/files/5857fe6f..d66ad0b8

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

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

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v5]

2022-04-28 Thread Joe Darcy
On Fri, 29 Apr 2022 02:12:19 GMT, Iris Clark  wrote:

>> Joe Darcy has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Change punctuation from review feedback.
>
> src/java.base/share/classes/java/lang/System.java line 743:
> 
>> 741:  * have the value {@code "1"}; after a second maintenance
>> 742:  * release, this property will have the value {@code "2"},
>> 743:  * and so on.
> 
> There should be no requirement that values be allocated sequentially.  In 
> other words, if JCP MR  does not require an RI, then it should not be 
> surprising if there is no JDK build with maintenance version .  As an 
> example, JSR 337 MR 1 and MR 2 both used the same RI.  If this system 
> property had existed during development of MR 1, it would return "1".  Since 
> no RI was submitted for MR 2, a build returning "2" should never exist.  MR 3 
> did update the RI, so it would return "3".

> Is there intent to backport this to the RI for JSR 337 MR 4?

I think that would be helpful, if not strictly necessary.

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v5]

2022-04-28 Thread Joe Darcy
On Fri, 29 Apr 2022 02:00:08 GMT, Iris Clark  wrote:

>> Joe Darcy has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Change punctuation from review feedback.
>
> src/java.base/share/classes/java/lang/VersionProps.java.template line 113:
> 
>> 111: props.put("java.specification.version", VERSION_SPECIFICATION);
>> 112: 
>> 113: // Uncomment next props.put call after the first maintenance 
>> release for a
> 
> Is "after" correct?  I think this value should be set to the target MR at the 
> beginning of RI development for the release, similar to what is done for 
> other version identification system properties.

Well, sure -- this versioning info should be updated in the same way we update 
versions for all the early access builds of a release.

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v5]

2022-04-28 Thread Iris Clark
On Fri, 29 Apr 2022 01:20:17 GMT, Joe Darcy  wrote:

>> 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.
>
> Joe Darcy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Change punctuation from review feedback.

Is there intent to backport this to the RI for JSR 337 MR 4?

src/java.base/share/classes/java/lang/System.java line 743:

> 741:  * have the value {@code "1"}; after a second maintenance
> 742:  * release, this property will have the value {@code "2"},
> 743:  * and so on.

There should be no requirement that values be allocated sequentially.  In other 
words, if JCP MR  does not require an RI, then it should not be surprising 
if there is no JDK build with maintenance version .  As an example, JSR 337 
MR 1 and MR 2 both used the same RI.  If this system property had existed 
during development of MR 1, it would return "1".  Since no RI was submitted for 
MR 2, a build returning "2" should never exist.  MR 3 did update the RI, so it 
would return "3".

src/java.base/share/classes/java/lang/VersionProps.java.template line 113:

> 111: props.put("java.specification.version", VERSION_SPECIFICATION);
> 112: 
> 113: // Uncomment next props.put call after the first maintenance 
> release for a

Is "after" correct?  I think this value should be set to the target MR at the 
beginning of RI development for the release, similar to what is done for other 
version identification system properties.

-

Changes requested by iris (Reviewer).

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


Re: Improve `finally` block exception handling

2022-04-28 Thread David Holmes
On 21/04/2022 7:30 am, 
some-java-user-99206970363698485...@vodafonemail.de wrote:

Thanks a lot for your feedback! My original message was a bit vague in parts, 
sorry for that.

The proposal consists of the following:
1. Forbidding usage of `break`, `continue`, `yield` and `return` in `finally` 
blocks
2. Adding exceptions as suppressed exceptions:
If exception E1 led to execution of the `finally` block and the block is 
left due to
another exception E2, then either E1 or E2 should be thrown with the other 
one added
as suppressed exception. For consistency with try-with-resources and 
because E1 is
most likely more relevant ideally E1 would be thrown and E2 should be 
suppressed. But
if you think the impact on backward compatibility would be too large, then 
E2 should
be thrown (the current behavior), but E1 should at least be added as 
suppressed exception.

The following replies to your comments hopefully make the rationale for these 
proposed
changes clearer.



The behaviour of try/catch/finally is not "obvious at all", you have to read 
what it means
and when you do that you clearly see what happens regarding exceptions.


For try-catch you see that the code catches some specific exception and then 
handles it in a
certain way, whether that is by rethrowing, logging or itentionally ignoring it.
The issue with `finally` is that in its current form, exceptions which occurred 
in the `try`
block might just silently disappear. Consider this simple example:
```
try {
 throw new RuntimeException();
} finally {
 return true;
}
```
Here it is not clear at all, unless you have read the JLS in detail, that the 
thrown exception
just vanishes. There is no explicit indication that the `finally` has any 
effect on the
thrown exception. Of course this is a contrived example, but consider the same 
situation where
the `try` block calls a method which might throw an exception (or the general 
case that a
VirtualMachineError occurs), and that the `return` (or any of the other 
affected statements)
is not the only statement in the `finally` block.

Similar confusing code can be written where the `try` or `catch` block returns 
a value
(or continues or breaks loop iteration), but the `finally` block overwrites 
that action:
```
try {
 return 1;
} finally {
 return 2;
}
```
Again, contrived, but consider the same code with additional statements in the 
`try` and
`finally` blocks. This breaks fundamental assumptions one has about the 
behavior of the
statements, such as `return`.


I still maintain this is simply a matter of user education. You have no 
business using a finally block if you have never bothered to even learn 
how it works. Sorry no sympathy from me on that one.



Are there any plans to forbid usage of `break`, `continue`, `yield` and 
`return` in
`finally` blocks?


Why would we do that? What would that gain?


It would prevent code such as the one shown above, where code, most likely 
unintentionally,
discards exceptions. Now also consider that inside the `try` block a 
VirtualMachineError
(or a subclass of it) may occur which you really should not catch. Yet the code 
in the
`finally` block silently discards it without you ever noticing the error, 
before it occurs
later in another part of the application again and has possibly already 
corrupted the state
of the application.


Again this is primarily user education. And there are times when you 
definitely do want to use those statements and you would have to jump 
through hoops to get the same effect if they were banned.


But in terms of "are there any plans ..." the answer is a clear no as 
the compatibility issues would make this a non-starter.





Similarly for `throw` and any implicitly thrown exceptions, is there a plan to 
at least
add the original exception as suppressed one to the newly thrown?


That suggestion is not completely without merit, but nor is it a "slam-dunk" obvious thing to do. 
The semantic implications of the exceptions matter, and semantics come from the programmers intent. There's 
no reasonable way to automagically determine that when an exception is created that another exception (that 
led to the finally block) should be inserted as a "suppressed exception". That would actually be 
completely wrong to do in many circumstances you would instead need to act when the exception would terminate 
the finally block and then add the original exception as the "suppressed" exception.


To clarify my suggestion: If a `finally` block is entered due to an exception 
E1, and is
exited due to an exception E2 (regardless of whether explicitly thrown by a 
`throw` statement),
and E1 != E2, then both exceptions should be preserved, with one being added as 
suppressed
exception to the other one.


That is not unreasonable: E2 suppresses E1.




But really the programmer is in the best position to decide how exceptions need 
to be handled.


Except that in a `finally` block they don't have access 

Re: RFR: 8285295: Need better testing for IdentityHashMap

2022-04-28 Thread Jaikiran Pai
On Fri, 29 Apr 2022 00:06:32 GMT, Stuart Marks  wrote:

>> test/jdk/java/util/IdentityHashMap/Basic.java line 257:
>> 
>>> 255: checkEntries(map.entrySet(), entry(k1b, v1b),
>>> 256:  entry(k2, v2));
>>> 257: }
>> 
>> Would an additional check `assertFalse(map.equals(map2));` be useful here 
>> (and other similar tests where we do "remove").
>
> I don't know if you noticed that previous versions checked the map's contents 
> with `map.equals(map2)` and either asserted true or false depending on the 
> situation. I pulled most of those out when I added `checkEntries`. The reason 
> is that `checkEntries` and `checkElements` are supposed to check the exact 
> contents of the map or the collection, and they fail if there is a missing, 
> different, or extra entry or element. If that's true we don't need to test 
> `map.equals`. I don't think it calling `map.equals` adds any value or does 
> any checking that the `checkX` methods don't already do.
> 
> Of course this relies on `checkEntries` and `checkElements` to do their jobs 
> properly, so we should make sure they do!
> 
> And also we need to test that the `equals` method is doing its job as well. 
> There are a couple tests for it already, and they test at least the basic 
> cases. But it's possible I might have missed something.
> 
> In any case, if we believe the `checkX` methods are sufficient, and if we 
> believe that the tests for `equals` are also sufficient, then I don't think 
> we need to add assertions about `equals` in any tests other than for `equals` 
> itself.

Hello Stuart,

Thank you for the explanation.

> In any case, if we believe the checkX methods are sufficient, and if we 
> believe that the tests for equals are also sufficient, then I don't think we 
> need to add assertions about equals in any tests other than for equals itself.

That makes sense.

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v5]

2022-04-28 Thread Jaikiran Pai
On Fri, 29 Apr 2022 01:20:17 GMT, Joe Darcy  wrote:

>> 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.
>
> Joe Darcy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Change punctuation from review feedback.

Thank you for the updates, Joe. Looks good to me.

-

Marked as reviewed by jpai (Committer).

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v4]

2022-04-28 Thread Joe Darcy
On Thu, 28 Apr 2022 23:56:27 GMT, Mark Reinhold  wrote:

>> Joe Darcy has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Respond to review feedback; make sequence of values explicit.
>
> src/java.base/share/classes/java/lang/System.java line 740:
> 
>> 738:  * > href="https://jcp.org/en/procedures/jcp2#3.6.4;>maintenance
>> 739:  * release (optional).
>> 740:  * After a first maintenance release, this property will
> 
> Separate independent clauses with a semicolon: `After a first maintenance 
> release this property will have the value {@code "1"}; after a second 
> maintenance release it will have the value {@code "2"}, and so on.`

PR and CSR updated.

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v5]

2022-04-28 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.

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

  Change punctuation from review feedback.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8437/files
  - new: https://git.openjdk.java.net/jdk/pull/8437/files/461407aa..2048aaac

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

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 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-28 Thread Guoxiong Li
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

Remind: please use the command `/jep JEP-425` [1] to mark this PR.

[1] 
https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/jep

-

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


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

2022-04-28 Thread Guoxiong Li
On Thu, 28 Apr 2022 18:10:57 GMT, Maurizio Cimadamore  
wrote:

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

Remind: please use the command `/jep JEP-424` [1] to mark this PR.

[1] 
https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/jep

-

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


Re: RFR: 8285295: Need better testing for IdentityHashMap

2022-04-28 Thread Stuart Marks
On Thu, 28 Apr 2022 13:17:59 GMT, Jaikiran Pai  wrote:

>> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch 
>> in the bug report that breaks `IdentityHashMap` now causes several cases in 
>> this new test to fail. There's more that could be done, but the new tests 
>> cover most of the core functions of `IdentityHashMap`. Unfortunately it 
>> seems difficult to merge this with the existing, comprehensive Collections 
>> tests (e.g., MOAT.java) because those tests implicity rely on 
>> `equals()`-based contract instead of the special-purpose `==`-based contract 
>> used by `IdentityHashMap`.
>
> test/jdk/java/util/IdentityHashMap/Basic.java line 257:
> 
>> 255: checkEntries(map.entrySet(), entry(k1b, v1b),
>> 256:  entry(k2, v2));
>> 257: }
> 
> Would an additional check `assertFalse(map.equals(map2));` be useful here 
> (and other similar tests where we do "remove").

I don't know if you noticed that previous versions checked the map's contents 
with `map.equals(map2)` and either asserted true or false depending on the 
situation. I pulled most of those out when I added `checkEntries`. The reason 
is that `checkEntries` and `checkElements` are supposed to check the exact 
contents of the map or the collection, and they fail if there is a missing, 
different, or extra entry or element. If that's true we don't need to test 
`map.equals`. I don't think it calling `map.equals` adds any value or does any 
checking that the `checkX` methods don't already do.

Of course this relies on `checkEntries` and `checkElements` to do their jobs 
properly, so we should make sure they do!

And also we need to test that the `equals` method is doing its job as well. 
There are a couple tests for it already, and they test at least the basic 
cases. But it's possible I might have missed something.

In any case, if we believe the `checkX` methods are sufficient, and if we 
believe that the tests for `equals` are also sufficient, then I don't think we 
need to add assertions about `equals` in any tests other than for `equals` 
itself.

-

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


Re: RFR: 8285295: Need better testing for IdentityHashMap

2022-04-28 Thread Stuart Marks
On Thu, 28 Apr 2022 13:22:34 GMT, Jaikiran Pai  wrote:

>> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch 
>> in the bug report that breaks `IdentityHashMap` now causes several cases in 
>> this new test to fail. There's more that could be done, but the new tests 
>> cover most of the core functions of `IdentityHashMap`. Unfortunately it 
>> seems difficult to merge this with the existing, comprehensive Collections 
>> tests (e.g., MOAT.java) because those tests implicity rely on 
>> `equals()`-based contract instead of the special-purpose `==`-based contract 
>> used by `IdentityHashMap`.
>
> test/jdk/java/util/IdentityHashMap/Basic.java line 337:
> 
>> 335: public void testPutOverwrite() {
>> 336: Box newVal = new Box(v1a);
>> 337: map.put(k1a, newVal);
> 
> Should we capture the return value and assert that it's not null and is 
> identity equal to `v1a`?
> 
> Perhaps, similarly at a few other places where we do put and putIfAbsent?

Yes, good point, I'll add checks of the return values in the appropriate 
places. There are several, including `remove`, `computeX`, `put`, etc.

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v4]

2022-04-28 Thread Mark Reinhold
On Thu, 28 Apr 2022 20:54:29 GMT, Joe Darcy  wrote:

>> 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.
>
> Joe Darcy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Respond to review feedback; make sequence of values explicit.

src/java.base/share/classes/java/lang/System.java line 740:

> 738:  *  href="https://jcp.org/en/procedures/jcp2#3.6.4;>maintenance
> 739:  * release (optional).
> 740:  * After a first maintenance release, this property will

Separate independent clauses with a semicolon: `After a first maintenance 
release this property will have the value {@code "1"}; after a second 
maintenance release it will have the value {@code "2"}, and so on.`

-

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


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

2022-04-28 Thread Jie Fu
On Thu, 28 Apr 2022 19:48:18 GMT, Paul Sandoz  wrote:

> It should be possible for you finalize now.

Done.
Thanks @PaulSandoz .

-

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


java.net.http.HttpClient do NOT support Digest authentication while HttpURLConnection do

2022-04-28 Thread Farkas Levente
Hi,

Even though many tutorial said that the new java.net.http.HttpClient
support Digest authentication it's not true:
https://github.com/openjdk/jdk/blob/master/src/java.net.http/share/classes/jdk/internal/net/http/AuthenticationFilter.java#L278
But at the same time HttpURLConnection support it through the simple:
---
Authenticator.setDefault(new Authenticator() {
@Override
protected PasswordAuthentication getPasswordAuthentication() {

return new PasswordAuthentication (
"username",
"password".toCharArray()
);
}
});
---
Is it a bug or you don't even plan to support it with HttpClient?
Regards.

-- 
  Levente   "Si vis pacem para bellum!"


Re: RFR: 8285523: Improve test java/io/FileOutputStream/OpenNUL.java

2022-04-28 Thread Sergey Bylokhov
On Thu, 28 Apr 2022 15:43:09 GMT, Andrew John Hughes  wrote:

>> The new test added as part of the 
>> [JDK-8285445](https://bugs.openjdk.java.net/browse/JDK-8285445) cannot 
>> trigger that bug and pass w/ and w/o fix.
>> 
>> An updated test validates the "default" case when the 
>> `jdk.io.File.enableADS` property is not set, in this case the ADS should be 
>> [accepted](https://github.com/openjdk/jdk/blob/9d9f4e502f6ddc3116ed9b80f7168a1edfce839e/src/java.base/windows/classes/java/io/WinNTFileSystem.java#L59).
>
> test/jdk/java/io/FileOutputStream/OpenNUL.java line 31:
> 
>> 29:  * @run main/othervm OpenNUL
>> 30:  * @run main/othervm -Djdk.io.File.enableADS OpenNUL
>> 31:  * @run main/othervm -Djdk.io.File.enableADS=FalsE OpenNUL
> 
> Is there a reason for the mixed case? Just to check equalsIgnoreCase is being 
> used?

Yes only for equalsIgnoreCase

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v4]

2022-04-28 Thread Sean Mullan
On Thu, 28 Apr 2022 20:54:29 GMT, Joe Darcy  wrote:

>> 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.
>
> Joe Darcy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Respond to review feedback; make sequence of values explicit.

Marked as reviewed by mullan (Reviewer).

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v2]

2022-04-28 Thread Joe Darcy
On Thu, 28 Apr 2022 20:15:44 GMT, Joe Darcy  wrote:

> > Also, don't forget to update the CSR with the new specification text, for 
> > the record. Otherwise, looks good!
> 
> Will do; I'll update the CSR to reflect that change as well as to be more 
> explicit about the sequence of values, once that is determined.

CSR now updated accordingly.

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v4]

2022-04-28 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.

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

  Respond to review feedback; make sequence of values explicit.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8437/files
  - new: https://git.openjdk.java.net/jdk/pull/8437/files/fc7730d6..461407aa

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

  Stats: 6 lines in 1 file changed: 5 ins; 0 del; 1 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: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

2022-04-28 Thread Brian Burkhalter
On Thu, 28 Apr 2022 20:13:48 GMT, Uwe Schindler  wrote:

> By the way: FileOutputStream has exactly the same problem with 
> `write(byte[])`. I see no test for it, but I assume this is now also fixed. 
> That's a longstanding issue in Lucene (this is why we use a 
> ChunkedOutputStream when writing files.

Yes, `write(byte[])` is also fixed: 
[io_util.c#L164](https://github.com/bplb/jdk/blob/40d46f8f4463dbd7dbe10651910826e90ca4dbdd/src/java.base/share/native/libjava/io_util.c#L164).

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v3]

2022-04-28 Thread Mark Reinhold
On Thu, 28 Apr 2022 20:15:36 GMT, Joe Darcy  wrote:

>> 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.
>
> Joe Darcy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Respond to review feedback.

Marked as reviewed by mr (Lead).

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v2]

2022-04-28 Thread Sean Mullan
On Thu, 28 Apr 2022 20:07:41 GMT, Joe Darcy  wrote:

>> src/java.base/share/conf/security/java.policy line 34:
>> 
>>> 32:"java.specification.version", "read";
>>> 33: permission java.util.PropertyPermission
>>> 34:"java.specification.maintenance.version", "read";
>> 
>> For consistency, you should probably also add a hard-coded permission to the 
>> `sun.security.provider.PolicyFile.initStaticPolicy()` method which is used 
>> as a fallback if there is a problem parsing the `java.policy` file. I doubt 
>> this will cause any issues as I don't suspect many/any applications will 
>> suddenly start reading this property and running with an SM but again it is 
>> mainly for consistency as all the other default permissions in this file are 
>> also granted in that method.
>
> Will do; thanks. Just to double-check, under the current proposal, this 
> property will be undefined most of the time. I assume it is fine for the 
> permissions to grant the ability to read a property that is not actually 
> there.

Yes.

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v2]

2022-04-28 Thread Joe Darcy
On Thu, 28 Apr 2022 18:53:11 GMT, Mark Reinhold  wrote:

> Also, don't forget to update the CSR with the new specification text, for the 
> record. Otherwise, looks good!

Will do; I'll update the CSR to reflect that change as well as to be more 
explicit about the sequence of values, once that is determined.

-

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


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

2022-04-28 Thread Joe Darcy
On Thu, 28 Apr 2022 10:24:46 GMT, Jaikiran Pai  wrote:

> Hello Joe, should the property description have a note stating what kind of a 
> value this property holds, if at all present? Would it be free form text or 
> an integer value?



> 

There is some discussion about the sequence of values in comments on the JBS 
issue. I think it would be reasonable to have the sequence:

undefined -> 1 -> 2 -> 3 ...

where 1, 2, 3 are strings. I'll update the text to be more explicit about the 
eventual sequence once the discussion is concluded; thanks.

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v2]

2022-04-28 Thread Joe Darcy
On Thu, 28 Apr 2022 19:37:00 GMT, Sean Mullan  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 five additional commits 
>> since the last revision:
>> 
>>  - Respond to review feedback.
>>  - Respond to CSR feedback.
>>  - Merge branch 'master' into JDK-8285497
>>  - Update comment in template.
>>  - JDK-8285497: Add system property for Java SE specification maintenance 
>> version
>
> src/java.base/share/conf/security/java.policy line 34:
> 
>> 32:"java.specification.version", "read";
>> 33: permission java.util.PropertyPermission
>> 34:"java.specification.maintenance.version", "read";
> 
> For consistency, you should probably also add a hard-coded permission to the 
> `sun.security.provider.PolicyFile.initStaticPolicy()` method which is used as 
> a fallback if there is a problem parsing the `java.policy` file. I doubt this 
> will cause any issues as I don't suspect many/any applications will suddenly 
> start reading this property and running with an SM but again it is mainly for 
> consistency as all the other default permissions in this file are also 
> granted in that method.

Will do; thanks. Just to double-check, under the current proposal, this 
property will be undefined most of the time. I assume it is fine for the 
permissions to grant the ability to read a property that is not actually there.

-

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


Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

2022-04-28 Thread Uwe Schindler
On Thu, 28 Apr 2022 20:02:36 GMT, Brian Burkhalter  wrote:

>> Modify native multi-byte read-write code used by the `java.io` classes to 
>> limit the size of the allocated native buffer thereby decreasing off-heap 
>> memory footprint and increasing throughput.
>
> Brian Burkhalter has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   6478546: Decrease malloc'ed buffer maximum size to 64 kB

By the way: FileOutputStream has exactly the same problem with `write(byte[])`. 
I see no test for it, but I assume this is now also fixed. That's a 
longstanding issue in Lucene (this is why we use a ChunkedOutputStream when 
writing files.

-

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


Re: RFR: JDK-8285497: Add system property for Java SE specification maintenance version [v3]

2022-04-28 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.

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

  Respond to review feedback.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8437/files
  - new: https://git.openjdk.java.net/jdk/pull/8437/files/101ad872..fc7730d6

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

  Stats: 4 lines in 2 files changed: 3 ins; 0 del; 1 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: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]

2022-04-28 Thread Brian Burkhalter
> Modify native multi-byte read-write code used by the `java.io` classes to 
> limit the size of the allocated native buffer thereby decreasing off-heap 
> memory footprint and increasing throughput.

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

  6478546: Decrease malloc'ed buffer maximum size to 64 kB

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8235/files
  - new: https://git.openjdk.java.net/jdk/pull/8235/files/8bb1e14f..40d46f8f

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

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

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


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

2022-04-28 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

It should be possible for you finalize now.

-

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


Re: RFR: JDK-8285764: Add system property for Java SE specification maintenance version [v2]

2022-04-28 Thread Sean Mullan
On Thu, 28 Apr 2022 17:36:32 GMT, Joe Darcy  wrote:

>> 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.
>
> 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 five additional commits since 
> the last revision:
> 
>  - Respond to review feedback.
>  - Respond to CSR feedback.
>  - Merge branch 'master' into JDK-8285497
>  - Update comment in template.
>  - JDK-8285497: Add system property for Java SE specification maintenance 
> version

src/java.base/share/conf/security/java.policy line 34:

> 32:"java.specification.version", "read";
> 33: permission java.util.PropertyPermission
> 34:"java.specification.maintenance.version", "read";

For consistency, you should probably also add a hard-coded permission to the 
`sun.security.provider.PolicyFile.initStaticPolicy()` method which is used as a 
fallback if there is a problem parsing the `java.policy` file. I doubt this 
will cause any issues as I don't suspect many/any applications will suddenly 
start reading this property and running with an SM but again it is mainly for 
consistency as all the other default permissions in this file are also granted 
in that method.

-

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


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

2022-04-28 Thread Erik Gahlin
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 egahlin (Reviewer).

-

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


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

2022-04-28 Thread Mandy Chung
On Thu, 28 Apr 2022 18:24:33 GMT, Andrey Turbanov  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 seven additional commits 
>> since the last revision:
>> 
>>  - Update copyright years.
>>  - Merge branch 'master' into JDK-8285676
>>  - Respond to more review feedback.
>>  - Respond to more review feedback.
>>  - 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.base/share/classes/java/lang/ref/PhantomReference.java line 48:
> 
>> 46:  * The {@link #refersTo(Object) refersTo} method can be used to test
>> 47:  * whether some object is the referent of a phantom reference.
>> 48:  * @param the type of the referent
> 
> Shouldn't there be a space after `@param` ?

Good catch.  Sorry I missed it. This occurs in all `java/lang/ref` files.

-

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


Re: RFR: JDK-8285764: Add system property for Java SE specification maintenance version [v2]

2022-04-28 Thread Mark Reinhold
On Thu, 28 Apr 2022 17:36:32 GMT, Joe Darcy  wrote:

>> 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.
>
> 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 five additional commits since 
> the last revision:
> 
>  - Respond to review feedback.
>  - Respond to CSR feedback.
>  - Merge branch 'master' into JDK-8285497
>  - Update comment in template.
>  - JDK-8285497: Add system property for Java SE specification maintenance 
> version

Also, don't forget to update the CSR with the new specification text, for the 
record.
Otherwise, looks good!

src/java.base/share/classes/java/lang/VersionProps.java.template line 113:

> 111: props.put("java.specification.version", VERSION_SPECIFICATION);
> 112: 
> 113: // Uncomment next props.put call after the first maintenance 
> review for a

s/review/release/

-

Changes requested by mr (Lead).

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


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

2022-04-28 Thread Andrey Turbanov
On Thu, 28 Apr 2022 18:05:39 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 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 seven additional commits since 
> the last revision:
> 
>  - Update copyright years.
>  - Merge branch 'master' into JDK-8285676
>  - Respond to more review feedback.
>  - Respond to more review feedback.
>  - 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.base/share/classes/java/lang/ref/PhantomReference.java line 48:

> 46:  * The {@link #refersTo(Object) refersTo} method can be used to test
> 47:  * whether some object is the referent of a phantom reference.
> 48:  * @param the type of the referent

Shouldn't there be a space after `@param` ?

-

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


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

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

  Downcall and upcall tests use wrong layout which is missing some trailing 
padding

-

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

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

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

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


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

2022-04-28 Thread Joe Darcy
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.

This pull request has now been integrated.

Changeset: bba456a8
Author:Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/bba456a8dbf9027e4b015567c17a79fc7441aa08
Stats: 102 lines in 40 files changed: 76 ins; 0 del; 26 mod

8285676: Add missing @param tags for type parameters on classes and interfaces

Reviewed-by: wetmore, smarks, dfuchs, prr, alanb, mchung

-

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


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

2022-04-28 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 seven additional commits since the 
last revision:

 - Update copyright years.
 - Merge branch 'master' into JDK-8285676
 - Respond to more review feedback.
 - Respond to more review feedback.
 - 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/aaa8f828..cb1fe1c2

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

  Stats: 687 lines in 59 files changed: 610 ins; 8 del; 69 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: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v2]

2022-04-28 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"

@PaulSandoz Could you please take a look at the Java changes when you find 
time. This PR from @XiaohongGong is a very good step towards long standing 
Vector API  wish list for better tail loop handling.

-

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


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

2022-04-28 Thread Mandy Chung
On Thu, 28 Apr 2022 16:58:40 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 mchung (Reviewer).

-

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


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

2022-04-28 Thread Joe Darcy
On Thu, 28 Apr 2022 12:31:31 GMT, Sean Mullan  wrote:

> Should this be added to the default permissions in the 
> `conf/security/java.policy` file along with other similar properties?

Seems reasonable; good catch.

-

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


Re: RFR: JDK-8285764: Add system property for Java SE specification maintenance version [v2]

2022-04-28 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.

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

 - Respond to review feedback.
 - Respond to CSR feedback.
 - Merge branch 'master' into JDK-8285497
 - Update comment in template.
 - JDK-8285497: Add system property for Java SE specification maintenance 
version

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8437/files
  - new: https://git.openjdk.java.net/jdk/pull/8437/files/4ea4708b..101ad872

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

  Stats: 670 lines in 38 files changed: 614 ins; 8 del; 48 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: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v4]

2022-04-28 Thread Alan Bateman
On Thu, 28 Apr 2022 16:58:40 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 alanb (Reviewer).

-

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-28 Thread Joe Darcy
On Thu, 28 Apr 2022 08:10:38 GMT, Alan Bateman  wrote:

>> Joe Darcy has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Respond to more review feedback.
>
> src/java.base/share/classes/java/nio/file/WatchEvent.java line 51:
> 
>> 49: /**
>> 50:  * An event kind, for the purposes of identification.
>> 51:  * @param  the type of the context value
> 
> This is okay but the it differs slightly to the type parameters specified 
> further up. I think the issue here is that it was just wasn't copied down to 
> WatchEvent.Kind.

Okay -- I'll for the T type parameter of the Kind interface I'll reuse the 
wording of the T type parameter of the enclosing WatchEvent interface. (The 
type variable on Kind could be renamed to show that the two type parameters are 
distinct.)

-

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


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

2022-04-28 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/db4919a9..aaa8f828

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

  Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 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-28 Thread Joe Darcy
On Thu, 28 Apr 2022 08:08:37 GMT, Alan Bateman  wrote:

>> Joe Darcy has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Respond to more review feedback.
>
> src/java.base/share/classes/java/nio/file/SecureDirectoryStream.java line 55:
> 
>> 53:  * against the original path of the directory (irrespective of if 
>> the
>> 54:  * directory is moved since it was opened).
>> 55:  * @param  the type of path
> 
> It may not be a path. The type parameter is specified in the super 
> interfaces, can you copy that down instead?

Will change in the next push.

-

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


Re: RFR: 8285523: Improve test java/io/FileOutputStream/OpenNUL.java

2022-04-28 Thread Brian Burkhalter
On Mon, 25 Apr 2022 04:35:13 GMT, Sergey Bylokhov  wrote:

> The new test added as part of the 
> [JDK-8285445](https://bugs.openjdk.java.net/browse/JDK-8285445) cannot 
> trigger that bug and pass w/ and w/o fix.
> 
> An updated test validates the "default" case when the `jdk.io.File.enableADS` 
> property is not set, in this case the ADS should be 
> [accepted](https://github.com/openjdk/jdk/blob/9d9f4e502f6ddc3116ed9b80f7168a1edfce839e/src/java.base/windows/classes/java/io/WinNTFileSystem.java#L59).

Marked as reviewed by bpb (Reviewer).

-

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


Re: RFR: 8285523: Improve test java/io/FileOutputStream/OpenNUL.java

2022-04-28 Thread Andrew John Hughes
On Mon, 25 Apr 2022 04:35:13 GMT, Sergey Bylokhov  wrote:

> The new test added as part of the 
> [JDK-8285445](https://bugs.openjdk.java.net/browse/JDK-8285445) cannot 
> trigger that bug and pass w/ and w/o fix.
> 
> An updated test validates the "default" case when the `jdk.io.File.enableADS` 
> property is not set, in this case the ADS should be 
> [accepted](https://github.com/openjdk/jdk/blob/9d9f4e502f6ddc3116ed9b80f7168a1edfce839e/src/java.base/windows/classes/java/io/WinNTFileSystem.java#L59).

Changes look right to me. The test seems to have been based on the situation 
pre-patch, where setting the property to `true` worked around the bug. With the 
patch, only the `false` case actually exhibits the bug.

test/jdk/java/io/FileOutputStream/OpenNUL.java line 31:

> 29:  * @run main/othervm OpenNUL
> 30:  * @run main/othervm -Djdk.io.File.enableADS OpenNUL
> 31:  * @run main/othervm -Djdk.io.File.enableADS=FalsE OpenNUL

Is there a reason for the mixed case? Just to check equalsIgnoreCase is being 
used?

-

Marked as reviewed by andrew (Reviewer).

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


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

2022-04-28 Thread Alan Bateman
On Thu, 28 Apr 2022 15:10:18 GMT, Markus Grönlund  wrote:

>> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadSubmitFailedEvent.java 
>> line 35:
>> 
>>> 33: 
>>> 34: @Category({"Java Runtime"})
>>> 35: @Label("Virtual thread submit task failed")
>> 
>> The label is a bit a long, would "Virtual Thread Submit Failed" work?
>
> It works. Thanks.

Yes, this is okay.

-

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


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

2022-04-28 Thread Markus Grönlund
On Thu, 28 Apr 2022 14:44:05 GMT, Erik Gahlin  wrote:

>> Alan Bateman has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp line 168:
> 
>> 166:   assert(!tl->is_excluded(), "invariant");
>> 167:   if (virtual_thread) {
>> 168: // TODO: blob cache for virtual threads
> 
> Fix this now or after integration?

Well spotted. It is an optimization and can happen after integration. I will 
create an enhancement for tracking; thank you.

> src/hotspot/share/jfr/metadata/metadata.xml line 121:
> 
>> 119: thread="true" stackTrace="true">
>> 120: > description="Thread enlisted as a carrier" />
>> 121: > description="Class of the continuation" />
> 
> "Continuation class" = >" Continuation Class"
> numFrames = frames
> numRefs = references
> "Number of interpreted frames" => "Interpreted Frames"
> "Number of references" => "References"
> "Stack size in bytes" => "Stack Size" contentType"bytes"

Will fix.

> src/hotspot/share/jfr/metadata/metadata.xml line 130:
> 
>> 128: thread="true" stackTrace="true">
>> 129: > description="Thread enlisted as a carrier" />
>> 130: > description="Class of the continuation" />
> 
> contClass => continuationClass
> "Continuation class" => "Continuation Class"
> "Class of the continuation" Remove (not needed)
> "Number of interpreted frames" => "Interpreted Frames"
> numFrames => frames
> "Number of references" => "References"
> numRefs => references
> "Stack size in bytes" => "Stack Size" contentType="bytes"

Will fix.

> src/hotspot/share/jfr/metadata/metadata.xml line 138:
> 
>> 136:   > category="Java Virtual Machine" label="Continuation Freeze Young" 
>> thread="true" stackTrace="false" startTime="false">
>> 137: 
>> 138: 
> 
> "Allocated new" => "Allocated New"

Will fix, thanks.

> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadEndEvent.java line 35:
> 
>> 33: 
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread end")
> 
> "Virtual thread end" => "Virtual Thread End"
> Remove description.

Will fix.

> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadPinnedEvent.java line 
> 35:
> 
>> 33: 
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread pinned")
> 
> "Virtual thread pinned" => "Virtual Thread Pinned"
> Remove description

Will fix.

> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadStartEvent.java line 35:
> 
>> 33: 
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread start")
> 
> "virtual thread start" => "Virtual Thread Start"
> Remove description

Will fix, thanks.

> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadSubmitFailedEvent.java 
> line 35:
> 
>> 33: 
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread submit task failed")
> 
> The label is a bit a long, would "Virtual Thread Submit Failed" work?

It works. Thanks.

-

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


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

2022-04-28 Thread Kim Barrett
On Wed, 27 Apr 2022 15:32:35 GMT, Roger Riggs  wrote:

>> Kim Barrett has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   update copyright, @bug list
>
> LGTM

Thanks for reviews @RogerRiggs and @mlchung

-

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


Integrated: 8285690: CloneableReference subtest should not throw CloneNotSupportedException

2022-04-28 Thread Kim Barrett
On Wed, 27 Apr 2022 09:24:24 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.

This pull request has now been integrated.

Changeset: 2d8d1402
Author:Kim Barrett 
URL:   
https://git.openjdk.java.net/jdk/commit/2d8d1402147f6ddd15732ce7098a8438317a2681
Stats: 5 lines in 1 file changed: 2 ins; 0 del; 3 mod

8285690: CloneableReference subtest should not throw CloneNotSupportedException

Reviewed-by: rriggs, mchung

-

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


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

2022-04-28 Thread Markus Grönlund
On Thu, 28 Apr 2022 14:41:04 GMT, Erik Gahlin  wrote:

>> Alan Bateman has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp line 94:
> 
>> 92: static const size_t global_buffer_size = 512 * K;
>> 93: 
>> 94: static const size_t thread_local_buffer_prealloc_count = 32;
> 
> Why is more memory needed? Moore's law or something specific to virtual 
> threads?

The carrier thread will write the metadata for the mounted virtual threads 
lazily with virtual threads as part of the event write. They write this thread 
locally, and the memory increase accommodates fewer roundtrips to fetch new 
buffers. The previous size was small because it would only hold at most one 
entry.

-

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


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

2022-04-28 Thread Markus Grönlund
On Thu, 28 Apr 2022 14:37:22 GMT, Erik Gahlin  wrote:

>> Alan Bateman has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.cpp line 81:
> 
>> 79:   be_writer.write(size);
>> 80:   be_writer.write(time);
>> 81:   be_writer.write(JfrTicks::now().value() - time);
> 
> Why is this changed?

It was a small optimization attempted before the current system of writing 
virtual thread checkpoints in bulk. Will restore it, thank you.

-

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


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

2022-04-28 Thread Sibabrata Sahoo
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 will do a small change in method signature patch(Path path, int fromLine, int 
toLine, List from, String to) where the argument "List from" to 
be "String from". Also a new open Test will be added into this change-set.

-

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


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

2022-04-28 Thread Erik Gahlin
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/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp line 168:

> 166:   assert(!tl->is_excluded(), "invariant");
> 167:   if (virtual_thread) {
> 168: // TODO: blob cache for virtual threads

Fix this now or after integration?

src/hotspot/share/jfr/metadata/metadata.xml line 121:

> 119: thread="true" stackTrace="true">
> 120:  description="Thread enlisted as a carrier" />
> 121:  description="Class of the continuation" />

"Continuation class" = >" Continuation Class"
numFrames = frames
numRefs = references
"Number of interpreted frames" => "Interpreted Frames"
"Number of references" => "References"
"Stack size in bytes" => "Stack Size" contentType"bytes"

src/hotspot/share/jfr/metadata/metadata.xml line 130:

> 128: thread="true" stackTrace="true">
> 129:  description="Thread enlisted as a carrier" />
> 130:  description="Class of the continuation" />

contClass => continuationClass
"Continuation class" => "Continuation Class"
"Class of the continuation" Remove (not needed)
"Number of interpreted frames" => "Interpreted Frames"
numFrames => frames
"Number of references" => "References"
numRefs => references
"Stack size in bytes" => "Stack Size" contentType="bytes"

src/hotspot/share/jfr/metadata/metadata.xml line 138:

> 136:category="Java Virtual Machine" label="Continuation Freeze Young" 
> thread="true" stackTrace="false" startTime="false">
> 137: 
> 138: 

"Allocated new" => "Allocated New"

src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp line 94:

> 92: static const size_t global_buffer_size = 512 * K;
> 93: 
> 94: static const size_t thread_local_buffer_prealloc_count = 32;

Why is more memory needed? Moore's law or something specific to virtual threads?

src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.cpp line 81:

> 79:   be_writer.write(size);
> 80:   be_writer.write(time);
> 81:   be_writer.write(JfrTicks::now().value() - time);

Why is this changed?

src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadEndEvent.java line 35:

> 33: 
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread end")

"Virtual thread end" => "Virtual Thread End"
Remove description.

src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadPinnedEvent.java line 35:

> 33: 
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread pinned")

"Virtual thread pinned" => "Virtual Thread Pinned"
Remove description

src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadStartEvent.java line 35:

> 33: 
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread start")

"virtual thread start" => "Virtual Thread Start"
Remove description

src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadSubmitFailedEvent.java 
line 35:

> 33: 
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread submit task failed")

The label is a bit a long, would "Virtual Thread Submit Failed" work?

-

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


Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v8]

2022-04-28 Thread Jan Lahoda
> This is a (preliminary) patch for javac implementation for the third preview 
> of pattern matching for switch (type patterns in switches).
> 
> Draft JLS:
> http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwitchPlusRecordPatterns-20220407/specs/patterns-switch-jls.html
> 
> The changes are:
> -there are no guarded patterns anymore, guards are not bound to the 
> CaseElement (JLS 15.28)
> -a new contextual keyword `when` is used to add a guard, instead of `&&`
> -`null` selector value is handled on switch level (if a switch has `case 
> null`, it is used, otherwise a NPE is thrown), rather than on pattern 
> matching level.
> -total patterns are allowed in `instanceof`
> -`java.lang.MatchException` is added for the case where a switch is 
> exhaustive (due to sealed types) at compile-time, but not at runtime.
> 
> Feedback is welcome!
> 
> Thanks!

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

 - Reducing MatchException constructors.
 - Merge branch 'master' into type-patterns-third
 - Reference-type pattern is not applicable at a selector of a primitive type - 
fixing.
 - Merge branch 'master' into type-patterns-third
 - Cleanup, fixing tests.
 - Cleanup - more total -> unconditional pattern renaming.
 - Adjusting to review feedback.
 - Cleanup.
 - Fixing tests.
 - Adding a test for a NPE from guards.
 - ... and 10 more: https://git.openjdk.java.net/jdk/compare/4919525d...115605ae

-

Changes: https://git.openjdk.java.net/jdk/pull/8182/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8182=07
  Stats: 860 lines in 52 files changed: 424 ins; 244 del; 192 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8182.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8182/head:pull/8182

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


Re: RFR: 8285295: Need better testing for IdentityHashMap

2022-04-28 Thread Jaikiran Pai
On Fri, 22 Apr 2022 03:37:27 GMT, Stuart Marks  wrote:

> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch 
> in the bug report that breaks `IdentityHashMap` now causes several cases in 
> this new test to fail. There's more that could be done, but the new tests 
> cover most of the core functions of `IdentityHashMap`. Unfortunately it seems 
> difficult to merge this with the existing, comprehensive Collections tests 
> (e.g., MOAT.java) because those tests implicity rely on `equals()`-based 
> contract instead of the special-purpose `==`-based contract used by 
> `IdentityHashMap`.

test/jdk/java/util/IdentityHashMap/Basic.java line 337:

> 335: public void testPutOverwrite() {
> 336: Box newVal = new Box(v1a);
> 337: map.put(k1a, newVal);

Should we capture the return value and assert that it's not null and is 
identity equal to `v1a`?

Perhaps, similarly at a few other places where we do put and putIfAbsent?

-

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


Re: RFR: 8285295: Need better testing for IdentityHashMap

2022-04-28 Thread Jaikiran Pai
On Fri, 22 Apr 2022 03:37:27 GMT, Stuart Marks  wrote:

> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch 
> in the bug report that breaks `IdentityHashMap` now causes several cases in 
> this new test to fail. There's more that could be done, but the new tests 
> cover most of the core functions of `IdentityHashMap`. Unfortunately it seems 
> difficult to merge this with the existing, comprehensive Collections tests 
> (e.g., MOAT.java) because those tests implicity rely on `equals()`-based 
> contract instead of the special-purpose `==`-based contract used by 
> `IdentityHashMap`.

test/jdk/java/util/IdentityHashMap/Basic.java line 244:

> 242: public void testKeySetNoRemove() {
> 243: Set keySet = map.keySet();
> 244: keySet.remove(new Box(k1a));

Should we assert here that this returns `false`?

test/jdk/java/util/IdentityHashMap/Basic.java line 253:

> 251: public void testKeySetRemove() {
> 252: Set keySet = map.keySet();
> 253: keySet.remove(k1a);

Similarly should we assert `true` here and few other places in this PR which 
does the remove?

test/jdk/java/util/IdentityHashMap/Basic.java line 257:

> 255: checkEntries(map.entrySet(), entry(k1b, v1b),
> 256:  entry(k2, v2));
> 257: }

Would an additional check `assertFalse(map.equals(map2));` be useful here (and 
other similar tests where we do "remove").

test/jdk/java/util/IdentityHashMap/Basic.java line 325:

> 323: Box newKey = new Box(k1a);
> 324: Box newVal = new Box(v1a);
> 325: map.put(newKey, newVal);

Should we capture the return value and assert that it is `null`?

-

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


Re: RFR: JDK-8285730: unify _WIN32_WINNT settings

2022-04-28 Thread Erik Joelsson
On Thu, 28 Apr 2022 07:16:30 GMT, Matthias Baesken  wrote:

> Hi Erik/David/Phil, we already have a good central place where we set the 
> definition of WIN32_LEAN_AND_MEAN
> 
> autoconf/flags-cflags.m4:460: ALWAYS_DEFINES_JDK="-DWIN32_LEAN_AND_MEAN 
> -D_CRT_SECURE_NO_DEPRECATE autoconf/flags-cflags.m4:462: 
> ALWAYS_DEFINES_JVM="-DNOMINMAX -DWIN32_LEAN_AND_MEAN"
> 
> should we add there the _WIN32_WINNT=0x0601 define (and remove the differing 
> defines instead at the other places) ? (defines of _WIN32_WINNT in 3rd party 
> code would stay like wepoll.c and harfbuzz).

Seems reasonable to me at least.

-

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


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

2022-04-28 Thread Sean Mullan
On Wed, 27 Apr 2022 22:27:34 GMT, Joe Darcy  wrote:

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

Should this be added to the default permissions in the 
`lib/security/java.policy` file along with other similar properties?

-

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


Re: RFR: 8285295: Need better testing for IdentityHashMap

2022-04-28 Thread ExE Boss
On Wed, 27 Apr 2022 03:28:09 GMT, Stuart Marks  wrote:

>> test/jdk/java/util/IdentityHashMap/Basic.java line 77:
>> 
>>> 75: E[] contents = (E[]) c.toArray();
>>> 76: 
>>> 77: assertEquals(c.size(), given.length);
>> 
>> I believe testng has the expected values in front in the `assertEquals` 
>> methods, as embodied in the exception messages, so this should be 
>> `assertEquals(given.length, c.size());`. Applies to other places.
>
> No, TestNG is `assertEquals(actual, expected)` which is irritatingly the 
> opposite of JUnit.
> 
> https://github.com/cbeust/testng/blob/master/testng-asserts/src/main/java/org/testng/asserts/Assertion.java#L151
> 
> This will make things quite tedious when/if we convert to JUnit.

There’s a reason why I prefer using [AssertJ], where the calls are:

Assertions.assertThat(actual)
.isEqualTo(expected);


[AssertJ]: https://assertj.github.io/doc

-

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


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

2022-04-28 Thread Jaikiran Pai
On Wed, 27 Apr 2022 22:27:34 GMT, Joe Darcy  wrote:

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

Hello Joe, should the property description have a note stating what kind of a 
value this property holds, if at all present? Would it be free form text or an 
integer value?

-

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


Integrated: 8283994: Make Xerces DatatypeException stackless

2022-04-28 Thread Aleksey Shipilev
On Wed, 30 Mar 2022 11:38:59 GMT, Aleksey Shipilev  wrote:

> See bug report for more details. This change improves 
> SPECjvm2008:xml.validation for about +3%:
> 
> 
>  baseline: 298.353 ± 1.008  ops/min
>  patched:  309.912 ± 1.347  ops/min
> 
> Of course, the real improvements might be even higher, as exception might be 
> thrown from much deeper call hierarchy.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug, `jaxp_all`
>  - [x] Linux x86_64 fastdebug, `javax/xml`

This pull request has now been integrated.

Changeset: 85f8d14e
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk/commit/85f8d14edf0128e94bfc8102619a6ddbc37ead70
Stats: 8 lines in 1 file changed: 6 ins; 0 del; 2 mod

8283994: Make Xerces DatatypeException stackless

Reviewed-by: joehw, jpai

-

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


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

2022-04-28 Thread Alan Bateman
On Thu, 28 Apr 2022 01:34:19 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.

src/java.base/share/classes/java/nio/file/SecureDirectoryStream.java line 55:

> 53:  * against the original path of the directory (irrespective of if 
> the
> 54:  * directory is moved since it was opened).
> 55:  * @param  the type of path

It may not be a path. The type parameter is specified in the super interfaces, 
can you copy that down instead?

src/java.base/share/classes/java/nio/file/WatchEvent.java line 51:

> 49: /**
> 50:  * An event kind, for the purposes of identification.
> 51:  * @param  the type of the context value

This is okay but the it differs slightly to the type parameters specified 
further up. I think the issue here is that it was just wasn't copied down to 
WatchEvent.Kind.

-

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-28 Thread Daniel Fuchs
On Thu, 28 Apr 2022 01:34:19 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.

Thanks for the updates Joe. Your new wording looks good to me.

-

Marked as reviewed by dfuchs (Reviewer).

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


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

2022-04-28 Thread Alan Bateman
On Wed, 27 Apr 2022 19:43:22 GMT, Paul Sandoz  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/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.

Indeed, the layering is a bit awkward and I plan to replace it. It came about 
as a short term solution to avoid double bookkeeping of virtual threads.

-

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


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

2022-04-28 Thread Andrew Haley
On Wed, 27 Apr 2022 19:21:58 GMT, Alan Bateman  wrote:

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

Yes, it's dead. Thanks.

-

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


Re: RFR: JDK-8285730: unify _WIN32_WINNT settings

2022-04-28 Thread Matthias Baesken
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).

Hi Erik/David/Phil, we already have a good central place where we set the 
definition of WIN32_LEAN_AND_MEAN 

autoconf/flags-cflags.m4:460:ALWAYS_DEFINES_JDK="-DWIN32_LEAN_AND_MEAN 
-D_CRT_SECURE_NO_DEPRECATE \
autoconf/flags-cflags.m4:462:ALWAYS_DEFINES_JVM="-DNOMINMAX 
-DWIN32_LEAN_AND_MEAN"

should we add there the _WIN32_WINNT=0x0601  define  (and remove the differing 
defines instead at the other places) ?
(defines of _WIN32_WINNT in 3rd party code would stay like wepoll.c and 
harfbuzz).

-

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


Re: RFR: JDK-8285730: unify _WIN32_WINNT settings

2022-04-28 Thread Matthias Baesken
On Wed, 27 Apr 2022 15:10:51 GMT, Alan Bateman  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.

Hi Alan, I agree (thats why I did not change the define in harfbuzz,  but I 
missed that wepoll.c is 3rd party code as well).

-

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


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

2022-04-28 Thread Jatin Bhateja
On Wed, 20 Apr 2022 02:44:39 GMT, Xiaohong Gong  wrote:

>>> The blend should be with the intended-to-store vector, so that masked lanes 
>>> contain the need-to-store elements and unmasked lanes contain the loaded 
>>> elements, which would be stored back, which results in unchanged values.
>> 
>> It may not work if memory is beyond legal accessible address space of the 
>> process, a corner case could be a page boundary.  Thus re-composing the 
>> intermediated vector which partially contains actual updates but effectively 
>> perform full vector write to destination address may not work in all 
>> scenarios.
>
> Thanks for the comment! So how about adding the check for the valid array 
> range like the masked vector load?
> Codes like:
> 
> public final
> void intoArray(byte[] a, int offset,
>VectorMask m) {
> if (m.allTrue()) {
> intoArray(a, offset);
> } else {
> ByteSpecies vsp = vspecies();
> if (offset >= 0 && offset <= (a.length - vsp.length())) { // 
> a full range check
> intoArray0(a, offset, m, /* usePred */ false);
>// can be vectorized by load+blend_store
> } else {
> checkMaskFromIndexSize(offset, vsp, m, 1, a.length);
> intoArray0(a, offset, m, /* usePred */ true); 
>// only be vectorized by the predicated store
> }
> }
> }

Thanks, this looks ok since out of range condition will not be intrinsified if 
targets does not support predicated vector store.

-

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