date:20230306

On Mon, 6 Mar 2023 06:54:27 GMT, Jaikiran Pai  wrote:

>After running the test you can share with us the logs. We could provide more 
>suggestions what to debug further, but I think you or someone with access to 
>that system will have to do the actual investigation..

Output for above patch, I can share whole JTR file if you want. 

Feeding input of length 524288
Num written 525312
Num written 0
Deflater isn't expecting any more data but numWritten is 0 and finished is false
Num written 0
Deflater isn't expecting any more data but numWritten is 0 and finished is false
Num written 0
Deflater isn't expecting any more data but numWritten is 0 and finished is false
Num written 0
Deflater isn't expecting any more data but numWritten is 0 and finished is false
Num written 0
Deflater isn't expecting any more data but numWritten is 0 and finished is false
Num written 0
Deflater isn't expecting any more data but numWritten is 0 and finished is false
Num written 0
Deflater isn't expecting any more data but numWritten is 0 and finished is false
Num written 0
Deflater isn't expecting any more data but numWritten is 0 and finished is false
Num written 0
Deflater isn't expecting any more data but numWritten is 0 and finished is false
Num written 0
Deflater isn't expecting any more data but numWritten is 0 and finished is false
Num written 0
Deflater isn't expecting any more data but numWritten is 0 and finished is false
Num written 0
Deflater isn't expecting any more data but numWritten is 0 and finished is false
Num written 0
Deflater isn't expecting any more data but numWritten is 0 an
result: Error. Agent error: java.lang.Exception: Agent 2 timed out with a 
timeout of 480 seconds; check console log for any additional details

test result: Error. Agent error: java.lang.Exception: Agent 2 timed out with a 
timeout of 480 seconds; check console log for any additional details

-

PR: https://git.openjdk.org/jdk/pull/12283

Re: RFR: 8297605: DelayQueue javadoc is confusing

2023-03-06 Thread Viktor Klang

The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

--
On Thu, 2 Mar 2023 20:00:56 GMT, Martin Buchholz  wrote:

> Inviting @DougLea and @viktorklang-ora to review.
> 
> As usual, I couldn't resist more fiddling.  
> - Added missing tests for take, remove(), remove(Object).
> - Improved DelayQueueTest's testing infrastructure
> - added more test assertions
> - linkified new javadoc definitions

@Martin-Buchholz I think the updated documentation is clearer. We need some 
extra reviewers like @AlanBateman or @pron to sign off :)

-

PR: https://git.openjdk.org/jdk/pull/12838

Re: RFR: 8294982: Implementation of Classfile API [v44]

> This is root pull request with Classfile API implementation, tests and 
> benchmarks initial drop into JDK.
> 
> Following pull requests consolidating JDK class files parsing, generating, 
> and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) 
> will chain to this one.
> 
> Classfile API development is tracked at:
> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch
> 
> Development branch of consolidated JDK class files parsing, generating, and 
> transforming is at:
> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch
> 
> Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online 
> API 
> documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html)
>  is also available.
> 
> Please take you time to review this non-trivial JDK addition.
> 
> Thank you,
> Adam

Adam Sotona has updated the pull request incrementally with four additional 
commits since the last revision:

 - Update src/java.base/share/classes/jdk/internal/classfile/impl/Util.java
   
   Co-authored-by: Paul Sandoz 
 - Update 
src/java.base/share/classes/jdk/internal/classfile/impl/ClassPrinterImpl.java
   
   Co-authored-by: Paul Sandoz 
 - Update 
src/java.base/share/classes/jdk/internal/classfile/impl/BootstrapMethodEntryImpl.java
   
   Co-authored-by: Paul Sandoz 
 - Update 
src/java.base/share/classes/jdk/internal/classfile/impl/BootstrapMethodEntryImpl.java
   
   Co-authored-by: Paul Sandoz 

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/10982/files
  - new: https://git.openjdk.org/jdk/pull/10982/files/173dc1e7..b44f47ba

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=10982&range=43
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10982&range=42-43

  Stats: 19 lines in 3 files changed: 0 ins; 12 del; 7 mod
  Patch: https://git.openjdk.org/jdk/pull/10982.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10982/head:pull/10982

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: 8294982: Implementation of Classfile API [v45]

The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

--
> This is root pull request with Classfile API implementation, tests and 
> benchmarks initial drop into JDK.
> 
> Following pull requests consolidating JDK class files parsing, generating, 
> and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) 
> will chain to this one.
> 
> Classfile API development is tracked at:
> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch
> 
> Development branch of consolidated JDK class files parsing, generating, and 
> transforming is at:
> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch
> 
> Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online 
> API 
> documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html)
>  is also available.
> 
> Please take you time to review this non-trivial JDK addition.
> 
> Thank you,
> Adam

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

  Update 
src/java.base/share/classes/jdk/internal/classfile/impl/AttributeHolder.java
  
  Co-authored-by: Paul Sandoz 

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/10982/files
  - new: https://git.openjdk.org/jdk/pull/10982/files/b44f47ba..a40842c4

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=10982&range=44
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10982&range=43-44

  Stats: 6 lines in 1 file changed: 0 ins; 5 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/10982.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10982/head:pull/10982

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: 8294982: Implementation of Classfile API [v43]

On Fri, 3 Mar 2023 21:56:39 GMT, Paul Sandoz  wrote:

>> Adam Sotona has updated the pull request incrementally with three additional 
>> commits since the last revision:
>> 
>>  - fixed AccessFlags javadoc
>>  - TransformImpl.FakeXyzTransform renamed to UnresolvedXyzTransform
>>  - removed obsolete generic parameter from AbstractDirectBuilder
>
> src/java.base/share/classes/jdk/internal/classfile/impl/AttributeHolder.java 
> line 73:
> 
>> 71: if (a.attributeMapper() == am)
>> 72: iterator.remove();
>> 73: }
> 
> Suggestion:
> 
> attributes.removeIf(a -> a.attributeMappter() == am);
> 
> But presumably use an inner class instead. I can understand because of that 
> if you want to keep the existing code instead.

It seems to be OK to use lambda here (not on critical JDK bootstrap path), 
thanks.

-

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: 8302791: Add specific ClassLoader object to Proxy IllegalArgumentException message [v4]

2023-03-06 Thread Ravali Yatham

On Thu, 2 Mar 2023 20:00:24 GMT, Mandy Chung  wrote:

>>> Thanks @AlanBateman. Regarding the separator, Have seen this being used in 
>>> ClassLoader.java for nameAndId method. Hence used the same for consistency. 
>>> I've reformatted L886 now, Please kindly check.
>> 
>> Thanks for checking, the updated version looks good.
>
> I suggest to add a `JavaLangAccess::getLoaderNameID(ClassLoader loader)` so 
> that Proxy (and other classes) can use the same representation.
> 
> The patch will look like this:
> 
> 
> diff --git a/src/java.base/share/classes/java/lang/ClassLoader.java 
> b/src/java.base/share/classes/java/lang/ClassLoader.java
> index 2bc9a22700a..f412ecfa3ae 100644
> --- a/src/java.base/share/classes/java/lang/ClassLoader.java
> +++ b/src/java.base/share/classes/java/lang/ClassLoader.java
> @@ -407,6 +407,10 @@ public abstract class ClassLoader {
>  return nid;
>  }
>  
> +String nameAndId() {
> +return nameAndId;
> +}
> +
>  /**
>   * Creates a new class loader of the specified name and using the
>   * specified parent class loader for delegation.
> diff --git a/src/java.base/share/classes/java/lang/System.java 
> b/src/java.base/share/classes/java/lang/System.java
> index 501ed47fcad..c138ea9fef5 100644
> --- a/src/java.base/share/classes/java/lang/System.java
> +++ b/src/java.base/share/classes/java/lang/System.java
> @@ -2663,6 +2663,10 @@ public final class System {
>Continuation 
> continuation) {
>  return StackWalker.newInstance(options, null, contScope, 
> continuation);
>  }
> +
> +public String getLoaderNameID(ClassLoader loader) {
> +return loader.nameAndId();
> +}
>  });
>  }
>  }
> diff --git a/src/java.base/share/classes/java/lang/reflect/Proxy.java 
> b/src/java.base/share/classes/java/lang/reflect/Proxy.java
> index 6064dcf5b6b..a2a9a03e6c4 100644
> --- a/src/java.base/share/classes/java/lang/reflect/Proxy.java
> +++ b/src/java.base/share/classes/java/lang/reflect/Proxy.java
> @@ -878,7 +878,7 @@ public class Proxy implements java.io.Serializable {
>  }
>  if (type != c) {
>  throw new IllegalArgumentException(c.getName() +
> -" referenced from a method is not visible from class 
> loader");
> +" referenced from a method is not visible from class 
> loader: " + JLA.getLoaderNameID(ld));
>  }
>  }
>  
> diff --git 
> a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java 
> b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java
> index 39adfb2130a..4720e589efb 100644
> --- a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java
> +++ b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java
> @@ -564,4 +564,6 @@ public interface JavaLangAccess {
>  StackWalker newStackWalkerInstance(Set options,
> ContinuationScope contScope,
> Continuation continuation);
> +
> +String getLoaderNameID(ClassLoader loader);
>  }

@mlchung - Thanks for suggesting this so as to reuse the existing `nameAndId` 
functionality in Proxy class. I've pushed new commit, Please kindly review.

-

PR: https://git.openjdk.org/jdk/pull/12641

Re: RFR: 8302791: Add specific ClassLoader object to Proxy IllegalArgumentException message [v6]

2023-03-06 Thread Ravali Yatham

> Added specific class loader object to proxy IllegalArgumentException from 
> which the class was not visible
> 
> The bug report for the same: https://bugs.openjdk.org/browse/JDK-8302791

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

  Added JavaLangAccess::getLoaderNameID(ClassLoader loader)

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/12641/files
  - new: https://git.openjdk.org/jdk/pull/12641/files/ba561eef..2dd5fbc5

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=12641&range=05
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=12641&range=04-05

  Stats: 24 lines in 4 files changed: 14 ins; 9 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/12641.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12641/head:pull/12641

PR: https://git.openjdk.org/jdk/pull/12641

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v2]

On Thu, 2 Feb 2023 08:27:55 GMT, Amit Kumar  wrote:

>> DeInflate.java test fails on s390x platform because size for out1 array 
>> which is responsible for storing the compressed data is insufficient. And 
>> being unable to write whole compressed data on array, on s390 whole data 
>> can't be recovered after compression. So this fix increase Array size (for 
>> s390).
>
> Amit Kumar has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   change acc to Alan comments

Thank you Amit for running this test. So the custom zlib implementation does 
indeed returns false for `needsInput()`, which is a good thing.

Based on the tests so far, what this suggests to me is that the `deflate` 
implementation of this custom `zlib` library compresses the data to a size 
larger than what this test expects. It isn't clear how large it is going to be 
or whether it will always be the same size after deflate, based on what you 
note:

> But I guess this behaviour could be explained (by zEDC). On s390x, there is 
> additional instruction present and it could be enabled by setting DFLTCC=1. 
> And for accelerator the size of compressed data could go2 times the size of 
> actual data. Again this is not deterministic as well, i.e. for same data 
> there could be different valid deflate stream.

This specific test `DeInflate` notes that it's there for basic deflate and 
inflate testing. As far as I can see, there's nothing in the specification of 
`Deflater#deflate()` which states that the current size of the output buffer 
that the test uses is mandated/expected by spec:

> byte[] dataOut1 = new byte[dataIn.length + 1024];

So `1024` is a reasonable extra length that the test has been using 
successfully so far. It appears that this is not enough on s390x with this 
custom implementation of zlib.
I believe it doesn't violate any spec. So your proposed change (which Alan 
recommended) to use a `ByteArrayOutputStream` to keep accumulating the deflated 
(and later inflated) data, until the deflater is `finished()` sounds fine to 
me. The important thing here is that the inflated content from this deflated 
content matches the original input. From what I see in this discussion, with 
these changes, it does indeed match and thus the test passes. So I think that's 
a good thing.

Now coming to this proposed patch, now that you are using local 
ByteArrayOutputStream(s) for the deflate/inflate part in this `check()` method, 
I think the `out1` and `out2` should no longer be needed in this method and 
thus the method signature can be changed to remove these params. I see that 
this is the only place where this method is used, so changing the method 
signature of `check()` should be OK and shouldn't impact other parts of the 
test.

While we are at it, just for the sake of testing, undo the changes I suggested 
in my last reply and use the current PR's code (afresh) and print out the 
length of the final deflated content, something like:

out1 = baos.toByteArray();
System.out.println("Deflated length is: " + out1.length + " for input of 
length: " + len);

I'm curious how large the deflated content would be.

Finally, are you or someone in your team, in contact with the author(s) of the 
custom zlib implementation 
https://github.com/iii-i/zlib/commit/113203437eda67261848b14b6c80a33ff7e33d34? 
Would you be able to ask for their inputs on whether this (large?) difference 
in the deflated content size expected and are there any specific input (sizes) 
that this shows up or is it for all inputs?

Finally, I would wait to hear from Alan or Lance on whether these changes are 
OK, given the results of the experiments we have seen so far.

-

PR: https://git.openjdk.org/jdk/pull/12283

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v2]

On Thu, 2 Feb 2023 08:27:55 GMT, Amit Kumar  wrote:

>> DeInflate.java test fails on s390x platform because size for out1 array 
>> which is responsible for storing the compressed data is insufficient. And 
>> being unable to write whole compressed data on array, on s390 whole data 
>> can't be recovered after compression. So this fix increase Array size (for 
>> s390).
>
> Amit Kumar has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   change acc to Alan comments

Hi Jaikiran, Thank you so much for your inputs :-) 

>While we are at it, just for the sake of testing, undo the changes I suggested 
>in my last reply and use the current PR's code (afresh) and print out the 
>length of the final deflated content, something like:
>```
>out1 = baos.toByteArray();
>System.out.println("Deflated length is: " + out1.length + " for input of 
>length: " + len);
>```
>I'm curious how large the deflated content would be.


I guess this is what you want to see ?


> `initial size: 525312`
> 
> error I'm facing is on level 1 not on -1.
> 
> ```
> level:1, strategy: 0, dowrap: false
>   required size: 553018  <- there is an issue
>   required size: 283432
>   required size: 526225  <- there is an issue
>   required size: 301127
>   required size: 409196
> level:1, strategy: 0, dowrap: true
>   required size: 553012  <- there is an issue
>   required size: 44541
>   required size: 414324
>   required size: 289230
>   required size: 176785
> ```
> 
> this is result for executing test case 2nd time:
> 
> ```
> level:1, strategy: 0, dowrap: false
>   required size: 552979  <- there is an issue
>   required size: 549979  <- there is an issue
>   required size: 49738
>   required size: 116661
>   required size: 439574
> level:1, strategy: 0, dowrap: true
>   required size: 552973  <- there is an issue
>   required size: 466730
>   required size: 509795
>   required size: 94907
>   required size: 476173
> ```
> 
> All other levels are okay, this could be explained by Ilya's patch. What 
> states that `hardware` compression will be used on level 1 and `software` 
> compression will be used on all other levels.


>Capacity with out1 * 32: 557056 <-> Buffer we need 552981
>Capacity with out1 * 32: 557056 <-> Buffer we need 552975
>Initial size of out1: 525312

-

PR: https://git.openjdk.org/jdk/pull/12283

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v2]

On Thu, 2 Feb 2023 08:27:55 GMT, Amit Kumar  wrote:

>> DeInflate.java test fails on s390x platform because size for out1 array 
>> which is responsible for storing the compressed data is insufficient. And 
>> being unable to write whole compressed data on array, on s390 whole data 
>> can't be recovered after compression. So this fix increase Array size (for 
>> s390).
>
> Amit Kumar has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   change acc to Alan comments

> I guess this is what you want to see ?
>
>
>level:1, strategy: 0, dowrap: false
>  required size: 553018  <- there is an issue

Yes, thank you. So to summarize, an input of size `524288` bytes gets 
"compressed"/deflated to a size of `553018` i.e. it _adds_ `28730` bytes, when 
level 1 deflater level (`BEST_SPEED`) is used and the custom zlib's hardware 
instruction is triggered. That, in my opinion, is odd (adding that many 
additional bytes for compressed data). So I think it would be good to get the 
team which did the zlib change involved in this discussion to understand if 
this large an impact is expected. And if so, is it for all inputs or if there's 
some issue that needs to be looked into in the implementation. Would you or 
someone in your team be able to get their attention to this?

-

PR: https://git.openjdk.org/jdk/pull/12283

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v2]

On Thu, 2 Feb 2023 08:27:55 GMT, Amit Kumar  wrote:

>> DeInflate.java test fails on s390x platform because size for out1 array 
>> which is responsible for storing the compressed data is insufficient. And 
>> being unable to write whole compressed data on array, on s390 whole data 
>> can't be recovered after compression. So this fix increase Array size (for 
>> s390).
>
> Amit Kumar has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   change acc to Alan comments

Hi JaiKiran, I already sent a request to Ilya, for his input on this PR.

-

PR: https://git.openjdk.org/jdk/pull/12283

Re: RFR: 8294982: Implementation of Classfile API [v46]

> This is root pull request with Classfile API implementation, tests and 
> benchmarks initial drop into JDK.
> 
> Following pull requests consolidating JDK class files parsing, generating, 
> and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) 
> will chain to this one.
> 
> Classfile API development is tracked at:
> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch
> 
> Development branch of consolidated JDK class files parsing, generating, and 
> transforming is at:
> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch
> 
> Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online 
> API 
> documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html)
>  is also available.
> 
> Please take you time to review this non-trivial JDK addition.
> 
> Thank you,
> Adam

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

  removed unused methods, fields and parameters
  added missing overrides
  fixed pointless operations and possible null pointer dereferences

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/10982/files
  - new: https://git.openjdk.org/jdk/pull/10982/files/a40842c4..61ff1c7c

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=10982&range=45
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10982&range=44-45

  Stats: 297 lines in 35 files changed: 58 ins; 121 del; 118 mod
  Patch: https://git.openjdk.org/jdk/pull/10982.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10982/head:pull/10982

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: 8294982: Implementation of Classfile API [v43]

On Fri, 3 Mar 2023 21:44:24 GMT, Paul Sandoz  wrote:

>> Adam Sotona has updated the pull request incrementally with three additional 
>> commits since the last revision:
>> 
>>  - fixed AccessFlags javadoc
>>  - TransformImpl.FakeXyzTransform renamed to UnresolvedXyzTransform
>>  - removed obsolete generic parameter from AbstractDirectBuilder
>
> src/java.base/share/classes/jdk/internal/classfile/impl/AbstractInstruction.java
>  line 683:
> 
>> 681: public UnboundInstruction(Opcode op, int size) {
>> 682: super(op, size);
>> 683: }
> 
> Unused?

fixed, thanks.

> src/java.base/share/classes/jdk/internal/classfile/impl/AnnotationReader.java 
> line 99:
> 
>> 97: }
>> 98: 
>> 99: public static List> 
>> readParameterAnnotations(ClassReader classReader, int p, boolean isVisible) {
> 
> Parameter `isVisible` is unused, but method is called with true/false values.

fixed, thanks.

> src/java.base/share/classes/jdk/internal/classfile/impl/AttributeHolder.java 
> line 76:
> 
>> 74: }
>> 75: 
>> 76: List> attributes() {
> 
> Unused

fixed, thanks.

> src/java.base/share/classes/jdk/internal/classfile/impl/BufferedMethodBuilder.java
>  line 54:
> 
>> 52: private final List elements = new ArrayList<>();
>> 53: private final SplitConstantPool constantPool;
>> 54: private final ClassEntry thisClass;
> 
> Unused. Can be removed as can the associated constructor parameter.

fixed, thanks.

-

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: 8303392: Runtime.exec and ProcessBuilder.start should use System logger [v2]

2023-03-06 Thread Daniel Fuchs

On Sat, 4 Mar 2023 14:22:33 GMT, Roger Riggs  wrote:

>> Runtime.exec and ProcessBuilder.start methods create a new operating system 
>> process with the program and arguments. Many applications configure a 
>> logging subsystem to monitor application events. Logging a process start 
>> message with the program, arguments, and stack trace can identify the caller 
>> and purpose.
>> Logging the process start event is complementary to the process start event 
>> generated for JFR (Java Flight Recorder).
>
> Roger Riggs has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add javadoc @implNote to Runtime.exec and ProcessBuilder methods

src/java.base/share/classes/java/lang/ProcessBuilder.java line 1148:

> 1146: ", pid: " + process.pid());
> 1147: } catch (Throwable thEx) {/* ignore */}
> 1148: }

