Re: RFR: 8286858: Remove dead code in sun.reflect.misc.MethodUtil

[Iris Clark]

On Sun, 15 May 2022 12:47:10 GMT, Andrey Turbanov  wrote:

> They are unused and leftover since JDK 7.  It's good to remove them.

Nice clean up.

-

Marked as reviewed by iris (Reviewer).

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

Re: RFR: 8286571: java source launcher from a minimal jdk image containing jdk.compiler fails with --enable-preview option [v2]

[Jaikiran Pai]

On Tue, 17 May 2022 13:08:32 GMT, Adam Sotona  wrote:

>> ### Problem description
>> Minimal jdk image created by jlink with the only jdk.compiler module and its 
>> dependencies
>> fails to run java source launcher correctly (for example when --source N is 
>> specified).
>> Failing source launcher is only one the expressions of internal jdk.compiler 
>> malfunction, another example is failure to run javac with --release option.
>> 
>> ### Root cause
>> Module jdk.compiler requires zip filesystem (jdk.zipfs module) to parse 
>> ct.sym file and so to provide full functionality.
>> Module jdk.zipfs is not included in the minimal JDK image, because module 
>> jdk.compiler does not declare it as "requires" in its module info.
>> 
>> ### Alternative patch
>> The problem can be alternatively resolved by complex code checks in 
>> jdk.compiler to provide user with valid error message, however that solution 
>> would be just a workaround for jdk.compiler dual functionality (with or 
>> without presence of jdk.zipfs module). Compiler functionality without access 
>> to ct.sym through jdk.zipfs is very limited. 
>> 
>> ### Proposed fix
>> This patch fixes the problem by explicit declaration of jdk.compiler 
>> dependency on jdk.zipfs.
>> Plus added specific test case.
>> 
>> Please review the patch or raise objections against declaration of 
>> jdk.compiler dependent on jdk.zipfs.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fix of LimitedImage test

Hello Adam, I don't have necessary knowledge of this area, so I don't know what 
approach would be preferred.

But a couple of questions:

- If we do decide to add the `jdk.zipfs` dependency to `jdk.compiler` module, 
do you think the `LimitedImage` test is still relevant? Or should it be 
removed? The comment on that test states:
> @summary Verify javac behaves properly in absence of zip/jar 
> FileSystemProvider

and it does so by using `--limit-modules`. But now with the `jdk.zipfs` being a 
dependency, the zip/jar FileSystemProvider will not be absent and the test then 
would seem unnecessary.

- In the JBS issue corresponding to this PR, you noted that:

>There are actually two different issues:
>
>First in JDKPlatformProvider, which silently swallows 
>ProviderNotFoundException. 

Should something be done there? I see that the catch block happens to reside in 
the `static` block of that class (which also happens to be a implementation of 
a Java `service`), so I'm unsure what if anything can be done there.

-

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

Re: RFR: 8286858: Remove dead code in sun.reflect.misc.MethodUtil

[Mandy Chung]

On Sun, 15 May 2022 12:47:10 GMT, Andrey Turbanov  wrote:

> They are unused and leftover since JDK 7.  It's good to remove them.

Looks good.   Do you know why there are test failures (they look unrelated)?

-

Marked as reviewed by mchung (Reviewer).

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

Integrated: 8281712: [REDO] AArch64: Implement string_compare intrinsic in SVE

[Ningsheng Jian]

On Mon, 16 May 2022 07:21:27 GMT, Ningsheng Jian  wrote:

> This is the REDO of JDK-8269559 and JDK-8275448. Those two backouts finally 
> turned to be some system zlib issue in AArch64 macOS, and is not related to 
> the patch itself. See [1][2] for details.
> 
> This patch is generally the same as JDK-8275448, which uses SVE to optimize 
> string_compare intrinsics for long string comparisons. I did a rebase with 
> small tweaks to get better performance on recent Neoverse hardware. Test data 
> on systems with different SVE vector sizes:
> 
> 
>casedelta size  128-bits  256-bits  512-bits
> compareToLL  2   24 0.17% 0.58% 0.00%
> compareToLL  2   36 0.00%2.25%  0.04%
> compareToLL  2   72 -4.40%   3.87%  -12.82%
> compareToLL  2   1284.55%58.31% 13.53%
> compareToLL  2   25619.39%   69.77% 82.03%
> compareToLL  2   5121.81%68.38% 170.93%
> compareToLU  2   24 25.57%   46.98% 54.61%
> compareToLU  2   36 36.03%   70.26% 94.33%
> compareToLU  2   72 35.86%   90.58% 146.04%
> compareToLU  2   12870.82%   119.19%266.22%
> compareToLU  2   25680.77%   146.33%420.01%
> compareToLU  2   51294.62%   171.72%530.87%
> compareToUL  2   24 20.82%   34.48% 62.14%
> compareToUL  2   36 39.77%   60.79% 69.77%
> compareToUL  2   72 35.46%   84.34% 121.90%
> compareToUL  2   12867.77%   110.97%220.53%
> compareToUL  2   25677.05%   160.29%331.30%
> compareToUL  2   51291.88%   184.57%524.21%
> compareToUU  2   24 -0.13%   0.40%  0.00%
> compareToUU  2   36 -9.18%   12.84% -13.93%
> compareToUU  2   72 1.67%60.61% 6.69%
> compareToUU  2   12813.51%   60.33% 55.27%
> compareToUU  2   2562.55%62.17% 153.26%
> compareToUU  2   5124.12%68.62% 201.68%
> 
> JTreg tests passed on SVE hardware.
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8275448
> [2] https://bugs.openjdk.java.net/browse/JDK-8282954

This pull request has now been integrated.

Changeset: b5526e5e
Author:Ningsheng Jian 
URL:   
https://git.openjdk.java.net/jdk/commit/b5526e5e5935658ed1d39938441ae1a3417c0545
Stats: 443 lines in 7 files changed: 433 ins; 0 del; 10 mod

8281712: [REDO] AArch64: Implement string_compare intrinsic in SVE

Co-authored-by: Tat Wai Chong 
Reviewed-by: thartmann, ngasson

-

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

Re: RFR: 8281712: [REDO] AArch64: Implement string_compare intrinsic in SVE

[Ningsheng Jian]

On Tue, 17 May 2022 08:37:52 GMT, Nick Gasson  wrote:

> LGTM!

Thank you, Nick!

-

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

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

[Ichiroh Takiguchi]

On Mon, 2 May 2022 15:00:07 GMT, Roger Riggs  wrote:

>> Ichiroh Takiguchi has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8285517: System.getenv() returns unexpected value if environment variable 
>> has non ASCII character
>
> Can you clarify the use of different charset properties for the 
> ProcessEnvironment vs the CommandLine strings?
> 
> They are used to decode or encode strings in the APIs to native functions 
> respectively.
> If I understand `native.encoding` was introduced to reflect the default 
> encoding of file contents, while `sun.jnu.encoding` is used to encode 
> filenames.
> 
> I think I would have expected to see `sun.jnu.encoding` to be used in all 
> contexts when encoding/decoding strings to the native representations.  
> (Assuming its not required for backward compatibility).

Hello @RogerRiggs .
Do you have any comment ?

-

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

Re: RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner [v3]

[Brent Christian]

> Please review this change to replace the finalizer in 
> `AbstractLdapNamingEnumeration` with a Cleaner.
> 
> The pieces of state required for cleanup (`LdapCtx homeCtx`, `LdapResult 
> res`, and `LdapClient enumClnt`) are moved to a static inner class . From 
> there, the change is fairly mechanical.
> 
> Details of note: 
> 1. Some operations need to change the state values (the update() method is 
> probably the most interesting).
> 2. Subclasses need to access `homeCtx`; I added a `homeCtx()` method to read 
> `homeCtx` from the superclass's `state`.
> 
> The test case is based on a copy of 
> `com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java`. A more minimal test case 
> might be possible, but this was done for expediency.
> 
> The test only confirms that the new Cleaner use does not keep the object 
> reachable. It only tests `LdapSearchEnumeration` (not `LdapNamingEnumeration` 
> or `LdapBindingEnumeration`, though all are subclasses of 
> `AbstractLdapNamingEnumeration`). 
> 
> Thanks.

Brent Christian has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains ten commits:

 - Merge
 - Rename inner class to EnumCtx ; use homeCtx() in 
AbstractLdapNamingEnumeration for consistencty ; new instance vars can be final
 - fix whitespace
 - Merge branch 'master' into remove-finalizers
 - Test changes to test new cleaner code
 - Rename test to LdapEnumeration
 - Create copy of AddNewEntry test to test AbstractLdapNamingEnumeration Cleaner
 - Merge branch 'master' into remove-finalizers
 - Replace AbstractLdapNamingEnumeration finalizer with Cleaner

-

Changes: https://git.openjdk.java.net/jdk/pull/8311/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8311=02
  Stats: 262 lines in 7 files changed: 194 ins; 20 del; 48 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8311.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8311/head:pull/8311

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

Integrated: 8282280: Update Xerces to Version 2.12.2

[Joe Wang]

On Mon, 16 May 2022 19:34:11 GMT, Joe Wang  wrote:

> Update to Xerces 2.12.2.
> 
> The update also fixes JDK-8144117. Added a test.
> 
> Tests: local XML tests passed. Tier2 passed. Two JCK tests failed with this 
> update. See related issue report.

This pull request has now been integrated.

Changeset: 72bd41b8
Author:Joe Wang 
URL:   
https://git.openjdk.java.net/jdk/commit/72bd41b844e03da4bcb19c2cb38d96975a9ebceb
Stats: 310 lines in 17 files changed: 296 ins; 3 del; 11 mod

8282280: Update Xerces to Version 2.12.2

Reviewed-by: lancea, naoto

-

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

Re: RFR: 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate [v10]

[Naoto Sato]

> Supporting `IsoFields` temporal fields in chronologies that are similar to 
> ISO chronology. Corresponding CSR has also been drafted.

Naoto Sato 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 13 additional commits since the 
last revision:

 - Merge branch 'master' into JDK-8279185
 - Addressing review comments
 - Revert to use the default method
 - Removed unnecessary package names
 - Modified class desc of `IsoFields`
 - abstract class -> top level interface
 - interface -> abstract class
 - Removed the method
 - Changed to use a type to determine ISO based or not
 - Renamed the new method
 - ... and 3 more: https://git.openjdk.java.net/jdk/compare/d7dfd3a6...3f69909f

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7683/files
  - new: https://git.openjdk.java.net/jdk/pull/7683/files/45edbc3b..3f69909f

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

  Stats: 243466 lines in 3650 files changed: 182993 ins; 40828 del; 19645 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7683.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7683/head:pull/7683

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

Re: RFR: 8286122: [macos]: App bundle cannot upload to Mac App Store due to info.plist embedded in java exe [v2]

[Kevin Rushforth]

On Thu, 12 May 2022 04:15:50 GMT, Alexander Matveev  
wrote:

>> - It is not possible to support native JDK commands such as "java" inside 
>> Mac App Store bundles due to embedded info.plist. Workarounds suggested in 
>> JDK-8286122 does not seems to be visible.
>>  - With proposed fix we will enforce "--strip-native-commands" option for 
>> jlink, so native JDK commands are not included when generating Mac App Store 
>> bundles.
>>  - Custom runtime provided via --runtime-image should not contain native 
>> commands as well, otherwise jpackage will throw error.
>>  - Added two tests to validate fix.
>
> Alexander Matveev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8286122: [macos]: App bundle cannot upload to Mac App Store due to 
> info.plist embedded in java exe [v2]