Is it really useful to try & catch a potential exception here, or wouldn't be 
better to let it propagate to the caller? An exception here would indicate a 
bug somewhere either in the logging backend or in the logging configuration - 
or possibly OOM or any other kind of VM error and maybe it would be better to 
just fail in that case, and let the VM exit (unless that exception is caught 
down the road by the caller?)

I understand why we would not want an exception in logging to prevent 
System.exit() from exiting, but surely the same constraint doesn't hold for 
`ProcessBuilder::start`?

-

PR: https://git.openjdk.org/jdk/pull/12862

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v2]

2023-03-06 Thread Ilya Leoshkevich

On Thu, 2 Feb 2023 08:27:55 GMT, Amit Kumar  wrote:

>> DeInflate.java test fails on s390x platform because size for out1 array 
>> which is responsible for storing the compressed data is insufficient. And 
>> being unable to write whole compressed data on array, on s390 whole data 
>> can't be recovered after compression. So this fix increase Array size (for 
>> s390).
>
> Amit Kumar has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   change acc to Alan comments

> Finally, are you or someone in your team, in contact with the author(s) of 
> the custom zlib implementation 
> [iii-i/zlib@1132034](https://github.com/iii-i/zlib/commit/113203437eda67261848b14b6c80a33ff7e33d34)?
>  Would you be able to ask for their inputs on whether this (large?) 
> difference in the deflated content size expected and are there any specific 
> input (sizes) that this shows up or is it for all inputs?

This can happen for any poorly compressible inputs, regardless of their size. 
Here is the calculation for the worst case: 
https://github.com/iii-i/zlib/blob/dfltcc-20230109/contrib/s390/dfltcc.h#L17 
(TL;DR: the size can double), but, of course, we don't expect this to be 
happening often.

Here are some benchmarking results: 
https://github.com/iii-i/zlib-ng/wiki/Performance-with-dfltcc-patch-applied-and-dfltcc-support-built-on-dfltcc-enabled-machine.
 The interesting data is in column `compressed_size` where `level` is 1. This 
is the hardware compressed size divided by the software compressed size. Most 
of the time it's +5-20%, in a few cases it compresses better than software, and 
only in one case (kennedy.xls) we see a big difference of +44%.

-

PR: https://git.openjdk.org/jdk/pull/12283

Re: RFR: 8294982: Implementation of Classfile API [v43]

The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

--
On Fri, 3 Mar 2023 23:12:17 GMT, Paul Sandoz  wrote:

>> Adam Sotona has updated the pull request incrementally with three additional 
>> commits since the last revision:
>> 
>>  - fixed AccessFlags javadoc
>>  - TransformImpl.FakeXyzTransform renamed to UnresolvedXyzTransform
>>  - removed obsolete generic parameter from AbstractDirectBuilder
>
> src/java.base/share/classes/jdk/internal/classfile/impl/Util.java line 161:
> 
>> 159: var desc = cd.descriptorString();
>> 160: return switch (desc.charAt(0)) {
>> 161: case '[' -> desc;
> 
> Is this correct? Arrays don't have an internal name.

It is a workaround to get CP class entry name string from descriptor, so the  
`toInternalName` is not exact.
I'll handle the array descriptors separately and let this method to handle real 
class internal names only.

-

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v2]

On Mon, 6 Mar 2023 13:10:57 GMT, Ilya Leoshkevich  wrote:

>> Amit Kumar has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   change acc to Alan comments
>
>> Finally, are you or someone in your team, in contact with the author(s) of 
>> the custom zlib implementation 
>> [iii-i/zlib@1132034](https://github.com/iii-i/zlib/commit/113203437eda67261848b14b6c80a33ff7e33d34)?
>>  Would you be able to ask for their inputs on whether this (large?) 
>> difference in the deflated content size expected and are there any specific 
>> input (sizes) that this shows up or is it for all inputs?
> 
> This can happen for any poorly compressible inputs, regardless of their size. 
> Here is the calculation for the worst case: 
> https://github.com/iii-i/zlib/blob/dfltcc-20230109/contrib/s390/dfltcc.h#L17 
> (TL;DR: the size can double), but, of course, we don't expect this to be 
> happening often.
> 
> Here are some benchmarking results: 
> https://github.com/iii-i/zlib-ng/wiki/Performance-with-dfltcc-patch-applied-and-dfltcc-support-built-on-dfltcc-enabled-machine.
>  The interesting data is in column `compressed_size` where `level` is 1. This 
> is the hardware compressed size divided by the software compressed size. Most 
> of the time it's +5-20%, in a few cases it compresses better than software, 
> and only in one case (kennedy.xls) we see a big difference of +44%.
> 
> P.S. Here we are compressing random data, if I read the testcase correctly 
> (`rnd.nextBytes(dataIn);`), so poor results are expected. Ideally we should 
> emit stored blocks, but the hardware engine does not support this at the 
> moment.

@iii-i Thank you Ilya for those details. I haven't fully caught up with them, 
but while you are here - In the benchmarking table, are the "compress_cpu" and 
the "compress_wall" too ratios of hardware compress time to software compress 
time for level 1 (BEST_SPEED)?

-

PR: https://git.openjdk.org/jdk/pull/12283

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v2]

2023-03-06 Thread Ilya Leoshkevich

On Thu, 2 Feb 2023 08:27:55 GMT, Amit Kumar  wrote:

>> DeInflate.java test fails on s390x platform because size for out1 array 
>> which is responsible for storing the compressed data is insufficient. And 
>> being unable to write whole compressed data on array, on s390 whole data 
>> can't be recovered after compression. So this fix increase Array size (for 
>> s390).
>
> Amit Kumar has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   change acc to Alan comments

Right, and that's the main selling point of the accelerator - it can be up to 
100x faster than software (this is reached with the enwik8, mozilla and sao 
benchmarks).

-

PR: https://git.openjdk.org/jdk/pull/12283

Re: Testing no memory leak occurs via references

2023-03-06 Thread Aleksei Ivanov

Thank you, Stuart and Roger, for the suggestion.

I didn't know about ForceGC class. We should use the common library, I
have updated my PR [7] with ForceGC.

Albert and Thomas suggested using WhiteBox API. It would probably work
too, however, I decided ForceGC is simpler and easier to use, so I went
for it. It does its job and it's used in core-libs tests.

Thanks to everyone who provided their input, I appreciate your help.

--
Regards,
Alexey

[7] https://github.com/openjdk/jdk/pull/12594

On 03/03/2023 22:54, Stuart Marks wrote:

As Roger mentioned, there is a ForceGC utility in the test library:

test/lib/jdk/test/lib/util/ForceGC.java

and it's used in a variety of places in the core libs tests.
Essentially it sets up a PhantomReference and a ReferenceQueue and
runs System.gc() in a loop. I'd strongly recommend using this in
preference to allocating a lot of memory in order to provoke
OutOfMemoryError. That technique has historically been a cause of test
flakiness, and it still is, as you've discovered.

There is also MemoryMXBean.gc(), which does the same thing System.gc()
does -- it calls Runtime.getRuntime().gc().

It's true that System.gc() may sometimes be ignored -- for instance if
Epsilon GC is enabled -- but for practical purposes, on Hotspot using
a standard collector, calling it will eventually cause garbage
collection and reference processing.

If at some point the behavior provided by System.gc() is inadequate
for our testing, we'll need to plumb a JDK-specific interface that has
stronger semantics, and then convert ForceGC to use it so that
individual tests won't have to be updated.

There are still some tests that allocate lots of memory in order to
provoke OOME and consequently reference processing. They probably need
to be run in /othervm mode in order to set custom heap sizes and to
avoid interfering with other tests. It would be interesting to see if
those could be adjusted to use something ForceGC so that they can
share the JVM with other tests and also avoid allocating lots of memory.

s'marks

On 3/3/23 10:02 AM, Aleksei Ivanov wrote:

Hello,

In clientlibs, there's occasionally a need to verify an object isn't
leaked. For this purpose, WeakReference or PhantomReference is used.

Then, we need to make the reference object be cleared, so a GC cycle
need to be triggered. The common approach is generating
OutOfMemoryError, catching it and verifying whether the reference is
cleared.

Some tests use a utility method regtesthelpers/Util.generateOOME [1].

For example, these tests follow the above approach:
https://github.com/openjdk/jdk/blob/master/test/jdk/javax/swing/border/TestTitledBorderLeak.java
https://github.com/openjdk/jdk/blob/master/test/jdk/java/awt/List/ListGarbageCollectionTest/AwtListGarbageCollectionTest.java

The AwtListGarbageCollectionTest.java test started to fail pretty
often in the end of January 2023.

I followed a piece of advice provided in a JBS comment for
JDK-8300727 [2] and replaced generating OOME with a simple call to
System.gc() along with adding a loop for re-trying.

The specification for System.gc() [3] mentions that this call can be
ignored, which started a discussion in the PR #12594 [4] that
System.gc() should not be used, at the very least without generating
OOME in addition to invoking System.gc().

At the same time, many tests for Reference objects, such as
ReferenceEnqueue.java [5] and PhantomReferentClearing.java [6], rely
solely on System.gc.

What would be your recommendation? Are there best practices in
core-libs and hotspot for testing for memory leaks that clientlibs
should follow?

--
Regards,
Alexey

[1]
https://github.com/openjdk/jdk/blob/29ee7c3b70ded8cd124ca5b4a38a2aee7c39068b/test/jdk/javax/swing/regtesthelpers/Util.java#L87

[2] https://bugs.openjdk.org/browse/JDK-8300727
[3]
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/System.html#gc()

[4] https://github.com/openjdk/jdk/pull/12594
[5]
https://github.com/openjdk/jdk/blob/f612dcfebea7ffd4390f833646ad45d6f0ebd04f/test/jdk/java/lang/ref/ReferenceEnqueue.java#L54-L60
[6]
https://github.com/openjdk/jdk/blob/f612dcfebea7ffd4390f833646ad45d6f0ebd04f/test/jdk/java/lang/ref/PhantomReferentClearing.java#L85-L92

RFR: JDK-8302360 Atomic*.compareAndExchange Javadoc unclear

2023-03-06 Thread Viktor Klang

I think the following proposal is very non-invasive and merely draws attention 
to the fact that "witness value" is a specific term related to the notion of 
these atomic methods.

It's a small change which I think provides additional clarity, see JBS for the 
discussion on the topic.

-

Commit messages:
 - JDK-8302360 : Adding emphasis to the witness value term in the JavaDoc

Changes: https://git.openjdk.org/jdk/pull/12880/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12880&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8302360
  Stats: 34 lines in 8 files changed: 0 ins; 0 del; 34 mod
  Patch: https://git.openjdk.org/jdk/pull/12880.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12880/head:pull/12880

PR: https://git.openjdk.org/jdk/pull/12880

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v2]

On Mon, 6 Mar 2023 13:10:57 GMT, Ilya Leoshkevich  wrote:

> P.S. Here we are compressing random data, if I read the testcase correctly 
> (rnd.nextBytes(dataIn);), 

Yes, you read it right.

-

PR: https://git.openjdk.org/jdk/pull/12283

Re: Testing no memory leak occurs via references

2023-03-06 Thread Albert Yang

Upon a cursory inspection of ForceGC.java, it seems that the fundamental logic 
involves waiting for a certain duration and relying on chance. However, I am of 
the opinion that utilizing the WhiteBox API can provide greater determinism and 
potentially strengthen some of the assertions.

> I decided ForceGC is simpler and easier to use

I was not aware of your specific requirements, so I cannot say for certain 
which approach is best. (However, it is worth noting that the WhiteBox API can 
be utilized to implement ForceGC if necessary.)

/Albert

Re: RFR: 8294982: Implementation of Classfile API [v47]

> This is root pull request with Classfile API implementation, tests and 
> benchmarks initial drop into JDK.
> 
> Following pull requests consolidating JDK class files parsing, generating, 
> and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) 
> will chain to this one.
> 
> Classfile API development is tracked at:
> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch
> 
> Development branch of consolidated JDK class files parsing, generating, and 
> transforming is at:
> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch
> 
> Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online 
> API 
> documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html)
>  is also available.
> 
> Please take you time to review this non-trivial JDK addition.
> 
> Thank you,
> Adam

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

 - simplified CodeImpl.SINGLETON_INSTRUCTIONS initialization
 - fixed handling of array descriptors by Util::toInternalName

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/10982/files
  - new: https://git.openjdk.org/jdk/pull/10982/files/61ff1c7c..46fffe40

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=10982&range=46
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10982&range=45-46

  Stats: 60 lines in 7 files changed: 0 ins; 34 del; 26 mod
  Patch: https://git.openjdk.org/jdk/pull/10982.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10982/head:pull/10982

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: 8294982: Implementation of Classfile API [v43]

On Fri, 3 Mar 2023 22:35:48 GMT, Paul Sandoz  wrote:

>> Adam Sotona has updated the pull request incrementally with three additional 
>> commits since the last revision:
>> 
>>  - fixed AccessFlags javadoc
>>  - TransformImpl.FakeXyzTransform renamed to UnresolvedXyzTransform
>>  - removed obsolete generic parameter from AbstractDirectBuilder
>
> src/java.base/share/classes/jdk/internal/classfile/impl/CodeImpl.java line 52:
> 
>> 50: static final Instruction[] SINGLETON_INSTRUCTIONS = new 
>> Instruction[256];
>> 51: 
>> 52: static {
> 
> Can we loop through all `Opcode` values filter for `sizeIfFixed == 1` and 
> switch on the kind? If so that would avoid the need for explicit lists and 
> simplify the code.

Yes, that is good idea.
Fixed, thanks.

-

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v42]

On Fri, 3 Mar 2023 19:42:53 GMT, Roger Riggs  wrote:

>> Jim Laskey has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Tighten up reporting of string template errors (fewer messages)
>
> src/java.base/share/classes/java/util/FormatProcessor.java line 66:
> 
>> 64:  * 
>> 65:  * {@link FormatProcessor}  format specification uses and exceptions are 
>> the same as
>> 66:  * those of {@link Formatter}.
> 
> Suggestion:
> 
>  * The {@link FormatProcessor} format specification uses and exceptions are 
> the same as
>  * those of {@link Formatter}.

Changing.

> src/java.base/share/classes/java/util/FormatProcessor.java line 134:
> 
>> 132:  * format string from the fragments, gathers up the values and
>> 133:  * evaluates the expression
>> 134:  * {@code new Formatter(locale).format(format, values).toString()}.
> 
> Should this be described using the "as if"... phrasing to avoid a literal 
> requirement in the spec that is inflexible?

Changing

> src/java.base/share/classes/java/util/FormatProcessor.java line 175:
> 
>> 173:  * {@link FormatProcessor#FMT} ({@code static final 
>> FormatProcessor}).
>> 174:  * 
>> 175:  * Other {@link FormatProcessor} can be specialized if stored as 
>> static final.
> 
> Suggestion:
> 
>  * Other {@link FormatProcessor}s can be specialized if stored as a 
> static final.

Changing

> src/java.base/share/classes/java/util/FormatProcessor.java line 187:
> 
>> 185:  * @throws  IllegalFormatException
>> 186:  *  If a format specifier contains an illegal syntax, a 
>> format
>> 187:  *  specifier that is incompatible with the given arguments,
> 
> Suggestion:
> 
>  *  specifier is incompatible with the given arguments,

Existing statement is consistent with those in Formatter. Reads more correctly 
as is.

-

PR: https://git.openjdk.org/jdk/pull/10889

Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v42]

On Sat, 4 Mar 2023 19:34:36 GMT, Marius Volkhart  wrote:

>> Jim Laskey has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Tighten up reporting of string template errors (fewer messages)
>
> src/java.base/share/classes/java/lang/template/Carriers.java line 396:
> 
>> 394: 
>> 395: /**
>> 396:  * Wrapper object for carrier data. Instances types are stored in 
>> the {@code objects}
> 
> Suggestion:
> 
>  * Wrapper object for carrier data. Instance types are stored in the 
> {@code objects}

Changing

> src/java.base/share/classes/java/lang/template/StringTemplate.java line 76:
> 
>> 74:  * produced that returns the same lists from {@link 
>> StringTemplate#fragments()} and
>> 75:  * {@link StringTemplate#values()} as shown above. The {@link 
>> StringTemplate#STR} template
>> 76:  * processor uses these lists to yield an interpolated string. the value 
>> of {@code s} will
> 
> Suggestion:
> 
>  * processor uses these lists to yield an interpolated string. The value of 
> {@code s} will

Changing

-

PR: https://git.openjdk.org/jdk/pull/10889

Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v42]

The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

--
On Fri, 3 Mar 2023 20:17:24 GMT, Roger Riggs  wrote:

>> Jim Laskey has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Tighten up reporting of string template errors (fewer messages)
>
> src/java.base/share/classes/java/lang/template/ProcessorLinkage.java line 37:
> 
>> 35: 
>> 36: /**
>> 37:  * Policies using this additional interface have the flexibility to 
>> specialize
> 
> Since it is 'sealed' it may clarify the use to say "Builtin policies"...

Changing

> src/java.base/share/classes/java/lang/template/StringProcessor.java line 58:
> 
>> 56: /**
>> 57:  * Constructs a {@link String} based on the template fragments and 
>> values in the
>> 58:  * supplied {@link StringTemplate} object.
> 
> Some inconsistency in the use of link/linkplain and the capitalization of 
> stringTemplate, the instance or the type.
> (As compared to TemplateProcessor.process(stringTemplate))
> Suggestion:
> 
>  * supplied {@link StringTemplate} object.

Changing

> src/java.base/share/classes/java/util/FormatProcessor.java line 42:
> 
>> 40:  * {@link Formatter} specifications and values found in the {@link 
>> StringTemplate}.
>> 41:  * Unlike {@link Formatter}, {@link FormatProcessor} uses the value from 
>> the
>> 42:  * embedded expression that immediately follows, no whitespace, after the
> 
> Suggestion:
> 
>  * embedded expression that immediately follows, without whitespace, the

Changing

> src/java.base/share/classes/java/util/FormatProcessor.java line 80:
> 
>> 78:  * int x = 10;
>> 79:  * int y = 20;
>> 80:  * String result = thaiFMT."%d\{x} + %d\{y} = %d\{x + y}";
> 
> The inclusion of format specifiers that yield the same results as the default 
> (%s) may mislead developers into thinking they need the format specifier. 
> Making the examples look more complicated than necessary.
> Can the examples, show customized output.
> 
> Suggestion:
> 
>  * String result = thaiFMT."%d{x} + %d{y} = %d{x + y}";

Changing

-

PR: https://git.openjdk.org/jdk/pull/10889

Re: RFR: 8294982: Implementation of Classfile API [v48]

> This is root pull request with Classfile API implementation, tests and 
> benchmarks initial drop into JDK.
> 
> Following pull requests consolidating JDK class files parsing, generating, 
> and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) 
> will chain to this one.
> 
> Classfile API development is tracked at:
> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch
> 
> Development branch of consolidated JDK class files parsing, generating, and 
> transforming is at:
> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch
> 
> Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online 
> API 
> documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html)
>  is also available.
> 
> Please take you time to review this non-trivial JDK addition.
> 
> Thank you,
> Adam

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

  simplified initialization of terminal builder in chained builders

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/10982/files
  - new: https://git.openjdk.org/jdk/pull/10982/files/46fffe40..0e43af66

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=10982&range=47
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10982&range=46-47

  Stats: 14 lines in 3 files changed: 0 ins; 2 del; 12 mod
  Patch: https://git.openjdk.org/jdk/pull/10982.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10982/head:pull/10982

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: 8294982: Implementation of Classfile API [v20]

On Wed, 1 Mar 2023 10:05:27 GMT, Adam Sotona  wrote:

>> src/java.base/share/classes/jdk/internal/classfile/impl/ChainedFieldBuilder.java
>>  line 48:
>> 
>>> 46: this.consumer = consumer;
>>> 47: FieldBuilder b = downstream;
>>> 48: while (b instanceof ChainedFieldBuilder cb)
>> 
>> I'm probably missing something - but if `b` is another chained builder, 
>> instead of using a loop, can't we just skip to its  `terminal` field? Also, 
>> the `downstream` field seems unused. (same considerations apply for 
>> `ChainedMethodBuilder` and `ChainedClassBuilder`).
>> 
>> I note that `NonTerminalCodeBuilder` seems to be already doing what I 
>> suggest here (e.g. skip to `terminal`).
>
> I would have to test possible side-effects of the proposed shortcut to give 
> you answer.
> Thanks for pointing it out.

Proposed shortcut seems to be OK, I've fixed it in `ChainedFieldBuilder`, 
`ChainedMethodBuilder` and `ChainedClassBuilder`. And removed unused 
`ChainedFieldBuilder.downstream` field.

-

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: 8303582: Reduce duplication in jdk/java/foreign tests

2023-03-06 Thread Maurizio Cimadamore

On Fri, 3 Mar 2023 15:08:01 GMT, Jorn Vernee  wrote:

> Port of: https://github.com/openjdk/panama-foreign/pull/804 which reduces 
> duplication in the test code by switching test value generation over to a 
> common method in `NativeTestHelper`.

Looks good (already reviewed in the panama repo)

-

Marked as reviewed by mcimadamore (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12856

Integrated: 8303582: Reduce duplication in jdk/java/foreign tests

On Fri, 3 Mar 2023 15:08:01 GMT, Jorn Vernee  wrote:

> Port of: https://github.com/openjdk/panama-foreign/pull/804 which reduces 
> duplication in the test code by switching test value generation over to a 
> common method in `NativeTestHelper`.

This pull request has now been integrated.

Changeset: dccfe8a2
Author:Jorn Vernee 
URL:   
https://git.openjdk.org/jdk/commit/dccfe8a2eedcead7f33f161f410222c7651398ef
Stats: 503 lines in 13 files changed: 138 ins; 272 del; 93 mod

8303582: Reduce duplication in jdk/java/foreign tests

Reviewed-by: mcimadamore

-

PR: https://git.openjdk.org/jdk/pull/12856

Re: Testing no memory leak occurs via references


Hi Albert,

The only  downside of the WhiteBox API is the need to link and run with 
native code.

It make adhoc testing more difficult.

I would not say ForceGC relies on chance, since it is testing the 
specific condition as requested by the caller.
It uses mechanisms available to normal applications and conditions they 
would encounter and not wait longer than necessary.


Regards, Roger


On 3/6/23 8:51 AM, Albert Yang wrote:

Upon a cursory inspection of ForceGC.java, it seems that the fundamental logic 
involves waiting for a certain duration and relying on chance. However, I am of 
the opinion that utilizing the WhiteBox API can provide greater determinism and 
potentially strengthen some of the assertions.


I decided ForceGC is simpler and easier to use

I was not aware of your specific requirements, so I cannot say for certain 
which approach is best. (However, it is worth noting that the WhiteBox API can 
be utilized to implement ForceGC if necessary.)

/Albert

Re: RFR: 8303604: Passing by-value structs whose size is not power of 2 doesn't work on all platforms (mainline) [v2]

The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

--
> Port of: https://github.com/openjdk/panama-foreign/pull/806 which fixes an 
> issue with passing structs whose size is not a power of two on SysV and 
> AArch64 platforms.

Jorn Vernee has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains four commits:

 - Merge branch 'master' into OOB
 - 8303017: Passing by-value structs whose size is not power of 2 doesn't work 
on all platforms
   
   Reviewed-by: mcimadamore
 - remove byteWidthOfPrimitive for now
 - 8302990: Reduce duplication in test code
   8293827: Padding computed by CallGeneratorHelper can be incorrect
   
   Reviewed-by: mcimadamore

-

Changes: https://git.openjdk.org/jdk/pull/12863/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12863&range=01
  Stats: 705 lines in 11 files changed: 652 ins; 2 del; 51 mod
  Patch: https://git.openjdk.org/jdk/pull/12863.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12863/head:pull/12863

PR: https://git.openjdk.org/jdk/pull/12863

Re: Testing no memory leak occurs via references

2023-03-06 Thread Aleksei Ivanov


On 06/03/2023 13:51, Albert Yang wrote:

Upon a cursory inspection of ForceGC.java, it seems that the fundamental logic 
involves waiting for a certain duration and relying on chance. However, I am of 
the opinion that utilizing the WhiteBox API can provide greater determinism and 
potentially strengthen some of the assertions.


Yes, it calls System.gc in a loop and waits for a reference to become 
cleared.


(It looks as if the body of ForceGC duplicates what one would have in 
the passed BooleanSupplier which again tests if a reference is cleared.)



I decided ForceGC is simpler and easier to use

I was not aware of your specific requirements, so I cannot say for certain 
which approach is best. (However, it is worth noting that the WhiteBox API can 
be utilized to implement ForceGC if necessary.)


My test is written to ensure awt.List gets garbage-collected when there 
are no strong references to it. Before JDK-8040076 had been fixed it wasn't.


So the test creates awt.List, adds it to a frame, calls 
setMultipleMode(true) to enable multi-selection in the list component, 
removes it from the frame. At this point, I want to confirm the awt.List 
can be garbage-collected.


The original test created a very long String to cause OutOfMemoryError 
and then verified whether the WeakReference to awt.List is cleared or not.


In my first fix, I replaced generating OutOfMemoryError with a call to 
System.gc() in a loop and waited for the reference object to be cleared. 
Usually, the reference is cleared in the second iteration if not in the 
first one.


Essentially, ForceGC does the same thing. So, it replaced my custom code 
with ForceGC.


--
Alexey

Integrated: 8303604: Passing by-value structs whose size is not power of 2 doesn't work on all platforms (mainline)

The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

--
On Fri, 3 Mar 2023 19:27:24 GMT, Jorn Vernee  wrote:

> Port of: https://github.com/openjdk/panama-foreign/pull/806 which fixes an 
> issue with passing structs whose size is not a power of two on SysV and 
> AArch64 platforms.

This pull request has now been integrated.

Changeset: 5977f266
Author:Jorn Vernee 
URL:   
https://git.openjdk.org/jdk/commit/5977f266d04a7a9890665d433d0a2ab627573ca4
Stats: 705 lines in 11 files changed: 652 ins; 2 del; 51 mod

8303604: Passing by-value structs whose size is not power of 2 doesn't work on 
all platforms (mainline)

Reviewed-by: mcimadamore

-

PR: https://git.openjdk.org/jdk/pull/12863

Integrated: 8303486: [REDO] Update ProcessTools.startProcess(...) to exit early if process exit before linePredicate is printed.

2023-03-06 Thread Leonid Mesnik

On Wed, 1 Mar 2023 20:50:38 GMT, Leonid Mesnik  wrote:

> It is the 2nd attempt to fix
> [JDK-8303133](https://bugs.openjdk.org/browse/JDK-8303133) Update 
> ProcessTools.startProcess(...) to exit early if the process exit before 
> linePredicate is printed.
> 
> The first fix failed because it runs
> Utils.waitForCondition(BooleanSupplier condition, long timeout, long 
> sleepTime) { ..}
> with 0 as no timeout and not -1 as required.

This pull request has now been integrated.

Changeset: ae8730fd
Author:Leonid Mesnik 
URL:   
https://git.openjdk.org/jdk/commit/ae8730fd62b382564cda8749763b939aa2939225
Stats: 30 lines in 2 files changed: 18 ins; 3 del; 9 mod

8303486: [REDO] Update ProcessTools.startProcess(...) to exit early if process 
exit before linePredicate is printed.

Reviewed-by: dholmes

-

PR: https://git.openjdk.org/jdk/pull/12815

Re: UUID creation performance


Hi Brett,

You might be trying to over optimize, making the source more complex 
than is useful or necessary.
I think it is likely that on most architectures the varhandles take 
advantage of appropriate machine instructions and both have the same 
performance.
I expect the performance difference to be barely noticeable, and if so 
keep it simple and direct.


Even so, a localized static cache inUUID of whichever VarHandle is 
native would serve the purpose better than a runtime check.


Regards, Roger


On 3/5/23 6:49 PM, Brett Okken wrote:

The new ByteArray class works great for the nameUUIDFromBytes method,
which must be in big endian.
For randomUUID, byte order does not matter, so using native would be
fastest, but there does not appear to be a utility class for that.
Is there a preference of just having a native order VarHandle to use
in UUID vs. having a utility method which chooses which utility class
to call based on the native order vs. some other option?

Thanks,
Brett

On Wed, Mar 1, 2023 at 9:08 AM Roger Riggs  wrote:

Hi,

That's an interesting idea.  Recently VarHandle access methods were
created by JDK-8300236 [1] [2]
in the jdk.internal.util package. See the ByteArray and
ByteArrayLittleEndian classes.

See how that would affect performance and leverage existing VarHandles.

Thanks, Roger

[1] https://bugs.openjdk.org/browse/JDK-8300236
[2] 
https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/12076__;!!ACWV5N9M2RV99hQ!LF6TU_iFxi-lfSpZXjna9bCMPRs_gRvdXsfL5Tih6k2oYN94Hfb3sV_gBzdTvty-gXm0rzSgEZ0_xdQdYa7b05LArw$

On 3/1/23 7:50 AM, Brett Okken wrote:

Is there any interest in updating the static UUID.randomUUID() and
UUID.nameUUIDFromBytes(byte[]) factory methods to use either a
ByteBuffer or byteArrayViewVarHandle to convert the byte[] to 2 long
values then do the bit twiddling?
These methods are really dominated by time to create/populate the
byte[], but this does reduce the time to create the 2 long values by
at least half.
It would also allow the removal of the private UUID(byte[] data).

  public static UUID randomUUID() {
  SecureRandom ng = Holder.numberGenerator;

  byte[] randomBytes = new byte[16];
  ng.nextBytes(randomBytes);
  final ByteBuffer bb = ByteBuffer.wrap(randomBytes);
  bb.order(ByteOrder.nativeOrder());

  long msb = bb.getLong();
  long lsb = bb.getLong();

  msb &= 0x0FFFL;  /* clear version*/
  msb |= 0x4000L;  /* set to version 4 */

  lsb &= 0x3FFFL;  /* clear variant*/
  lsb |= 0x8000L;  /* set to IETF variant  */

  return new UUID(msb, lsb);
  }

  public static UUID nameUUIDFromBytes(byte[] name) {
  MessageDigest md;
  try {
  md = MessageDigest.getInstance("MD5");
  } catch (NoSuchAlgorithmException nsae) {
  throw new InternalError("MD5 not supported", nsae);
  }
  byte[] md5Bytes = md.digest(name);

  // default byte order is BIG_ENDIAN
  final ByteBuffer bb = ByteBuffer.wrap(md5Bytes);

  long msb = bb.getLong();
  long lsb = bb.getLong();

  msb &= 0x0FFFL;  /* clear version*/
  msb |= 0x3000L;  /* set to version 3 */

  lsb &= 0x3FFFL;  /* clear variant*/
  lsb |= 0x8000L;  /* set to IETF variant  */

  return new UUID(msb, lsb);
  }