I'm just catching up with this thread. Based on the analysis, it's pretty clear 
that any solution that actually allows bundling native launchers is going to 
take more research, and is out of scope for this bug. Some combination of 
[JDK-8286850](https://bugs.openjdk.java.net/browse/JDK-8286850) and/or 
providing Java launchers that don't have an `Info.plist` is likely needed to 
provide a complete solution.

Given that, I think that this PR seems a reasonable fix to avoid creating an 
app bundle that can't be uploaded to the app store.

-

Marked as reviewed by kcr (Author).

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

Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable

[Naoto Sato]

On Wed, 11 May 2022 18:00:31 GMT, Gaurav Chaudhari  
wrote:

> This fix ensures that when a lookup for a custom TZ code fails, and an 
> attempt is made to find the GMT offset in order to get the current time, 
> Daylight savings rules are applied correctly.

Thanks for contributing the fix. This issue has been reported in the past, but 
closed as will not fix (https://bugs.openjdk.java.net/browse/JDK-6992725), as 
implementing POSIX TZ fully requires a sizable amount of work for a rare 
situation. Returning a fixed offset ID with the `TZ` set to observing DST is 
thus a compromised solution. Even if the fix would return a custom zone 
observing the DST does not mean the zone is correct in winter. Having said that 
I agree that the derived fixed offset zone should reflect the *current* offset, 
as well as macOS's implementation.

src/java.base/unix/native/libjava/TimeZone_md.c line 609:

> 607: }
> 608: 
> 609: offset = (gmt.tm_hour - localtm.tm_hour)*3600 + (gmt.tm_min - 
> localtm.tm_min)*60;

Since it is not using `timezone` anymore, we can reverse the subtraction, i.e., 
`localtime` - `gmtime` so that the weird sign switch below can be eliminated.

test/jdk/java/util/TimeZone/CustomTzIDCheckDST.java line 2:

> 1: /*
> 2:  * Copyright (c) 2001, 2022, Oracle and/or its affiliates. All rights 
> reserved.

This is a new test case, the year should be only 2022.

test/jdk/java/util/TimeZone/CustomTzIDCheckDST.java line 27:

> 25:  * Specifically called by runCustomTzIDCheckDST.sh to check if Daylight 
> savings is
> 26:  * properly followed with a custom TZ code set through environment 
> variables.
> 27:  * */

Nit: Need a new line.

test/jdk/java/util/TimeZone/runCustomTzIDCheckDST.sh line 1:

> 1: #

I'd change this script into a java test case (using `.sh` is not recommended). 
In fact, this causes a failure invoking `javac` with `-Xmx768m` (from 
TESTVMOPTS)
There are libraries under `jdk/test/lib/` for building test jvm process and 
tools.

test/jdk/java/util/TimeZone/runCustomTzIDCheckDST.sh line 40:

> 38: 
> 39: OS=`uname -s`
> 40: case "$OS" in 

In case other than Linux/AIX, it would be helpful to emit some message that the 
test is ignored.

-

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

Re: RFR: JDK-8281001 Examine the behavior of Class::forName if the caller is null [v2]

[Tim Prinzing]

> The Class::forName behavior change to match JNI FindClass is a compatible 
> change and seems pretty attractive as it would be expected that 
> Class::forName would give the same behavior as FindClass which uses the 
> system classloader.  The test for 8281006 was enhanced to test for this 
> change.  Merged master to pick up fixes to unrelated test failures to reduce 
> noise.

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

  Added javadoc comment

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8711/files
  - new: https://git.openjdk.java.net/jdk/pull/8711/files/5b10f0d2..9d054ea6

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

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

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

Re: RFR: 8282280: Update Xerces to Version 2.12.2 [v2]

[Lance Andersen]

On Tue, 17 May 2022 00:39:37 GMT, Joe Wang  wrote:

>> Update to Xerces 2.12.2.
>> 
>> The update also fixes JDK-8144117. Added a test.
>> 
>> Tests: local XML tests passed. Tier2 passed. Two JCK tests failed with this 
>> update. See related issue report.
>
> Joe Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   provider description

Marked as reviewed by lancea (Reviewer).

-

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

Integrated: 8213045: Add BigDecimal.TWO

[Brian Burkhalter]

On Mon, 16 May 2022 21:29:22 GMT, Brian Burkhalter  wrote:

> Add constant `java.math.BigDecimal.TWO`.

This pull request has now been integrated.

Changeset: 1d8e92ae
Author:Brian Burkhalter 
URL:   
https://git.openjdk.java.net/jdk/commit/1d8e92ae0d2d0d6740e2171abef45545439e6414
Stats: 17 lines in 2 files changed: 12 ins; 2 del; 3 mod

8213045: Add BigDecimal.TWO

Reviewed-by: darcy

-

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

Re: RFR: 8284960: Integration of JEP 426: Vector API (Fourth Incubator) [v6]

[Jatin Bhateja]

> Hi All,
> 
> Patch adds the planned support for new vector operations and APIs targeted 
> for [JEP 426: Vector API (Fourth 
> Incubator).](https://bugs.openjdk.java.net/browse/JDK-8280173)
> 
> Following is the brief summary of changes:-
> 
> 1)  Extends the scope of existing lanewise API for following new vector 
> operations.
>-  VectorOperations.BIT_COUNT: counts the number of one-bits
>- VectorOperations.LEADING_ZEROS_COUNT: counts the number of leading zero 
> bits
>- VectorOperations.TRAILING_ZEROS_COUNT: counts the number of trailing 
> zero bits
>- VectorOperations.REVERSE: reversing the order of bits
>- VectorOperations.REVERSE_BYTES: reversing the order of bytes
>- compress and expand bits: Semantics are based on Hacker's Delight 
> section 7-4 Compress, or Generalized Extract.
> 
> 2)  Adds following new APIs to perform cross lane vector compress and 
> expansion operations under the influence of a mask.
>- Vector.compress
>- Vector.expand 
>- VectorMask.compress
> 
> 3) Adds predicated and non-predicated versions of following new APIs to load 
> and store the contents of vector from foreign MemorySegments. 
>   - Vector.fromMemorySegment
>   - Vector.intoMemorySegment
> 
> 4) C2 Compiler IR enhancements and optimized X86 and AARCH64 backend support 
> for each newly added operation.
> 
> 
>  Patch has been regressed over AARCH64 and X86 targets different AVX levels. 
> 
>  Kindly review and share your feedback.
> 
>  Best Regards,
>  Jatin

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

  8284960: Adding --enable-preview in vectorAPI benchmarks.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8425/files
  - new: https://git.openjdk.java.net/jdk/pull/8425/files/df7eb90e..0b7f84bb

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

  Stats: 21 lines in 10 files changed: 7 ins; 4 del; 10 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8425.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8425/head:pull/8425

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

Integrated: 8286788: Test java/lang/Thread/virtual/ThreadAPI.testGetStackTrace3 fails

[Alan Bateman]

On Tue, 17 May 2022 11:15:19 GMT, Alan Bateman  wrote:

> This is a test fix. ThreadAPI.testGetStackTrace3 tests Thread::getStackTrace 
> on a thread doing a selection operation. The test is not reliable as it 
> expects to see the "select" method in the stack trace after waiting 200ms. 
> The test is changed to poll the stack trace so that it's no longer dependent 
> on sleep.
> 
> The update includes a drive-by change to the test description to use 
> `@enablePreview`.

This pull request has now been integrated.

Changeset: 8535d51d
Author:Alan Bateman 
URL:   
https://git.openjdk.java.net/jdk/commit/8535d51db7e1c33218c4254e774de4ca4ca60023
Stats: 10 lines in 1 file changed: 3 ins; 2 del; 5 mod

8286788: Test java/lang/Thread/virtual/ThreadAPI.testGetStackTrace3 fails

Reviewed-by: darcy, jpai

-

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

Re: RFR: JDK-8281001 Examine the behavior of Class::forName if the caller is null

[Mandy Chung]

On Tue, 17 May 2022 16:55:14 GMT, Tim Prinzing  wrote:

> I like the idea of moving all the null caller tests to a clearly named 
> directory to avoid duplication.

+1.   You can do this refactoring in a separate JBS issue and then update this 
PR when the tests are moved.

-

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

Re: RFR: JDK-8281001 Examine the behavior of Class::forName if the caller is null

[Tim Prinzing]

On Mon, 16 May 2022 18:55:42 GMT, Mandy Chung  wrote:

> `Class::forName(String)` javadoc should specify which class loader to use 
> when invoked with null caller similar to the other APIs you fixed for null 
> callers.
> 
> I think this new test case does not belong to `NullCallerGetResource.java` 
> which is a test for `Module::getResource`. It's better to be placed under the 
> `test/jdk/java/lang/Class/forName` directory that makes it clear it's a test 
> for `Class::forName`.
> 
> Alternatively, we could move all the tests for null caller under a new 
> clearly-named directory, maybe `test/jdk/jdk/nullCaller`. This may allow to 
> do some refactoring and remove the duplicated code in these test cases. What 
> do you think?

I like the idea of moving all the null caller tests to a clearly named 
directory to avoid duplication.

-

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v4]

[Jorn Vernee]

> Hi,
> 
> This PR brings over commits from the panama-foreign repo. These commits 
> mostly pertain to the switch to ASM and away from MethodHandle combinators 
> for the binding recipe specialization. But, there is one more commit which 
> adds freeing of downcall stubs, since those changes were mostly Java as well.
> 
> Thanks,
> Jorn
> 
> Testing: `run-test-jdk_foreign` with `-Dgenerator.sample.factor=-1` on 
> Windows and Linux.

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

 - Merge branch 'pr/7959' into JEP-19-ASM
 - BootstrapMethodError -> ExceptionInInitializerError
 - Use unaligned layout constants when filling in reconstituted structs (was 
accidentally dropped change)
 - Fix LinkUpcall benchmark
 - 8286306: Upcall wrapper class sharing
   
   Reviewed-by: mcimadamore
 - Polish
 - 8281595: ASM-ify scope acquire/release for down call parameters
   8281387: Some downcall shapes show unexpected allocations
   
   Co-authored-by: Maurizio Cimadamore 
   Reviewed-by: mcimadamore
 - 8281228: Preview branch's CLinker.downcallHandle crashes inside asm
   
   Reviewed-by: sundar, jvernee
 - 8278414: Replace binding recipe customization using MH combinators with 
bytecode spinning
   
   Reviewed-by: mcimadamore
 - 8276648: Downcall stubs are never freed
   
   Reviewed-by: mcimadamore

-

Changes: https://git.openjdk.java.net/jdk/pull/8685/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8685=03
  Stats: 2182 lines in 24 files changed: 1396 ins; 660 del; 126 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8685.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8685/head:pull/8685

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

Integrated: JDK-8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses

[Joe Darcy]

On Sun, 15 May 2022 18:36:07 GMT, Joe Darcy  wrote:

> Make the javadoc in the InputStream and OutputStream subclasses in core libs 
> DRY-er by use of inheritDoc. (Any analagous changes to AudioInputStream in 
> client libs will be done another a separate bug.) When the time comes, will 
> do any necessary merging for the changes in[JDK-8286200.
> 
> Please also review the CSR to cover the introduction of implspec and implNote 
> tags: https://bugs.openjdk.java.net/browse/JDK-8286784

This pull request has now been integrated.

Changeset: 8e602b86
Author:Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/8e602b862db305e6f28b13f9fb0f7ff2cab89bae
Stats: 201 lines in 13 files changed: 60 ins; 45 del; 96 mod

8286783: Expand use of @inheritDoc in InputStream and OutputStream subclasses

Reviewed-by: alanb

-

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

Re: RFR: 8282280: Update Xerces to Version 2.12.2 [v2]

[Naoto Sato]

On Tue, 17 May 2022 00:39:37 GMT, Joe Wang  wrote:

>> Update to Xerces 2.12.2.
>> 
>> The update also fixes JDK-8144117. Added a test.
>> 
>> Tests: local XML tests passed. Tier2 passed. Two JCK tests failed with this 
>> update. See related issue report.
>
> Joe Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   provider description

Marked as reviewed by naoto (Reviewer).

-

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

Re: RFR: 8286788: Test java/lang/Thread/virtual/ThreadAPI.testGetStackTrace3 fails

[Jaikiran Pai]

On Tue, 17 May 2022 11:15:19 GMT, Alan Bateman  wrote:

> This is a test fix. ThreadAPI.testGetStackTrace3 tests Thread::getStackTrace 
> on a thread doing a selection operation. The test is not reliable as it 
> expects to see the "select" method in the stack trace after waiting 200ms. 
> The test is changed to poll the stack trace so that it's no longer dependent 
> on sleep.
> 
> The update includes a drive-by change to the test description to use 
> `@enablePreview`.

Looks fine to me.

The documentation of `@enablePreview` states:
> If a test declares that it uses preview features, these additional options 
> will be provided automatically, for all @run main and @compile actions, 
> including implicit @compile actions generated by @build actions.

The test here is using `testng` and not `main`, but I think it's more a 
documentation issue since it appears that the test is running fine in GitHub 
Actions job:


2022-05-17T12:12:50.5073267Z TEST: java/lang/Thread/virtual/ThreadAPI.java
2022-05-17T12:12:50.5073898Z   build: 0.138 seconds
2022-05-17T12:12:50.5074531Z   compile: 0.138 seconds
2022-05-17T12:12:50.5074992Z   testng: 32.421 seconds
2022-05-17T12:12:50.5076274Z TEST RESULT: Passed. Execution successful
2022-05-17T12:12:50.5076900Z

-

Marked as reviewed by jpai (Committer).

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

Integrated: 8286810: Use public [Double|Float].PRECISION fields in jdk.internal.math.[Double|Float]Consts

[Raffaello Giulietti]

On Mon, 16 May 2022 14:48:43 GMT, Raffaello Giulietti  
wrote:

> Please review these simple changes in jdk.internal.math.[Double|Float]Consts

This pull request has now been integrated.

Changeset: ea713c37
Author:Raffaello Giulietti 
Committer: Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/ea713c37fb7eb628c46ad8838425a0029f24be9d
Stats: 41 lines in 2 files changed: 12 ins; 5 del; 24 mod

8286810: Use public [Double|Float].PRECISION fields in 
jdk.internal.math.[Double|Float]Consts

Reviewed-by: bpb, rriggs, darcy

-

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

Re: RFR: 8286788: Test java/lang/Thread/virtual/ThreadAPI.testGetStackTrace3 fails

[Joe Darcy]

On Tue, 17 May 2022 11:15:19 GMT, Alan Bateman  wrote:

> This is a test fix. ThreadAPI.testGetStackTrace3 tests Thread::getStackTrace 
> on a thread doing a selection operation. The test is not reliable as it 
> expects to see the "select" method in the stack trace after waiting 200ms. 
> The test is changed to poll the stack trace so that it's no longer dependent 
> on sleep.
> 
> The update includes a drive-by change to the test description to use 
> `@enablePreview`.

Marked as reviewed by darcy (Reviewer).

test/jdk/java/lang/Thread/virtual/ThreadAPI.java line 28:

> 26:  * @bug 8284161 8286788
> 27:  * @summary Test Thread API with virtual threads
> 28:  * @enablePreview

Good to see there is directed jtreg support for this.

-

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

Re: RFR: 8283689: Update the foreign linker VM implementation [v23]

[Jorn Vernee]

> Hi,
> 
> This PR updates the VM implementation of the foreign linker, by bringing over 
> commits from the panama-foreign repo.
> 
> This is split off from the main JEP integration for 19, since we have limited 
> resources to handle this. As such, this PR might fall over to 20, but it 
> would be nice if we could get it into 19.
> 
> I've written up an overview of the Linker architecture here: 
> http://cr.openjdk.java.net/~jvernee/docs/FL_Overview.html it might be useful 
> to read that first.
> 
> This patch moves from the "legacy" implementation, to what is currently 
> implemented in the panama-foreign repo, except for replacing the use of 
> method handle combinators with ASM. That will come in a later path. To recap. 
> This PR contains the following changes:
> 
> 1. VM stubs for downcalls are now generated up front, instead of lazily by C2 
> [1].
> 2. the VM support for upcalls/downcalls now support all possible call shapes. 
> And VM stubs and Java code implementing the buffered invocation strategy has 
> been removed  [2], [3], [4], [5].
> 3. The existing C2 intrinsification support for the `linkToNative` method 
> handle linker was no longer needed and has been removed [6] (support might be 
> re-added in another form later).
> 4. Some other cleanups, such as: OptimizedEntryBlob (for upcalls) now 
> implements RuntimeBlob directly. Binding to java classes has been rewritten 
> to use javaClasses.h/cpp (this wasn't previously possible due to these java 
> classes being in an incubator module) [7], [8], [9].
> 
> While the patch mostly consists of VM changes, there are also some Java 
> changes to support (2).
> 
> The original commit structure has been mostly retained, so it might be useful 
> to look at a specific commit, or the corresponding patch in the 
> [panama-foreign](https://github.com/openjdk/panama-foreign/pulls?q=is%3Apr) 
> repo as well. I've also left some inline comments to explain some of the 
> changes, which will hopefully make reviewing easier.
> 
> Testing: Tier1-4
> 
> Thanks,
> Jorn
> 
> [1]: 
> https://github.com/openjdk/jdk/pull/7959/commits/048b88156814579dca1f70742061ad24942fd358
> [2]: 
> https://github.com/openjdk/jdk/pull/7959/commits/2fbbef472b4c2b4fee5ede2f18cd81ab61e88f49
> [3]: 
> https://github.com/openjdk/jdk/pull/7959/commits/8a957a4ed9cc8d1f708ea8777212eb51ab403dc3
> [4]: 
> https://github.com/openjdk/jdk/pull/7959/commits/35ba1d964f1de4a77345dc58debe0565db4b0ff3
> [5]: 
> https://github.com/openjdk/jdk/pull/7959/commits/4e72aae22920300c5ffa16fed805b62ed9092120
> [6]: 
> https://github.com/openjdk/jdk/pull/7959/commits/08e22e1b468c5c8f0cfd7135c72849944068aa7a
> [7]: 
> https://github.com/openjdk/jdk/pull/7959/commits/451cd9edf54016c182dab21a8b26bd8b609fc062
> [8]: 
> https://github.com/openjdk/jdk/pull/7959/commits/4c851d2795afafec3a3ab17f4142ee098692068f
> [9]: 
> https://github.com/openjdk/jdk/pull/7959/commits/d025377799424f31512dca2ffe95491cd5ae22f9

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

 - Merge branch 'master' into JEP-19-VM-IMPL2
 - ifdef NOT_PRODUCT -> ifndef PRODUCT
 - Missing ASSERT -> NOT_PRODUCT
 - Cleanup UL usage
 - Fix failure with SPEC disabled (accidentally dropped change)
 - indentation
 - fix space
 - Merge branch 'master' into JEP-19-VM-IMPL2
 - Undo spurious changes.
 - Merge branch 'JEP-19-VM-IMPL2' of https://github.com/JornVernee/jdk into 
JEP-19-VM-IMPL2
 - ... and 95 more: https://git.openjdk.java.net/jdk/compare/af07919e...c3c1421b

-

Changes: https://git.openjdk.java.net/jdk/pull/7959/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7959=22
  Stats: 6914 lines in 155 files changed: 2577 ins; 3219 del; 1118 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7959.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7959/head:pull/7959

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

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

[Naoto Sato]

On Tue, 17 May 2022 01:19:30 GMT, Ichiroh Takiguchi  
wrote:

>> On JDK19 with Linux ja_JP.eucjp locale,
>> System.getenv() returns unexpected value if environment variable has 
>> Japanese EUC characters.
>> It seems this issue happens because of JEP 400.
>> Arguments for ProcessBuilder have same kind of issue.
>
> Ichiroh Takiguchi has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8285517: System.getenv() returns unexpected value if environment variable 
> has non ASCII character

Looks good. Thanks for the fix!

-

Marked as reviewed by naoto (Reviewer).

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

Re: RFR: 8213045: Add BigDecimal.TWO [v2]

[Raffaello Giulietti]

On Mon, 16 May 2022 22:34:21 GMT, Brian Burkhalter  wrote:

>> Add constant `java.math.BigDecimal.TWO`.
>
> Brian Burkhalter has refreshed the contents of this pull request, and 
> previous commits have been removed. The incremental views will show 
> differences compared to the previous content of the PR. The pull request 
> contains one new commit since the last revision:
> 
>   8213045: Add BigDecimal.TWO

Looks good

-

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v3]

[Jorn Vernee]

On Tue, 17 May 2022 14:28:11 GMT, Rémi Forax  wrote:

>> What about using MethodTypeDesc/ClassDesc as building block?
>
> yes, it should be less expensive, the ClassDesc still need to be constructed 
> from Class to allow refactoring.

Can't wait for constant folding :)

I'll pull the `methodType(void.class).descriptorString()` into a separate 
constant, and reuse that.

I don't think it's worth it to spend too much time here at this point though, 
since this code is only executed once.

-

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

RFR: 8286788: Test java/lang/Thread/virtual/ThreadAPI.testGetStackTrace3 fails

[Alan Bateman]

This is a test fix. ThreadAPI.testGetStackTrace3 tests Thread::getStackTrace on 
a thread doing a selection operation. The test is not reliable as it expects to 
see the "select" method in the stack trace after waiting 200ms. The test is 
changed to poll the stack trace so that it's no longer dependent on sleep.

The update includes a drive-by change to the test description to use 
`@enablePreview`.

-

Commit messages:
 - Initial commit

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

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v3]

[Rémi Forax]

On Tue, 17 May 2022 11:57:01 GMT, Jorn Vernee  wrote:

>> src/java.base/share/classes/jdk/internal/foreign/abi/SoftReferenceCache.java 
>> line 42:
>> 
>>> 40: 
>>> 41: private class Node {
>>> 42: private SoftReference ref;
>> 
>> this code looks like a double check locking so ref should be volatile
>
> Thanks. I thought the `moniterenter` & `monitorexit` were enough here for 
> visibility. I've read up on this and it looks like `volatile` is needed to 
> correctly order the constructor and apply call (and contents) before the 
> write to the `ref` field.

yes, otherwise another thread than the one inside the 
`monitorenter`/`monitorexit` that is initializing the object can see the object 
half initialized.

-

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v3]

[Rémi Forax]

On Tue, 17 May 2022 08:16:32 GMT, Maurizio Cimadamore  
wrote:

>> src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java 
>> line 98:
>> 
>>> 96: private static final String CLASS_DATA_DESC = 
>>> methodType(Object.class, MethodHandles.Lookup.class, String.class, 
>>> Class.class).descriptorString();
>>> 97: private static final String RELEASE0_DESC = 
>>> methodType(void.class).descriptorString();
>>> 98: private static final String ACQUIRE0_DESC = 
>>> methodType(void.class).descriptorString();
>> 
>> calling methodType() is quite slow because of the cache of MethodType so 
>> maybe those initialization should be done explicitly in a static block to 
>> avoid to recompute methodType(long).descriptorString() several times
>
> What about using MethodTypeDesc/ClassDesc as building block?

yes, it should be less expensive, the ClassDesc still need to be constructed 
from Class to allow refactoring.

-

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

Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable

[Gaurav Chaudhari]

On Wed, 11 May 2022 21:20:52 GMT, Dalibor Topic  wrote:

>> This fix ensures that when a lookup for a custom TZ code fails, and an 
>> attempt is made to find the GMT offset in order to get the current time, 
>> Daylight savings rules are applied correctly.
>
> Hi, please send me an e-mail at dalibor.to...@oracle.com, so that I can mark 
> your account as verified.

I've sent an email yesterday @robilad

-

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

Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable

[Dalibor Topic]

On Wed, 11 May 2022 18:00:31 GMT, Gaurav Chaudhari  
wrote:

> This fix ensures that when a lookup for a custom TZ code fails, and an 
> attempt is made to find the GMT offset in order to get the current time, 
> Daylight savings rules are applied correctly.

Hi, please send me an e-mail at dalibor.to...@oracle.com, so that I can mark 
your account as verified.

-

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

RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable

[Gaurav Chaudhari]

This fix ensures that when a lookup for a custom TZ code fails, and an attempt 
is made to find the GMT offset in order to get the current time, Daylight 
savings rules are applied correctly.

-

Commit messages:
 - 8285838: Fix for TZ environment variable DST rules

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

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

Re: RFR: 8286694: Incorrect argument processing in java launcher

[Yasumasa Suenaga]

On Tue, 17 May 2022 04:39:06 GMT, David Holmes  wrote:

>> GCC 12 reports as following:
>
> You forced me to look at this in depth. The values are set via the build 
> system. In the build system it is impossible to get just -J because the -J is 
> only added as a prefix for an existing string. So basically this is 
> impossible code to reach unless you manually override the build system 
> definition. So as far as I can see this is a classic case where we want an 
> assert.
> 
> 
> if (arg[0] == '-' && arg[1] == 'J') {
> assert(arg[2] != '\0', "Invalid JAVA_ARGS or EXTRA_JAVA_ARGS defined by 
> build");
> *nargv++ = JLI_StringDup(arg + 2);
> }

@dholmes-ora Thanks for your comment!

We cannot use `assert(cond, message)` at here because it is not HotSpot code. 
In imageFile.cpp for libjimage. It uses `assert(cond && message)`, so I use 
this style in new commit, and the warning has gone.

I can change to "normal" `assert` usage at here if it is strange.

-

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

Re: RFR: 8286694: Incorrect argument processing in java launcher [v2]

[Yasumasa Suenaga]

> GCC 12 reports as following:

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

 - Use assert() to check jargv
 - Merge remote-tracking branch 'upstream/master' into JDK-8286694
 - 8286694: Incorrect argument processing in java launcher

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8694/files
  - new: https://git.openjdk.java.net/jdk/pull/8694/files/fbde50b2..339dc135

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

  Stats: 6334 lines in 527 files changed: 3166 ins; 1506 del; 1662 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8694.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8694/head:pull/8694

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

Re: RFR: 8286562: GCC 12 reports some compiler warnings [v5]

[Yasumasa Suenaga]

On Tue, 17 May 2022 04:52:44 GMT, Kim Barrett  wrote:

>> src/hotspot/share/opto/memnode.cpp line 1413:
>> 
>>> 1411:bt == T_BYTE|| bt == T_SHORT ||
>>> 1412:bt == T_INT || bt == T_LONG, "wrong type = 
>>> %s", type2name(bt));
>>> 1413: PRAGMA_DIAG_POP
>> 
>> The problem here is the definition of type2name, which returns NULL for an 
>> unknown BasicType.  It would probably be better if it returned a 
>> recognizable string for that situation, eliminating this warning at it's 
>> source.
>
> While looking at type2name, I noticed the comment for the immediately 
> preceding type2name_tab is wrong.

The warning has gone in new commit, and I fixed the comment for `type2name_tab` 
in it.

-

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

Re: RFR: 8286562: GCC 12 reports some compiler warnings [v5]

[Yasumasa Suenaga]

On Tue, 17 May 2022 03:06:49 GMT, Kim Barrett  wrote:

>> Yasumasa Suenaga has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Revert change for java.c , parse_manifest.c , LinuxPackage.c
>
> src/hotspot/share/opto/type.cpp line 4300:
> 
>> 4298: PRAGMA_FORMAT_OVERFLOW_IGNORED
>> 4299:   fatal("not an element type: %s", type2name(etype));
>> 4300: PRAGMA_DIAG_POP
> 
> Another occurrence of type2name returning NULL for unknown BasicType.

The warning has gone in new commit due to change of `type2name`.

-

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

Re: RFR: 8286562: GCC 12 reports some compiler warnings [v7]

[Yasumasa Suenaga]

> I saw some compiler warnings when I tried to build OpenJDK with GCC 12.0.1 on 
> Fedora 36.
> As you can see, the warnings spreads several areas. Let me know if I should 
> separate them by area.
> 
> * -Wstringop-overflow
> * src/hotspot/share/oops/array.hpp
> * 
> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
> 
> In member function 'void Array::at_put(int, const T&) [with T = unsigned 
> char]',
> inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at 
> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64,
> inlined from 'void ConstantPool::method_at_put(int, int, int)' at 
> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:380:15,
> inlined from 'ConstantPool* 
> BytecodeConstantPool::create_constant_pool(JavaThread*) const' at 
> /home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/bytecodeAssembler.cpp:85:26:

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

  revert changes for memnode.cpp and type.cpp

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8646/files
  - new: https://git.openjdk.java.net/jdk/pull/8646/files/73c306d7..88cbf46d

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

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

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

Re: RFR: 8286562: GCC 12 reports some compiler warnings [v5]

[Yasumasa Suenaga]

On Tue, 17 May 2022 03:14:05 GMT, Kim Barrett  wrote:

>> Yasumasa Suenaga has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Revert change for java.c , parse_manifest.c , LinuxPackage.c
>
> src/hotspot/share/classfile/bytecodeAssembler.cpp line 95:
> 
>> 93: ShouldNotReachHere();
>> 94: }
>> 95: PRAGMA_DIAG_POP
> 
> One of these is another of the symbol_at_put cases that I don't understand.  
> Are there other cases in the switch that warn?  And if so, which ones and 
> how?  There are no details of this one in the PR cover description.