BenchmarkMode  Cnt   Score   Error  Units
UUIDBenchmark.jdk_name   avgt3  11.885 ± 4.025  ns/op
UUIDBenchmark.jdk_random avgt3  11.656 ± 0.987  ns/op
UUIDBenchmark.longs  avgt3   7.618 ± 1.047  ns/op
UUIDBenchmark.longs_bb   avgt3   7.755 ± 1.643  ns/op
UUIDBenchmark.longs_name avgt3   8.467 ± 1.784  ns/op
UUIDBenchmark.longs_name_bb  avgt3   8.455 ± 1.662  ns/op
UUIDBenchmark.randomBytesavgt3   6.132 ± 0.447  ns/op


@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Warmup(iterations = 3, time = 2, timeUnit = TimeUnit.SECONDS)
@Measurement(iterations = 3, time = 2, timeUnit = TimeUnit.SECONDS)
@Fork(1)
@State(Scope.Benchmark)
public class UUIDBenchmark {

  private static final VarHandle LONGS_ACCESS =
MethodHandles.byteArrayViewVarHandle(long[].class,
ByteOrder.nativeOrder());

  private static final VarHandle BE_LONGS_ACCESS =
MethodHandles.byteArrayViewVarHandle(long[].class,
ByteOrder.BIG_ENDIAN);

  @Benchmark
  public byte[] randomBytes() {
  final byte[] bytes = new byte[16];
  randomBytes(bytes);
  return bytes;
  }

  @Benchmark
  public void jdk_random(Blackhole bh) {
  final byte[] data = new byte[16];
  randomBytes(data);
  data[6]  &= 0x0f;  /* clear version*/
  data[6]  |= 0x40;  /* set to version 4 */
  data[8]  &= 0x3f;

Re: RFR: JDK-8302360 Atomic*.compareAndExchange Javadoc unclear

2023-03-06 Thread Martin Buchholz

On Mon, 6 Mar 2023 13:23:59 GMT, Viktor Klang  wrote:

> I think the following proposal is very non-invasive and merely draws 
> attention to the fact that "witness value" is a specific term related to the 
> notion of these atomic methods.
> 
> It's a small change which I think provides additional clarity, see JBS for 
> the discussion on the topic.

Marked as reviewed by martin (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/12880

Re: RFR: 8303440: The "ZonedDateTime.parse" may not accept the "UTC+XX" zone id [v3]

On Sat, 4 Mar 2023 02:22:39 GMT, Naoto Sato  wrote:

>> This change is to fix a regression caused by the fix to 
>> [JDK-8234347](https://bugs.openjdk.org/browse/JDK-8234347). The previous fix 
>> was simply disabling offset-based parsing if the parser was text-based. 
>> Instead, check if it is an offset or not by explicitly comparing the 
>> character with '+'/'-' and continue offset parsing if it is.
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   now -> zdt

LGTM

-

Marked as reviewed by rriggs (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12868

RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter

The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

--
Implemented `Float.floatToFloat16` and `Float.float16ToFloat` intrinsics in 
Interpreter and C1 compiler to produce the same results as C2 intrinsics on 
x64, Aarch64 and RISC-V - all platforms where C2 intrinsics for these Java 
methods were implemented originally.

Replaced `SharedRuntime::f2hf()` and `hf2f()` C runtime functions with calls to 
runtime stubs which use the same HW instructions as C2 intrinsics. Only for 
64-bit x64 because 32-bit x86 stub does not work: result is passed through FPU 
register and NaN values become different from C2 intrinsic. This runtime stub 
is only used to calculate constant values during C2 compilation and can be 
skipped.

I added new tests based on Tobias's `TestAll.java` And copied 
`jdk/lang/Float/Binary16Conversion*.java` tests to run them with `-Xcomp` to 
make sure code is compiled by C1 or C2. I modified `Binary16ConversionNaN.java` 
to compare results from Interpreter, C1 and C2.

Tested tier1-5, Xcomp, stress

-

Commit messages:
 - Remove ConvF2HFNode::Identity(). Updated tests
 - Copyright year update
 - Check float16 instructions support on platforms for C1
 - Update Copyright year. Add missing UnlockDiagnosticVMOptions flag in new test
 - Implement Aarch64 and RiscV parts, remove 32-bit x86 runtime stub
 - Merge branch 'master' into 8302976
 - 8302976: C2 intrinsification of Float.floatToFloat16 and 
Float.float16ToFloat yields different result than the interpreter

Changes: https://git.openjdk.org/jdk/pull/12869/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12869&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8302976
  Stats: 1432 lines in 48 files changed: 1284 ins; 97 del; 51 mod
  Patch: https://git.openjdk.org/jdk/pull/12869.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12869/head:pull/12869

PR: https://git.openjdk.org/jdk/pull/12869

Re: RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter

On Fri, 3 Mar 2023 21:41:35 GMT, Vladimir Kozlov  wrote:

> Implemented `Float.floatToFloat16` and `Float.float16ToFloat` intrinsics in 
> Interpreter and C1 compiler to produce the same results as C2 intrinsics on 
> x64, Aarch64 and RISC-V - all platforms where C2 intrinsics for these Java 
> methods were implemented originally.
> 
> Replaced `SharedRuntime::f2hf()` and `hf2f()` C runtime functions with calls 
> to runtime stubs which use the same HW instructions as C2 intrinsics. Only 
> for 64-bit x64 because 32-bit x86 stub does not work: result is passed 
> through FPU register and NaN values become different from C2 intrinsic. This 
> runtime stub is only used to calculate constant values during C2 compilation 
> and can be skipped.
> 
> I added new tests based on Tobias's `TestAll.java` And copied 
> `jdk/lang/Float/Binary16Conversion*.java` tests to run them with `-Xcomp` to 
> make sure code is compiled by C1 or C2. I modified 
> `Binary16ConversionNaN.java` to compare results from Interpreter, C1 and C2.
> 
> Tested tier1-5, Xcomp, stress

GHA failure on linux-x86 in test 
compiler/vectorization/runner/LoopRangeStrideTest.java is due to 
[JDK-8303105](https://bugs.openjdk.org/browse/JDK-8303105)

-

PR: https://git.openjdk.org/jdk/pull/12869

Re: RFR: 8303577: [JVMCI] OOME causes crash while translating exceptions

2023-03-06 Thread Tom Rodriguez

On Fri, 3 Mar 2023 15:40:01 GMT, Doug Simon  wrote:

> JDK-8297431 added code for special handling of OutOfMemoryError when 
> translating an exception between libjvmci and HotSpot[1].
> Unfortunately, this code was deleted by JDK-8298099 when moving the exception 
> translation mechanism to VMSupport[2].
> This causes the VM to crash when an OOME occurs while translating an 
> exception from HotSpot to libjvmci.
> This PR revives the deleted code.
> 
> This bug was found by running 
> `test/jdk/java/util/concurrent/locks/Lock/OOMEInAQS.java` on libgraal. The 
> fix now makes the test pass.
> 
> Note that the code modified in 
> `src/java.base/share/classes/jdk/internal/vm/VMSupport.java` is JVMCI 
> specific and was original added by 
> [JDK-8298099](https://git.openjdk.org/jdk/pull/11513).
> 
> [1] 
> https://github.com/openjdk/jdk/commit/952e10055135613e8ea2b818a4f35842936f5633#diff-4d3a3b7e7e12e1d5b4cf3e4677d9e0de5e9df3bbf1bbfa0d8d43d12098d67dc4R222-R234
> [2] 
> https://github.com/openjdk/jdk/commit/8b69a2e434ad2fa3369079622b57afb973d5bd9a#diff-7292551772c27b7152af03cbbad90a897c5e37c6a97d4026be835e6d8fe1R121-R125

Marked as reviewed by never (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/12857

Re: RFR: 8303577: [JVMCI] OOME causes crash while translating exceptions

2023-03-06 Thread Doug Simon

On Fri, 3 Mar 2023 18:05:51 GMT, Vladimir Kozlov  wrote:

>> JDK-8297431 added code for special handling of OutOfMemoryError when 
>> translating an exception between libjvmci and HotSpot[1].
>> Unfortunately, this code was deleted by JDK-8298099 when moving the 
>> exception translation mechanism to VMSupport[2].
>> This causes the VM to crash when an OOME occurs while translating an 
>> exception from HotSpot to libjvmci.
>> This PR revives the deleted code.
>> 
>> This bug was found by running 
>> `test/jdk/java/util/concurrent/locks/Lock/OOMEInAQS.java` on libgraal. The 
>> fix now makes the test pass.
>> 
>> Note that the code modified in 
>> `src/java.base/share/classes/jdk/internal/vm/VMSupport.java` is JVMCI 
>> specific and was original added by 
>> [JDK-8298099](https://git.openjdk.org/jdk/pull/11513).
>> 
>> [1] 
>> https://github.com/openjdk/jdk/commit/952e10055135613e8ea2b818a4f35842936f5633#diff-4d3a3b7e7e12e1d5b4cf3e4677d9e0de5e9df3bbf1bbfa0d8d43d12098d67dc4R222-R234
>> [2] 
>> https://github.com/openjdk/jdk/commit/8b69a2e434ad2fa3369079622b57afb973d5bd9a#diff-7292551772c27b7152af03cbbad90a897c5e37c6a97d4026be835e6d8fe1R121-R125
>
> Good.

Thanks for the reviews @vnkozlov and @tkrodriguez .

-

PR: https://git.openjdk.org/jdk/pull/12857

Integrated: 8303577: [JVMCI] OOME causes crash while translating exceptions

2023-03-06 Thread Doug Simon

The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

--
On Fri, 3 Mar 2023 15:40:01 GMT, Doug Simon  wrote:

> JDK-8297431 added code for special handling of OutOfMemoryError when 
> translating an exception between libjvmci and HotSpot[1].
> Unfortunately, this code was deleted by JDK-8298099 when moving the exception 
> translation mechanism to VMSupport[2].
> This causes the VM to crash when an OOME occurs while translating an 
> exception from HotSpot to libjvmci.
> This PR revives the deleted code.
> 
> This bug was found by running 
> `test/jdk/java/util/concurrent/locks/Lock/OOMEInAQS.java` on libgraal. The 
> fix now makes the test pass.
> 
> Note that the code modified in 
> `src/java.base/share/classes/jdk/internal/vm/VMSupport.java` is JVMCI 
> specific and was original added by 
> [JDK-8298099](https://git.openjdk.org/jdk/pull/11513).
> 
> [1] 
> https://github.com/openjdk/jdk/commit/952e10055135613e8ea2b818a4f35842936f5633#diff-4d3a3b7e7e12e1d5b4cf3e4677d9e0de5e9df3bbf1bbfa0d8d43d12098d67dc4R222-R234
> [2] 
> https://github.com/openjdk/jdk/commit/8b69a2e434ad2fa3369079622b57afb973d5bd9a#diff-7292551772c27b7152af03cbbad90a897c5e37c6a97d4026be835e6d8fe1R121-R125

This pull request has now been integrated.

Changeset: cac81ddc
Author:Doug Simon 
URL:   
https://git.openjdk.org/jdk/commit/cac81ddc9259168a5b12c290ae2ce7db25a729fc
Stats: 31 lines in 5 files changed: 22 ins; 0 del; 9 mod

8303577: [JVMCI] OOME causes crash while translating exceptions

Reviewed-by: kvn, never

-

PR: https://git.openjdk.org/jdk/pull/12857

Re: RFR: 8303440: The "ZonedDateTime.parse" may not accept the "UTC+XX" zone id [v3]

On Sat, 4 Mar 2023 02:22:39 GMT, Naoto Sato  wrote:

>> This change is to fix a regression caused by the fix to 
>> [JDK-8234347](https://bugs.openjdk.org/browse/JDK-8234347). The previous fix 
>> was simply disabling offset-based parsing if the parser was text-based. 
>> Instead, check if it is an offset or not by explicitly comparing the 
>> character with '+'/'-' and continue offset parsing if it is.
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   now -> zdt

Marked as reviewed by lancea (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/12868

Re: RFR: 8303392: Runtime.exec and ProcessBuilder.start should use System logger [v2]

On Mon, 6 Mar 2023 12:44:57 GMT, Daniel Fuchs  wrote:

>> Roger Riggs has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Add javadoc @implNote to Runtime.exec and ProcessBuilder methods
>
> src/java.base/share/classes/java/lang/ProcessBuilder.java line 1148:
> 
>> 1146: ", pid: " + process.pid());
>> 1147: } catch (Throwable thEx) {/* ignore */}
>> 1148: }
> 
> Is it really useful to try & catch a potential exception here, or wouldn't be 
> better to let it propagate to the caller? An exception here would indicate a 
> bug somewhere either in the logging backend or in the logging configuration - 
> or possibly OOM or any other kind of VM error and maybe it would be better to 
> just fail in that case, and let the VM exit (unless that exception is caught 
> down the road by the caller?)
> 
> I understand why we would not want an exception in logging to prevent 
> System.exit() from exiting, but surely the same constraint doesn't hold for 
> `ProcessBuilder::start`?

Since adding logging injects additional code during start(), the idea was to 
handle exceptions to avoid breaking existing code.
And since the logger is cached during the static initialization of the 
ProcessBuilder class, a mis-configured logger would cause 
ExceptionInInitializer, exposing the cause early, though in a pretty ugly way. 
I may need to reconsider that.

-

PR: https://git.openjdk.org/jdk/pull/12862

Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]

On Wed, 1 Mar 2023 06:37:45 GMT, Martin Doerr  wrote:

>>> * Uploaded my simple reproducer to 
>>> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>> 
>> Thanks!
>> 
>>> * Using oversized load / stores is problematic. Don't forget that 
>>> OpenJDK still supports Big Endian platforms (AIX, s390x).
>> 
>> You're right. I realized that it's also problematic for heap segments, for 
>> which we can't do oversized accesses. I am working on another solution that 
>> splits up the loads/stores into power-of-two sized chunks: 
>> https://github.com/openjdk/panama-foreign/compare/foreign-memaccess+abi...JornVernee:panama-foreign:OOB
>>  That patch is just a POC at this point though. Also, I don't think it works 
>> for BE at the moment (need to flip the offset for BE, I think. Just like we 
>> do in Unsafe).
>> 
>>> * The result of `NativeCallingConvention::calling_convention` is 
>>> interpreted as size, but it returns the max offset. That's off by one slot. 
>>> Should I file a bug for that? (PPC64 is not affected because it doesn't use 
>>> the result.)
>> 
>> I'm not sure there's an issue there. Note that the 'max offset' is computed 
>> as `reg.offset() + reg.stack_size()`, so that should get us the size we need 
>> to allocate for the stack arguments. (e.g. 2 ints being passed at offset 0 
>> and 4, would make max offset 4 + 4 = 8, which gives the size needed for the 
>> 2 ints). Computing the max offset instead of just summing the sizes of the 
>> stack arguments is needed since stack arguments can be sparsely placed in 
>> some cases on Mac/AArch64.
>> 
>>> * Since the membar on the return path was mentioned: I think it would 
>>> be good to enable UseSystemMemoryBarrier by default on operating systems 
>>> which support it. Maybe we should discuss this with @robehn.
>> 
>> ~I don't think we've done that much testing with UseSystemMemoryBarrier 
>> since it was added~. I'm a bit nervous about turning it on by default since 
>> it's currently also used for JNI. Let's see what Robbin thinks.
>
> @JornVernee: Thanks a lot for your detailed review! I have quite a few TODOs 
> which include:
> - Include my tests for the HFA corner cases.
> - Try to improve handling of the overlapping registers as you suggested.
> - Check nesting of HFA.
> 
> There will surely be more when looking into Big Endian support after merging 
> with your recent work on 
> https://github.com/openjdk/panama-foreign/compare/foreign-memaccess+abi...JornVernee:panama-foreign:OOB
> We should get rid of oversized accesses on PPC64, too.
> Thanks for sharing your plans to intrisify `linkToNative` in C2 later. I 
> guess we should do more preparation work on all platforms when that gets 
> addressed.

@TheRealMDoerr I've moved the support for structs/unions that are not a power 
of 2 in size to this repo, so you should be able to merge the master branch to 
get it now.

-

PR: https://git.openjdk.org/jdk/pull/12708

Re: RFR: 8303392: Runtime.exec and ProcessBuilder.start should use System logger [v2]

On Sun, 5 Mar 2023 06:14:49 GMT, Thomas Stuefe  wrote:

> I think to be even more useful it could make sense to print the current 
> directory the child is started in as well as the env var array, possibly only 
> with a finer logging level.

There are concerns about exposing potentially sensitive information in 
exceptions and logs.
And it would need to apply to all process arguments, not just directory and 
environment.
For example, there is a property "jdk.includeInExceptions" that is used to only 
include detail information if it is set.
Unless specifically enabled, the information such as host names and file names 
are not exposed.
Restricting the detail to finer grained logging levels might be the/an answer.

-

PR: https://git.openjdk.org/jdk/pull/12862

Re: RFR: 8294982: Implementation of Classfile API [v36]

On Thu, 2 Mar 2023 23:28:23 GMT, Paul Sandoz  wrote:

>> Adam Sotona has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   StackMapFrameInfo extracted to top level from StackMapTableAttribute
>
> src/java.base/share/classes/jdk/internal/classfile/CodeBuilder.java line 165:
> 
>> 163:  * @return this builder
>> 164:  */
>> 165: default CodeBuilder transforming(CodeTransform transform, 
>> Consumer handler) {
> 
> The functionality of this method, `transforming`, and 
> `ClassfileBuilder::transform`, are in effect equivalent in their 
> transforming: adding the results of transformed code to the builder. They 
> differ in the source of code elements.
> 
> The latter's behaviour can be implemented using the former, with a  consumer 
> that passes all elements of a code model to the builder e.g. `builder -> 
> model.forEach(builder::with)`.
> 
> The difference in naming initially confused me. To me this suggests the 
> method names should be the same? (perhaps with the transformer being 
> consistently the last argument?).

The `CodeBuilder::transforming` solves a bit different use cases than all the 
other transform.
It is designed to be able to use code transformations on a code building 
handler within a single pass.
Main reason is support of features like `StackTracker` in a form of code 
transformation. `StackTracker` (or any other similar tool requiring to monitor 
or affect code building) is passed as a transformation of a code fragment, 
while it can immediately serve as a source of information necessary to generate 
follow-up bytecode of the same method (in the same pass). 
Example of such use case is here: 
https://github.com/openjdk/jdk/blob/0e43af667ba6c6bda61461c260688bc46d3f3474/src/java.base/share/classes/jdk/internal/classfile/components/CodeStackTracker.java#L49

These code generation/transformation cases must be handled in a single pass and 
`CodeBuilder::transforming` method has no similar peer in any other (method, 
field or class) builder, because it is not necessary.

-

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: 8302791: Add specific ClassLoader object to Proxy IllegalArgumentException message [v6]

2023-03-06 Thread Mandy Chung

On Mon, 6 Mar 2023 11:08:42 GMT, Ravali Yatham  wrote:

>> Added specific class loader object to proxy IllegalArgumentException from 
>> which the class was not visible
>> 
>> The bug report for the same: https://bugs.openjdk.org/browse/JDK-8302791
>
> Ravali Yatham has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Added JavaLangAccess::getLoaderNameID(ClassLoader loader)

Looks good.  Thanks for the change.

-

Marked as reviewed by mchung (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12641

Re: RFR: 8294982: Implementation of Classfile API [v49]

> This is root pull request with Classfile API implementation, tests and 
> benchmarks initial drop into JDK.
> 
> Following pull requests consolidating JDK class files parsing, generating, 
> and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) 
> will chain to this one.
> 
> Classfile API development is tracked at:
> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch
> 
> Development branch of consolidated JDK class files parsing, generating, and 
> transforming is at:
> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch
> 
> Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online 
> API 
> documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html)
>  is also available.
> 
> Please take you time to review this non-trivial JDK addition.
> 
> Thank you,
> Adam

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

  snippets and tests synced with jdk.jfr class instrumentation source code

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/10982/files
  - new: https://git.openjdk.org/jdk/pull/10982/files/0e43af66..6df1297e

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=10982&range=48
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10982&range=47-48

  Stats: 13 lines in 2 files changed: 1 ins; 3 del; 9 mod
  Patch: https://git.openjdk.org/jdk/pull/10982.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10982/head:pull/10982

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: 8294982: Implementation of Classfile API [v36]

2023-03-06 Thread Paul Sandoz

On Mon, 6 Mar 2023 17:02:46 GMT, Adam Sotona  wrote:

>> src/java.base/share/classes/jdk/internal/classfile/CodeBuilder.java line 165:
>> 
>>> 163:  * @return this builder
>>> 164:  */
>>> 165: default CodeBuilder transforming(CodeTransform transform, 
>>> Consumer handler) {
>> 
>> The functionality of this method, `transforming`, and 
>> `ClassfileBuilder::transform`, are in effect equivalent in their 
>> transforming: adding the results of transformed code to the builder. They 
>> differ in the source of code elements.
>> 
>> The latter's behaviour can be implemented using the former, with a  consumer 
>> that passes all elements of a code model to the builder e.g. `builder -> 
>> model.forEach(builder::with)`.
>> 
>> The difference in naming initially confused me. To me this suggests the 
>> method names should be the same? (perhaps with the transformer being 
>> consistently the last argument?).
>
> The `CodeBuilder::transforming` solves a bit different use cases than all the 
> other transform.
> It is designed to be able to use code transformations on a code building 
> handler within a single pass.
> Main reason is support of features like `StackTracker` in a form of code 
> transformation. `StackTracker` (or any other similar tool requiring to 
> monitor or affect code building) is passed as a transformation of a code 
> fragment, while it can immediately serve as a source of information necessary 
> to generate follow-up bytecode of the same method (in the same pass). 
> Example of such use case is here: 
> https://github.com/openjdk/jdk/blob/0e43af667ba6c6bda61461c260688bc46d3f3474/src/java.base/share/classes/jdk/internal/classfile/components/CodeStackTracker.java#L49
> 
> These code generation/transformation cases must be handled in a single pass 
> and `CodeBuilder::transforming` method has no similar peer in any other 
> (method, field or class) builder, because it is not necessary.

The use-case seems fine to me (and that it only makes sense for building code). 
I still think it's a "transform", but with a different source. Subtly changing 
the name makes it seem different and fundamentally it is not AFAICT. If there 
is a separate name I think it should reflect the difference in source input to 
the transformation, rather than differentiate via the present participle.

-

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: 8294982: Implementation of Classfile API [v36]

On Thu, 2 Mar 2023 19:56:08 GMT, Paul Sandoz  wrote:

>> Adam Sotona has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   StackMapFrameInfo extracted to top level from StackMapTableAttribute
>
> src/java.base/share/classes/jdk/internal/classfile/components/snippet-files/PackageSnippets.java
>  line 149:
> 
>> 147: var instrumentorCodeMap = instrumentor.methods().stream()
>> 148:   
>> .filter(instrumentedMethodsFilter)
>> 149:   
>> .collect(Collectors.toMap(mm -> mm.methodName().stringValue() + 
>> mm.methodType().stringValue(), mm -> mm.code().orElse(null)));
> 
> Should we be filtering out abstract methods before the `collect` and/or 
> replace with:
> 
> mm -> mm.code().orElseThrow()
> 
> ?

Abstract methods should not be selected for instrumentation, so `orElseThrow()` 
is a good idea.
Fixed, thanks.

> src/java.base/share/classes/jdk/internal/classfile/components/snippet-files/PackageSnippets.java
>  line 171:
> 
>> 169: 
>> 170: //store stacked method 
>> parameters into locals
>> 171: var storeStack = new 
>> LinkedList();
> 
> FWIW you can use an ArrayDeque + push + forEach, in the spirit of 
> highlighting Deque over LinkedList for stack-based usage (since this is an 
> example with visibility).

fixed, thanks.

-

PR: https://git.openjdk.org/jdk/pull/10982

Integrated: 8303440: The "ZonedDateTime.parse" may not accept the "UTC+XX" zone id

2023-03-06 Thread Naoto Sato

The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

--
On Fri, 3 Mar 2023 20:51:14 GMT, Naoto Sato  wrote:

> This change is to fix a regression caused by the fix to 
> [JDK-8234347](https://bugs.openjdk.org/browse/JDK-8234347). The previous fix 
> was simply disabling offset-based parsing if the parser was text-based. 
> Instead, check if it is an offset or not by explicitly comparing the 
> character with '+'/'-' and continue offset parsing if it is.

This pull request has now been integrated.

Changeset: cfb0a25a
Author:Naoto Sato 
URL:   
https://git.openjdk.org/jdk/commit/cfb0a25a4ee1a9cebd88c84fa622c46fe1c89bae
Stats: 81 lines in 2 files changed: 79 ins; 0 del; 2 mod

8303440: The "ZonedDateTime.parse" may not accept the "UTC+XX" zone id

Reviewed-by: iris, jpai, rriggs, lancea

-

PR: https://git.openjdk.org/jdk/pull/12868

Re: RFR: 8294982: Implementation of Classfile API [v36]

The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

--
On Mon, 6 Mar 2023 17:15:25 GMT, Paul Sandoz  wrote:

>> The `CodeBuilder::transforming` solves a bit different use cases than all 
>> the other transform.
>> It is designed to be able to use code transformations on a code building 
>> handler within a single pass.
>> Main reason is support of features like `StackTracker` in a form of code 
>> transformation. `StackTracker` (or any other similar tool requiring to 
>> monitor or affect code building) is passed as a transformation of a code 
>> fragment, while it can immediately serve as a source of information 
>> necessary to generate follow-up bytecode of the same method (in the same 
>> pass). 
>> Example of such use case is here: 
>> https://github.com/openjdk/jdk/blob/0e43af667ba6c6bda61461c260688bc46d3f3474/src/java.base/share/classes/jdk/internal/classfile/components/CodeStackTracker.java#L49
>> 
>> These code generation/transformation cases must be handled in a single pass 
>> and `CodeBuilder::transforming` method has no similar peer in any other 
>> (method, field or class) builder, because it is not necessary.
>
> The use-case seems fine to me (and that it only makes sense for building 
> code). I still think it's a "transform", but with a different source. Subtly 
> changing the name makes it seem different and fundamentally it is not AFAICT. 
> If there is a separate name I think it should reflect the difference in 
> source input to the transformation, rather than differentiate via the present 
> participle.

I see what you mean. `transforming` was selected to represent the continuation 
of the code building process, similar to `trying` and `catching`. While 
`transform` may imply that the actual code builder gets transformed into 
something else. For example `MethodModel::transformCode` takes `CodeModel`, 
applies `CodeTransform` and after that the code of the actual method is 
finished.
What about `transformBlock(BlockCodeBuilder, CodeTransform)`?

-

PR: https://git.openjdk.org/jdk/pull/10982

Re: RFR: 8294982: Implementation of Classfile API [v36]

2023-03-06 Thread Paul Sandoz

On Mon, 6 Mar 2023 17:34:35 GMT, Adam Sotona  wrote:

>> The use-case seems fine to me (and that it only makes sense for building 
>> code). I still think it's a "transform", but with a different source. Subtly 
>> changing the name makes it seem different and fundamentally it is not 
>> AFAICT. If there is a separate name I think it should reflect the difference 
>> in source input to the transformation, rather than differentiate via the 
>> present participle.
>
> I see what you mean. `transforming` was selected to represent the 
> continuation of the code building process, similar to `trying` and 
> `catching`. While `transform` may imply that the actual code builder gets 
> transformed into something else. For example `MethodModel::transformCode` 
> takes `CodeModel`, applies `CodeTransform` and after that the code of the 
> actual method is finished.
> What about `transformBlock(BlockCodeBuilder, CodeTransform)`?

I am unsure how you might use `BlockCodeBuilder`. If the current signature is 
not changed then `transformFromHandler` seems reasonable (since its the handler 
that pushes elements into its given builder).

The other `transform` is implicitly "transform model".

-

PR: https://git.openjdk.org/jdk/pull/10982

ZipOutputStream & directories

2023-03-06 Thread Eirik Bjørsnøs

Hi,

I noticed that ZipOutputStream violates APPNOTE.txt when writing directory
entries:

Zero-byte files, directories, and other file types that
> contain no content MUST NOT include file data.


Nobody seems to have complained about this violation (I searched JBS), so
in itself it might not be worth pursuing a fix. However, in addition to
breaking the specification, this also causes suboptimal performance both on
the write and read paths.

Before I go ahead and suggest any particular solution, I wanted to share
some analysis:

ZipOutputStream writes directories as any other entry, so by default it
ends up writing a LOC header with the DEFLATED method and the data
descriptor bit set. This is followed by a final, empty DEFLATE block
(0x0300) and then a signed data descriptor with the CRC, csize (2) and size
(0), each 4 bytes:

--  Local File Header  --
00  signature  0x04034b50
04  version20
06  flags  0x0808
08  method 8  Deflated
10  time   0x9812 19:00:36
12  date   0x5666 2023-03-06
14  crc0x
18  csize  0
22  size   0
26  nlen   9
28  elen   0
30  name   9 bytes'META-INF/'

--  File Data  --
39  data   2 bytes

--  Data Descriptor  --
41  signature  0x08074b50
45  crc0x
49  csize  2
53  size   0

In total, this means that an extra 18 bytes is output, which is a waste of
storage space and IO cycles while writing and reading/parsing the
unnecessary headers and data. If the directory entry has the STORED method
and the sizes and crc is set to 0, then the file data and data descriptors
are skipped and we reclaim 18 bytes.

The use of DEFLATE has an adverse negative impact on performance, since
native ZLIB code needs to be called to produce and read (in ZipInputStream)
the empty DEFLATE block. Worse, Deflater.reset and Inflater.reset are
called, which seems to incur a significant overhead.

In fact, when configuring the STORED method and explicitly setting csize,
size and crc to 0, we see considerable benchmark improvements writing and
reading directory entries:

DEFLATED:

Benchmark(size)  Mode  Cnt  Score   Error  Units
ZipEmptyStreams.readDirStream   512  avgt   15  0.278 ± 0.005  ms/op
ZipEmptyStreams.readDirStream  1024  avgt   15  0.550 ± 0.006  ms/op
ZipEmptyStreams.writeDirStreams 512  avgt   15  2.379 ± 0.043  ms/op
ZipEmptyStreams.writeDirStreams1024  avgt   15  4.845 ± 0.087  ms/op

STORED:

Benchmark   (size)  Mode  Cnt  Score   Error  Units
ZipEmptyStreams.readDirStream  512  avgt   15  0.082 ± 0.001  ms/op
ZipEmptyStreams.readDirStream 1024  avgt   15  0.176 ± 0.015  ms/op
ZipEmptyStreams.writeDirStream 512  avgt   15  0.233 ± 0.021  ms/op
ZipEmptyStreams.writeDirStream1024  avgt   15  0.460 ± 0.028  ms/op


Possible actions:

- Do nothing: Nobody complained so far, so if is ain't broke, don't fix it.
- Update Javadocs of ZipInputStream.putNextEntry to recommend that users
should use STORED  and configure sizes & crc for directory entries for
correctness and performance
- Change the default compression method for directories to STORED and set
sizes & crc to 0. This would add the risk of changing behavior of existing
code.

For context, 7% of entries in the dependencies of Spring Petclinic are
directories.

Any thoughts or input?

Thanks,
Eirik.

Re: ZipOutputStream & directories

Hi Eirik,

Please see below

On Mar 6, 2023, at 1:43 PM, Eirik Bjørsnøs 
mailto:eir...@gmail.com>> wrote:


Hi,

I noticed that ZipOutputStream violates APPNOTE.txt when writing directory 
entries:

You are referring to section:



 4.3.8 File data


  Immediately following the local header for a file
  SHOULD be placed the compressed or stored data for the file.
  If the file is encrypted, the encryption header for the file
  SHOULD be placed after the local header and before the file
  data. The series of [local file header][encryption header]
  [file data][data descriptor] repeats for each file in the
  .ZIP archive.

  Zero-byte files, directories, and other file types that
  contain no content MUST NOT include file data.

——

I don’t think changing the default will cause any harm but I have been 
surprised by even the most trivial change before.

The jar tool  already addresses this in Main.java:addFile() on a quick glance.

So I guess I do not see a huge downside, however we might want to look at a 
property to adjust in the unlikely event the unexpected occurs?

Alan thoughts?




Zero-byte files, directories, and other file types that
contain no content MUST NOT include file data.

Nobody seems to have complained about this violation (I searched JBS), so in 
itself it might not be worth pursuing a fix. However, in addition to breaking 
the specification, this also causes suboptimal performance both on the write 
and read paths.

Before I go ahead and suggest any particular solution, I wanted to share some 
analysis:

ZipOutputStream writes directories as any other entry, so by default it ends up 
writing a LOC header with the DEFLATED method and the data descriptor bit set. 
This is followed by a final, empty DEFLATE block (0x0300) and then a signed 
data descriptor with the CRC, csize (2) and size (0), each 4 bytes:

--  Local File Header  --
00  signature  0x04034b50
04  version20
06  flags  0x0808
08  method 8  Deflated
10  time   0x9812 19:00:36
12  date   0x5666 2023-03-06
14  crc0x
18  csize  0
22  size   0
26  nlen   9
28  elen   0
30  name   9 bytes'META-INF/'

--  File Data  --
39  data   2 bytes

--  Data Descriptor  --
41  signature  0x08074b50
45  crc0x
49  csize  2
53  size   0

In total, this means that an extra 18 bytes is output, which is a waste of 
storage space and IO cycles while writing and reading/parsing the unnecessary 
headers and data. If the directory entry has the STORED method and the sizes 
and crc is set to 0, then the file data and data descriptors are skipped and we 
reclaim 18 bytes.

The use of DEFLATE has an adverse negative impact on performance, since native 
ZLIB code needs to be called to produce and read (in ZipInputStream) the empty 
DEFLATE block. Worse, Deflater.reset and Inflater.reset are called, which seems 
to incur a significant overhead.

In fact, when configuring the STORED method and explicitly setting csize, size 
and crc to 0, we see considerable benchmark improvements writing and reading 
directory entries:

DEFLATED:

Benchmark(size)  Mode  Cnt  Score   Error  Units
ZipEmptyStreams.readDirStream   512  avgt   15  0.278 ± 0.005  ms/op
ZipEmptyStreams.readDirStream  1024  avgt   15  0.550 ± 0.006  ms/op
ZipEmptyStreams.writeDirStreams 512  avgt   15  2.379 ± 0.043  ms/op
ZipEmptyStreams.writeDirStreams1024  avgt   15  4.845 ± 0.087  ms/op

STORED:

Benchmark   (size)  Mode  Cnt  Score   Error  Units
ZipEmptyStreams.readDirStream  512  avgt   15  0.082 ± 0.001  ms/op
ZipEmptyStreams.readDirStream 1024  avgt   15  0.176 ± 0.015  ms/op
ZipEmptyStreams.writeDirStream 512  avgt   15  0.233 ± 0.021  ms/op
ZipEmptyStreams.writeDirStream1024  avgt   15  0.460 ± 0.028  ms/op


Possible actions:

- Do nothing: Nobody complained so far, so if is ain't broke, don't fix it.
- Update Javadocs of ZipInputStream.putNextEntry to recommend that users should 
use STORED  and configure sizes & crc for directory entries for correctness and 
performance
- Change the default compression method for directories to STORED and set sizes 
& crc to 0. This would add the risk of changing behavior of existing code.

For context, 7% of entries in the dependencies of Spring Petclinic are 
directories.

Any thoughts or input?

Thanks,
Eirik.

[cid:E1C4E2F0-ECD0-4C9D-ADB4-B16CA7BCB7FC@home]






Lance Andersen | Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com

Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v43]

> Enhance the Java programming language with string templates, which are 
> similar to string literals but contain embedded expressions. A string 
> template is interpreted at run time by replacing each expression with the 
> result of evaluating that expression, possibly after further validation and 
> transformation. This is a [preview language feature and 
> API](http://openjdk.java.net/jeps/12).

Jim Laskey has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains 59 commits:

 - Merge branch 'master' into 8285932
 - Javadoc corrections and bug fix for StringTemplate::combine
 - Tighten up reporting of string template errors (fewer messages)
 - Merge branch 'master' into 8285932
 - Merge branch 'master' into 8285932
 - Minor correction to javadoc
 - Merge branch 'master' into 8285932
 - CSR review
 - Bring up to date
 - Merge branch 'master' into 8285932
 - ... and 49 more: https://git.openjdk.org/jdk/compare/cfb0a25a...5d79f650

-

Changes: https://git.openjdk.org/jdk/pull/10889/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=10889&range=42
  Stats: 9552 lines in 81 files changed: 9452 ins; 24 del; 76 mod
  Patch: https://git.openjdk.org/jdk/pull/10889.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10889/head:pull/10889

PR: https://git.openjdk.org/jdk/pull/10889

Integrated: 8298939: Refactor open/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.sh to jtreg java test

2023-03-06 Thread Matthew Donovan

On Mon, 9 Jan 2023 19:54:24 GMT, Matthew Donovan  wrote:

> Removed SSLSocketParametersTest.sh script (which just called a Java file) and 
> configured the java code to run directly with jtreg

This pull request has now been integrated.

Changeset: ccfe1675
Author:Matthew Donovan 
Committer: Stuart Marks 
URL:   
https://git.openjdk.org/jdk/commit/ccfe1675a2a82accbca0ecd8bd6f1c167a1c06c6
Stats: 238 lines in 2 files changed: 34 ins; 149 del; 55 mod

8298939: Refactor open/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.sh to 
jtreg java test

Reviewed-by: dfuchs, smarks

-

PR: https://git.openjdk.org/jdk/pull/11910

Re: ZipOutputStream & directories

2023-03-06 Thread Alan Bateman


On 06/03/2023 19:16, Lance Andersen wrote:

:

So I guess I do not see a huge downside, however we might want to look 
at a property to adjust in the unlikely event the unexpected occurs?


Alan thoughts?

Yeah, the jar tool does the right thing and writes directories entries 
with the STORED method and a size of 0.


Eirik's mail mentions that 7% of Spring Petclinic dependences are 
directories. It might be interesting to dig into that to see how they 
are generated, is it mostly maven-jar-plugin and if so, which APIs is it 
using?


Changing ZOS.putNextEntry to ignore the method/size provided by the 
caller would change long standing behavior, and Hyrum's Law has it that 
somebody will notice. This may be something for an API note rather than 
an implementation change.


-Alan

Re: ZipOutputStream & directories



On Mar 6, 2023, at 2:44 PM, Alan Bateman 
mailto:alan.bate...@oracle.com>> wrote:

On 06/03/2023 19:16, Lance Andersen wrote:
:

So I guess I do not see a huge downside, however we might want to look at a 
property to adjust in the unlikely event the unexpected occurs?

Alan thoughts?

Yeah, the jar tool does the right thing and writes directories entries with the 
STORED method and a size of 0.

Eirik's mail mentions that 7% of Spring Petclinic dependences are directories. 
It might be interesting to dig into that to see how they are generated, is it 
mostly maven-jar-plugin and if so, which APIs is it using?

Changing ZOS.putNextEntry to ignore the method/size provided by the caller 
would change long standing behavior, and Hyrum's Law has it that somebody will 
notice. This may be something for an API note rather than an implementation 
change.

That is what I was thinking  (once bitten twice shy).   I agree an API note is 
probably best

Best
Lance

-Alan

[cid:E1C4E2F0-ECD0-4C9D-ADB4-B16CA7BCB7FC@home]






Lance Andersen | Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com

Re: ZipOutputStream & directories

2023-03-06 Thread Eirik Bjørsnøs

On Mon, Mar 6, 2023 at 8:45 PM Alan Bateman  wrote:

> Changing ZOS.putNextEntry to ignore the method/size provided by the
> caller would change long standing behavior, and Hyrum's Law has it that
> somebody will notice.

I was not thinking of changing anything if the caller actually specified a
method. This would be just for the default/unspecified case, in which case
ZipEntry.method is -1.

So:

if (e.method == -1) {

if (e.isDirectory()) {
   // New code:
   e.setMethod(STORED);
   e.setSize(0);
   e.setCrc(0);
} else {
// Today's code:
e.method = method;  // use default method
}
}

But I guess this doesn't completely alleviate concerns, since there might
be code out there doing:

zo.putNextEntry("directory/");
zo.write("Hello".getBytes());

(This would be very weird code, but people might have found it convenient
to attach data to directories somehow).

This may be something for an API note rather than
> an implementation change.
>

Am I right that this would not be a specification change, so it would not
require a CSR?

Thanks,

Re: RFR: 8303480: Miscellaneous fixes to mostly invisible doc comments [v2]

2023-03-06 Thread Pavel Rappo

> Please review this superficial documentation cleanup that was triggered by 
> unrelated analysis of doc comments in JDK API.
> 
> The only effect that this multi-area PR has on the JDK API Documentation 
> (i.e. the observable effect on the generated HTML pages) can be summarized as 
> follows:
> 
> 
> diff -ur build/macosx-aarch64/images/docs-before/api/serialized-form.html 
> build/macosx-aarch64/images/docs-after/api/serialized-form.html
> --- build/macosx-aarch64/images/docs-before/api/serialized-form.html  
> 2023-03-02 11:47:44
> +++ build/macosx-aarch64/images/docs-after/api/serialized-form.html   
> 2023-03-02 11:48:45
> @@ -17084,7 +17084,7 @@
>   throws  href="java.base/java/io/IOException.html" title="class in 
> java.io">IOException,
>  ClassNotFoundException
>  readObject is called to restore the 
> state of the
> - (@code BasicPermission} from a stream.
> + BasicPermission from a stream.
>  
>  Parameters:
>  s - the ObjectInputStream from which data 
> is read
> 
> Notes
> -
> 
> * I'm not an expert in any of the affected areas, except for jdk.javadoc, and 
> I was merely after misused tags. Because of that, I would appreciate reviews 
> from experts in other areas.
> * I discovered many more issues than I included in this PR. The excluded 
> issues seem to occur in infrequently updated third-party code (e.g. 
> javax.xml), which I assume we shouldn't touch unless necessary.
> * I will update copyright years after (and if) the fix had been approved, as 
> required.

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

 - Merge branch 'master' into 8303480
 - Initial commit

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/12826/files
  - new: https://git.openjdk.org/jdk/pull/12826/files/d2f4a553..87166408

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=12826&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=12826&range=00-01

  Stats: 13433 lines in 415 files changed: 9003 ins; 2610 del; 1820 mod
  Patch: https://git.openjdk.org/jdk/pull/12826.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12826/head:pull/12826

PR: https://git.openjdk.org/jdk/pull/12826

Re: RFR: 8303480: Miscellaneous fixes to mostly invisible doc comments [v2]

2023-03-06 Thread Jonathan Gibbons

On Fri, 3 Mar 2023 11:31:04 GMT, Alexey Ivanov  wrote:

>> Yes, iff means if-and-only-if and is used for extra precision in formal 
>> logic, mathematics. As @pavelrappo points out it's a relatively common 
>> occurrence in the OpenJDK sources, though perhaps not in the public 
>> javadocs. Perhaps a bit pretentious, but mostly a terse way to say "return 
>> true if the BSM method type exactly matches X, otherwise false".
>> 
>> The broken link stems from the fact that the method I was targeting (a way 
>> to use condy for lambda proxy singletons rather than a 
>> `MethodHandle.constant`) was never integrated. We'll look at either getting 
>> that done (@briangoetz suggested the time might be ready for it) or remove 
>> this currently pointless static bootstrap specialization test.
>
>> Yes, iff means if-and-only-if and is used for extra precision in formal 
>> logic, mathematics.
> 
> I've never come across it before. With your explanations, it makes perfect 
> sense.

I would recommend (separately) changing `iff` to the expanded form `if and only 
if`

-

PR: https://git.openjdk.org/jdk/pull/12826

Re: ZipOutputStream & directories

Adding an API Note will require a CSR.

And yes,  what we often think as the right change tends to bite us (given this 
code has been in the wild since the 90s) so best to err on the side of 
documentation vs. assuming this change would not cause more problems than it is 
worth.

Best
Lance

On Mar 6, 2023, at 3:20 PM, Eirik Bjørsnøs 
mailto:eir...@gmail.com>> wrote:

On Mon, Mar 6, 2023 at 8:45 PM Alan Bateman 
mailto:alan.bate...@oracle.com>> wrote:
Changing ZOS.putNextEntry to ignore the method/size provided by the
caller would change long standing behavior, and Hyrum's Law has it that
somebody will notice.

I was not thinking of changing anything if the caller actually specified a 
method. This would be just for the default/unspecified case, in which case 
ZipEntry.method is -1.

So:

if (e.method == -1) {

if (e.isDirectory()) {
   // New code:
   e.setMethod(STORED);
   e.setSize(0);
   e.setCrc(0);
} else {
// Today's code:
e.method = method;  // use default method
}
}

But I guess this doesn't completely alleviate concerns, since there might be 
code out there doing:

zo.putNextEntry("directory/");
zo.write("Hello".getBytes());

(This would be very weird code, but people might have found it convenient to 
attach data to directories somehow).

This may be something for an API note rather than
an implementation change.

Am I right that this would not be a specification change, so it would not 
require a CSR?

Thanks,


[cid:E1C4E2F0-ECD0-4C9D-ADB4-B16CA7BCB7FC@home]






Lance Andersen | Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com

Re: RFR: 8303480: Miscellaneous fixes to mostly invisible doc comments [v2]

2023-03-06 Thread Jonathan Gibbons

The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

--
On Mon, 6 Mar 2023 20:22:48 GMT, Pavel Rappo  wrote:

>> Please review this superficial documentation cleanup that was triggered by 
>> unrelated analysis of doc comments in JDK API.
>> 
>> The only effect that this multi-area PR has on the JDK API Documentation 
>> (i.e. the observable effect on the generated HTML pages) can be summarized 
>> as follows:
>> 
>> 
>> diff -ur 
>> build/macosx-aarch64/images/docs-before/api/serialized-form.html 
>> build/macosx-aarch64/images/docs-after/api/serialized-form.html
>> --- build/macosx-aarch64/images/docs-before/api/serialized-form.html 
>> 2023-03-02 11:47:44
>> +++ build/macosx-aarch64/images/docs-after/api/serialized-form.html  
>> 2023-03-02 11:48:45
>> @@ -17084,7 +17084,7 @@
>>   throws > href="java.base/java/io/IOException.html" title="class in 
>> java.io">IOException,
>>  ClassNotFoundException
>>  readObject is called to restore the 
>> state of the
>> - (@code BasicPermission} from a stream.
>> + BasicPermission from a stream.
>>  
>>  Parameters:
>>  s - the ObjectInputStream from which data 
>> is read
>> 
>> Notes
>> -
>> 
>> * I'm not an expert in any of the affected areas, except for jdk.javadoc, 
>> and I was merely after misused tags. Because of that, I would appreciate 
>> reviews from experts in other areas.
>> * I discovered many more issues than I included in this PR. The excluded 
>> issues seem to occur in infrequently updated third-party code (e.g. 
>> javax.xml), which I assume we shouldn't touch unless necessary.
>> * I will update copyright years after (and if) the fix had been approved, as 
>> required.
>
> Pavel Rappo has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains two additional 
> commits since the last revision:
> 
>  - Merge branch 'master' into 8303480
>  - Initial commit

Marked as reviewed by jjg (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/12826

Re: RFR: 8303480: Miscellaneous fixes to mostly invisible doc comments [v2]

On Mon, 6 Mar 2023 20:22:48 GMT, Pavel Rappo  wrote:

>> Please review this superficial documentation cleanup that was triggered by 
>> unrelated analysis of doc comments in JDK API.
>> 
>> The only effect that this multi-area PR has on the JDK API Documentation 
>> (i.e. the observable effect on the generated HTML pages) can be summarized 
>> as follows:
>> 
>> 
>> diff -ur 
>> build/macosx-aarch64/images/docs-before/api/serialized-form.html 
>> build/macosx-aarch64/images/docs-after/api/serialized-form.html
>> --- build/macosx-aarch64/images/docs-before/api/serialized-form.html 
>> 2023-03-02 11:47:44
>> +++ build/macosx-aarch64/images/docs-after/api/serialized-form.html  
>> 2023-03-02 11:48:45
>> @@ -17084,7 +17084,7 @@
>>   throws > href="java.base/java/io/IOException.html" title="class in 
>> java.io">IOException,
>>  ClassNotFoundException
>>  readObject is called to restore the 
>> state of the
>> - (@code BasicPermission} from a stream.
>> + BasicPermission from a stream.
>>  
>>  Parameters:
>>  s - the ObjectInputStream from which data 
>> is read
>> 
>> Notes
>> -
>> 
>> * I'm not an expert in any of the affected areas, except for jdk.javadoc, 
>> and I was merely after misused tags. Because of that, I would appreciate 
>> reviews from experts in other areas.
>> * I discovered many more issues than I included in this PR. The excluded 
>> issues seem to occur in infrequently updated third-party code (e.g. 
>> javax.xml), which I assume we shouldn't touch unless necessary.
>> * I will update copyright years after (and if) the fix had been approved, as 
>> required.
>
> Pavel Rappo has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains two additional 
> commits since the last revision:
> 
>  - Merge branch 'master' into 8303480
>  - Initial commit

Marked as reviewed by lancea (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/12826

Re: ZipOutputStream & directories

2023-03-06 Thread Eirik Bjørsnøs

>
> Eirik's mail mentions that 7% of Spring Petclinic dependences are
> directories. It might be interesting to dig into that to see how they
> are generated, is it mostly maven-jar-plugin and if so, which APIs is it
> using?
>

Some stats:

Of 109 JAR files, 2 do not have a manifest file, and 28 do not have a
"Created-By" attribute.

The distribution of "Created-By" of the remaining files:

1.4.2_09 (Apple Computer, Inc.): 1

Apache Maven Bundle Plugin: 30
Apache Maven Bundle Plugin 5.1.4: 2
Apache Maven Bundle Plugin 5.1.7: 2
Apache Maven 3.6.0: 1
Apache Maven 3.2.3: 1
Apache Maven 3.6.3: 2
Maven Jar Plugin 3.2.0: 5
Maven JAR Plugin 3.2.2: 2

1.6.0_32-b05 (Sun Microsystems Inc.): 1
1.8.0_25 (Oracle Corporation): 1
1.8.0_202 (Oracle Corporation): 1
1.8.0_252 (Oracle Corporation): 1
1.8.0_231 (Oracle Corporation): 1
1.8.0_241-b07 (Oracle Corporation): 1
1.8.0_281-b09 (Oracle Corporation): 1
1.8.0_333 (Oracle Corporation): 14
11.0.2 (Oracle Corporation 11.0.2+9): 1
11.0.8 (GraalVM Community): 1

11.0.11 (AdoptOpenJDK): 1
11.0.14 (Eclipse Adoptium): 1
11.0.13 (Eclipse Adoptium 11.0.13+8): 6
18.0.1+10 (Eclipse Adoptium): 1

I also checked which method was used for directories:

4 files had no directories.
65 files had DEFLATE only directories
34 files has STORED only directories
6 files (interestingly!) has a mix of DEFLATED and STORED directories.

Here is the count of directories of each method for each file:

jakarta.xml.bind-api-2.3.3.jar:
  STORED 17
jsonassert-1.5.1.jar:
  STORED 9
checker-qual-3.5.0.jar:
  DEFLATED 59
byte-buddy-agent-1.12.13.jar:
  STORED 13
spring-core-5.3.22.jar:
  DEFLATED 53
jboss-logging-3.4.3.Final.jar:
  STORED 7
spring-boot-actuator-2.7.3.jar:
  DEFLATED 85
spring-web-5.3.22.jar:
  DEFLATED 60
slf4j-api-1.7.36.jar:
  STORED 9
junit-jupiter-5.8.2.jar:
  DEFLATED 1
spring-aspects-5.3.22.jar:
  DEFLATED 15
micrometer-core-1.9.3.jar:
  DEFLATED 50
byte-buddy-1.12.13.jar:
  DEFLATED 48
spring-context-5.3.22.jar:
  DEFLATED 69
objenesis-3.2.jar:
  STORED 12
json-smart-2.4.8.jar:
  DEFLATED 11
spring-boot-starter-tomcat-2.7.3.jar:
  DEFLATED 1
thymeleaf-3.0.15.RELEASE.jar:
  STORED 42
postgresql-42.3.6.jar:
  DEFLATED 57
spring-context-support-5.3.22.jar:
  DEFLATED 17
junit-jupiter-engine-5.8.2.jar:
  DEFLATED 13
spring-boot-starter-2.7.3.jar:
  DEFLATED 1
jaxb-runtime-2.3.6.jar:
  STORED 39
mysql-connector-java-8.0.30.jar:
  STORED 36
log4j-api-2.17.2.jar:
  STORED 23
spring-boot-starter-jdbc-2.7.3.jar:
  DEFLATED 1
spring-boot-devtools-2.7.3.jar:
  DEFLATED 24
spring-boot-test-autoconfigure-2.7.3.jar:
  DEFLATED 37
spring-data-commons-2.7.2.jar:
  STORED 49
spring-boot-starter-actuator-2.7.3.jar:
  DEFLATED 1
log4j-to-slf4j-2.17.2.jar:
  STORED 9
spring-boot-starter-aop-2.7.3.jar:
  DEFLATED 1
jakarta.persistence-api-2.2.3.jar:
  STORED 9
logback-core-1.2.11.jar:
  STORED 41
spring-jcl-5.3.22.jar:
  DEFLATED 7
junit-platform-commons-1.8.2.jar:
  DEFLATED 16
assertj-core-3.22.0.jar:
  STORED 76
spring-test-5.3.22.jar:
  DEFLATED 50
json-path-2.7.0.jar:
  DEFLATED 16
spring-boot-starter-web-2.7.3.jar:
  DEFLATED 1
spring-beans-5.3.22.jar:
  DEFLATED 16
accessors-smart-2.4.8.jar:
  DEFLATED 8
jackson-core-2.13.3.jar:
  STORED 2
  DEFLATED 20
junit-platform-engine-1.8.2.jar:
  DEFLATED 13
spring-aop-5.3.22.jar:
  DEFLATED 21
asm-9.1.jar:
  DEFLATED 5
spring-boot-starter-test-2.7.3.jar:
  DEFLATED 1
classmate-1.5.1.jar:
  DEFLATED 10
jandex-2.4.2.Final.jar:
  DEFLATED 7
snakeyaml-1.30.jar:
  DEFLATED 35
HdrHistogram-2.1.12.jar:
  DEFLATED 7
thymeleaf-extras-java8time-3.0.4.RELEASE.jar:
  STORED 8
spring-boot-actuator-autoconfigure-2.7.3.jar:
  DEFLATED 106
thymeleaf-spring5-3.0.15.RELEASE.jar:
  STORED 24
jakarta.activation-1.2.2.jar:
  STORED 11
tomcat-embed-el-9.0.65.jar:
mockito-core-4.5.1.jar:
  DEFLATED 69
jakarta.transaction-api-1.3.3.jar:
  STORED 6
aspectjweaver-1.9.7.jar:
  DEFLATED 47
spring-boot-starter-validation-2.7.3.jar:
  DEFLATED 1
LatencyUtils-2.0.3.jar:
  STORED 6
junit-jupiter-params-5.8.2.jar:
  DEFLATED 29
spring-boot-starter-data-jpa-2.7.3.jar:
  DEFLATED 1
spring-boot-starter-logging-2.7.3.jar:
  DEFLATED 1
jakarta.activation-api-1.2.2.jar:
  STORED 6
spring-boot-starter-json-2.7.3.jar:
  DEFLATED 1
spring-boot-starter-thymeleaf-2.7.3.jar:
  DEFLATED 1
jackson-datatype-jsr310-2.13.3.jar:
  STORED 2
  DEFLATED 15
tomcat-embed-websocket-9.0.65.jar:
spring-jdbc-5.3.22.jar:
  DEFLATED 20
jackson-databind-2.13.3.jar:
  STORED 2
  DEFLATED 30
jul-to-slf4j-1.7.36.jar:
  STORED 7
jackson-annotations-2.13.3.jar:
  DEFLATED 8
tomcat-embed-core-9.0.65.jar:
hibernate-core-5.6.10.Final.jar:
  DEFLATED 344
istack-commons-runtime-3.0.12.jar:
  STORED 15
mockito-junit-jupiter-4.5.1.jar:
  DEFLATED 5
apiguardian-api-1.1.2.jar:
  DEFLATED 4
jakarta.annotation-api-1.3.5.jar:
  STORED 8
xmlunit-core-2.9.0.jar:
  STORED 14
bootstrap-5.1.3.jar:
  DEFLATED 5
hibernate-commons-annotations-5.1.2.Final.jar:
  DEFLATED 11
spring-webmvc-5.3.22.jar:
  DEFLATED 32
hibernate-validator-6.2.4.F

Re: RFR: 8303480: Miscellaneous fixes to mostly invisible doc comments [v2]

On Mon, 6 Mar 2023 20:22:48 GMT, Pavel Rappo  wrote:

>> Please review this superficial documentation cleanup that was triggered by 
>> unrelated analysis of doc comments in JDK API.
>> 
>> The only effect that this multi-area PR has on the JDK API Documentation 
>> (i.e. the observable effect on the generated HTML pages) can be summarized 
>> as follows:
>> 
>> 
>> diff -ur 
>> build/macosx-aarch64/images/docs-before/api/serialized-form.html 
>> build/macosx-aarch64/images/docs-after/api/serialized-form.html
>> --- build/macosx-aarch64/images/docs-before/api/serialized-form.html 
>> 2023-03-02 11:47:44
>> +++ build/macosx-aarch64/images/docs-after/api/serialized-form.html  
>> 2023-03-02 11:48:45
>> @@ -17084,7 +17084,7 @@
>>   throws > href="java.base/java/io/IOException.html" title="class in 
>> java.io">IOException,
>>  ClassNotFoundException
>>  readObject is called to restore the 
>> state of the
>> - (@code BasicPermission} from a stream.
>> + BasicPermission from a stream.
>>  
>>  Parameters:
>>  s - the ObjectInputStream from which data 
>> is read
>> 
>> Notes
>> -
>> 
>> * I'm not an expert in any of the affected areas, except for jdk.javadoc, 
>> and I was merely after misused tags. Because of that, I would appreciate 
>> reviews from experts in other areas.
>> * I discovered many more issues than I included in this PR. The excluded 
>> issues seem to occur in infrequently updated third-party code (e.g. 
>> javax.xml), which I assume we shouldn't touch unless necessary.
>> * I will update copyright years after (and if) the fix had been approved, as 
>> required.
>
> Pavel Rappo has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains two additional 
> commits since the last revision:
> 
>  - Merge branch 'master' into 8303480
>  - Initial commit

Marked as reviewed by rriggs (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/12826

Re: RFR: 8217496: Matcher.group() can return null after usePattern

2023-03-06 Thread Stuart Marks

On Thu, 2 Mar 2023 17:13:28 GMT, Ian Graves  wrote:

> Updates to the documentation to describe behavior in Matcher.group().

LGTM

-

Marked as reviewed by smarks (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12835

Re: RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter

On Fri, 3 Mar 2023 21:41:35 GMT, Vladimir Kozlov  wrote:

> Implemented `Float.floatToFloat16` and `Float.float16ToFloat` intrinsics in 
> Interpreter and C1 compiler to produce the same results as C2 intrinsics on 
> x64, Aarch64 and RISC-V - all platforms where C2 intrinsics for these Java 
> methods were implemented originally.
> 
> Replaced `SharedRuntime::f2hf()` and `hf2f()` C runtime functions with calls 
> to runtime stubs which use the same HW instructions as C2 intrinsics. Only 
> for 64-bit x64 because 32-bit x86 stub does not work: result is passed 
> through FPU register and NaN values become different from C2 intrinsic. This 
> runtime stub is only used to calculate constant values during C2 compilation 
> and can be skipped.
> 
> I added new tests based on Tobias's `TestAll.java` And copied 
> `jdk/lang/Float/Binary16Conversion*.java` tests to run them with `-Xcomp` to 
> make sure code is compiled by C1 or C2. I modified 
> `Binary16ConversionNaN.java` to compare results from Interpreter, C1 and C2.
> 
> Tested tier1-5, Xcomp, stress

@fyang, please help to verify that new tests passed on RISC-V with these 
changes and review these changes. Thanks!

I tested x86 (64- and 32-bit) and AArch64.

-

PR: https://git.openjdk.org/jdk/pull/12869

Re: RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter

On Mon, 6 Mar 2023 23:54:44 GMT, Vladimir Kozlov  wrote:

>> Implemented `Float.floatToFloat16` and `Float.float16ToFloat` intrinsics in 
>> Interpreter and C1 compiler to produce the same results as C2 intrinsics on 
>> x64, Aarch64 and RISC-V - all platforms where C2 intrinsics for these Java 
>> methods were implemented originally.
>> 
>> Replaced `SharedRuntime::f2hf()` and `hf2f()` C runtime functions with calls 
>> to runtime stubs which use the same HW instructions as C2 intrinsics. Only 
>> for 64-bit x64 because 32-bit x86 stub does not work: result is passed 
>> through FPU register and NaN values become different from C2 intrinsic. This 
>> runtime stub is only used to calculate constant values during C2 compilation 
>> and can be skipped.
>> 
>> I added new tests based on Tobias's `TestAll.java` And copied 
>> `jdk/lang/Float/Binary16Conversion*.java` tests to run them with `-Xcomp` to 
>> make sure code is compiled by C1 or C2. I modified 
>> `Binary16ConversionNaN.java` to compare results from Interpreter, C1 and C2.
>> 
>> Tested tier1-5, Xcomp, stress
>
> @fyang, please help to verify that new tests passed on RISC-V with these 
> changes and review these changes. Thanks!
> 
> I tested x86 (64- and 32-bit) and AArch64.

@vnkozlov Thanks a lot for taking this up. Is the following in the PR 
description still true:
"Replaced SharedRuntime::f2hf() and hf2f() C runtime functions with calls to 
runtime stubs which use the same HW instructions as C2 intrinsics. Only for 
64-bit x64 because 32-bit x86 stub does not work: result is passed through FPU 
register and NaN values become different from C2 intrinsic."
>From the PR it looks to me that for x86_64 you have the changes in place for 
>SharedRuntime and the same result is produced across SharedRuntime, 
>interpreter, c1, and c2.
For x86 32-bit also things are consistent across. Only the SharedRuntime 
optimization doesnt happen for x86 32bit as StubRoutines::hf2f() and 
StubRoutines::f2hf() are set as null. The fallback is handled correctly in 
interpreter, c1, and c2.

-

PR: https://git.openjdk.org/jdk/pull/12869

Re: RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter

On Tue, 7 Mar 2023 00:19:03 GMT, Sandhya Viswanathan  
wrote:

> For x86 32-bit also things are consistent across. Only the SharedRuntime 
> optimization doesnt happen for x86 32bit as StubRoutines::hf2f() and 
> StubRoutines::f2hf() are set as null. The fallback is handled correctly in 
> interpreter, c1, and c2.

Correct, it is consistent.  Only optimization to calculate constant value 
during compile time is skipped. C2 will generate HW instruction for `ConvF2HF` 
node as if its input was not constant. That is it.

It is possible to add similar Stub routines  for AArch64 and RISC-V to be 
called from C2 but I am not expert in those platforms so I skipped them.

-

PR: https://git.openjdk.org/jdk/pull/12869

Re: RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter

On Fri, 3 Mar 2023 21:41:35 GMT, Vladimir Kozlov  wrote:

> Implemented `Float.floatToFloat16` and `Float.float16ToFloat` intrinsics in 
> Interpreter and C1 compiler to produce the same results as C2 intrinsics on 
> x64, Aarch64 and RISC-V - all platforms where C2 intrinsics for these Java 
> methods were implemented originally.
> 
> Replaced `SharedRuntime::f2hf()` and `hf2f()` C runtime functions with calls 
> to runtime stubs which use the same HW instructions as C2 intrinsics. Only 
> for 64-bit x64 because 32-bit x86 stub does not work: result is passed 
> through FPU register and NaN values become different from C2 intrinsic. This 
> runtime stub is only used to calculate constant values during C2 compilation 
> and can be skipped.
> 
> I added new tests based on Tobias's `TestAll.java` And copied 
> `jdk/lang/Float/Binary16Conversion*.java` tests to run them with `-Xcomp` to 
> make sure code is compiled by C1 or C2. I modified 
> `Binary16ConversionNaN.java` to compare results from Interpreter, C1 and C2.
> 
> Tested tier1-5, Xcomp, stress

Note, I removed `ConvF2HFNode::Identity()` optimization because tests show that 
it produces different NaN results due to skipped conversion.

-

PR: https://git.openjdk.org/jdk/pull/12869

RFR: 8299779: Test tools/jpackage/share/jdk/jpackage/tests/MainClassTest.java timed out

2023-03-06 Thread Alexander Matveev

- Fixed by increasing test timeout. Fix verified on host which reproduced issue.

-

Commit messages:
 - 8299779: Test tools/jpackage/share/jdk/jpackage/tests/MainClassTest.java 
timed out

Changes: https://git.openjdk.org/jdk/pull/12896/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12896&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8299779
  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/12896.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12896/head:pull/12896

PR: https://git.openjdk.org/jdk/pull/12896

Re: RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter

On Fri, 3 Mar 2023 21:41:35 GMT, Vladimir Kozlov  wrote:

> Implemented `Float.floatToFloat16` and `Float.float16ToFloat` intrinsics in 
> Interpreter and C1 compiler to produce the same results as C2 intrinsics on 
> x64, Aarch64 and RISC-V - all platforms where C2 intrinsics for these Java 
> methods were implemented originally.
> 
> Replaced `SharedRuntime::f2hf()` and `hf2f()` C runtime functions with calls 
> to runtime stubs which use the same HW instructions as C2 intrinsics. Only 
> for 64-bit x64 because 32-bit x86 stub does not work: result is passed 
> through FPU register and NaN values become different from C2 intrinsic. This 
> runtime stub is only used to calculate constant values during C2 compilation 
> and can be skipped.
> 
> I added new tests based on Tobias's `TestAll.java` And copied 
> `jdk/lang/Float/Binary16Conversion*.java` tests to run them with `-Xcomp` to 
> make sure code is compiled by C1 or C2. I modified 
> `Binary16ConversionNaN.java` to compare results from Interpreter, C1 and C2.
> 
> Tested tier1-5, Xcomp, stress

src/hotspot/cpu/x86/macroAssembler_x86.hpp line 199:

> 197:   void flt_to_flt16(Register dst, XMMRegister src, XMMRegister tmp) {
> 198: // Instruction requires different XMM registers
> 199: vcvtps2ph(tmp, src, 0x04, Assembler::AVX_128bit);

vcvtps2ph can have source and destination as same. Did you mean to say here in 
the comment that "Instruction requires XMM register as destination"?

src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 3928:

> 3926:   }
> 3927: 
> 3928:   if (VM_Version::supports_f16c() || VM_Version::supports_avx512vl()) {

We could check for VM_Version::supports_float16() here instead.

src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 3931:

> 3929: // For results consistency both intrinsics should be enabled.
> 3930: if 
> (vmIntrinsics::is_intrinsic_available(vmIntrinsics::_float16ToFloat) &&
> 3931: 
> vmIntrinsics::is_intrinsic_available(vmIntrinsics::_floatToFloat16)) {

Should this also check for InlineIntrinsics?

src/hotspot/cpu/x86/templateInterpreterGenerator_x86_64.cpp line 346:

> 344:   }
> 345:   // For AVX CPUs only. f16c support is disabled if UseAVX == 0.
> 346:   if (VM_Version::supports_f16c() || VM_Version::supports_avx512vl()) {

We could check for VM_Version::supports_float16() here instead.

-

PR: https://git.openjdk.org/jdk/pull/12869

Re: RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter

On Tue, 7 Mar 2023 00:52:37 GMT, Vladimir Kozlov  wrote:

> Note, I removed `ConvF2HFNode::Identity()` optimization because tests show 
> that it produces different NaN results due to skipped conversion.

Yes, removing the Identity optimization is correct.  It doesn't hold for NaN 
inputs.

-

PR: https://git.openjdk.org/jdk/pull/12869

Re: RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter

On Fri, 3 Mar 2023 21:41:35 GMT, Vladimir Kozlov  wrote:

> Implemented `Float.floatToFloat16` and `Float.float16ToFloat` intrinsics in 
> Interpreter and C1 compiler to produce the same results as C2 intrinsics on 
> x64, Aarch64 and RISC-V - all platforms where C2 intrinsics for these Java 
> methods were implemented originally.
> 
> Replaced `SharedRuntime::f2hf()` and `hf2f()` C runtime functions with calls 
> to runtime stubs which use the same HW instructions as C2 intrinsics. Only 
> for 64-bit x64 because 32-bit x86 stub does not work: result is passed 
> through FPU register and NaN values become different from C2 intrinsic. This 
> runtime stub is only used to calculate constant values during C2 compilation 
> and can be skipped.
> 
> I added new tests based on Tobias's `TestAll.java` And copied 
> `jdk/lang/Float/Binary16Conversion*.java` tests to run them with `-Xcomp` to 
> make sure code is compiled by C1 or C2. I modified 
> `Binary16ConversionNaN.java` to compare results from Interpreter, C1 and C2.
> 
> Tested tier1-5, Xcomp, stress

Other than the minor comments above, the x86 side changes look good to me.

-

PR: https://git.openjdk.org/jdk/pull/12869

Re: RFR: JDK-8302801: Remove fdlibm C sources [v4]

2023-03-06 Thread Joe Darcy

On Sun, 5 Mar 2023 17:10:08 GMT, Joe Darcy  wrote:

> PS Successful Mach 5 job of default builds and tier 1 tests with this make 
> line present.

PPS And for extra measure as suggested by David Holmes, a tier 1 through 5 
build job was also successful.

-

PR: https://git.openjdk.org/jdk/pull/12821

Re: RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter

On Fri, 3 Mar 2023 21:41:35 GMT, Vladimir Kozlov  wrote:

> Implemented `Float.floatToFloat16` and `Float.float16ToFloat` intrinsics in 
> Interpreter and C1 compiler to produce the same results as C2 intrinsics on 
> x64, Aarch64 and RISC-V - all platforms where C2 intrinsics for these Java 
> methods were implemented originally.
> 
> Replaced `SharedRuntime::f2hf()` and `hf2f()` C runtime functions with calls 
> to runtime stubs which use the same HW instructions as C2 intrinsics. Only 
> for 64-bit x64 because 32-bit x86 stub does not work: result is passed 
> through FPU register and NaN values become different from C2 intrinsic. This 
> runtime stub is only used to calculate constant values during C2 compilation 
> and can be skipped.
> 
> I added new tests based on Tobias's `TestAll.java` And copied 
> `jdk/lang/Float/Binary16Conversion*.java` tests to run them with `-Xcomp` to 
> make sure code is compiled by C1 or C2. I modified 
> `Binary16ConversionNaN.java` to compare results from Interpreter, C1 and C2.
> 
> Tested tier1-5, Xcomp, stress

Thank you for review @sviswa7. I will address you comments.

-

PR: https://git.openjdk.org/jdk/pull/12869

Re: RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter

On Tue, 7 Mar 2023 01:04:00 GMT, Sandhya Viswanathan  
wrote:

>> Implemented `Float.floatToFloat16` and `Float.float16ToFloat` intrinsics in 
>> Interpreter and C1 compiler to produce the same results as C2 intrinsics on 
>> x64, Aarch64 and RISC-V - all platforms where C2 intrinsics for these Java 
>> methods were implemented originally.
>> 
>> Replaced `SharedRuntime::f2hf()` and `hf2f()` C runtime functions with calls 
>> to runtime stubs which use the same HW instructions as C2 intrinsics. Only 
>> for 64-bit x64 because 32-bit x86 stub does not work: result is passed 
>> through FPU register and NaN values become different from C2 intrinsic. This 
>> runtime stub is only used to calculate constant values during C2 compilation 
>> and can be skipped.
>> 
>> I added new tests based on Tobias's `TestAll.java` And copied 
>> `jdk/lang/Float/Binary16Conversion*.java` tests to run them with `-Xcomp` to 
>> make sure code is compiled by C1 or C2. I modified 
>> `Binary16ConversionNaN.java` to compare results from Interpreter, C1 and C2.
>> 
>> Tested tier1-5, Xcomp, stress
>
> src/hotspot/cpu/x86/macroAssembler_x86.hpp line 199:
> 
>> 197:   void flt_to_flt16(Register dst, XMMRegister src, XMMRegister tmp) {
>> 198: // Instruction requires different XMM registers
>> 199: vcvtps2ph(tmp, src, 0x04, Assembler::AVX_128bit);
> 
> vcvtps2ph can have source and destination as same. Did you mean to say here 
> in the comment that "Instruction requires XMM register as destination"?

`flt_to_flt16` is used in `x86.ad` instruction which requires preserving `src` 
register.
I did not want to add an other macroassembler instruction for src->src case.
I will add this to this comment.

> src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 3928:
> 
>> 3926:   }
>> 3927: 
>> 3928:   if (VM_Version::supports_f16c() || VM_Version::supports_avx512vl()) {
> 
> We could check for VM_Version::supports_float16() here instead.

Yes.

> src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 3931:
> 
>> 3929: // For results consistency both intrinsics should be enabled.
>> 3930: if 
>> (vmIntrinsics::is_intrinsic_available(vmIntrinsics::_float16ToFloat) &&
>> 3931: 
>> vmIntrinsics::is_intrinsic_available(vmIntrinsics::_floatToFloat16)) {
> 
> Should this also check for InlineIntrinsics?

`vmIntrinsics::disabled_by_jvm_flags()` checks `InlineIntrinsics`. See 
`vmIntrinsics.cpp` changes.

> src/hotspot/cpu/x86/templateInterpreterGenerator_x86_64.cpp line 346:
> 
>> 344:   }
>> 345:   // For AVX CPUs only. f16c support is disabled if UseAVX == 0.
>> 346:   if (VM_Version::supports_f16c() || VM_Version::supports_avx512vl()) {
> 
> We could check for VM_Version::supports_float16() here instead.

Yes. And I need to remove `!InlineIntrinsics` check at line 340.

-

PR: https://git.openjdk.org/jdk/pull/12869

Re: RFR: 8303499: [s390x] ProblemList StressStackOverflow

2023-03-06 Thread David Holmes

The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.

--
On Thu, 2 Mar 2023 16:22:24 GMT, Amit Kumar  wrote:

> This PR adds StressStackOverflow.java test to ProblemList. The feature Scope 
> Value is not yet implemented on s390x-arch which is Incubating state. 
> Whenever fix will be given to 
> [JDK-8303498](https://bugs.openjdk.org/browse/JDK-8303498) then we will 
> remove it.

Wouldn't this also be an issue on AIX?

-

PR: https://git.openjdk.org/jdk/pull/12834

Re: RFR: 8303499: [s390x] ProblemList StressStackOverflow

On Tue, 7 Mar 2023 02:23:52 GMT, David Holmes  wrote:

>> This PR adds StressStackOverflow.java test to ProblemList. The feature Scope 
>> Value is not yet implemented on s390x-arch which is Incubating state. 
>> Whenever fix will be given to 
>> [JDK-8303498](https://bugs.openjdk.org/browse/JDK-8303498) then we will 
>> remove it.
>
> Wouldn't this also be an issue on AIX?

Yes @dholmes-ora it is, and I informed Tyler Steele about it.

-

PR: https://git.openjdk.org/jdk/pull/12834

Re: RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter

On Tue, 7 Mar 2023 01:59:25 GMT, Vladimir Kozlov  wrote:

>> src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 3931:
>> 
>>> 3929: // For results consistency both intrinsics should be enabled.
>>> 3930: if 
>>> (vmIntrinsics::is_intrinsic_available(vmIntrinsics::_float16ToFloat) &&
>>> 3931: 
>>> vmIntrinsics::is_intrinsic_available(vmIntrinsics::_floatToFloat16)) {
>> 
>> Should this also check for InlineIntrinsics?
>
> `vmIntrinsics::disabled_by_jvm_flags()` checks `InlineIntrinsics`. See 
> `vmIntrinsics.cpp` changes.

Yes you are right.

-

PR: https://git.openjdk.org/jdk/pull/12869

Re: RFR: JDK-8302360 Atomic*.compareAndExchange Javadoc unclear

2023-03-06 Thread David Holmes

On Mon, 6 Mar 2023 13:23:59 GMT, Viktor Klang  wrote:

> I think the following proposal is very non-invasive and merely draws 
> attention to the fact that "witness value" is a specific term related to the 
> notion of these atomic methods.
> 
> It's a small change which I think provides additional clarity, see JBS for 
> the discussion on the topic.

Changes requested by dholmes (Reviewer).

src/java.base/share/classes/java/util/concurrent/atomic/AtomicReference.java 
line 357:

> 355:  * @param newValue the new value
> 356:  * @return the witness value, which is the current value
> 357:  * at the time of the operation.

This change is not appropriate as discussed in JBS. And unclear why you changed 
only this version? Leftover?

-

PR: https://git.openjdk.org/jdk/pull/12880

Re: RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter [v2]

> Implemented `Float.floatToFloat16` and `Float.float16ToFloat` intrinsics in 
> Interpreter and C1 compiler to produce the same results as C2 intrinsics on 
> x64, Aarch64 and RISC-V - all platforms where C2 intrinsics for these Java 
> methods were implemented originally.
> 
> Replaced `SharedRuntime::f2hf()` and `hf2f()` C runtime functions with calls 
> to runtime stubs which use the same HW instructions as C2 intrinsics. Only 
> for 64-bit x64 because 32-bit x86 stub does not work: result is passed 
> through FPU register and NaN values become different from C2 intrinsic. This 
> runtime stub is only used to calculate constant values during C2 compilation 
> and can be skipped.
> 
> I added new tests based on Tobias's `TestAll.java` And copied 
> `jdk/lang/Float/Binary16Conversion*.java` tests to run them with `-Xcomp` to 
> make sure code is compiled by C1 or C2. I modified 
> `Binary16ConversionNaN.java` to compare results from Interpreter, C1 and C2.
> 
> Tested tier1-5, Xcomp, stress

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

  Address review comments

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/12869/files
  - new: https://git.openjdk.org/jdk/pull/12869/files/2eb47bf5..9302d4bc

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=12869&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=12869&range=00-01

  Stats: 86 lines in 8 files changed: 15 ins; 24 del; 47 mod
  Patch: https://git.openjdk.org/jdk/pull/12869.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12869/head:pull/12869

PR: https://git.openjdk.org/jdk/pull/12869

Re: RFR: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter

On Tue, 7 Mar 2023 01:26:44 GMT, Sandhya Viswanathan  
wrote:

>> Implemented `Float.floatToFloat16` and `Float.float16ToFloat` intrinsics in 
>> Interpreter and C1 compiler to produce the same results as C2 intrinsics on 
>> x64, Aarch64 and RISC-V - all platforms where C2 intrinsics for these Java 
>> methods were implemented originally.
>> 
>> Replaced `SharedRuntime::f2hf()` and `hf2f()` C runtime functions with calls 
>> to runtime stubs which use the same HW instructions as C2 intrinsics. Only 
>> for 64-bit x64 because 32-bit x86 stub does not work: result is passed 
>> through FPU register and NaN values become different from C2 intrinsic. This 
>> runtime stub is only used to calculate constant values during C2 compilation 
>> and can be skipped.
>> 
>> I added new tests based on Tobias's `TestAll.java` And copied 
>> `jdk/lang/Float/Binary16Conversion*.java` tests to run them with `-Xcomp` to 
>> make sure code is compiled by C1 or C2. I modified 
>> `Binary16ConversionNaN.java` to compare results from Interpreter, C1 and C2.
>> 
>> Tested tier1-5, Xcomp, stress
>
> Other than the minor comments above, the x86 side changes look good to me.

@sviswa7 I update changes based on your comments. Please, look: 
[9302d4b](https://github.com/openjdk/jdk/pull/12869/commits/9302d4bc00f8f1d8e774a260eb6aacb2d51a2dd4)

-

PR: https://git.openjdk.org/jdk/pull/12869

Re: RFR: JDK-8297605 DelayQueue javadoc is confusing

2023-03-06 Thread Martin Buchholz

On Thu, 2 Mar 2023 15:59:33 GMT, Martin Buchholz  wrote:

> Right. But remove(Object) unlike remove() doesn't consider the expiration 
> time. Confusing!

Actually, confusion extends to **three** methods with the same name:
- `Queue.remove()`
- `Collection.remove(Object)`
- `Iterator.remove()`

-

PR: https://git.openjdk.org/jdk/pull/12729

Re: RFR: 8302791: Add specific ClassLoader object to Proxy IllegalArgumentException message [v6]

2023-03-06 Thread Ravali Yatham

On Mon, 6 Mar 2023 17:07:14 GMT, Mandy Chung  wrote:

>> Ravali Yatham has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Added JavaLangAccess::getLoaderNameID(ClassLoader loader)
>
> Looks good.  Thanks for the change.

@mlchung @AlanBateman - I have issued the integrate command. Now it is ready to 
be sponsored. Please do the needful

-

PR: https://git.openjdk.org/jdk/pull/12641

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v2]