Most of switch cases are warned. Please see 
[logs](https://github.com/openjdk/jdk/files/8708578/hotspot_variant-server_libjvm_objs_bytecodeAssembler.o.log)

-

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

Re: RFR: 8280035: Use Class.isInstance instead of Class.isAssignableFrom where applicable [v2]

[Andrey Turbanov]

On Thu, 31 Mar 2022 08:03:23 GMT, Andrey Turbanov  wrote:

>> Method `Class.isAssignableFrom` is often used in form of:
>> 
>> if (clazz.isAssignableFrom(obj.getClass())) {
>> Such condition could be simplified to more shorter and performarnt code
>> 
>> if (clazz.isInstance(obj)) {
>> 
>> Replacement is equivalent if it's known that `obj != null`.
>> In JDK codebase there are many such places.
>> 
>> I tried to measure performance via JMH
>> 
>> Class integerClass = Integer.class;
>> Class numberClass = Number.class;
>> 
>> Object integerObject = 45666;
>> Object doubleObject = 8777d;
>> 
>> @Benchmark
>> public boolean integerInteger_isInstance() {
>> return integerClass.isInstance(integerObject);
>> }
>> 
>> @Benchmark
>> public boolean integerInteger_isAssignableFrom() {
>> return integerClass.isAssignableFrom(integerObject.getClass());
>> }
>> 
>> @Benchmark
>> public boolean integerDouble_isInstance() {
>> return integerClass.isInstance(doubleObject);
>> }
>> 
>> @Benchmark
>> public boolean integerDouble_isAssignableFrom() {
>> return integerClass.isAssignableFrom(doubleObject.getClass());
>> }
>> 
>> @Benchmark
>> public boolean numberDouble_isInstance() {
>> return numberClass.isInstance(doubleObject);
>> }
>> 
>> @Benchmark
>> public boolean numberDouble_isAssignableFrom() {
>> return numberClass.isAssignableFrom(doubleObject.getClass());
>> }
>> 
>> @Benchmark
>> public boolean numberInteger_isInstance() {
>> return numberClass.isInstance(integerObject);
>> }
>> 
>> @Benchmark
>> public boolean numberInteger_isAssignableFrom() {
>> return numberClass.isAssignableFrom(integerObject.getClass());
>> }
>> 
>> @Benchmark
>> public void numberIntegerDouble_isInstance(Blackhole bh) {
>> bh.consume(numberClass.isInstance(integerObject));
>> bh.consume(numberClass.isInstance(doubleObject));
>> }
>> 
>> @Benchmark
>> public void integerIntegerDouble_isAssignableFrom(Blackhole bh) {
>> bh.consume(integerClass.isAssignableFrom(integerObject.getClass()));
>> bh.consume(integerClass.isAssignableFrom(doubleObject.getClass()));
>> }
>> 
>> `isInstance` is a bit faster than `isAssignableFrom`
>> 
>> Benchmark  Mode  Cnt  Score   Error  Units
>> integerDouble_isAssignableFrom avgt5  1,173 ± 0,026  ns/op
>> integerDouble_isInstance   avgt5  0,939 ± 0,038  ns/op
>> integerIntegerDouble_isAssignableFrom  avgt5  2,106 ± 0,068  ns/op
>> numberIntegerDouble_isInstance avgt5  1,516 ± 0,046  ns/op
>> integerInteger_isAssignableFromavgt5  1,175 ± 0,029  ns/op
>> integerInteger_isInstance  avgt5  0,886 ± 0,017  ns/op
>> numberDouble_isAssignableFrom  avgt5  1,172 ± 0,007  ns/op
>> numberDouble_isInstanceavgt5  0,891 ± 0,030  ns/op
>> numberInteger_isAssignableFrom avgt5  1,169 ± 0,014  ns/op
>> numberInteger_isInstance   avgt5  0,887 ± 0,016  ns/op
>
> Andrey Turbanov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8280035: Use Class.isInstance instead of Class.isAssignableFrom where 
> applicable
>   apply suggestion to avoid second Method.invoke call

Any more reviewers, please?)

-

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

Re: RFR: 8286562: GCC 12 reports some compiler warnings [v6]

[Yasumasa Suenaga]

> I saw some compiler warnings when I tried to build OpenJDK with GCC 12.0.1 on 
> Fedora 36.
> As you can see, the warnings spreads several areas. Let me know if I should 
> separate them by area.
> 
> * -Wstringop-overflow
> * src/hotspot/share/oops/array.hpp
> * 
> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
> 
> In member function 'void Array::at_put(int, const T&) [with T = unsigned 
> char]',
> inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at 
> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64,
> inlined from 'void ConstantPool::method_at_put(int, int, int)' at 
> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:380:15,
> inlined from 'ConstantPool* 
> BytecodeConstantPool::create_constant_pool(JavaThread*) const' at 
> /home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/bytecodeAssembler.cpp:85:26:

Yasumasa Suenaga has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains seven commits:

 - Add assert to check the range of BasicType
 - Merge remote-tracking branch 'upstream/master' into HEAD
 - Revert change for java.c , parse_manifest.c , LinuxPackage.c
 - Calculate char offset before realloc()
 - Use return value from JLI_Snprintf
 - Avoid pragma error in before GCC 12
 - 8286562: GCC 12 reports some compiler warnings

-

Changes: https://git.openjdk.java.net/jdk/pull/8646/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8646=05
  Stats: 56 lines in 11 files changed: 46 ins; 1 del; 9 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8646.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8646/head:pull/8646

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

Re: RFR: 8286571: java source launcher from a minimal jdk image containing jdk.compiler fails with --enable-preview option [v2]

[Adam Sotona]

> ### Problem description
> Minimal jdk image created by jlink with the only jdk.compiler module and its 
> dependencies
> fails to run java source launcher correctly (for example when --source N is 
> specified).
> Failing source launcher is the only one expression of internal jdk.compiler 
> malfunction, another example is failure to run javac with --release option.
> 
> ### Root cause
> Module jdk.compiler requires zip filesystem (jdk.zipfs module) to parse 
> ct.sym file and so to provide full functionality.
> However module jdk.zipfs is not included in the minimal JDK image, because 
> module jdk.compiler does not declare it as "requires" in its module info.
> 
> ### Alternative patch
> The problem can be alternatively resolved by complex code checks in 
> jdk.compiler to provide user with valid error message, however that solution 
> would be just a workaround for jdk.compiler dual functionality (with or 
> without presence of jdk.zipfs module).
> 
> ### Proposed fix
> This patch does fixes the problem by explicit declaration of jdk.compiler 
> dependency on jdk.zipfs.
> Plus added specific test case.
> 
> Please review the patch or raise objections against declaration of 
> jdk.compiler dependent on jdk.zipfs.
> 
> Thanks,
> Adam

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

  fix of LimitedImage test

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8747/files
  - new: https://git.openjdk.java.net/jdk/pull/8747/files/40884fd8..6bdf4a9a

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

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

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

RFR: 8286571: java source launcher from a minimal jdk image containing jdk.compiler fails with --enable-preview option

[Adam Sotona]

### Problem description
Minimal jdk image created by jlink with the only jdk.compiler module and its 
dependencies
fails to run java source launcher correctly (for example when --source N is 
specified).
Failing source launcher is the only one expression of internal jdk.compiler 
malfunction, another example is failure to run javac with --release option.

### Root cause
Module jdk.compiler requires zip filesystem (jdk.zipfs module) to parse ct.sym 
file and so to provide full functionality.
However module jdk.zipfs is not included in the minimal JDK image, because 
module jdk.compiler does not declare it as "requires" in its module info.

### Alternative patch
The problem can be alternatively resolved by complex code checks in 
jdk.compiler to provide user with valid error message, however that solution 
would be just a workaround for jdk.compiler dual functionality (with or without 
presence of jdk.zipfs module).

### Proposed fix
This patch does fixes the problem by explicit declaration of jdk.compiler 
dependency on jdk.zipfs.
Plus added specific test case.

Please review the patch or raise objections against declaration of jdk.compiler 
dependent on jdk.zipfs.

Thanks,
Adam

-

Commit messages:
 - 8286571: java source launcher from a minimal jdk image containing 
jdk.compiler fails with --enable-preview option

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

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

Re: RFR: 8286562: GCC 12 reports some compiler warnings [v5]

[Yasumasa Suenaga]

On Tue, 17 May 2022 03:02:55 GMT, Kim Barrett  wrote:

>> Yasumasa Suenaga has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Revert change for java.c , parse_manifest.c , LinuxPackage.c
>
> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
>  line 103:
> 
>> 101: PRAGMA_STRINGOP_OVERFLOW_IGNORED
>> 102:   *dest = op(bits, *dest);
>> 103: PRAGMA_DIAG_POP
> 
> I see no stringop here.  I'm still trying to understand what the compiler is 
> complaining about.

I guess GCC cannot understand `assert(dest != NULL` immediately preceding it.


In file included from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdLoadBarrier.inline.hpp:33,
 from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.inline.hpp:30,
 from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/support/jfrJdkJfrEvent.cpp:30:
In function 'void set_form(jbyte, jbyte*) [with jbyte (* op)(jbyte, jbyte) = 
traceid_or]',
inlined from 'void set(jbyte, jbyte*)' at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp:129:23,
inlined from 'static void JfrTraceIdBits::store(jbyte, const T*) [with T = 
Klass]' at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp:135:6,
inlined from 'static void JfrTraceId::tag_as_jdk_jfr_event(const Klass*)' 
at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.inline.hpp:106:3,
inlined from 'static void JdkJfrEvent::tag_as(const Klass*)' at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/support/jfrJdkJfrEvent.cpp:176:35:

-

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

Re: RFR: 8286562: GCC 12 reports some compiler warnings [v5]

[Yasumasa Suenaga]

On Tue, 17 May 2022 01:43:25 GMT, Kim Barrett  wrote:

>> Yasumasa Suenaga has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Revert change for java.c , parse_manifest.c , LinuxPackage.c
>
> src/hotspot/share/classfile/classFileParser.cpp line 5970:
> 
>> 5968: PRAGMA_STRINGOP_OVERFLOW_IGNORED
>> 5969:   _cp->symbol_at_put(hidden_index, _class_name);
>> 5970: PRAGMA_DIAG_POP
> 
> I don't understand these warning suppressions for symbol_at_put (here and 
> elsewhere).  I don't see any stringops here.  What is the compiler 
> complaining about?  (There's no mention of classfile stuff in the review 
> cover message.)

Like the others, it is caused by `Array::at_put()`.


In file included from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/annotations.hpp:28,
 from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/instanceKlass.hpp:29,
 from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/javaClasses.hpp:30,
 from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/precompiled/precompiled.hpp:35:
In member function 'void Array::at_put(int, const T&) [with T = unsigned 
char]',
inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64,
inlined from 'void ConstantPool::symbol_at_put(int, Symbol*)' at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:362:15,
inlined from 'void 
ClassFileParser::mangle_hidden_class_name(InstanceKlass*)' at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/classFileParser.cpp:5966:21:

-

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

Re: RFR: 8283689: Update the foreign linker VM implementation [v22]

[Robbin Ehn]

On Tue, 17 May 2022 10:38:39 GMT, Jorn Vernee  wrote:

>> Hi,
>> 
>> This PR updates the VM implementation of the foreign linker, by bringing 
>> over commits from the panama-foreign repo.
>> 
>> This is split off from the main JEP integration for 19, since we have 
>> limited resources to handle this. As such, this PR might fall over to 20, 
>> but it would be nice if we could get it into 19.
>> 
>> I've written up an overview of the Linker architecture here: 
>> http://cr.openjdk.java.net/~jvernee/docs/FL_Overview.html it might be useful 
>> to read that first.
>> 
>> This patch moves from the "legacy" implementation, to what is currently 
>> implemented in the panama-foreign repo, except for replacing the use of 
>> method handle combinators with ASM. That will come in a later path. To 
>> recap. This PR contains the following changes:
>> 
>> 1. VM stubs for downcalls are now generated up front, instead of lazily by 
>> C2 [1].
>> 2. the VM support for upcalls/downcalls now support all possible call 
>> shapes. And VM stubs and Java code implementing the buffered invocation 
>> strategy has been removed  [2], [3], [4], [5].
>> 3. The existing C2 intrinsification support for the `linkToNative` method 
>> handle linker was no longer needed and has been removed [6] (support might 
>> be re-added in another form later).
>> 4. Some other cleanups, such as: OptimizedEntryBlob (for upcalls) now 
>> implements RuntimeBlob directly. Binding to java classes has been rewritten 
>> to use javaClasses.h/cpp (this wasn't previously possible due to these java 
>> classes being in an incubator module) [7], [8], [9].
>> 
>> While the patch mostly consists of VM changes, there are also some Java 
>> changes to support (2).
>> 
>> The original commit structure has been mostly retained, so it might be 
>> useful to look at a specific commit, or the corresponding patch in the 
>> [panama-foreign](https://github.com/openjdk/panama-foreign/pulls?q=is%3Apr) 
>> repo as well. I've also left some inline comments to explain some of the 
>> changes, which will hopefully make reviewing easier.
>> 
>> Testing: Tier1-4
>> 
>> Thanks,
>> Jorn
>> 
>> [1]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/048b88156814579dca1f70742061ad24942fd358
>> [2]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/2fbbef472b4c2b4fee5ede2f18cd81ab61e88f49
>> [3]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/8a957a4ed9cc8d1f708ea8777212eb51ab403dc3
>> [4]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/35ba1d964f1de4a77345dc58debe0565db4b0ff3
>> [5]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/4e72aae22920300c5ffa16fed805b62ed9092120
>> [6]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/08e22e1b468c5c8f0cfd7135c72849944068aa7a
>> [7]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/451cd9edf54016c182dab21a8b26bd8b609fc062
>> [8]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/4c851d2795afafec3a3ab17f4142ee098692068f
>> [9]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/d025377799424f31512dca2ffe95491cd5ae22f9
>
> Jorn Vernee has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   ifdef NOT_PRODUCT -> ifndef PRODUCT

Looks good, thanks.

-

Marked as reviewed by rehn (Reviewer).

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

Integrated: 8285485: Fix typos in corelibs

[Magnus Ihse Bursie]

On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie  wrote:

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

This pull request has now been integrated.

Changeset: e68024c2
Author:Magnus Ihse Bursie 
URL:   
https://git.openjdk.java.net/jdk/commit/e68024c2d28d634ebfde7f2fdcc35f5d7b07d704
Stats: 295 lines in 101 files changed: 0 ins; 0 del; 295 mod

8285485: Fix typos in corelibs

Reviewed-by: jpai, sundar, naoto, lancea

-

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v3]

[Jorn Vernee]

On Tue, 17 May 2022 05:51:58 GMT, Rémi Forax  wrote:

>> Jorn Vernee has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   BootstrapMethodError -> ExceptionInInitializerError
>
> src/java.base/share/classes/jdk/internal/foreign/abi/SoftReferenceCache.java 
> line 42:
> 
>> 40: 
>> 41: private class Node {
>> 42: private SoftReference ref;
> 
> this code looks like a double check locking so ref should be volatile

Thanks. I thought the `moniterenter` & `monitorexit` were enough here for 
visibility. I've read up on this and it looks like `volatile` is needed to 
correctly order the constructor and apply call (and contents) before the write 
to the `ref` field.

-

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v3]

[Jorn Vernee]

On Tue, 17 May 2022 06:13:04 GMT, Rémi Forax  wrote:

>> Jorn Vernee has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   BootstrapMethodError -> ExceptionInInitializerError
>
> src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java 
> line 943:
> 
>> 941: Z, B, S, C, I, J, F, D, L;
>> 942: 
>> 943: static BasicType of(Class cls) {
> 
> This seems a duplication of ASM Type class for no good reason, 
> Type.getOpcode(ILOAD) or Type.getOpcode(IRETURN) gives you the proper variant 
> opcode

Didn't know about that. Neat!

-

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

Re: RFR: 8283689: Update the foreign linker VM implementation [v22]

[Jorn Vernee]

> Hi,
> 
> This PR updates the VM implementation of the foreign linker, by bringing over 
> commits from the panama-foreign repo.
> 
> This is split off from the main JEP integration for 19, since we have limited 
> resources to handle this. As such, this PR might fall over to 20, but it 
> would be nice if we could get it into 19.
> 
> I've written up an overview of the Linker architecture here: 
> http://cr.openjdk.java.net/~jvernee/docs/FL_Overview.html it might be useful 
> to read that first.
> 
> This patch moves from the "legacy" implementation, to what is currently 
> implemented in the panama-foreign repo, except for replacing the use of 
> method handle combinators with ASM. That will come in a later path. To recap. 
> This PR contains the following changes:
> 
> 1. VM stubs for downcalls are now generated up front, instead of lazily by C2 
> [1].
> 2. the VM support for upcalls/downcalls now support all possible call shapes. 
> And VM stubs and Java code implementing the buffered invocation strategy has 
> been removed  [2], [3], [4], [5].
> 3. The existing C2 intrinsification support for the `linkToNative` method 
> handle linker was no longer needed and has been removed [6] (support might be 
> re-added in another form later).
> 4. Some other cleanups, such as: OptimizedEntryBlob (for upcalls) now 
> implements RuntimeBlob directly. Binding to java classes has been rewritten 
> to use javaClasses.h/cpp (this wasn't previously possible due to these java 
> classes being in an incubator module) [7], [8], [9].
> 
> While the patch mostly consists of VM changes, there are also some Java 
> changes to support (2).
> 
> The original commit structure has been mostly retained, so it might be useful 
> to look at a specific commit, or the corresponding patch in the 
> [panama-foreign](https://github.com/openjdk/panama-foreign/pulls?q=is%3Apr) 
> repo as well. I've also left some inline comments to explain some of the 
> changes, which will hopefully make reviewing easier.
> 
> Testing: Tier1-4
> 
> Thanks,
> Jorn
> 
> [1]: 
> https://github.com/openjdk/jdk/pull/7959/commits/048b88156814579dca1f70742061ad24942fd358
> [2]: 
> https://github.com/openjdk/jdk/pull/7959/commits/2fbbef472b4c2b4fee5ede2f18cd81ab61e88f49
> [3]: 
> https://github.com/openjdk/jdk/pull/7959/commits/8a957a4ed9cc8d1f708ea8777212eb51ab403dc3
> [4]: 
> https://github.com/openjdk/jdk/pull/7959/commits/35ba1d964f1de4a77345dc58debe0565db4b0ff3
> [5]: 
> https://github.com/openjdk/jdk/pull/7959/commits/4e72aae22920300c5ffa16fed805b62ed9092120
> [6]: 
> https://github.com/openjdk/jdk/pull/7959/commits/08e22e1b468c5c8f0cfd7135c72849944068aa7a
> [7]: 
> https://github.com/openjdk/jdk/pull/7959/commits/451cd9edf54016c182dab21a8b26bd8b609fc062
> [8]: 
> https://github.com/openjdk/jdk/pull/7959/commits/4c851d2795afafec3a3ab17f4142ee098692068f
> [9]: 
> https://github.com/openjdk/jdk/pull/7959/commits/d025377799424f31512dca2ffe95491cd5ae22f9

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

  ifdef NOT_PRODUCT -> ifndef PRODUCT

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7959/files
  - new: https://git.openjdk.java.net/jdk/pull/7959/files/406f3e83..c3abb732

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7959=21
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7959=20-21

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

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v2]

[Jorn Vernee]

On Mon, 16 May 2022 12:58:51 GMT, Maurizio Cimadamore  
wrote:

>> Jorn Vernee has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Use unaligned layout constants when filling in reconstituted structs (was 
>> accidentally dropped change)
>
> src/java.base/share/classes/jdk/internal/foreign/abi/CallingSequence.java 
> line 109:
> 
>> 107:  * @return the caller method type.
>> 108:  */
>> 109: public MethodType callerMethodType() {
> 
> Would it make sense to either rename these, or to point to the fact that 
> these are equivalent to Linker::downcallType/upcallType ?

I don't think renaming them to down/upcallType makes sense, since both down/up 
calls use both the caller and callee method type. There can also be things that 
make these not equivalent to downcallType/upcallType, such as the SysV need to 
pass the number of floating point arguments for variadic calls as well: 
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/foreign/abi/x64/sysv/CallArranger.java#L109-L111

-

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v3]

[Jorn Vernee]

On Tue, 17 May 2022 08:32:54 GMT, Maurizio Cimadamore  
wrote:

>> Jorn Vernee has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   BootstrapMethodError -> ExceptionInInitializerError
>
> src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java 
> line 131:
> 
>> 129: private int[] scopeSlots;
>> 130: private int curScopeLocalIdx = -1;
>> 131: private int RETURN_ALLOCATOR_IDX = -1;
> 
> These are not constants, so it is odd to see the name capitalized

Right, habit of writing lambda forms, where name variables are capitalized.

-

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

Re: RFR: 8283689: Update the foreign linker VM implementation [v21]

[Jorn Vernee]

On Tue, 17 May 2022 08:27:41 GMT, Maurizio Cimadamore  
wrote:

>> Jorn Vernee has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Missing ASSERT -> NOT_PRODUCT
>
> src/java.base/share/classes/jdk/internal/foreign/abi/ProgrammableInvoker.java 
> line 66:
> 
>> 64: private static final boolean USE_SPEC = Boolean.parseBoolean(
>> 65: 
>> GetPropertyAction.privilegedGetProperty("jdk.internal.foreign.ProgrammableInvoker.USE_SPEC",
>>  "true"));
>> 66: private static final boolean USE_INTRINSICS = Boolean.parseBoolean(
> 
> Do we need to update TestMatrix given that we're removing one dimension in 
> the invokers?

Looks like that already happened as part of the main JEP integration.

-

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v2]

[Maurizio Cimadamore]

On Fri, 13 May 2022 13:23:41 GMT, Jorn Vernee  wrote:

>> Hi,
>> 
>> This PR brings over commits from the panama-foreign repo. These commits 
>> mostly pertain to the switch to ASM and away from MethodHandle combinators 
>> for the binding recipe specialization. But, there is one more commit which 
>> adds freeing of downcall stubs, since those changes were mostly Java as well.
>> 
>> Thanks,
>> Jorn
>> 
>> Testing: `run-test-jdk_foreign` with `-Dgenerator.sample.factor=-1` on 
>> Windows and Linux.
>
> Jorn Vernee has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use unaligned layout constants when filling in reconstituted structs (was 
> accidentally dropped change)

src/java.base/share/classes/jdk/internal/foreign/abi/CallingSequence.java line 
109:

> 107:  * @return the caller method type.
> 108:  */
> 109: public MethodType callerMethodType() {

Would it make sense to either rename these, or to point to the fact that these 
are equivalent to Linker::downcallType/upcallType ?

-

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v3]

[Maurizio Cimadamore]

On Tue, 17 May 2022 05:54:39 GMT, Rémi Forax  wrote:

>> Jorn Vernee has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   BootstrapMethodError -> ExceptionInInitializerError
>
> src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java 
> line 98:
> 
>> 96: private static final String CLASS_DATA_DESC = 
>> methodType(Object.class, MethodHandles.Lookup.class, String.class, 
>> Class.class).descriptorString();
>> 97: private static final String RELEASE0_DESC = 
>> methodType(void.class).descriptorString();
>> 98: private static final String ACQUIRE0_DESC = 
>> methodType(void.class).descriptorString();
> 
> calling methodType() is quite slow because of the cache of MethodType so 
> maybe those initialization should be done explicitly in a static block to 
> avoid to recompute methodType(long).descriptorString() several times

What about using MethodTypeDesc/ClassDesc as building block?

-

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v3]

[Maurizio Cimadamore]

On Mon, 16 May 2022 17:40:41 GMT, Jorn Vernee  wrote:

>> Hi,
>> 
>> This PR brings over commits from the panama-foreign repo. These commits 
>> mostly pertain to the switch to ASM and away from MethodHandle combinators 
>> for the binding recipe specialization. But, there is one more commit which 
>> adds freeing of downcall stubs, since those changes were mostly Java as well.
>> 
>> Thanks,
>> Jorn
>> 
>> Testing: `run-test-jdk_foreign` with `-Dgenerator.sample.factor=-1` on 
>> Windows and Linux.
>
> Jorn Vernee has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   BootstrapMethodError -> ExceptionInInitializerError

src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 257:

> 255:  * the context's allocator is accessed.
> 256:  */
> 257: public static Context ofSession() {

Thanks for fixing this

src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java 
line 131:

> 129: private int[] scopeSlots;
> 130: private int curScopeLocalIdx = -1;
> 131: private int RETURN_ALLOCATOR_IDX = -1;

These are not constants, so it is odd to see the name capitalized

src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java 
line 291:

> 289: emitGetStatic(Binding.Context.class, "DUMMY", 
> BINDING_CONTEXT_DESC);
> 290: } else {
> 291: emitInvokeStatic(Binding.Context.class, "ofSession", 
> OF_SESSION_DESC);

Maybe this is micro-optimization, but I realized that in reality we probably 
don't need any scope/session if there are no "ToSegment" bindings.

src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java 
line 336:

> 334: 
> 335: if (callingSequence.forUpcall()) {
> 336: // for upcalls, recipes have a result, which we handle 
> here

I find this part a bit confusing. The comment speaks about recipes input and 
outputs, hinting at the fact that downcall bindings have inputs, while upcall 
bindings have outputs.
In reality, if we look at the bindings themselves, they look pretty symmetric:

* UNBOX_ADDRESS = pops an Addressable and push a long on the stack
* BOX_ADDRESS = pops a long and push a MemoryAddress on the stack

That is, in both cases it appears we have an input and an output (and the 
binding is just the function which maps one into another).

I think the input/output asymmetry comes in when we consider the full recipes. 
In downcalls, for a given argument we will have the following sequence:

Binding0, Binding1, ... BindingN-1, VMStore

While for upcalls we will have:

VMLoad, Binding1, Binding2, ... BindingN

So (ignoring return buffer for simplicity), for downcalls, it is obvious that 
before we can execute Binding0, we need some kind of "input value" (e.g. the 
value of some local). For upcalls, this is not needed, as the VMLoad binding 
will basically do that for free.
When we finished executing the bindings, again, for downcalls there's nothing 
to do, as the chain always ends with a VMStore (which stores binding result 
into some local), while for upcalls we do need to set the output value 
explicitly.

In other words, it seems to me that having VMLoad and VMStore in the chains of 
bindings to be interpreted/specialized does more harm than good here. These low 
level bindings make sense for the VM code, as the VM needs to know how to load 
a value from a register, or how to store a value into a register. But in terms 
of the specializing Java code, these bindings are immaterial. At the same time, 
the fact that we have these bindings, and that they are virtualized, makes the 
code hard to follow, as some preparation happens in 
`BindingSpecializer::specialize`, while some other preparation happens inside 
`BindingSpecializer::doBindings`, as part of the virtualized impl of 
VMLoad/VMStore. And, this virtualized implementation ends up doing similar 
steps as what the preparation code before/after the binding execution does 
(e.g. for downcalls/VMStore we call setOutput, while for upcalls/VMLoad we call 
getInput).

All I'm saying here is that, for bindings to work, we _always_ need to call 
both getInput and setOutput - but it is easy to overlook this when looking at 
the code, because the getInput/setOutput calls are spread across two different 
places in the specializing code, perhaps making things more asymmetric than 
they need to be. (I realize I'm simplifying here, and that some details of the 
return buffer are not 100% quite as symmetric).

-

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

Re: RFR: 8281712: [REDO] AArch64: Implement string_compare intrinsic in SVE

[Nick Gasson]

On Mon, 16 May 2022 07:21:27 GMT, Ningsheng Jian  wrote:

> This is the REDO of JDK-8269559 and JDK-8275448. Those two backouts finally 
> turned to be some system zlib issue in AArch64 macOS, and is not related to 
> the patch itself. See [1][2] for details.
> 
> This patch is generally the same as JDK-8275448, which uses SVE to optimize 
> string_compare intrinsics for long string comparisons. I did a rebase with 
> small tweaks to get better performance on recent Neoverse hardware. Test data 
> on systems with different SVE vector sizes:
> 
> 
>casedelta size  128-bits  256-bits  512-bits
> compareToLL  2   24 0.17% 0.58% 0.00%
> compareToLL  2   36 0.00%2.25%  0.04%
> compareToLL  2   72 -4.40%   3.87%  -12.82%
> compareToLL  2   1284.55%58.31% 13.53%
> compareToLL  2   25619.39%   69.77% 82.03%
> compareToLL  2   5121.81%68.38% 170.93%
> compareToLU  2   24 25.57%   46.98% 54.61%
> compareToLU  2   36 36.03%   70.26% 94.33%
> compareToLU  2   72 35.86%   90.58% 146.04%
> compareToLU  2   12870.82%   119.19%266.22%
> compareToLU  2   25680.77%   146.33%420.01%
> compareToLU  2   51294.62%   171.72%530.87%
> compareToUL  2   24 20.82%   34.48% 62.14%
> compareToUL  2   36 39.77%   60.79% 69.77%
> compareToUL  2   72 35.46%   84.34% 121.90%
> compareToUL  2   12867.77%   110.97%220.53%
> compareToUL  2   25677.05%   160.29%331.30%
> compareToUL  2   51291.88%   184.57%524.21%
> compareToUU  2   24 -0.13%   0.40%  0.00%
> compareToUU  2   36 -9.18%   12.84% -13.93%
> compareToUU  2   72 1.67%60.61% 6.69%
> compareToUU  2   12813.51%   60.33% 55.27%
> compareToUU  2   2562.55%62.17% 153.26%
> compareToUU  2   5124.12%68.62% 201.68%
> 
> JTreg tests passed on SVE hardware.
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8275448
> [2] https://bugs.openjdk.java.net/browse/JDK-8282954

LGTM!

-

Marked as reviewed by ngasson (Reviewer).

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

Re: RFR: 8283689: Update the foreign linker VM implementation [v21]

[Maurizio Cimadamore]

On Mon, 16 May 2022 16:15:49 GMT, Jorn Vernee  wrote:

>> Hi,
>> 
>> This PR updates the VM implementation of the foreign linker, by bringing 
>> over commits from the panama-foreign repo.
>> 
>> This is split off from the main JEP integration for 19, since we have 
>> limited resources to handle this. As such, this PR might fall over to 20, 
>> but it would be nice if we could get it into 19.
>> 
>> I've written up an overview of the Linker architecture here: 
>> http://cr.openjdk.java.net/~jvernee/docs/FL_Overview.html it might be useful 
>> to read that first.
>> 
>> This patch moves from the "legacy" implementation, to what is currently 
>> implemented in the panama-foreign repo, except for replacing the use of 
>> method handle combinators with ASM. That will come in a later path. To 
>> recap. This PR contains the following changes:
>> 
>> 1. VM stubs for downcalls are now generated up front, instead of lazily by 
>> C2 [1].
>> 2. the VM support for upcalls/downcalls now support all possible call 
>> shapes. And VM stubs and Java code implementing the buffered invocation 
>> strategy has been removed  [2], [3], [4], [5].
>> 3. The existing C2 intrinsification support for the `linkToNative` method 
>> handle linker was no longer needed and has been removed [6] (support might 
>> be re-added in another form later).
>> 4. Some other cleanups, such as: OptimizedEntryBlob (for upcalls) now 
>> implements RuntimeBlob directly. Binding to java classes has been rewritten 
>> to use javaClasses.h/cpp (this wasn't previously possible due to these java 
>> classes being in an incubator module) [7], [8], [9].
>> 
>> While the patch mostly consists of VM changes, there are also some Java 
>> changes to support (2).
>> 
>> The original commit structure has been mostly retained, so it might be 
>> useful to look at a specific commit, or the corresponding patch in the 
>> [panama-foreign](https://github.com/openjdk/panama-foreign/pulls?q=is%3Apr) 
>> repo as well. I've also left some inline comments to explain some of the 
>> changes, which will hopefully make reviewing easier.
>> 
>> Testing: Tier1-4
>> 
>> Thanks,
>> Jorn
>> 
>> [1]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/048b88156814579dca1f70742061ad24942fd358
>> [2]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/2fbbef472b4c2b4fee5ede2f18cd81ab61e88f49
>> [3]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/8a957a4ed9cc8d1f708ea8777212eb51ab403dc3
>> [4]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/35ba1d964f1de4a77345dc58debe0565db4b0ff3
>> [5]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/4e72aae22920300c5ffa16fed805b62ed9092120
>> [6]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/08e22e1b468c5c8f0cfd7135c72849944068aa7a
>> [7]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/451cd9edf54016c182dab21a8b26bd8b609fc062
>> [8]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/4c851d2795afafec3a3ab17f4142ee098692068f
>> [9]: 
>> https://github.com/openjdk/jdk/pull/7959/commits/d025377799424f31512dca2ffe95491cd5ae22f9
>
> Jorn Vernee has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Missing ASSERT -> NOT_PRODUCT

src/java.base/share/classes/jdk/internal/foreign/abi/ProgrammableInvoker.java 
line 66:

> 64: private static final boolean USE_SPEC = Boolean.parseBoolean(
> 65: 
> GetPropertyAction.privilegedGetProperty("jdk.internal.foreign.ProgrammableInvoker.USE_SPEC",
>  "true"));
> 66: private static final boolean USE_INTRINSICS = Boolean.parseBoolean(

Do we need to update TestMatrix given that we're removing one dimension in the 
invokers?

-

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

RFR: 8286858: Remove dead code in sun.reflect.misc.MethodUtil

[Andrey Turbanov]

They are unused and leftover since JDK 7.  It's good to remove them.

-

Commit messages:
 - [PATCH] Remove unused code in sun.reflect.misc.MethodUtil

Changes: https://git.openjdk.java.net/jdk/pull/8715/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8715=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8286858
  Stats: 179 lines in 1 file changed: 4 ins; 170 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8715.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8715/head:pull/8715

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

Re: [External] : Re: RFR: 8286122: [macos]: App bundle cannot upload to Mac App Store due to info.plist embedded in java exe

[Michael Hall]

> On May 17, 2022, at 12:15 AM, Alexander Matveev 
>  wrote:
> 
> Hi Michael,
> 
> I filed following enhancement for signing user provided app image and yes it 
> will be two step process. Invoke jpackage to generate image, then user can 
> modified it and then invoke jpackage again to sign it.
> https://bugs.openjdk.java.net/browse/JDK-8286850
> 
> For JDK-8286122, I will integrate proposed fix as is. Error will be thrown if 
> user tries to include native commands for Mac App Store image.
> 
> Still not sure if we will provide solution for including native commands with 
> Mac App Store. Including special versions of native commands with jpackage 
> will work only if runtime is generated by jpackage. It will not solve issue 
> with user provided runtime. Also, it is not clear if app updates will require 
> same unique ID based on UUID across app versions.

This enhancement doesn’t directly concern this issue. Although given this 
enhancement it would be possible to provide a temporary ‘hack’ fix without 
building it into jpackage. Where the executables are changed to make them 
unique in the post-processing step. If it is not your intention to address this 
issue given this enhancement you would probably have to check the provided 
application bundle to be sure it doesn’t have the colliding Info.plist native 
commands. What Apple DTS provided may of had commands for this? I don’t 
remember. Or you would have to just fail any passed application bundles with 
native java commands.

From https://bugs.openjdk.java.net/browse/JDK-8286850 


> Generating DMG or PKG from —app-image with signing enabled will not sign app 
> image as it currently do.

I am not sure you would want to change this. If the developer doesn’t want to 
do any post-processing of the application bundle then they should be able to 
use the current invocations unchanged to do this?
I would think if you want post-processing you would generate unsigned 
applications, post-process, use the enhancement to sign. 


> 
> Also, many functionality provided by native JDK commands can be used via 
> ToolProvider. For example if you include jdk.jpackage module with your app, 
> you should able to use jpackage functionality from your app via ToolProvider 
> without actual jpackage native command.
> 
> Thanks,
> Alexander

Which might be another way this could be useful. If any 3rd parties want to 
generate their own application bundles they could still use jpackage as the 
definitive way to sign those.

Thanks 
Mike
> 

Re: RFR: 8286212: Cgroup v1 initialization causes NPE on some systems [v2]

[Severin Gehwolf]

On Tue, 17 May 2022 07:14:34 GMT, Thomas Stuefe  wrote:

>> src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 92:
>> 
>>> 90:   }
>>> 91:   ss.print_raw(_root, last_matching_slash_pos);
>>> 92:   _path = os::strdup(ss.base());
>> 
>> Do you mean `Find the longest common prefix`? Maybe give an example in the 
>> comments? Text parsing in C code is really difficult to understand.
>
> Maybe factor out the search like this
> 
> // Return length of common starting substring. E.g. "cat" for ("cattle", 
> "catnip");
> static int find_common_starting_substring(const char* s1, const char* s2) {
> ...
> }
> 
> That way its clearer and we can find and move this to utilities if we ever 
> need this in other places.

OK.

-

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

Re: RFR: 8286212: Cgroup v1 initialization causes NPE on some systems [v2]

[Severin Gehwolf]

On Tue, 17 May 2022 05:55:47 GMT, Ioi Lam  wrote:

>> Severin Gehwolf has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Use stringStream to simplify controller path assembly
>
> test/hotspot/gtest/runtime/test_os_linux_cgroups.cpp line 63:
> 
>> 61: ASSERT_STREQ(expected_cg_paths[i], ctrl->subsystem_path());
>> 62:   }
>> 63: }
> 
> I found it hard to relate the different paths. Could you create a new struct 
> like this?
> 
> 
> struct TestCase {
> char* mount_path;
> char* root_paths;
> char* cgroup_path;
> char* expected_cg_paths;
> } = {
>   {  "/sys/fs/cgroup/memory", // mount
>"/",   // root,
>

Yes, makes sense. Will do.

-

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

Re: RFR: 8284960: Integration of JEP 426: Vector API (Fourth Incubator) [v5]

[Jatin Bhateja]

> Hi All,
> 
> Patch adds the planned support for new vector operations and APIs targeted 
> for [JEP 426: Vector API (Fourth 
> Incubator).](https://bugs.openjdk.java.net/browse/JDK-8280173)
> 
> Following is the brief summary of changes:-
> 
> 1)  Extends the scope of existing lanewise API for following new vector 
> operations.
>-  VectorOperations.BIT_COUNT: counts the number of one-bits
>- VectorOperations.LEADING_ZEROS_COUNT: counts the number of leading zero 
> bits
>- VectorOperations.TRAILING_ZEROS_COUNT: counts the number of trailing 
> zero bits
>- VectorOperations.REVERSE: reversing the order of bits
>- VectorOperations.REVERSE_BYTES: reversing the order of bytes
>- compress and expand bits: Semantics are based on Hacker's Delight 
> section 7-4 Compress, or Generalized Extract.
> 
> 2)  Adds following new APIs to perform cross lane vector compress and 
> expansion operations under the influence of a mask.
>- Vector.compress
>- Vector.expand 
>- VectorMask.compress
> 
> 3) Adds predicated and non-predicated versions of following new APIs to load 
> and store the contents of vector from foreign MemorySegments. 
>   - Vector.fromMemorySegment
>   - Vector.intoMemorySegment
> 
> 4) C2 Compiler IR enhancements and optimized X86 and AARCH64 backend support 
> for each newly added operation.
> 
> 
>  Patch has been regressed over AARCH64 and X86 targets different AVX levels. 
> 
>  Kindly review and share your feedback.
> 
>  Best Regards,
>  Jatin

Jatin Bhateja has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 13 commits:

 - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960
 - 8284960: Review comments resolution.
 - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960
 - 8284960: Correcting a typo.
 - 8284960: Integrating changes from panama-vector (Add @since 19 tags).
 - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960
 - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960
 - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960
 - 8284960: AARCH64 backend changes.
 - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960
 - ... and 3 more: https://git.openjdk.java.net/jdk/compare/5e5500cb...df7eb90e

-

Changes: https://git.openjdk.java.net/jdk/pull/8425/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8425=04
  Stats: 38068 lines in 254 files changed: 16705 ins; 16921 del; 4442 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8425.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8425/head:pull/8425

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

Re: RFR: 8286212: Cgroup v1 initialization causes NPE on some systems [v2]

[Thomas Stuefe]

On Tue, 17 May 2022 06:18:25 GMT, Ioi Lam  wrote:

>> Severin Gehwolf has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Use stringStream to simplify controller path assembly
>
> src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 92:
> 
>> 90:   }
>> 91:   ss.print_raw(_root, last_matching_slash_pos);
>> 92:   _path = os::strdup(ss.base());
> 
> Do you mean `Find the longest common prefix`? Maybe give an example in the 
> comments? Text parsing in C code is really difficult to understand.

Maybe factor out the search like this

// Return length of common starting substring. E.g. "cat" for ("cattle", 
"catnip");
static int find_common_starting_substring(const char* s1, const char* s2) {
...
}

That way its clearer and we can find and move this to utilities if we ever need 
this in other places.

-

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

Re: RFR: 8281712: [REDO] AArch64: Implement string_compare intrinsic in SVE

[Ningsheng Jian]

On Tue, 17 May 2022 06:09:11 GMT, Tobias Hartmann  wrote:

> Sure, I already submitted testing yesterday, it all passed.

Thank you, Tobias!

-

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

Re: RFR: 8286212: Cgroup v1 initialization causes NPE on some systems [v2]

[Ioi Lam]

On Thu, 12 May 2022 18:09:40 GMT, Severin Gehwolf  wrote:

>> Please review this change to the cgroup v1 subsystem which makes it more 
>> resilient on some of the stranger systems. Unfortunately, I wasn't able to 
>> re-create a similar system as the reporter. The idea of using the longest 
>> substring match for the cgroupv1 file paths was based on the fact that on 
>> systemd systems processes run in separate scopes and the maven forked test 
>> runner might exhibit this property. For that it makes sense to use the 
>> common ancestor path. Nothing changes in the common cases where the 
>> `cgroup_path` matches `_root` and when the `_root` is `/` (container the 
>> former, host system the latter).
>> 
>> In addition, the code paths which are susceptible to throw NPE have been 
>> hardened to catch those situations. Should it happen in the future it makes 
>> more sense (to me) to not have accurate container detection in favor of 
>> continuing to keep running.
>> 
>> Finally, with the added unit-tests a bug was uncovered on the "substring" 
>> match case of cgroup paths in hotspot. `p` returned from `strstr` can never 
>> point to `_root` as it's used as the "needle" to find in "haystack" 
>> `cgroup_path` (not the other way round).
>> 
>> Testing:
>> - [x] Added unit tests
>> - [x] GHA
>> - [x] Container tests on cgroups v1 Linux. Continue to pass
>
> Severin Gehwolf has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Use stringStream to simplify controller path assembly

src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 92:

> 90:   }
> 91:   ss.print_raw(_root, last_matching_slash_pos);
> 92:   _path = os::strdup(ss.base());

Do you mean `Find the longest common prefix`? Maybe give an example in the 
comments? Text parsing in C code is really difficult to understand.

-

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v3]

[Rémi Forax]

On Mon, 16 May 2022 17:40:41 GMT, Jorn Vernee  wrote:

>> Hi,
>> 
>> This PR brings over commits from the panama-foreign repo. These commits 
>> mostly pertain to the switch to ASM and away from MethodHandle combinators 
>> for the binding recipe specialization. But, there is one more commit which 
>> adds freeing of downcall stubs, since those changes were mostly Java as well.
>> 
>> Thanks,
>> Jorn
>> 
>> Testing: `run-test-jdk_foreign` with `-Dgenerator.sample.factor=-1` on 
>> Windows and Linux.
>
> Jorn Vernee has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   BootstrapMethodError -> ExceptionInInitializerError

src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java 
line 943:

> 941: Z, B, S, C, I, J, F, D, L;
> 942: 
> 943: static BasicType of(Class cls) {

This seems a duplication of ASM Type class for no good reason, 
Type.getOpcode(ILOAD) or Type.getOpcode(IRETURN) gives you the proper variant 
opcode

-

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v3]

[Rémi Forax]

On Mon, 16 May 2022 17:40:41 GMT, Jorn Vernee  wrote:

>> Hi,
>> 
>> This PR brings over commits from the panama-foreign repo. These commits 
>> mostly pertain to the switch to ASM and away from MethodHandle combinators 
>> for the binding recipe specialization. But, there is one more commit which 
>> adds freeing of downcall stubs, since those changes were mostly Java as well.
>> 
>> Thanks,
>> Jorn
>> 
>> Testing: `run-test-jdk_foreign` with `-Dgenerator.sample.factor=-1` on 
>> Windows and Linux.
>
> Jorn Vernee has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   BootstrapMethodError -> ExceptionInInitializerError

src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java 
line 967:

> 965: 
> 966: // unaligned constants
> 967: public final static ValueLayout.OfBoolean JAVA_BOOLEAN_UNALIGNED = 
> JAVA_BOOLEAN;

as far as i understand, those constants are accessed from the bytecode, they 
should be in their own class to cleanly separate the specializer part and the 
runtime part.

-

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

Re: RFR: 8281712: [REDO] AArch64: Implement string_compare intrinsic in SVE

[Tobias Hartmann]

On Mon, 16 May 2022 07:21:27 GMT, Ningsheng Jian  wrote:

> This is the REDO of JDK-8269559 and JDK-8275448. Those two backouts finally 
> turned to be some system zlib issue in AArch64 macOS, and is not related to 
> the patch itself. See [1][2] for details.
> 
> This patch is generally the same as JDK-8275448, which uses SVE to optimize 
> string_compare intrinsics for long string comparisons. I did a rebase with 
> small tweaks to get better performance on recent Neoverse hardware. Test data 
> on systems with different SVE vector sizes:
> 
> 
>casedelta size  128-bits  256-bits  512-bits
> compareToLL  2   24 0.17% 0.58% 0.00%
> compareToLL  2   36 0.00%2.25%  0.04%
> compareToLL  2   72 -4.40%   3.87%  -12.82%
> compareToLL  2   1284.55%58.31% 13.53%
> compareToLL  2   25619.39%   69.77% 82.03%
> compareToLL  2   5121.81%68.38% 170.93%
> compareToLU  2   24 25.57%   46.98% 54.61%
> compareToLU  2   36 36.03%   70.26% 94.33%
> compareToLU  2   72 35.86%   90.58% 146.04%
> compareToLU  2   12870.82%   119.19%266.22%
> compareToLU  2   25680.77%   146.33%420.01%
> compareToLU  2   51294.62%   171.72%530.87%
> compareToUL  2   24 20.82%   34.48% 62.14%
> compareToUL  2   36 39.77%   60.79% 69.77%
> compareToUL  2   72 35.46%   84.34% 121.90%
> compareToUL  2   12867.77%   110.97%220.53%
> compareToUL  2   25677.05%   160.29%331.30%
> compareToUL  2   51291.88%   184.57%524.21%
> compareToUU  2   24 -0.13%   0.40%  0.00%
> compareToUU  2   36 -9.18%   12.84% -13.93%
> compareToUU  2   72 1.67%60.61% 6.69%
> compareToUU  2   12813.51%   60.33% 55.27%
> compareToUU  2   2562.55%62.17% 153.26%
> compareToUU  2   5124.12%68.62% 201.68%
> 
> JTreg tests passed on SVE hardware.
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8275448
> [2] https://bugs.openjdk.java.net/browse/JDK-8282954

Marked as reviewed by thartmann (Reviewer).

Sure, I already submitted testing yesterday, it all passed.

-

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

Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v3]

[Rémi Forax]

On Mon, 16 May 2022 17:40:41 GMT, Jorn Vernee  wrote:

>> Hi,
>> 
>> This PR brings over commits from the panama-foreign repo. These commits 
>> mostly pertain to the switch to ASM and away from MethodHandle combinators 
>> for the binding recipe specialization. But, there is one more commit which 
>> adds freeing of downcall stubs, since those changes were mostly Java as well.
>> 
>> Thanks,
>> Jorn
>> 
>> Testing: `run-test-jdk_foreign` with `-Dgenerator.sample.factor=-1` on 
>> Windows and Linux.
>
> Jorn Vernee has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   BootstrapMethodError -> ExceptionInInitializerError

src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java 
line 157:

> 155: }
> 156: 
> 157: static MethodHandle doSpecialize(MethodHandle leafHandle, 
> CallingSequence callingSequence, ABIDescriptor abi) {

I think thise method should be split in to, one version for the upcall, one for 
the downcall, i do not see the point of merging the two codes.

src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java 
line 816:

> 814: return;
> 815: }
> 816: if (con instanceof Integer) {

those can use the instanceof + type pattern

-

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