Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v3]

[Aleksey Shipilev]

On Wed, 1 Jun 2022 06:04:14 GMT, Alan Bateman  wrote:

>> This patch adds an alternative virtual thread implementation where each 
>> virtual thread is backed by an OS thread. It doesn't scale but it can be 
>> used by ports that don't have continuations support in the VM. Aside from 
>> scalability, the lack of continuations support means:
>> 
>> 1. JVM TI is not supported when running with --enable-preview (the JVM TI 
>> spec allows for this) 
>> 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs 
>> and so needs JVM TI)
>> 
>> The VM option "VMContinuations" is added as an experimental option so it can 
>> be used by tests. A number of tests are changed to re-run with 
>> -XX:-VMContinuations. A new jtreg property is added so that tests that need 
>> the underlying VM support to be present can use "@requires vm.continuations" 
>> in the test description. A follow-up change would be to add "@requires 
>> vm.continuations" to the ~70 serviceability/jvmti/vthread that run with 
>> preview features enabled.
>
> Alan Bateman has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 11 commits:
> 
>  - Fixed another typo in comment
>  - Merge
>  - Fix typos in comments
>  - Allowing linking without intrinsic stub, contributed-by: rehn
>  - Continuation clinit should fail if no continuations support
>  - Fix merge issue with test
>  - Change VMContinuations to be experimental option, ensure JNI GetEnv 
> returns EVERSION
>  - Update
>  - Expend test coverage
>  - Merge
>  - ... and 1 more: 
> https://git.openjdk.java.net/jdk/compare/3deb58a8...a5c06cb6

> A new jtreg property is added so that tests that need the underlying VM 
> support to be present can use "@requires vm.continuations" in the test 
> description. A follow-up change would be to add "@requires vm.continuations" 
> to the ~70 serviceability/jvmti/vthread that run with preview features 
> enabled.

See #8990.

-

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

Integrated: 8287520: Shrink x86_32 problemlists after JDK-8287437

[Aleksey Shipilev]

On Mon, 30 May 2022 13:20:17 GMT, Aleksey Shipilev  wrote:

> [JDK-8287137](https://bugs.openjdk.java.net/browse/JDK-8287137) added a bunch 
> of tests into problemlist. Those lists basically exclude every test that runs 
> with --enable-preview. 
> [JDK-8287437](https://bugs.openjdk.java.net/browse/JDK-8287437) makes it 
> better: it only disables Loom parts, even with --enable-preview. This allows 
> testing x86_32 on other preview features and avoids accidental bug slippage 
> in non-Loom code paths. We can and should shrink the problem lists now.
> 
> Additional testing:
>  - [x] Removed tests with Linux x86_32 fastdebug; mostly pass, some non-Loom 
> failures
>  - [x] Linux x86_32 fastdebug `tier1`
>  - [x] GHA run

This pull request has now been integrated.

Changeset: 71599763
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk/commit/71599763359055c81afbe5e04d6034b7bb3f3606
Stats: 143 lines in 3 files changed: 0 ins; 142 del; 1 mod

8287520: Shrink x86_32 problemlists after JDK-8287437

Reviewed-by: alanb

-

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

Re: RFR: 8287520: Shrink x86_32 problemlists after JDK-8287437 [v2]

[Aleksey Shipilev]

> [JDK-8287137](https://bugs.openjdk.java.net/browse/JDK-8287137) added a bunch 
> of tests into problemlist. Those lists basically exclude every test that runs 
> with --enable-preview. 
> [JDK-8287437](https://bugs.openjdk.java.net/browse/JDK-8287437) makes it 
> better: it only disables Loom parts, even with --enable-preview. This allows 
> testing x86_32 on other preview features and avoids accidental bug slippage 
> in non-Loom code paths. We can and should shrink the problem lists now.
> 
> Additional testing:
>  - [x] Removed tests with Linux x86_32 fastdebug; mostly pass, some non-Loom 
> failures
>  - [x] Linux x86_32 fastdebug `tier1`
>  - [x] GHA run

Aleksey Shipilev 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:

 - Merge branch 'master' into JDK-8287520-loom-x86-32-shrink
 - Keep GetEventWriter on problemlist
 - Fix

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8946/files
  - new: https://git.openjdk.java.net/jdk/pull/8946/files/7c90ead6..a3c3db2b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8946&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8946&range=00-01

  Stats: 38619 lines in 252 files changed: 17091 ins; 16971 del; 4557 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8946.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8946/head:pull/8946

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

Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]

[Aleksey Shipilev]

On Tue, 31 May 2022 16:54:41 GMT, Boris Ulasevich  
wrote:

> I expected this change to fix the broken ARM32 port, but it doesn't work.

It would not fix ARM32, because the interpreter stubs need to be predicated on 
`Continuations::enabled()`. Also, as my ARM32 experiments show 
(https://github.com/openjdk/jdk/pull/8634/files#diff-027490ce3f4a92be9b489d9d2e54c7baaea87b7489399b198543c79f1ce1e2e3R4208-R4216)
 -- there is a breakage somewhere in C2 as well, which this patch would not 
seem to resolve as well. So, this is a stepping stone for *some* support, but 
it does not resolve porting situation fully.

-

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

Re: RFR: 8287520: Shrink x86_32 problemlists after JDK-8287437

[Aleksey Shipilev]

On Tue, 31 May 2022 11:44:27 GMT, Alan Bateman  wrote:

>> [JDK-8287137](https://bugs.openjdk.java.net/browse/JDK-8287137) added a 
>> bunch of tests into problemlist. Those lists basically exclude every test 
>> that runs with --enable-preview. 
>> [JDK-8287437](https://bugs.openjdk.java.net/browse/JDK-8287437) makes it 
>> better: it only disables Loom parts, even with --enable-preview. This allows 
>> testing x86_32 on other preview features and avoids accidental bug slippage 
>> in non-Loom code paths. We can and should shrink the problem lists now.
>> 
>> Additional testing:
>>  - [x] Removed tests with Linux x86_32 fastdebug; mostly pass, some non-Loom 
>> failures
>>  - [x] Linux x86_32 fastdebug `tier1`
>>  - [x] GHA run
>
> This looks okay.  If we get PR8939 in then it should be possible to do more 
> clear out of the exclude lists.

Thank you, @AlanBateman. Any other comments? I plan to integrate it soon.

-

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

Re: RFR: 8287520: Shrink x86_32 problemlists after JDK-8287437

[Aleksey Shipilev]

On Mon, 30 May 2022 13:20:17 GMT, Aleksey Shipilev  wrote:

> [JDK-8287137](https://bugs.openjdk.java.net/browse/JDK-8287137) added a bunch 
> of tests into problemlist. Those lists basically exclude every test that runs 
> with --enable-preview. 
> [JDK-8287437](https://bugs.openjdk.java.net/browse/JDK-8287437) makes it 
> better: it only disables Loom parts, even with --enable-preview. This allows 
> testing x86_32 on other preview features and avoids accidental bug slippage 
> in non-Loom code paths. We can and should shrink the problem lists now.
> 
> Additional testing:
>  - [x] Removed tests with Linux x86_32 fastdebug; mostly pass, some non-Loom 
> failures
>  - [x] Linux x86_32 fastdebug `tier1`
>  - [ ] GHA run

Open for reviews. The test failures on Windows x64 are known and are solved 
separately.

-

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

Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration [v3]

[Aleksey Shipilev]

On Tue, 24 May 2022 21:10:29 GMT, David Holmes  wrote:

> Just FYI. There is a mistake in this changeset. The JDK ProblemList already 
> contained:
> 
> `java/nio/channels/FileChannel/LargeMapTest.java 8286980 windows-all`
> 
> and this change then added:
> 
> `java/nio/channels/FileChannel/LargeMapTest.java 8286642 generic-i586`
> 
> which appears to have negated the first entry.

Whoops, sorry. It seems not to be a problem since 
[JDK-8287263](https://bugs.openjdk.java.net/browse/JDK-8287263) committed a few 
hours ago removed the `windows-all` entry and fixed the test. I checked other 
ProblemLists, and there are no other double entries.

-

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

Integrated: 8287137: Problemlist failing x86_32 tests after Loom integration

[Aleksey Shipilev]

On Mon, 23 May 2022 12:28:30 GMT, Aleksey Shipilev  wrote:

> [JDK-8284161](https://bugs.openjdk.java.net/browse/JDK-8284161) broke a lot 
> of x86_32 code. The x86_32 porting is done under 
> [JDK-8286642](https://bugs.openjdk.java.net/browse/JDK-8286642). Meanwhile, 
> we can problemlist the failing tests to get cleaner runs for other patches. 
> This should also make GHA runs cleaner.
> 
> Additional testing:
>  - [x] Linux x86_32 fastdebug `tier1` (some unrelated failures in hotspot)
>  - [x] Linux x86_32 fastdebug `tier2` (some unrelated failures in hotspot)
>  - [x] Linux x86_32 fastdebug `tier3` (clean now)
>  - [x] GHA run

This pull request has now been integrated.

Changeset: 0a82c4eb
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk/commit/0a82c4ebc3748f6dfbbcd72e4421fbe0ea89e0b0
Stats: 279 lines in 3 files changed: 279 ins; 0 del; 0 mod

8287137: Problemlist failing x86_32 tests after Loom integration

Reviewed-by: prr, mcimadamore

-

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

Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration [v3]

[Aleksey Shipilev]

On Mon, 23 May 2022 18:25:23 GMT, Aleksey Shipilev  wrote:

>> [JDK-8284161](https://bugs.openjdk.java.net/browse/JDK-8284161) broke a lot 
>> of x86_32 code. The x86_32 porting is done under 
>> [JDK-8286642](https://bugs.openjdk.java.net/browse/JDK-8286642). Meanwhile, 
>> we can problemlist the failing tests to get cleaner runs for other patches. 
>> This should also make GHA runs cleaner.
>> 
>> Additional testing:
>>  - [x] Linux x86_32 fastdebug `tier1` (some unrelated failures in hotspot)
>>  - [x] Linux x86_32 fastdebug `tier2` (some unrelated failures in hotspot)
>>  - [x] Linux x86_32 fastdebug `tier3` (clean now)
>>  - [x] GHA run
>
> Aleksey Shipilev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Another release test

Thank you!

-

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

Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration [v3]

[Aleksey Shipilev]

On Mon, 23 May 2022 18:25:23 GMT, Aleksey Shipilev  wrote:

>> [JDK-8284161](https://bugs.openjdk.java.net/browse/JDK-8284161) broke a lot 
>> of x86_32 code. The x86_32 porting is done under 
>> [JDK-8286642](https://bugs.openjdk.java.net/browse/JDK-8286642). Meanwhile, 
>> we can problemlist the failing tests to get cleaner runs for other patches. 
>> This should also make GHA runs cleaner.
>> 
>> Additional testing:
>>  - [x] Linux x86_32 fastdebug `tier1` (some unrelated failures in hotspot)
>>  - [x] Linux x86_32 fastdebug `tier2` (some unrelated failures in hotspot)
>>  - [x] Linux x86_32 fastdebug `tier3` (clean now)
>>  - [x] GHA run
>
> Aleksey Shipilev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Another release test

GHA run completes fine on x86_32. There are unrelated Windows x86_64 failures 
in javac land, to be handled separately. 

Please approve, @AlanBateman, @mcimadamore and others?

-

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

Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration [v3]

[Aleksey Shipilev]

> [JDK-8284161](https://bugs.openjdk.java.net/browse/JDK-8284161) broke a lot 
> of x86_32 code. The x86_32 porting is done under 
> [JDK-8286642](https://bugs.openjdk.java.net/browse/JDK-8286642). Meanwhile, 
> we can problemlist the failing tests to get cleaner runs for other patches. 
> This should also make GHA runs cleaner.
> 
> Additional testing:
>  - [x] Linux x86_32 fastdebug `tier1` (some unrelated failures in hotspot)
>  - [x] Linux x86_32 fastdebug `tier2` (some unrelated failures in hotspot)
>  - [x] Linux x86_32 fastdebug `tier3` (clean now)
>  - [ ] GHA run

Aleksey Shipilev has updated the pull request incrementally with one additional 
commit since the last revision:

  Another release test

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8843/files
  - new: https://git.openjdk.java.net/jdk/pull/8843/files/5a0cd8fe..e223e211

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8843&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8843&range=01-02

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

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

Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration [v2]

[Aleksey Shipilev]

> [JDK-8284161](https://bugs.openjdk.java.net/browse/JDK-8284161) broke a lot 
> of x86_32 code. The x86_32 porting is done under 
> [JDK-8286642](https://bugs.openjdk.java.net/browse/JDK-8286642). Meanwhile, 
> we can problemlist the failing tests to get cleaner runs for other patches. 
> This should also make GHA runs cleaner.
> 
> Additional testing:
>  - [x] Linux x86_32 fastdebug `tier1` (some unrelated failures in hotspot)
>  - [x] Linux x86_32 fastdebug `tier2` (some unrelated failures in hotspot)
>  - [x] Linux x86_32 fastdebug `tier3` (clean now)
>  - [ ] GHA run

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

 - Fix the tests in release JDKs
 - Merge branch 'master' into JDK-8287137-problemlist-x86_32
 - Updates
 - Revert test groups
 - Fix 2
 - Fix

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8843/files
  - new: https://git.openjdk.java.net/jdk/pull/8843/files/ba5c631e..5a0cd8fe

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8843&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8843&range=00-01

  Stats: 4568 lines in 145 files changed: 2586 ins; 1656 del; 326 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8843.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8843/head:pull/8843

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

Integrated: 8286956: Loom: Define test groups for development/porting use

[Aleksey Shipilev]

On Wed, 18 May 2022 11:10:58 GMT, Aleksey Shipilev  wrote:

> It would be beneficial to bring over the Loom-specific test groups from the 
> loom repo to aid development/porting work.
> 
> https://github.com/openjdk/loom/blob/fibers/test/jdk/TEST.groups#L97-L108
> https://github.com/openjdk/loom/blob/6f42923b3342e41d95b262733205283068802b40/test/hotspot/jtreg/TEST.groups#L111-L125
> 
> I had to drop `jdk/incubator/concurrent` from JDK definition, because there 
> are no tests in that folder and tests break.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug `hotspot_loom jdk_loom`
>  - [x] Linux AArch64 fastdebug `hotspot_loom jdk_loom`

This pull request has now been integrated.

Changeset: ac274c4c
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk/commit/ac274c4ca67555742065dc850823e924361f2ff7
Stats: 28 lines in 2 files changed: 28 ins; 0 del; 0 mod

8286956: Loom: Define test groups for development/porting use

Reviewed-by: alanb, zgu

-

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

Re: RFR: 8286956: Loom: Define test groups for development/porting use

[Aleksey Shipilev]

On Wed, 18 May 2022 11:10:58 GMT, Aleksey Shipilev  wrote:

> It would be beneficial to bring over the Loom-specific test groups from the 
> loom repo to aid development/porting work.
> 
> https://github.com/openjdk/loom/blob/fibers/test/jdk/TEST.groups#L97-L108
> https://github.com/openjdk/loom/blob/6f42923b3342e41d95b262733205283068802b40/test/hotspot/jtreg/TEST.groups#L111-L125
> 
> I had to drop `jdk/incubator/concurrent` from JDK definition, because there 
> are no tests in that folder and tests break.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug `hotspot_loom jdk_loom`
>  - [x] Linux AArch64 fastdebug `hotspot_loom jdk_loom`

All right, thank you!

-

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

Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration

[Aleksey Shipilev]

On Mon, 23 May 2022 15:34:11 GMT, Alan Bateman  wrote:

>> Those tests *do* run with preview enabled: 
>> https://github.com/openjdk/jdk/blob/master/test/jdk/java/util/stream/test/TEST.properties#L10-L11
>>  -- and I know that because I basically copy-pasted all jtreg failures that 
>> lead to `Unimplemented()` in x86_32 code.
>
> Okay, so the issue is SpliteratorTest uses preview APIs and having the 
> configuration in TEST.properties means that all the streams tests are run 
> with this option.

Yes, `TEST.properties` is way too broad. I'd keep the current patch as is, 
instead of trying to fix that one. I suspect there is some funky TestNG/othervm 
interaction going...

-

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

Re: RFR: 8287137: Problemlist failing x86_32 tests after Loom integration

[Aleksey Shipilev]

On Mon, 23 May 2022 15:13:02 GMT, Alan Bateman  wrote:

>> [JDK-8284161](https://bugs.openjdk.java.net/browse/JDK-8284161) broke a lot 
>> of x86_32 code. The x86_32 porting is done under 
>> [JDK-8286642](https://bugs.openjdk.java.net/browse/JDK-8286642). Meanwhile, 
>> we can problemlist the failing tests to get cleaner runs for other patches. 
>> This should also make GHA runs cleaner.
>> 
>> Additional testing:
>>  - [x] Linux x86_32 fastdebug `tier1` (some unrelated failures in hotspot)
>>  - [x] Linux x86_32 fastdebug `tier2` (some unrelated failures in hotspot)
>>  - [x] Linux x86_32 fastdebug `tier3` (clean now)
>>  - [ ] GHA run
>
> test/jdk/ProblemList.txt line 894:
> 
>> 892: 
>> java/util/stream/test/org/openjdk/tests/java/util/stream/CollectionAndMapModifyStreamTest.java
>>   8286642 generic-i586
>> 893: 
>> java/util/stream/test/org/openjdk/tests/java/util/stream/CollectorExample.java
>>   8286642 generic-i586
>> 894: 
>> java/util/stream/test/org/openjdk/tests/java/util/stream/CollectorToUnmodListTest.java
>>   8286642 generic-i586
> 
> I'm surprised that the stream tests are included as they don't run with 
> --enable-preview.

Those tests *do* run with preview enabled: 
https://github.com/openjdk/jdk/blob/master/test/jdk/java/util/stream/test/TEST.properties#L10-L11
 -- and I know that because I basically copy-pasted all jtreg failures that 
lead to `Unimplemented()` in x86_32 code.

-

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

RFR: 8287137: Problemlist failing x86_32 tests after Loom integration

[Aleksey Shipilev]

[JDK-8284161](https://bugs.openjdk.java.net/browse/JDK-8284161) broke a lot of 
x86_32 code. The x86_32 porting is done under 
[JDK-8286642](https://bugs.openjdk.java.net/browse/JDK-8286642). Meanwhile, we 
can problemlist the failing tests to get cleaner runs for other patches. This 
should also make GHA runs cleaner.

Additional testing:
 - [x] Linux x86_32 fastdebug `tier1` (some unrelated failures in hotspot)
 - [x] Linux x86_32 fastdebug `tier2` (some unrelated failures in hotspot)
 - [x] Linux x86_32 fastdebug `tier3` (clean now)
 - [ ] GHA run

-

Commit messages:
 - Updates
 - Revert test groups
 - Fix 2
 - Fix

Changes: https://git.openjdk.java.net/jdk/pull/8843/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8843&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8287137
  Stats: 276 lines in 3 files changed: 276 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8843.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8843/head:pull/8843

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

Re: RFR: 8286956: Loom: Define test groups for development/porting use

[Aleksey Shipilev]

On Wed, 18 May 2022 11:10:58 GMT, Aleksey Shipilev  wrote:

> It would be beneficial to bring over the Loom-specific test groups from the 
> loom repo to aid development/porting work.
> 
> https://github.com/openjdk/loom/blob/fibers/test/jdk/TEST.groups#L97-L108
> https://github.com/openjdk/loom/blob/6f42923b3342e41d95b262733205283068802b40/test/hotspot/jtreg/TEST.groups#L111-L125
> 
> I had to drop `jdk/incubator/concurrent` from JDK definition, because there 
> are no tests in that folder and tests break.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug `hotspot_loom jdk_loom`
>  - [x] Linux AArch64 fastdebug `hotspot_loom jdk_loom`

Anyone?

-

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

Re: RFR: 8286956: Loom: Define test groups for development/porting use

[Aleksey Shipilev]

On Wed, 18 May 2022 11:10:58 GMT, Aleksey Shipilev  wrote:

> It would be beneficial to bring over the Loom-specific test groups from the 
> loom repo to aid development/porting work.
> 
> https://github.com/openjdk/loom/blob/fibers/test/jdk/TEST.groups#L97-L108
> https://github.com/openjdk/loom/blob/6f42923b3342e41d95b262733205283068802b40/test/hotspot/jtreg/TEST.groups#L111-L125
> 
> I had to drop `jdk/incubator/concurrent` from JDK definition, because there 
> are no tests in that folder and tests break.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug `hotspot_loom jdk_loom`
>  - [x] Linux AArch64 fastdebug `hotspot_loom jdk_loom`

Thanks! Any other comments? I'd like Loom porters to have it sooner.

-

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

RFR: 8286956: Loom: Define test groups for development/porting use

[Aleksey Shipilev]

It would be beneficial to bring over the Loom-specific test groups from the 
loom repo to aid development/porting work.

https://github.com/openjdk/loom/blob/fibers/test/jdk/TEST.groups#L97-L108
https://github.com/openjdk/loom/blob/6f42923b3342e41d95b262733205283068802b40/test/hotspot/jtreg/TEST.groups#L111-L125

I had to drop `jdk/incubator/concurrent` from JDK definition, because there are 
no tests in that folder and tests break.

Additional testing:
 - [x] Linux x86_64 fastdebug `hotspot_loom jdk_loom`
 - [ ] Linux AArch64 fastdebug `hotspot_loom jdk_loom`

-

Commit messages:
 - Fix

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

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

Re: RFR: 8286473: Drop --enable-preview from Record related tests [v2]

[Aleksey Shipilev]

On Tue, 10 May 2022 14:54:39 GMT, Aleksey Shipilev  wrote:

>> There are plenty of tests failing on many architectures due to 
>> `--enable-preview` VM code introduced by Loom. This improvements eliminates 
>> some of the redundant `--enable-preview` clauses from the Record tests, 
>> since Records have been graduated from preview in JDK 16. 
>> 
>> Additional testing:
>>  - [x] Linux x86_64 fastdebug, affected tests still pass
>>  - [x]  Linux x86_32 fastdebug, affected tests start to pass
>
> Aleksey Shipilev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Review comments

Thanks for reviews. I see GHA are mostly clean, with the usual x86_32 failures.

-

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

Integrated: 8286473: Drop --enable-preview from Record related tests

[Aleksey Shipilev]

On Tue, 10 May 2022 12:03:09 GMT, Aleksey Shipilev  wrote:

> There are plenty of tests failing on many architectures due to 
> `--enable-preview` VM code introduced by Loom. This improvements eliminates 
> some of the redundant `--enable-preview` clauses from the Record tests, since 
> Records have been graduated from preview in JDK 16. 
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug, affected tests still pass
>  - [x]  Linux x86_32 fastdebug, affected tests start to pass

This pull request has now been integrated.

Changeset: 73c5e993
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk/commit/73c5e993e17f7435553edae79a1e8d70ece5493d
Stats: 30 lines in 3 files changed: 0 ins; 24 del; 6 mod

8286473: Drop --enable-preview from Record related tests

Reviewed-by: alanb, jpai, mchung

-

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

Re: RFR: 8286474: Drop --enable-preview from Sealed Classes related tests [v2]

[Aleksey Shipilev]

On Tue, 10 May 2022 14:58:15 GMT, Aleksey Shipilev  wrote:

>> There are plenty of tests failing on many architectures due to 
>> `--enable-preview` VM code introduced by Loom. This improvement eliminates 
>> some of the redundant `--enable-preview` clauses from the Sealed Classes 
>> tests, since Sealed Classes have been graduated from preview in JDK 17.
>> 
>> Additional testing:
>>  - [x] Linux x86_64 fastdebug, affected tests still pass
>>  - [x] Linux x86_32 fastdebug, affected tests start to pass
>
> Aleksey Shipilev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Review comments

Thanks!

-

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

Integrated: 8286474: Drop --enable-preview from Sealed Classes related tests

[Aleksey Shipilev]

On Tue, 10 May 2022 12:07:31 GMT, Aleksey Shipilev  wrote:

> There are plenty of tests failing on many architectures due to 
> `--enable-preview` VM code introduced by Loom. This improvement eliminates 
> some of the redundant `--enable-preview` clauses from the Sealed Classes 
> tests, since Sealed Classes have been graduated from preview in JDK 17.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug, affected tests still pass
>  - [x] Linux x86_32 fastdebug, affected tests start to pass

This pull request has now been integrated.

Changeset: d547a707
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk/commit/d547a707bf1f9e252213fdab7eaf076b5cf884b4
Stats: 6 lines in 2 files changed: 0 ins; 0 del; 6 mod

8286474: Drop --enable-preview from Sealed Classes related tests

Reviewed-by: alanb, jpai, mchung, lancea

-

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

Re: RFR: 8286474: Drop --enable-preview from Sealed Classes related tests [v2]

[Aleksey Shipilev]

On Tue, 10 May 2022 12:47:08 GMT, Alan Bateman  wrote:

>> Aleksey Shipilev has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Review comments
>
> test/jdk/java/lang/reflect/sealed_classes/SealedClassesReflectionTest.java 
> line 29:
> 
>> 27:  * @summary reflection test for sealed classes
>> 28:  * @compile -source ${jdk.version} SealedClassesReflectionTest.java
>> 29:  * @run testng/othervm SealedClassesReflectionTest
> 
> You should be able to drop` -source ${jdk.version}` too. It was required when 
> compiling with `--enable-preview`.

Fixed.

-

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

Re: RFR: 8286474: Drop --enable-preview from Sealed Classes related tests [v2]

[Aleksey Shipilev]

> There are plenty of tests failing on many architectures due to 
> `--enable-preview` VM code introduced by Loom. This improvement eliminates 
> some of the redundant `--enable-preview` clauses from the Sealed Classes 
> tests, since Sealed Classes have been graduated from preview in JDK 17.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug, affected tests still pass
>  - [x] Linux x86_32 fastdebug, affected tests start to pass

Aleksey Shipilev has updated the pull request incrementally with one additional 
commit since the last revision:

  Review comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8627/files
  - new: https://git.openjdk.java.net/jdk/pull/8627/files/9b1a55a3..1b74acf4

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8627&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8627&range=00-01

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

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

Re: RFR: 8286473: Drop --enable-preview from Record related tests [v2]

[Aleksey Shipilev]

On Tue, 10 May 2022 14:43:06 GMT, Alan Bateman  wrote:

>> Aleksey Shipilev has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Review comments
>
> test/jdk/java/nio/Buffer/BulkPutBuffer.java line 57:
> 
>> 55:  * @run testng/othervm BulkPutBuffer
>> 56:  */
>> 57: public class BulkPutBuffer {
> 
> You can drop the -source option from these tests too.

Done.

-

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

Re: RFR: 8286473: Drop --enable-preview from Record related tests [v2]

[Aleksey Shipilev]

> There are plenty of tests failing on many architectures due to 
> `--enable-preview` VM code introduced by Loom. This improvements eliminates 
> some of the redundant `--enable-preview` clauses from the Record tests, since 
> Records have been graduated from preview in JDK 16. 
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug, affected tests still pass
>  - [x]  Linux x86_32 fastdebug, affected tests start to pass

Aleksey Shipilev has updated the pull request incrementally with one additional 
commit since the last revision:

  Review comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8626/files
  - new: https://git.openjdk.java.net/jdk/pull/8626/files/d5f0b1be..626e673a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8626&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8626&range=00-01

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

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

RFR: 8286474: Drop --enable-preview from Sealed Classes related tests

[Aleksey Shipilev]

There are plenty of tests failing on many architectures due to 
`--enable-preview` VM code introduced by Loom. This improvement eliminates some 
of the redundant `--enable-preview` clauses from the Sealed Classes tests, 
since Sealed Classes have been graduated from preview in JDK 17.

Additional testing:
 - [x] Linux x86_64 fastdebug, affected tests still pass
 - [x] Linux x86_32 fastdebug, affected tests start to pass

-

Commit messages:
 - Some

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

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

RFR: 8286473: Drop --enable-preview from Record related tests

[Aleksey Shipilev]

There are plenty of tests failing on many architectures due to 
`--enable-preview` VM code introduced by Loom. This improvements eliminates 
some of the redundant `--enable-preview` clauses from the Record tests, since 
Records have been graduated from preview in JDK 16. 

Additional testing:
 - [x] Linux x86_64 fastdebug, affected tests still pass
 - [x]  Linux x86_32 fastdebug, affected tests start to pass

-

Commit messages:
 - Fix

Changes: https://git.openjdk.java.net/jdk/pull/8626/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8626&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8286473
  Stats: 28 lines in 3 files changed: 0 ins; 24 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8626.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8626/head:pull/8626

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

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

[Aleksey Shipilev]

On Thu, 5 May 2022 18:54:49 GMT, Alan Bateman  wrote:

> We mailed porters-dev in Sep 2021 to give a heads up that this feature would 
> require porting work so it shouldn't be a surprise. We have been open to 
> including other ports with the initial integration but it was never a goal to 
> have all the ports done in advance of that.

It is understandable to ship the preview feature not implemented on all 
platforms. It is even understandable to ship the final product feature that 
breaks some platforms in an clearly understandable way, prompting platform 
maintainers to implement the agreed-upon, final Java feature. What I am seeing, 
though, is that just running `java -version` (!) after integrating a *preview 
feature* is broken!

> Most of the new code in the VM is only used when running with 
> --enable-preview. You'll see several places that test 
> Continuations::enabled(). It should be possible to get these port running 
> without -enable-preview without much effort. The feature has to be 
> implemented to pass all the tests of course.

It is not as clear-cut, unfortunately. I see there are substantial changes in 
deopt machinery with post-call NOPs -- and there maybe more stuff lurking after 
that one is implemented -- so it is not as simple as changing `Unimplemented()` 
to guarding with `Continuations::enabled()`. So this looks to me that the core 
deopt machinery is affected, whether Loom is enabled or not. Is the impact of 
that deopt machinery change on the VM stability and VM ports discussed, 
understood, documented somewhere?

I would have honestly expected those core changes to be heavily conditionalized 
with either `ifdef`-s, or runtime checks, or both, so that both unimplemented 
platforms had the old behavior *and* the implemented platforms had a fallback 
to old behavior if preview feature was broken. The current code is fine for 
experimental Loom repo, but I firmly believe mainline should have much stronger 
safety/reliability requirements.

My fear is that an integration like this would wreck JDK 19 release. So my 
question stands: Are we breaking those platforms? Are we sure the unconditional 
VM changes are problem-free, implementable everywhere, etc? The answer might be 
"Yes, we are integrating, let the chips fall where they may", but I also 
believe that should be the call made by responsible platform/VM architects, who 
should explicitly weigh in and accept the risk of wide JDK 19 breakage.

-

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

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

[Aleksey Shipilev]

On Thu, 5 May 2022 09:35:42 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview).
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling _Unimplemented_) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. We 
>> can add additional labels, if required, as the PR progresses.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Extent Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 19 commits:
> 
>  - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f
>  - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5
>  - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05
>  - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89
>  - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a
>  - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4
>  - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec
>  - Merge with jdk-19+20
>  - Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>  - Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69
>  - ... and 9 more: 
> https://git.openjdk.java.net/jdk/compare/1bb4de2e...86bea8ec

I am sorry to be a buzzkill here, but this integration would break lots of 
platforms even when Loom functionality is not enabled/used. For example, 
running `java -version` on RISC-V runs into many issues: 
`TemplateInterpreterGenerator::generate_Continuation_doYield_entry` runs into 
`Unimplemented()`, `UnsafeCopyMemory` asserts about UCM table size, 
`NativeDeoptInstruction::is_deopt` would run into `Unimplemented()` while being 
called from signal handler.

This effectively blocks development on those platforms, and it seems to be too 
much breakage for a preview feature. Are we really breaking these platforms? Do 
we have plans in place with maintainers of those platforms to fix all this in 
JDK 19 timeframe? I'd suggest holding off the integration until such a plan and 
agreement is in place.

-

Changes requested by shade (Reviewer).

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

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

[Aleksey Shipilev]

On Wed, 4 May 2022 12:12:48 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview).
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling _Unimplemented_) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. We 
>> can add additional labels, if required, as the PR progresses.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Extent Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 17 commits:
> 
>  - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05
>  - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89
>  - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a
>  - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4
>  - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec
>  - Merge with jdk-19+20
>  - Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>  - Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69
>  - Refresh cf561073f48fad58e931a5285b92629aa83c9bd1
>  - Merge with jdk-19+19
>  - ... and 7 more: 
> https://git.openjdk.java.net/jdk/compare/4b2c8220...f06aff75

> The patch has support for x64 and aarch64 on the usual operating systems 
> (Linux, macOS, and Windows). There are stubs (calling _Unimplemented_) for 
> zero and some of the other ports. Additional ports can be contributed via PRs 
> against the fibers branch in the loom repo.

So, does this PR pass current GHA checks? I see they are not enabled for this 
PR. It would be unfortunate for this large integration to break builds/tests 
for smaller PRs that would follow it.

-

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

Integrated: 8283994: Make Xerces DatatypeException stackless

[Aleksey Shipilev]

On Wed, 30 Mar 2022 11:38:59 GMT, Aleksey Shipilev  wrote:

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

This pull request has now been integrated.

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

8283994: Make Xerces DatatypeException stackless

Reviewed-by: joehw, jpai

-

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

Re: RFR: 8283994: Make Xerces DatatypeException stackless [v2]

[Aleksey Shipilev]

On Wed, 6 Apr 2022 07:48:04 GMT, Aleksey Shipilev  wrote:

>> Any other reviews? I would like someone else to confirm my investigation 
>> that we don't use the stack traces out of these exceptions too...
>
>> Hello @shipilev, do you think this stackless nature of this specific 
>> `DatatypeException` type should be noted in its javadoc, just to avoid any 
>> surprises when someone in future ends up using this exception type as the 
>> "cause" of some other exception?
> 
> I don't think so. It seems to me the intent for these exceptions is to carry 
> error information without any stack trace info.

> @shipilev
> 
> > Any other reviews? I would like someone else to confirm my investigation 
> > that we don't use the stack traces out of these exceptions too...
> 
> I opened [XERCESJ‑1742](https://issues.apache.org/jira/browse/XERCESJ-1742) 
> to get this change upstreamed into **Xerces**, the resolution of which should 
> also answer your question about whether the stack traces are really unused.

Thanks! But I see there seem to be no upstream interest yet. So I am planning 
to integrate this to JDK first, and so asking for JDK-specific reviews again.

-

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

Re: RFR: 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method

[Aleksey Shipilev]

On Wed, 6 Apr 2022 09:42:21 GMT, Claes Redestad  wrote:

> Do I need a second reviewer for the hotspot changes?

FWIW, I think hotspot changes are quite simple, so maybe no need for second 
reviewer?

-

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

Re: RFR: 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method

[Aleksey Shipilev]

On Mon, 4 Apr 2022 09:58:35 GMT, Claes Redestad  wrote:

> As an alternative to #7667 I took a look at injecting an empty class array 
> from the VM. Turns out we already do this for exception types - see 
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/oops/method.cpp#L918
>  - and we can do similarly for the parameter types array. We still need to 
> parse the signature for the return type, though.
> 
> I've verified by dumping and inspecting heaps that this means we are not 
> allocating extra `Class[]` on `Method` reflection.

Looks fine. I would have thought to put `assert(parameter_count > 0)` near 
`mirrors->obj_at_put(index++, mirror);`, but I think it is fine as it is, since 
`obj_at_put` does its own bounds checking.

-

Marked as reviewed by shade (Reviewer).

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

Re: RFR: 8283994: Make Xerces DatatypeException stackless [v2]

[Aleksey Shipilev]

On Fri, 1 Apr 2022 08:10:48 GMT, Aleksey Shipilev  wrote:

>> Aleksey Shipilev has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Also update LastModified
>
> Any other reviews? I would like someone else to confirm my investigation that 
> we don't use the stack traces out of these exceptions too...

> Hello @shipilev, do you think this stackless nature of this specific 
> `DatatypeException` type should be noted in its javadoc, just to avoid any 
> surprises when someone in future ends up using this exception type as the 
> "cause" of some other exception?

I don't think so. It seems to me the intent for these exceptions is to carry 
error information without any stack trace info.

-

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

RFR: 8283994: Make Xerces DatatypeException stackless

[Aleksey Shipilev]

See bug report for more details. This change improves 
SPECjvm2008:xml.validation for about +3%:


 baseline: 298.353 ± 1.008  ops/min
 patched:  309.912 ± 1.347  ops/min

Of course, the real improvements might be even higher, as exception might be 
thrown from much deeper call hierarchy.

Additional testing:
 - [x] Linux x86_64 fastdebug, `jaxp_all`
 - [x] Linux x86_64 fastdebug, `javax/xml`

-

Commit messages:
 - Fix

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

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

Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v4]

[Aleksey Shipilev]

On Wed, 23 Mar 2022 02:03:26 GMT, Fei Yang  wrote:

>> This PR implements JEP 422: Linux/RISC-V Port [1].
>> The PR starts as a squashed merge of the 
>> https://openjdk.java.net/projects/riscv-port branch.
>> 
>> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive 
>> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are 
>> also carried out regularly. So it should be good enough to run most Java 
>> programs.
>> 
>> [1] https://openjdk.java.net/jeps/422
>
> Fei Yang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix copyright header

Looks okay to me.

-

Marked as reviewed by shade (Reviewer).

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

Re: RFR: 8282897: Fix call parameter to GetStringChars() in HostLocaleProviderAdapter_md.c

[Aleksey Shipilev]

On Thu, 10 Mar 2022 13:33:05 GMT, Zhengyu Gu  wrote:

> Please review this trivial patch to correct last parameter of 
> `GetStringChars()` call, which should be a  `jboolean*`, instead of `jboolean`

Looks fine to me.

-

Marked as reviewed by shade (Reviewer).

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

Integrated: 8281168: Micro-optimize VarForm.getMemberName for interpreter

[Aleksey Shipilev]

On Thu, 3 Feb 2022 07:20:28 GMT, Aleksey Shipilev  wrote:

> I was looking for easy things to do to improve `java.lang.invoke` cold 
> performance. One of the things is inlining `VarForm.getMemberName` a bit, so 
> that interpreter does not have to call through `getMemberNameOrNull`.
> 
> There is direct VarHandle benchmark in our corpus:
> 
> 
> $ CONF=linux-x86_64-server-release make run-test 
> TEST=micro:java.lang.invoke.VarHandleExact 
> MICRO="TIME=200ms;WARMUP_TIME=200ms;VM_OPTIONS=-Xint"
> 
> Benchmark Mode  Cnt ScoreError  Units
> 
> # -Xint
> # Baseline
> VarHandleExact.exact_exactInvocation  avgt   30   714.041 ±  5.882  ns/op
> VarHandleExact.generic_exactInvocationavgt   30   641.570 ± 11.681  ns/op
> VarHandleExact.generic_genericInvocation  avgt   30  1336.571 ± 11.873  ns/op
> 
> # -Xint
> # Patched
> VarHandleExact.exact_exactInvocation  avgt   30   678.495 ± 10.752  ns/op 
> ; +5%
> VarHandleExact.generic_exactInvocationavgt   30   573.320 ±  5.100  ns/op 
> ; +11%
> VarHandleExact.generic_genericInvocation  avgt   30  1338.593 ± 14.275  ns/op 
> 
> # (server, default)
> # Baseline
> VarHandleExact.exact_exactInvocation  avgt   30   0.620 ± 0.079  ns/op
> VarHandleExact.generic_exactInvocationavgt   30   0.602 ± 0.063  ns/op
> VarHandleExact.generic_genericInvocation  avgt   30  10.521 ± 0.065  ns/op
> 
> # (server, default)
> # Patched
> VarHandleExact.exact_exactInvocation  avgt   30   0.621 ± 0.070  ns/op
> VarHandleExact.generic_exactInvocationavgt   30   0.601 ± 0.061  ns/op
> VarHandleExact.generic_genericInvocation  avgt   30  10.499 ± 0.070  ns/op
> 
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug `tier1`
>  - [x] Linux x86_64 fastdebug `tier2`
>  - [x] Linux x86_64 fastdebug `tier3`

This pull request has now been integrated.

Changeset: fc772178
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk/commit/fc77217814eb1a346d7380299abdc2b01a69b4de
Stats: 7 lines in 1 file changed: 5 ins; 0 del; 2 mod

8281168: Micro-optimize VarForm.getMemberName for interpreter

Reviewed-by: redestad, vlivanov, mchung

-

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

Re: RFR: 8281168: Micro-optimize VarForm.getMemberName for interpreter [v2]

[Aleksey Shipilev]

On Tue, 8 Feb 2022 17:39:37 GMT, Mandy Chung  wrote:

> This change looks okay. One biggest cold startup overhead we measured for JEP 
> 416 is due to the overhead of spinning and loading classes of MH/VH. This 
> micro-optimization focuses on the performance of VH invocation. Do you see 
> class spinning and loading from your benchmark? Maybe in the generic 
> invocation case?

Not really, I am trying to see that `-Xint` does not have easy to fix 
impediments to cold performance. In this benchmark, bootstrapping stuff had 
already happened at warmup, it measures the overheads of doing all the MH/VH 
without the optimizing compiler assistance.

-

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

Re: RFR: 8281168: Micro-optimize VarForm.getMemberName for interpreter [v2]

[Aleksey Shipilev]

On Tue, 8 Feb 2022 14:16:15 GMT, Vladimir Ivanov  wrote:

>> Aleksey Shipilev 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:
>> 
>>  - Comment
>>  - Merge branch 'master' into JDK-8281168-int-varform
>>  - Fix
>
> src/java.base/share/classes/java/lang/invoke/VarForm.java line 112:
> 
>> 110: @ForceInline
>> 111: final MemberName getMemberName(int mode) {
>> 112: MemberName mn = memberName_table[mode];
> 
> It makes sense to leave a comment here why inlining `getMemberNameOrNull()` 
> makes sense.  Otherwise, looks good.

Added comment.

-

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

Re: RFR: 8281168: Micro-optimize VarForm.getMemberName for interpreter [v2]

[Aleksey Shipilev]

> I was looking for easy things to do to improve `java.lang.invoke` cold 
> performance. One of the things is inlining `VarForm.getMemberName` a bit, so 
> that interpreter does not have to call through `getMemberNameOrNull`.
> 
> There is direct VarHandle benchmark in our corpus:
> 
> 
> $ CONF=linux-x86_64-server-release make run-test 
> TEST=micro:java.lang.invoke.VarHandleExact 
> MICRO="TIME=200ms;WARMUP_TIME=200ms;VM_OPTIONS=-Xint"
> 
> Benchmark Mode  Cnt ScoreError  Units
> 
> # -Xint
> # Baseline
> VarHandleExact.exact_exactInvocation  avgt   30   714.041 ±  5.882  ns/op
> VarHandleExact.generic_exactInvocationavgt   30   641.570 ± 11.681  ns/op
> VarHandleExact.generic_genericInvocation  avgt   30  1336.571 ± 11.873  ns/op
> 
> # -Xint
> # Patched
> VarHandleExact.exact_exactInvocation  avgt   30   678.495 ± 10.752  ns/op 
> ; +5%
> VarHandleExact.generic_exactInvocationavgt   30   573.320 ±  5.100  ns/op 
> ; +11%
> VarHandleExact.generic_genericInvocation  avgt   30  1338.593 ± 14.275  ns/op 
> 
> # (server, default)
> # Baseline
> VarHandleExact.exact_exactInvocation  avgt   30   0.620 ± 0.079  ns/op
> VarHandleExact.generic_exactInvocationavgt   30   0.602 ± 0.063  ns/op
> VarHandleExact.generic_genericInvocation  avgt   30  10.521 ± 0.065  ns/op
> 
> # (server, default)
> # Patched
> VarHandleExact.exact_exactInvocation  avgt   30   0.621 ± 0.070  ns/op
> VarHandleExact.generic_exactInvocationavgt   30   0.601 ± 0.061  ns/op
> VarHandleExact.generic_genericInvocation  avgt   30  10.499 ± 0.070  ns/op
> 
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug `tier1`
>  - [x] Linux x86_64 fastdebug `tier2`
>  - [x] Linux x86_64 fastdebug `tier3`

Aleksey Shipilev 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:

 - Comment
 - Merge branch 'master' into JDK-8281168-int-varform
 - Fix

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7333/files
  - new: https://git.openjdk.java.net/jdk/pull/7333/files/62028379..a84a4e7e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7333&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7333&range=00-01

  Stats: 6290 lines in 289 files changed: 3892 ins; 1334 del; 1064 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7333.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7333/head:pull/7333

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

Re: RFR: 8281168: Micro-optimize VarForm.getMemberName for interpreter

[Aleksey Shipilev]

On Mon, 7 Feb 2022 13:34:24 GMT, Claes Redestad  wrote:

> Looks reasonable.

Thanks. Any more reviews needed, or I am pushing?

-

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

RFR: 8281168: Micro-optimize VarForm.getMemberName for interpreter

[Aleksey Shipilev]

I was looking for easy things to do to improve `java.lang.invoke` cold 
performance. One of the things is inlining `VarForm.getMemberName` a bit, so 
that interpreter does not have to call through `getMemberNameOrNull`.

There is direct VarHandle benchmark in our corpus:


$ CONF=linux-x86_64-server-release make run-test 
TEST=micro:java.lang.invoke.VarHandleExact 
MICRO="TIME=200ms;WARMUP_TIME=200ms;VM_OPTIONS=-Xint"

Benchmark Mode  Cnt ScoreError  Units

# -Xint
# Baseline
VarHandleExact.exact_exactInvocation  avgt   30   714.041 ±  5.882  ns/op
VarHandleExact.generic_exactInvocationavgt   30   641.570 ± 11.681  ns/op
VarHandleExact.generic_genericInvocation  avgt   30  1336.571 ± 11.873  ns/op

# -Xint
# Patched
VarHandleExact.exact_exactInvocation  avgt   30   678.495 ± 10.752  ns/op ; 
+5%
VarHandleExact.generic_exactInvocationavgt   30   573.320 ±  5.100  ns/op ; 
+11%
VarHandleExact.generic_genericInvocation  avgt   30  1338.593 ± 14.275  ns/op 

# (server, default)
# Baseline
VarHandleExact.exact_exactInvocation  avgt   30   0.620 ± 0.079  ns/op
VarHandleExact.generic_exactInvocationavgt   30   0.602 ± 0.063  ns/op
VarHandleExact.generic_genericInvocation  avgt   30  10.521 ± 0.065  ns/op

# (server, default)
# Patched
VarHandleExact.exact_exactInvocation  avgt   30   0.621 ± 0.070  ns/op
VarHandleExact.generic_exactInvocationavgt   30   0.601 ± 0.061  ns/op
VarHandleExact.generic_genericInvocation  avgt   30  10.499 ± 0.070  ns/op


Additional testing:
 - [x] Linux x86_64 fastdebug `tier1`
 - [x] Linux x86_64 fastdebug `tier2`
 - [x] Linux x86_64 fastdebug `tier3`

-

Commit messages:
 - Fix

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

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

Integrated: 8280889: java/lang/instrument/GetObjectSizeIntrinsicsTest.java fails with -XX:-UseCompressedOops

[Aleksey Shipilev]

On Fri, 28 Jan 2022 15:37:59 GMT, Aleksey Shipilev  wrote:

> Recent test regression after adding new cases in the test. Without compressed 
> oops, ~1G elements `Object[]` array takes >8G of memory, which fails the 
> test. The fix cuts it down to 512M when reference size is 8 bytes. 
> Additionally, `ObjectAlignmentInBytes=32` is excessive for new test configs.
> 
> Additional testing: 
>  - [x] Linux x86_64 fastdebug, default, affected test still passes
>  - [x] Linux x86_32 fastdebug, default, affected test still passes
>  - [x] Linux x86_64 fastdebug, `-XX:-UseCompressedOops`, affected test now 
> passes

This pull request has now been integrated.

Changeset: 251351f4
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk/commit/251351f49498ea39150b38860b8f73232fbaf05d
Stats: 10 lines in 1 file changed: 1 ins; 3 del; 6 mod

8280889: java/lang/instrument/GetObjectSizeIntrinsicsTest.java fails with 
-XX:-UseCompressedOops

Reviewed-by: sspitsyn, dcubed

-

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

Re: RFR: 8280889: java/lang/instrument/GetObjectSizeIntrinsicsTest.java fails with -XX:-UseCompressedOops

[Aleksey Shipilev]

On Fri, 28 Jan 2022 15:37:59 GMT, Aleksey Shipilev  wrote:

> Recent test regression after adding new cases in the test. Without compressed 
> oops, ~1G elements `Object[]` array takes >8G of memory, which fails the 
> test. The fix cuts it down to 512M when reference size is 8 bytes. 
> Additionally, `ObjectAlignmentInBytes=32` is excessive for new test configs.
> 
> Additional testing: 
>  - [x] Linux x86_64 fastdebug, default, affected test still passes
>  - [x] Linux x86_32 fastdebug, default, affected test still passes
>  - [x] Linux x86_64 fastdebug, `-XX:-UseCompressedOops`, affected test now 
> passes

Thanks!

-

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

RFR: 8280889: java/lang/instrument/GetObjectSizeIntrinsicsTest.java fails with -XX:-UseCompressedOops

[Aleksey Shipilev]

Recent test regression after adding new cases in the test. Without compressed 
oops, ~1G elements `Object[]` array takes >8G of memory, which fails the test. 
The fix cuts it down to 512M when reference size is 8 bytes. Additionally, 
`ObjectAlignmentInBytes=32` is excessive for new test configs.

Additional testing: 
 - [x] Linux x86_64 fastdebug, default, affected test still passes
 - [x] Linux x86_32 fastdebug, default, affected test still passes
 - [x] Linux x86_64 fastdebug, `-XX:-UseCompressedOops`, affected test now 
passes

-

Commit messages:
 - Fix

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

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

Integrated: 8280166: Extend java/lang/instrument/GetObjectSizeIntrinsicsTest.java test cases

[Aleksey Shipilev]

On Tue, 18 Jan 2022 18:33:29 GMT, Aleksey Shipilev  wrote:

> While working on JDK-8280003, I noticed that 
> java/lang/instrument/GetObjectSizeIntrinsicsTest.java does not test arrays 
> with more than 1-byte size elements, and no large arrays (past 4G limit) are 
> tested either. It would be better to add those test cases. 
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug, affected test still passes
>  - [x] Linux x86_32 fastdebug, affected test still passes
>  - [x] Linux AArch64 fastdebug, affected test still passes
>  - [x] Linux PPC64 fastdebug, affected test still passes

This pull request has now been integrated.

Changeset: 76fe03fe
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk/commit/76fe03fe01a7c824e2e9263de95b8bcbb4b9d752
Stats: 89 lines in 1 file changed: 62 ins; 0 del; 27 mod

8280166: Extend java/lang/instrument/GetObjectSizeIntrinsicsTest.java test cases

Reviewed-by: sspitsyn, lmesnik

-

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

Integrated: 8280041: Retry loop issues in java.io.ClassCache

[Aleksey Shipilev]

On Fri, 14 Jan 2022 19:27:18 GMT, Aleksey Shipilev  wrote:

> JDK-8277072 introduced java.io.ClassCache, but there seem to be at least two 
> issues with it:
>   - The cache cannot disambiguate between cleared SoftReference and the 
> accidental passing of `null` value; in that case, the retry loop would spin 
> indefinitely;
>   - If retry loop would spin several times, every time discovering a cleared 
> SoftReference, it would create and register new SoftReference on the 
> ReferenceQueue. However, it would not *drain* the RQ in the loop; in that 
> case, we might have the unbounded garbage accumulating in RQ; 
> 
> Attention @rkennke, @plevart.
> 
> Additional testing:
>   - [x] Linux x86_64 fastdebug `java/io/ObjectStreamClass`
>   - [x] Linux x86_64 fastdebug `tier1`
>   - [x] Linux x86_64 fastdebug `tier2`
>   - [x] Linux x86_64 fastdebug `tier3`

This pull request has now been integrated.

Changeset: cebaad1c
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk/commit/cebaad1c94c301304fd146526cac95bfeaac66bf
Stats: 202 lines in 5 files changed: 190 ins; 0 del; 12 mod

8280041: Retry loop issues in java.io.ClassCache

Co-authored-by: Peter Levart 
Reviewed-by: rkennke, rriggs, plevart

-

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

Re: RFR: 8280041: Retry loop issues in java.io.ClassCache [v5]

[Aleksey Shipilev]

On Tue, 25 Jan 2022 16:33:10 GMT, Aleksey Shipilev  wrote:

>> JDK-8277072 introduced java.io.ClassCache, but there seem to be at least two 
>> issues with it:
>>   - The cache cannot disambiguate between cleared SoftReference and the 
>> accidental passing of `null` value; in that case, the retry loop would spin 
>> indefinitely;
>>   - If retry loop would spin several times, every time discovering a cleared 
>> SoftReference, it would create and register new SoftReference on the 
>> ReferenceQueue. However, it would not *drain* the RQ in the loop; in that 
>> case, we might have the unbounded garbage accumulating in RQ; 
>> 
>> Attention @rkennke, @plevart.
>> 
>> Additional testing:
>>   - [x] Linux x86_64 fastdebug `java/io/ObjectStreamClass`
>>   - [x] Linux x86_64 fastdebug `tier1`
>>   - [x] Linux x86_64 fastdebug `tier2`
>>   - [x] Linux x86_64 fastdebug `tier3`
>
> Aleksey Shipilev 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 ten additional 
> commits since the last revision:
> 
>  - Drop unnecessary null check in clearStrong
>  - Merge branch 'master' into JDK-8280041-classcache-problems
>  - Guarantee progress for at least one thread
>  - Merge branch 'master' into JDK-8280041-classcache-problems
>  - Test summary
>  - GC sanity test
>  - NullValueTest
>  - Merge branch 'master' into JDK-8280041-classcache-problems
>  - Fix

All right, we seem to be good to go.

-

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

Re: RFR: 8280041: Retry loop issues in java.io.ClassCache [v4]

[Aleksey Shipilev]

On Tue, 25 Jan 2022 16:12:11 GMT, Aleksey Shipilev  wrote:

> > This looks good, although I don't know whether the additional check for 
> > strongReferent != null is needed in clearStrong(). This is all racy code 
> > and you have already got a non-null return from getStrong() in case you are 
> > calling clearStrong()...
> 
> This seems like a standard thing to do to avoid multi-threaded write 
> contention under the race. By the time one thread calls `clearStrong`, some 
> other thread might have already cleared it, and we don't have to clear again. 
> Granted, that is a micro-optimization, but I'd like to avoid surprises on 
> this path :)

Actually, the window after the first `strongVal != null` check is not that 
large that a second null check would matter. Dropped the null check in 
`clearStrong` for simplicity. (IIRC, it mattered a bit in prior internal 
iterations of my patch).

-

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

Re: RFR: 8280041: Retry loop issues in java.io.ClassCache [v5]

[Aleksey Shipilev]

> JDK-8277072 introduced java.io.ClassCache, but there seem to be at least two 
> issues with it:
>   - The cache cannot disambiguate between cleared SoftReference and the 
> accidental passing of `null` value; in that case, the retry loop would spin 
> indefinitely;
>   - If retry loop would spin several times, every time discovering a cleared 
> SoftReference, it would create and register new SoftReference on the 
> ReferenceQueue. However, it would not *drain* the RQ in the loop; in that 
> case, we might have the unbounded garbage accumulating in RQ; 
> 
> Attention @rkennke, @plevart.
> 
> Additional testing:
>   - [x] Linux x86_64 fastdebug `java/io/ObjectStreamClass`
>   - [x] Linux x86_64 fastdebug `tier1`
>   - [x] Linux x86_64 fastdebug `tier2`
>   - [x] Linux x86_64 fastdebug `tier3`

Aleksey Shipilev 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 ten additional 
commits since the last revision:

 - Drop unnecessary null check in clearStrong
 - Merge branch 'master' into JDK-8280041-classcache-problems
 - Guarantee progress for at least one thread
 - Merge branch 'master' into JDK-8280041-classcache-problems
 - Test summary
 - GC sanity test
 - NullValueTest
 - Merge branch 'master' into JDK-8280041-classcache-problems
 - Fix

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7092/files
  - new: https://git.openjdk.java.net/jdk/pull/7092/files/0f2dcd0d..17073ef2

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7092&range=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7092&range=03-04

  Stats: 330 lines in 51 files changed: 159 ins; 70 del; 101 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7092.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7092/head:pull/7092

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

Re: RFR: 8280166: Extend java/lang/instrument/GetObjectSizeIntrinsicsTest.java test cases [v3]

[Aleksey Shipilev]

On Tue, 18 Jan 2022 19:36:18 GMT, Chris Plummer  wrote:

>> Aleksey Shipilev 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 four additional 
>> commits since the last revision:
>> 
>>  - Update copyright dates
>>  - Merge branch 'master' into JDK-8280166-getobjectsize
>>  - Merge branch 'master' into JDK-8280166-getobjectsize
>>  - Fix
>
> test/jdk/java/lang/instrument/GetObjectSizeIntrinsicsTest.java line 326:
> 
>> 324: 
>> 325: public static void main(String[] args)throws Throwable {
>> 326: new GetObjectSizeIntrinsicsTest(args[0], (args.length >= 2 ? 
>> args[1] : "")).runTest();
> 
> Shouldn't this be `args.length == 2`?

@plummercj This does not look like a review-blocking comment, so I am going to 
proceed with integration, if there are no other comments.

-

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

Re: RFR: 8280041: Retry loop issues in java.io.ClassCache

[Aleksey Shipilev]

On Tue, 18 Jan 2022 20:07:10 GMT, Roger Riggs  wrote:

>> JDK-8277072 introduced java.io.ClassCache, but there seem to be at least two 
>> issues with it:
>>   - The cache cannot disambiguate between cleared SoftReference and the 
>> accidental passing of `null` value; in that case, the retry loop would spin 
>> indefinitely;
>>   - If retry loop would spin several times, every time discovering a cleared 
>> SoftReference, it would create and register new SoftReference on the 
>> ReferenceQueue. However, it would not *drain* the RQ in the loop; in that 
>> case, we might have the unbounded garbage accumulating in RQ; 
>> 
>> Attention @rkennke, @plevart.
>> 
>> Additional testing:
>>   - [x] Linux x86_64 fastdebug `java/io/ObjectStreamClass`
>>   - [x] Linux x86_64 fastdebug `tier1`
>>   - [x] Linux x86_64 fastdebug `tier2`
>>   - [x] Linux x86_64 fastdebug `tier3`
>
> It may not be worth it. If not, add the label "noreg-hard".
> 
> It is possible for add to the test description the modules to be opened:
> 
> * @modules java.base/java.io:open
> 
> jtreg will add the appropriate command line args when it is compiled and run.
> There are various examples in existing test/jdk/java/io... tests.

@RogerRiggs, are you happy with tests?

-

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

Re: RFR: 8280041: Retry loop issues in java.io.ClassCache [v4]

[Aleksey Shipilev]

On Mon, 24 Jan 2022 21:32:16 GMT, Peter Levart  wrote:

> This looks good, although I don't know whether the additional check for 
> strongReferent != null is needed in clearStrong(). This is all racy code and 
> you have already got a non-null return from getStrong() in case you are 
> calling clearStrong()...

This seems like a standard thing to do to avoid multi-threaded write contention 
under the race. By the time one thread calls `clearStrong`, some other thread 
might have already cleared it, and we don't have to clear again. Granted, that 
is a micro-optimization, but I'd like to avoid surprises on this path :)

-

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

Re: RFR: 8280166: Extend java/lang/instrument/GetObjectSizeIntrinsicsTest.java test cases [v2]

[Aleksey Shipilev]

On Fri, 21 Jan 2022 16:19:07 GMT, Leonid Mesnik  wrote:

> Please update copyright years.

Updated, thanks!

-

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

Re: RFR: 8280166: Extend java/lang/instrument/GetObjectSizeIntrinsicsTest.java test cases [v3]

[Aleksey Shipilev]

> While working on JDK-8280003, I noticed that 
> java/lang/instrument/GetObjectSizeIntrinsicsTest.java does not test arrays 
> with more than 1-byte size elements, and no large arrays (past 4G limit) are 
> tested either. It would be better to add those test cases. 
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug, affected test still passes
>  - [x] Linux x86_32 fastdebug, affected test still passes
>  - [x] Linux AArch64 fastdebug, affected test still passes
>  - [x] Linux PPC64 fastdebug, affected test still passes

Aleksey Shipilev 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 four additional 
commits since the last revision:

 - Update copyright dates
 - Merge branch 'master' into JDK-8280166-getobjectsize
 - Merge branch 'master' into JDK-8280166-getobjectsize
 - Fix

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7132/files
  - new: https://git.openjdk.java.net/jdk/pull/7132/files/dd9ce049..98653011

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7132&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7132&range=01-02

  Stats: 327 lines in 51 files changed: 159 ins; 67 del; 101 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7132.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7132/head:pull/7132

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

Re: RFR: 8280459: Suspicious integer division in Hashtable.readHashtable

[Aleksey Shipilev]

On Fri, 21 Jan 2022 15:19:38 GMT, Aleksey Shipilev  wrote:

> Found by Sonar. See details in the bug.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug `tier1`
>  - [x] Linux x86_64 fastdebug `java/util/Hashtable`

Thanks for reviews!

-

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

Integrated: 8280459: Suspicious integer division in Hashtable.readHashtable

[Aleksey Shipilev]

On Fri, 21 Jan 2022 15:19:38 GMT, Aleksey Shipilev  wrote:

> Found by Sonar. See details in the bug.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug `tier1`
>  - [x] Linux x86_64 fastdebug `java/util/Hashtable`

This pull request has now been integrated.

Changeset: d1569111
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk/commit/d1569111d7077dd95b95aea6c42616f85d85e781
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8280459: Suspicious integer division in Hashtable.readHashtable

Reviewed-by: rriggs, bpb

-

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

RFR: 8280459: Suspicious integer division in Hashtable.readHashtable

[Aleksey Shipilev]

Found by Sonar. See details in the bug.

Additional testing:
 - [x] Linux x86_64 fastdebug `tier1`
 - [x] Linux x86_64 fastdebug `java/util/Hashtable`

-

Commit messages:
 - Fix

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

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

Re: RFR: 8280166: Extend java/lang/instrument/GetObjectSizeIntrinsicsTest.java test cases [v2]

[Aleksey Shipilev]

> While working on JDK-8280003, I noticed that 
> java/lang/instrument/GetObjectSizeIntrinsicsTest.java does not test arrays 
> with more than 1-byte size elements, and no large arrays (past 4G limit) are 
> tested either. It would be better to add those test cases. 
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug, affected test still passes
>  - [x] Linux x86_32 fastdebug, affected test still passes
>  - [x] Linux AArch64 fastdebug, affected test still passes
>  - [x] Linux PPC64 fastdebug, affected test still passes

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

 - Merge branch 'master' into JDK-8280166-getobjectsize
 - Fix

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7132/files
  - new: https://git.openjdk.java.net/jdk/pull/7132/files/29e6fd24..dd9ce049

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7132&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7132&range=00-01

  Stats: 7693 lines in 284 files changed: 5179 ins; 1672 del; 842 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7132.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7132/head:pull/7132

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

Re: RFR: 8280041: Retry loop issues in java.io.ClassCache [v3]

[Aleksey Shipilev]

On Wed, 19 Jan 2022 13:10:55 GMT, Peter Levart  wrote:

> WDYT?

I like the idea of holding to a value strongly for a brief period of time, in 
order to guarantee progress! The sample code was a bit hard to follow, so I 
rewrote the loop a bit with comments, see new commit. This still passes tests.

-

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

Re: RFR: 8280041: Retry loop issues in java.io.ClassCache [v4]

[Aleksey Shipilev]

> JDK-8277072 introduced java.io.ClassCache, but there seem to be at least two 
> issues with it:
>   - The cache cannot disambiguate between cleared SoftReference and the 
> accidental passing of `null` value; in that case, the retry loop would spin 
> indefinitely;
>   - If retry loop would spin several times, every time discovering a cleared 
> SoftReference, it would create and register new SoftReference on the 
> ReferenceQueue. However, it would not *drain* the RQ in the loop; in that 
> case, we might have the unbounded garbage accumulating in RQ; 
> 
> Attention @rkennke, @plevart.
> 
> Additional testing:
>   - [x] Linux x86_64 fastdebug `java/io/ObjectStreamClass`
>   - [x] Linux x86_64 fastdebug `tier1`
>   - [x] Linux x86_64 fastdebug `tier2`
>   - [x] Linux x86_64 fastdebug `tier3`

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

 - Guarantee progress for at least one thread
 - Merge branch 'master' into JDK-8280041-classcache-problems
 - Test summary
 - GC sanity test
 - NullValueTest
 - Merge branch 'master' into JDK-8280041-classcache-problems
 - Fix

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7092/files
  - new: https://git.openjdk.java.net/jdk/pull/7092/files/49b852de..0f2dcd0d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7092&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7092&range=02-03

  Stats: 6687 lines in 233 files changed: 4438 ins; 1559 del; 690 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7092.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7092/head:pull/7092

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

Re: RFR: 8280166: Extend java/lang/instrument/GetObjectSizeIntrinsicsTest.java test cases

[Aleksey Shipilev]

On Tue, 18 Jan 2022 19:36:18 GMT, Chris Plummer  wrote:

>> While working on JDK-8280003, I noticed that 
>> java/lang/instrument/GetObjectSizeIntrinsicsTest.java does not test arrays 
>> with more than 1-byte size elements, and no large arrays (past 4G limit) are 
>> tested either. It would be better to add those test cases. 
>> 
>> Additional testing:
>>  - [x] Linux x86_64 fastdebug, affected test still passes
>>  - [x] Linux x86_32 fastdebug, affected test still passes
>>  - [x] Linux AArch64 fastdebug, affected test still passes
>>  - [x] Linux PPC64 fastdebug, affected test still passes
>
> test/jdk/java/lang/instrument/GetObjectSizeIntrinsicsTest.java line 326:
> 
>> 324: 
>> 325: public static void main(String[] args)throws Throwable {
>> 326: new GetObjectSizeIntrinsicsTest(args[0], (args.length >= 2 ? 
>> args[1] : "")).runTest();
> 
> Shouldn't this be `args.length == 2`?

@plummercj, are you good with this explanation?

-

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

Re: RFR: 8280041: Retry loop issues in java.io.ClassCache [v3]

[Aleksey Shipilev]

On Wed, 19 Jan 2022 10:42:02 GMT, Peter Levart  wrote:

> So, this patch is fine for making ClassCache more robust as a re-usable 
> component inside JDK (checking for non-null return of computeValue). The 2nd 
> change, that apparently shields against unbounded accumulation of garbage, 
> might not be enough in the sense that when garbage accumulated without 
> bounds, there would be not progress. So before the patch, we would at least 
> get an OOME and the loop would end. Now the loop could spin indefinitely. So 
> perhaps we must rather prevent this from happening in some other way... I 
> have an idea. Stay tuned.

I think playing the catch-up race with GC (which we will eventually win, IMO) 
is better than obscure OOME. Note this matches other places we do the 
weak-reference loops, for example in `MethodType`: 
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/invoke/MethodType.java#L1390-L1401

-

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

Re: RFR: 8280041: Retry loop issues in java.io.ClassCache [v3]

[Aleksey Shipilev]

> JDK-8277072 introduced java.io.ClassCache, but there seem to be at least two 
> issues with it:
>   - The cache cannot disambiguate between cleared SoftReference and the 
> accidental passing of `null` value; in that case, the retry loop would spin 
> indefinitely;
>   - If retry loop would spin several times, every time discovering a cleared 
> SoftReference, it would create and register new SoftReference on the 
> ReferenceQueue. However, it would not *drain* the RQ in the loop; in that 
> case, we might have the unbounded garbage accumulating in RQ; 
> 
> Attention @rkennke, @plevart.
> 
> Additional testing:
>   - [x] Linux x86_64 fastdebug `java/io/ObjectStreamClass`
>   - [x] Linux x86_64 fastdebug `tier1`
>   - [x] Linux x86_64 fastdebug `tier2`
>   - [x] Linux x86_64 fastdebug `tier3`

Aleksey Shipilev has updated the pull request incrementally with one additional 
commit since the last revision:

  Test summary

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7092/files
  - new: https://git.openjdk.java.net/jdk/pull/7092/files/88d23af1..49b852de

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7092&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7092&range=01-02

  Stats: 47 lines in 2 files changed: 1 ins; 34 del; 12 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7092.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7092/head:pull/7092

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

Re: RFR: 8280041: Retry loop issues in java.io.ClassCache

[Aleksey Shipilev]

On Tue, 18 Jan 2022 20:07:10 GMT, Roger Riggs  wrote:

> It may not be worth it. If not, add the label "noreg-hard".

Added.
 
> It is possible for add to the test description the modules to be opened:
> ```
> * @modules java.base/java.io:open
> ```
> 
> jtreg will add the appropriate command line args when it is compiled and run. 
> There are various examples in existing test/jdk/java/io... tests.

It is not about modules protection, it is about the plain Java rule: the class 
is package-private, you cannot subclass from it, unless you are in the same 
package, period. In older Java days you would "just" put the test in the same 
package, but we cannot do it now, because that would be a split package and 
test code would refuse to load. But there is a way to inject `java.io` classes, 
apparently! I added two tests in two new commits.

-

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

Re: RFR: 8280041: Retry loop issues in java.io.ClassCache [v2]

[Aleksey Shipilev]

> JDK-8277072 introduced java.io.ClassCache, but there seem to be at least two 
> issues with it:
>   - The cache cannot disambiguate between cleared SoftReference and the 
> accidental passing of `null` value; in that case, the retry loop would spin 
> indefinitely;
>   - If retry loop would spin several times, every time discovering a cleared 
> SoftReference, it would create and register new SoftReference on the 
> ReferenceQueue. However, it would not *drain* the RQ in the loop; in that 
> case, we might have the unbounded garbage accumulating in RQ; 
> 
> Attention @rkennke, @plevart.
> 
> Additional testing:
>   - [x] Linux x86_64 fastdebug `java/io/ObjectStreamClass`
>   - [x] Linux x86_64 fastdebug `tier1`
>   - [x] Linux x86_64 fastdebug `tier2`
>   - [x] Linux x86_64 fastdebug `tier3`

Aleksey Shipilev 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 four additional 
commits since the last revision:

 - GC sanity test
 - NullValueTest
 - Merge branch 'master' into JDK-8280041-classcache-problems
 - Fix

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7092/files
  - new: https://git.openjdk.java.net/jdk/pull/7092/files/14835de6..88d23af1

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7092&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7092&range=00-01

  Stats: 1925 lines in 92 files changed: 1467 ins; 230 del; 228 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7092.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7092/head:pull/7092

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

Re: RFR: 8280166: Extend java/lang/instrument/GetObjectSizeIntrinsicsTest.java test cases

[Aleksey Shipilev]

On Tue, 18 Jan 2022 19:36:18 GMT, Chris Plummer  wrote:

>> While working on JDK-8280003, I noticed that 
>> java/lang/instrument/GetObjectSizeIntrinsicsTest.java does not test arrays 
>> with more than 1-byte size elements, and no large arrays (past 4G limit) are 
>> tested either. It would be better to add those test cases. 
>> 
>> Additional testing:
>>  - [x] Linux x86_64 fastdebug, affected test still passes
>>  - [x] Linux x86_32 fastdebug, affected test still passes
>>  - [x] Linux AArch64 fastdebug, affected test still passes
>>  - [x] Linux PPC64 fastdebug, affected test still passes
>
> test/jdk/java/lang/instrument/GetObjectSizeIntrinsicsTest.java line 326:
> 
>> 324: 
>> 325: public static void main(String[] args)throws Throwable {
>> 326: new GetObjectSizeIntrinsicsTest(args[0], (args.length >= 2 ? 
>> args[1] : "")).runTest();
> 
> Shouldn't this be `args.length == 2`?

`>= 2` is more future-proof, I think, as it checks that _at least_ two 
arguments exists, which allows to read `args[1]`. Accidental addition of third 
argument would not silently break the test.

-

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

Re: RFR: 8280041: Retry loop issues in java.io.ClassCache

[Aleksey Shipilev]

On Tue, 18 Jan 2022 18:22:26 GMT, Roger Riggs  wrote:

> Can a test be written to verify the correct handling of nulls.

`java.io.ClassCache` is not public (for a reason), I struggle to find a way to 
access it from a jtreg test...

-

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

RFR: 8280166: Extend java/lang/instrument/GetObjectSizeIntrinsicsTest.java test cases

[Aleksey Shipilev]

While working on JDK-8280003, I noticed that 
java/lang/instrument/GetObjectSizeIntrinsicsTest.java does not test arrays with 
more than 1-byte size elements, and no large arrays (past 4G limit) are tested 
either. It would be better to add those test cases. 

Additional testing:
 - [x] Linux x86_64 fastdebug, affected test still passes
 - [x] Linux x86_32 fastdebug, affected test still passes
 - [ ] Linux AArch64 fastdebug, affected test still passes
 - [ ] Linux PPC64 fastdebug, affected test still passes

-

Commit messages:
 - Fix

Changes: https://git.openjdk.java.net/jdk/pull/7132/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7132&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8280166
  Stats: 88 lines in 1 file changed: 62 ins; 0 del; 26 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7132.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7132/head:pull/7132

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

Re: RFR: 8280041: Retry loop issues in java.io.ClassCache

[Aleksey Shipilev]

On Fri, 14 Jan 2022 20:50:24 GMT, Roman Kennke  wrote:

> One additional improvement would be to change r.get() == null to 
> r.refersTo(null) to check for cleared reference, that avoids reviving the 
> referent, e.g. in SATB GCs. I leave that up to you.

I think that does not work, because we want to strongly reference the `val` for 
eventual return.

-

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

RFR: 8280041: Retry loop issues in java.io.ClassCache

[Aleksey Shipilev]

JDK-8277072 introduced java.io.ClassCache, but there seem to be at least two 
issues with it:
  - The cache cannot disambiguate between cleared SoftReference and the 
accidental passing of `null` value; in that case, the retry loop would spin 
indefinitely;
  - If retry loop would spin several times, every time discovering a cleared 
SoftReference, it would create and register new SoftReference on the 
ReferenceQueue. However, it would not *drain* the RQ in the loop; in that case, 
we might have the unbounded garbage accumulating in RQ; 

Attention @rkennke, @plevart.

Additional testing:
  - [x] Linux x86_64 fastdebug `java/io/ObjectStreamClass`
  - [x] Linux x86_64 fastdebug `tier1`
  - [ ] Linux x86_64 fastdebug `tier2`
  - [ ] Linux x86_64 fastdebug `tier3`

-

Commit messages:
 - Fix

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

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

Re: RFR: JDK-8279954 - java/lang/StringBuffer(StringBuilder)/HugeCapacity.java intermittently fails

[Aleksey Shipilev]

On Fri, 14 Jan 2022 14:33:13 GMT, Jim Laskey  wrote:

> Tests were fatally failing (windows) on Github actions. Pumping up the memory 
> requirements will hopefully alleviate.

Looks fine.

-

Marked as reviewed by shade (Reviewer).

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

[jdk18] Integrated: 8279370: jdk.jpackage/share/native/applauncher/JvmLauncher.cpp fails to build with GCC 6.3.0

[Aleksey Shipilev]

On Mon, 3 Jan 2022 08:10:27 GMT, Aleksey Shipilev  wrote:

> Seems like a missing include. C++ docs say `offsetof` is from ``, 
> adding that include explicitly fixes the build. Seems to only happen with 
> older GCCs, but it seems to be a happy accident it works on newer ones, 
> probably through the transitive include somewhere.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug `tools/jpackage` tests
>  - [x]  Linux x86_64 release `tools/jpackage` tests

This pull request has now been integrated.

Changeset: 14a90e53
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk18/commit/14a90e536b86a8fb8d5f0272ec03359e44638da5
Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod

8279370: jdk.jpackage/share/native/applauncher/JvmLauncher.cpp fails to build 
with GCC 6.3.0

Reviewed-by: almatvee, asemenyuk

-

PR: https://git.openjdk.java.net/jdk18/pull/74

Re: [jdk18] RFR: 8279370: jdk.jpackage/share/native/applauncher/JvmLauncher.cpp fails to build with GCC 6.3.0

[Aleksey Shipilev]

On Mon, 3 Jan 2022 08:10:27 GMT, Aleksey Shipilev  wrote:

> Seems like a missing include. C++ docs say `offsetof` is from ``, 
> adding that include explicitly fixes the build. Seems to only happen with 
> older GCCs, but it seems to be a happy accident it works on newer ones, 
> probably through the transitive include somewhere.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug `tools/jpackage` tests
>  - [x]  Linux x86_64 release `tools/jpackage` tests

Thank you!

-

PR: https://git.openjdk.java.net/jdk18/pull/74

Re: [jdk18] RFR: 8279370: jdk.jpackage/share/native/applauncher/JvmLauncher.cpp fails to build with GCC 6.3.0

[Aleksey Shipilev]

On Mon, 3 Jan 2022 08:10:27 GMT, Aleksey Shipilev  wrote:

> Seems like a missing include. C++ docs say `offsetof` is from ``, 
> adding that include explicitly fixes the build. Seems to only happen with 
> older GCCs, but it seems to be a happy accident it works on newer ones, 
> probably through the transitive include somewhere.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug `tools/jpackage` tests
>  - [x]  Linux x86_64 release `tools/jpackage` tests

Still pending review... Or I can push under "triviality" rule, if any Reviewer 
agrees.

-

PR: https://git.openjdk.java.net/jdk18/pull/74

Re: RFR: 8279833: Loop optimization issue in String.encodeUTF8_UTF16 [v2]

[Aleksey Shipilev]

On Tue, 11 Jan 2022 13:02:21 GMT, Claes Redestad  wrote:

>> In `String.encodeUTF8_UTF16`, making the `char c` local to each loop helps 
>> the performance of the method by helping C2 optimize each individual loop 
>> better.
>> 
>> Results on the updated micros:
>> 19-b04:
>> 
>> Benchmark  (charsetName)  Mode  Cnt Score 
>> Error  Units
>> StringEncode.encodeUTF16   UTF-8  avgt   15   164.457 ±   
>> 7.296  ns/op
>> StringEncode.encodeUTF16LongEndUTF-8  avgt   15  2294.160 ±  
>> 40.580  ns/op
>> StringEncode.encodeUTF16LongStart  UTF-8  avgt   15  9128.698 ± 
>> 124.636  ns/op
>> 
>> 
>> Patch:
>> 
>> Benchmark  (charsetName)  Mode  Cnt Score
>> Error  Units
>> StringEncode.encodeUTF16   UTF-8  avgt   15   131.296 ±  
>> 6.693  ns/op
>> StringEncode.encodeUTF16LongEndUTF-8  avgt   15  2282.750 ± 
>> 46.891  ns/op
>> StringEncode.encodeUTF16LongStart  UTF-8  avgt   15  4786.965 ± 
>> 64.896  ns/op
>> 
>> 
>> Going back this seem to have been an issue since this method was introduced 
>> with JEP 254 in JDK 9:
>> 
>> 8u311:
>> 
>> Benchmark  (charsetName)  Mode  Cnt Score 
>> Error  Units
>> StringEncode.encodeUTF16   UTF-8  avgt   15   194.057 ±   
>> 3.913  ns/op
>> StringEncode.encodeUTF16LongEndUTF-8  avgt   15  3024.860 ±  
>> 88.386  ns/op
>> StringEncode.encodeUTF16LongStart  UTF-8  avgt   15  5282.849 ± 
>> 247.230  ns/op
>> 
>> 9:
>> 
>> Benchmark  (charsetName)  Mode  Cnt  Score 
>> Error  Units
>> StringEncode.encodeUTF16   UTF-8  avgt   15148.481 ±   
>> 9.374  ns/op
>> StringEncode.encodeUTF16LongEndUTF-8  avgt   15   2832.754 ± 
>> 263.372  ns/op
>> StringEncode.encodeUTF16LongStart  UTF-8  avgt   15  10447.115 ± 
>> 408.338  ns/op
>> 
>> 
>> (Interestingly JDK 9 seem faster on some of the micros than later 
>> iterations, while slower on others. The main issue is the slow non-ASCII 
>> loop, where the patch speeds things up ~2x. With the patch we're 
>> significantly faster than both JDK 8 and 9 on all measures.)
>> 
>> There's a JIT compiler bug hiding in plain sight here where the potentially 
>> uninitialized local `char c` appears to mess up optimization of the second 
>> loop. I'll file a separate bug for this (a standalone reproducer should be 
>> straightforward to produce). I think this patch is reasonable to put back 
>> into the JDK while we investigate if/how C2 can better handle this kind of 
>> condition. It might also be easier to backport, depending on whether the C2 
>> fix is trivial or not.
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove unused Blackholes

Looks fine, consider touchups in benchmark code.

test/micro/org/openjdk/bench/java/lang/StringEncode.java line 113:

> 111: 
> 112: @Benchmark
> 113: public byte[] encodeUTF16LongEnd(Blackhole bh) throws Exception {

Here and later: can drop `bh` argument, since you don't need it anymore.

-

Marked as reviewed by shade (Reviewer).

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

Integrated: 8279453: Disable tools/jar/ReproducibleJar.java on 32-bit platforms

[Aleksey Shipilev]

On Tue, 4 Jan 2022 16:46:09 GMT, Aleksey Shipilev  wrote:

> The real problem is Y2038 
> ([JDK-8279444](https://bugs.openjdk.java.net/browse/JDK-8279444)), which does 
> not look solvable at this time. So for test cleanliness, we might just 
> disable this test on 32-bit platforms.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug, affected test still passes
>  - [x]  Linux x86_32 fastdebug, affected test is now skipped

This pull request has now been integrated.

Changeset: a741b927
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk/commit/a741b927a3cdc8e339ae557c77886ea850aa06b6
Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod

8279453: Disable tools/jar/ReproducibleJar.java on 32-bit platforms

Reviewed-by: alanb, bpb

-

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

Re: RFR: 8279453: Disable tools/jar/ReproducibleJar.java on 32-bit platforms

[Aleksey Shipilev]

On Tue, 4 Jan 2022 16:46:09 GMT, Aleksey Shipilev  wrote:

> The real problem is Y2038 
> ([JDK-8279444](https://bugs.openjdk.java.net/browse/JDK-8279444)), which does 
> not look solvable at this time. So for test cleanliness, we might just 
> disable this test on 32-bit platforms.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug, affected test still passes
>  - [x]  Linux x86_32 fastdebug, affected test is now skipped

Thanks for reviews!

-

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

Re: [jdk18] RFR: 8279370: jdk.jpackage/share/native/applauncher/JvmLauncher.cpp fails to build with GCC 6.3.0

[Aleksey Shipilev]

On Mon, 3 Jan 2022 21:18:12 GMT, Victor Dyakov  wrote:

> it requires @alexeysemenyukoracle review (owner of JDK-8274856 changeset)

Well, this looks trivial to me. Should we really wait @alexeysemenyukoracle, 
who, I assume, might be on NY holiday break?

-

PR: https://git.openjdk.java.net/jdk18/pull/74

RFR: 8279453: Disable tools/jar/ReproducibleJar.java on 32-bit platforms

[Aleksey Shipilev]

The real problem is Y2038 
([JDK-8279444](https://bugs.openjdk.java.net/browse/JDK-8279444)), which does 
not look solvable at this time. So for test cleanliness, we might just disable 
this test on 32-bit platforms.

Additional testing:
 - [x] Linux x86_64 fastdebug, affected test still passes
 - [x]  Linux x86_32 fastdebug, affected test is now skipped

-

Commit messages:
 - Fix

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

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

[jdk18] RFR: 8279370: jdk.jpackage/share/native/applauncher/JvmLauncher.cpp fails to build with GCC 6.3.0

[Aleksey Shipilev]

Seems like a missing include. C++ docs say `offsetof` is from ``, 
adding that include explicitly fixes the build. Seems to only happen with older 
GCCs, but it seems to be a happy accident it works on newer ones, probably 
through the transitive include somewhere.

Additional testing:
 - [x] Linux x86_64 fastdebug `tools/jpackage` tests
 - [x]  Linux x86_64 release `tools/jpackage` tests

-

Commit messages:
 - Sort
 - Fix

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

PR: https://git.openjdk.java.net/jdk18/pull/74

Re: RFR: 8278341: Liveness check for global scope is not as fast as it could be

[Aleksey Shipilev]

On Tue, 7 Dec 2021 13:12:13 GMT, Maurizio Cimadamore  
wrote:

> When doing some unrelated performance measurements, I realized that segments 
> backed by global scope were still paying a relatively high cost for liveness 
> checks - that's because GlobalScopeImpl extends from SharedScopeImpl, and 
> does not override the `isAlive` method. This means that when checking for 
> liveness, we will still do (in some cases - e.g. when calling 
> `checkValidStateSlow`) a volatile VH get on the scope state - which is 
> useless in this case.
> 
> This simple patch adds the missing override.

Looks good!

-

Marked as reviewed by shade (Reviewer).

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

Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3

[Aleksey Shipilev]

On Tue, 30 Nov 2021 18:48:15 GMT, Aleksey Shipilev  wrote:

> OpenJDK tiered tests definitions have the catch-all `tier4` that runs all 
> tests not defined in the lower tiers. `hotspot:tier4` has lots of them, 
> mostly long-running vmTestbase tests, which take many hours even on a very 
> parallel machines.
> 
> This, unfortunately, has a chilling effect on `jdk:tier4`, which is seldom 
> run by contributors, because `hotspot:tier4` is in the way. But, there are 
> plenty of fast and stable tests in `jdk:tier4` that can be run in 
> `jdk:tier3`. `jdk_svc` is the example of such tests: management features 
> (including but not limited to JFR) are important to keep from regressions, 
> and significant subset of them runs pretty fast.
> 
> So, it makes sense to move some `jdk_svc` tests to `jdk:tier3` to expose it 
> to more contributors. I think the only group we don't want to run is 
> `svc_tools`, which includes lots of non-parallel tests that are rather slow.
> 
> Sample run before:
> 
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>jtreg:test/jdk:tier3174   174 0 0  
>  
> ==
> TEST SUCCESS
> 
> Finished building target 'run-test' in configuration 
> 'linux-x86_64-server-fastdebug'
> 
> real  2m38.242s
> user  80m7.216s
> sys   2m13.846s
> 
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>>> jtreg:test/jdk:tier4   2904  2901 3 0 <<
> ==
> TEST FAILURE
> 
> real  18m13.933s
> user  546m50.556s
> sys   25m7.086s
> 
> 
> Sample run after:
> 
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>jtreg:test/jdk:tier3   1296  1296 0 0  
>  
> ==
> TEST SUCCESS
> 
> Finished building target 'run-test' in configuration 
> 'linux-x86_64-server-fastdebug'
> 
> real  7m49.017s
> user  287m30.943s
> sys   13m20.060s
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>>> jtreg:test/jdk:tier4   1783  1780 3 0 <<
> ==
> TEST FAILURE
> 
> 
> real  12m19.973s
> user  351m48.561s
> sys   14m51.566s

All right, I'll be integrating it soon. Last call for comments :)

-

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

Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3

[Aleksey Shipilev]

On Tue, 30 Nov 2021 18:48:15 GMT, Aleksey Shipilev  wrote:

> OpenJDK tiered tests definitions have the catch-all `tier4` that runs all 
> tests not defined in the lower tiers. `hotspot:tier4` has lots of them, 
> mostly long-running vmTestbase tests, which take many hours even on a very 
> parallel machines.
> 
> This, unfortunately, has a chilling effect on `jdk:tier4`, which is seldom 
> run by contributors, because `hotspot:tier4` is in the way. But, there are 
> plenty of fast and stable tests in `jdk:tier4` that can be run in 
> `jdk:tier3`. `jdk_svc` is the example of such tests: management features 
> (including but not limited to JFR) are important to keep from regressions, 
> and significant subset of them runs pretty fast.
> 
> So, it makes sense to move some `jdk_svc` tests to `jdk:tier3` to expose it 
> to more contributors. I think the only group we don't want to run is 
> `svc_tools`, which includes lots of non-parallel tests that are rather slow.
> 
> Sample run before:
> 
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>jtreg:test/jdk:tier3174   174 0 0  
>  
> ==
> TEST SUCCESS
> 
> Finished building target 'run-test' in configuration 
> 'linux-x86_64-server-fastdebug'
> 
> real  2m38.242s
> user  80m7.216s
> sys   2m13.846s
> 
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>>> jtreg:test/jdk:tier4   2904  2901 3 0 <<
> ==
> TEST FAILURE
> 
> real  18m13.933s
> user  546m50.556s
> sys   25m7.086s
> 
> 
> Sample run after:
> 
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>jtreg:test/jdk:tier3   1296  1296 0 0  
>  
> ==
> TEST SUCCESS
> 
> Finished building target 'run-test' in configuration 
> 'linux-x86_64-server-fastdebug'
> 
> real  7m49.017s
> user  287m30.943s
> sys   13m20.060s
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>>> jtreg:test/jdk:tier4   1783  1780 3 0 <<
> ==
> TEST FAILURE
> 
> 
> real  12m19.973s
> user  351m48.561s
> sys   14m51.566s

Ok, good. I need some formal (R)eviews to get this going :)

-

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

Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3

[Aleksey Shipilev]

On Fri, 3 Dec 2021 07:34:16 GMT, Alan Bateman  wrote:

> No objection to the change, I think it is just a re-balancing of tiers for CI 
> systems.

Yes, quite. @dholmes-ora, are you happy with Alan's assessment?

-

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

Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3

[Aleksey Shipilev]

On Thu, 2 Dec 2021 12:15:51 GMT, David Holmes  wrote:

> I've solicited feedback from core-libs folk as this affects them the most. At 
> present we, Oracle, run the jdk_svc tests as part of hotspot testing, but 
> this change will suddenly cause jdk testing to include them. That is probably 
> not an issue for testing within the CI framework but developers running 
> jdk_tier3 locally, often, may have an unpleasant surprise at running over a 
> thousand additional tests that probably have zero relevance to what they are 
> trying to test.

Thanks, no problem, this is not urgent. 

I think tiered testing is by definition tests things broadly, beyond the scope 
of feature-centric test groups. I think not breaking JDK management APIs and 
JFR (that I believe includes JDK events too) would is the useful check for a 
high jdk:tier3.

-

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

Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3

[Aleksey Shipilev]

On Tue, 30 Nov 2021 18:48:15 GMT, Aleksey Shipilev  wrote:

> OpenJDK tiered tests definitions have the catch-all `tier4` that runs all 
> tests not defined in the lower tiers. `hotspot:tier4` has lots of them, 
> mostly long-running vmTestbase tests, which take many hours even on a very 
> parallel machines.
> 
> This, unfortunately, has a chilling effect on `jdk:tier4`, which is seldom 
> run by contributors, because `hotspot:tier4` is in the way. But, there are 
> plenty of fast and stable tests in `jdk:tier4` that can be run in 
> `jdk:tier3`. `jdk_svc` is the example of such tests: management features 
> (including but not limited to JFR) are important to keep from regressions, 
> and significant subset of them runs pretty fast.
> 
> So, it makes sense to move some `jdk_svc` tests to `jdk:tier3` to expose it 
> to more contributors. I think the only group we don't want to run is 
> `svc_tools`, which includes lots of non-parallel tests that are rather slow.
> 
> Sample run before:
> 
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>jtreg:test/jdk:tier3174   174 0 0  
>  
> ==
> TEST SUCCESS
> 
> Finished building target 'run-test' in configuration 
> 'linux-x86_64-server-fastdebug'
> 
> real  2m38.242s
> user  80m7.216s
> sys   2m13.846s
> 
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>>> jtreg:test/jdk:tier4   2904  2901 3 0 <<
> ==
> TEST FAILURE
> 
> real  18m13.933s
> user  546m50.556s
> sys   25m7.086s
> 
> 
> Sample run after:
> 
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>jtreg:test/jdk:tier3   1296  1296 0 0  
>  
> ==
> TEST SUCCESS
> 
> Finished building target 'run-test' in configuration 
> 'linux-x86_64-server-fastdebug'
> 
> real  7m49.017s
> user  287m30.943s
> sys   13m20.060s
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>>> jtreg:test/jdk:tier4   1783  1780 3 0 <<
> ==
> TEST FAILURE
> 
> 
> real  12m19.973s
> user  351m48.561s
> sys   14m51.566s

All right then. Thanks for followups! 

@egahlin, this affects JFR tests a bit: subsumes `jfr_tier_3` into `tier3`, 
basically. Are you good with this?

-

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

Re: RFR: JDK-8278014: [vectorapi] Remove test run script [v2]

[Aleksey Shipilev]

On Tue, 30 Nov 2021 23:35:08 GMT, Paul Sandoz  wrote:

>> Remove Vector API scripts for building and running tests. `jtreg` should be 
>> used instead.
>> 
>> Also updated the test generation script to remove options that assume 
>> mercurial as the code repository.
>
> Paul Sandoz has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update copywrite year.

Marked as reviewed by shade (Reviewer).

-

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

Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3

[Aleksey Shipilev]

On Wed, 1 Dec 2021 02:29:14 GMT, David Holmes  wrote:

> Hi @shipilev , We need to have someone look at the impact of this on our CI. 
> I don't think we run the tier4 group as defined in our tier 4 so we may not 
> see those execution time savings that offset the extra cost to tier3.

OK, I can wait for those who have more insight in Oracle testing pipelines 
check their workflows with this change. I have no insight in Oracle infra, so 
somebody else have to do it. Now that Igor left, who should we be talking to?

-

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

RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3

[Aleksey Shipilev]

OpenJDK tiered tests definitions have the catch-all `tier4` that runs all tests 
not defined in the lower tiers. `hotspot:tier4` has lots of them, mostly 
long-running vmTestbase tests, which take many hours even on a very parallel 
machines.

This, unfortunately, has a chilling effect on `jdk:tier4`, which is seldom run 
by contributors, because `hotspot:tier4` is in the way. But, there are plenty 
of fast and stable tests in `jdk:tier4` that can be run in `jdk:tier3`. 
`jdk_svc` is the example of such tests: management features (including but not 
limited to JFR) are important to keep from regressions, and significant subset 
of them runs pretty fast.

So, it makes sense to move some `jdk_svc` tests to `jdk:tier3` to expose it to 
more contributors. I think the only group we don't want to run is `svc_tools`, 
which includes lots of non-parallel tests that are rather slow.

Sample run before:


==
Test summary
==
   TEST  TOTAL  PASS  FAIL ERROR   
   jtreg:test/jdk:tier3174   174 0 0   
==
TEST SUCCESS

Finished building target 'run-test' in configuration 
'linux-x86_64-server-fastdebug'

real2m38.242s
user80m7.216s
sys 2m13.846s


==
Test summary
==
   TEST  TOTAL  PASS  FAIL ERROR   
>> jtreg:test/jdk:tier4   2904  2901 3 0 <<
==
TEST FAILURE

real18m13.933s
user546m50.556s
sys 25m7.086s


Sample run after:


==
Test summary
==
   TEST  TOTAL  PASS  FAIL ERROR   
   jtreg:test/jdk:tier3   1296  1296 0 0   
==
TEST SUCCESS

Finished building target 'run-test' in configuration 
'linux-x86_64-server-fastdebug'

real7m49.017s
user287m30.943s
sys 13m20.060s

==
Test summary
==
   TEST  TOTAL  PASS  FAIL ERROR   
>> jtreg:test/jdk:tier4   1783  1780 3 0 <<
==
TEST FAILURE


real12m19.973s
user351m48.561s
sys 14m51.566s

-

Commit messages:
 - Fix

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

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

Re: RFR: JDK-8276422 Add command-line option to disable finalization

[Aleksey Shipilev]

On Thu, 18 Nov 2021 07:40:34 GMT, David Holmes  wrote:

>> Do we even have to have a flag on Java side? It looks like these calls are 
>> only done as the upcalls from VM, so we might just keep the flag on VM side?
>
> @shipilev not sure what you mean by  "a flag on the Java side". The Java code 
> just queries the VM for the finalization enabled/disabled state and uses that 
> to control things.

Yeah, "flag" is `Holder.ENABLED` here. I mean, are Java methods 
`registerFinalizer` and `runFinalization` called only by VM? If so, can VM 
check the whole thing on VM side, without going to Java and asking back from 
there?

-

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

Re: RFR: JDK-8276422 Add command-line option to disable finalization

[Aleksey Shipilev]

On Thu, 18 Nov 2021 07:13:55 GMT, David Holmes  wrote:

>> Huh, good catch! This was mostly left over from an earlier version of the 
>> flag that used system properties, which aren't initialized until after the 
>> Finalizer class is initialized.
>> 
>> It might be the case that the Holder can be removed at this point, since the 
>> finalization-enabled bit is no longer in a system property and is in a 
>> native class member that should be available before the VM is started.
>> 
>> I say "might" though because this occurs early in system startup, and weird 
>> things potentially happen. For example, suppose the first object with a 
>> finalizer is created before the Finalizer class is initialized. The VM will 
>> perform an upcall to Finalizer::register. An ordinary call to a static 
>> method will ensure the class is initialized before proceeding with the call, 
>> but this VM upcall is a special case I'll have to investigate this some 
>> more.
>
> @stuart-marks not sure I see how anything is different here compared to the 
> existing logic. The `Finalizer` class is explicitly initialized quite early 
> in the init process, but if a preceding class's initialization created an 
> object with a finalizer then that same upcall would be involved.

Do we even have to have a flag on Java side? It looks like these calls are only 
done as the upcalls from VM, so we might just keep the flag on VM side?

-

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

Re: RFR: JDK-8276422 Add command-line option to disable finalization

[Aleksey Shipilev]

On Thu, 18 Nov 2021 01:34:36 GMT, Stuart Marks  wrote:

> Pretty much what it says. The new option controls a static member in 
> InstanceKlass that's consulted to determine whether the finalization 
> machinery is activated for instances when a class is loaded. A new native 
> method is added so that this state can be queried from Java. This is used to 
> control whether a finalizer thread is created and to disable the `System` and 
> `Runtime::runFinalization` methods. Includes tests for the above.

>From the brief look, it is OK. Minor nits.

src/hotspot/share/prims/jvm.cpp line 694:

> 692: 
> 693: JVM_ENTRY(jboolean, JVM_IsFinalizationEnabled(JNIEnv * env))
> 694: return InstanceKlass::finalization_enabled() ? JNI_TRUE : JNI_FALSE;

Suggestion:

  return InstanceKlass::finalization_enabled() ? JNI_TRUE : JNI_FALSE;

-

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

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved

[Aleksey Shipilev]

On Tue, 16 Nov 2021 05:24:38 GMT, Vicente Romero  wrote:

> Please review this PR which aims to optimize the implementation of the 
> `toString` method we provide for records. A benchmark comparing the 
> implementation we are providing for records with lombok found out that lombok 
> is much faster mainly because our implementation uses `String::format`. This 
> fix is basically delegating on StringConcatFactory::makeConcatWithConstants 
> which is faster.
> 
> TIA
> 
> This is the result of the benchmark comparing records to lombok with vanilla 
> JDK:
> 
> Benchmark  Mode  CntScoreError  Units
> MyBenchmark.base   avgt40.849 ±  0.111  ns/op
> MyBenchmark.equals_record  avgt47.343 ±  2.740  ns/op
> MyBenchmark.equals_value   avgt46.644 ±  1.920  ns/op
> MyBenchmark.record_hash_code   avgt45.763 ±  3.882  ns/op
> MyBenchmark.record_to_string   avgt4  262.626 ± 12.574  ns/op 
>   <-- Before
> MyBenchmark.value_class_to_string  avgt4   30.325 ± 21.389  ns/op
> MyBenchmark.value_hash_codeavgt45.048 ±  3.936  ns/op
> 
> 
> after this patch:
> 
> Benchmark  Mode  Cnt   Score   Error  Units
> MyBenchmark.base   avgt4   0.680 ± 0.185  ns/op
> MyBenchmark.equals_record  avgt4   5.599 ± 1.348  ns/op
> MyBenchmark.equals_value   avgt4   5.718 ± 4.633  ns/op
> MyBenchmark.record_hash_code   avgt4   4.628 ± 4.368  ns/op
> MyBenchmark.record_to_string   avgt4  26.791 ± 1.817  ns/op   
>   <--- After
> MyBenchmark.value_class_to_string  avgt4  35.473 ± 2.626  ns/op
> MyBenchmark.value_hash_codeavgt4   6.152 ± 5.101  ns/op

There is a limit on the number of arguments that you can feed into `SCF`, what 
happens if we reach it? E.g. what if we have the Record with too many 
components?

-

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

Re: RFR: 8276700: Improve java.lang.ref.Cleaner javadocs

[Aleksey Shipilev]

On Fri, 22 Oct 2021 08:03:34 GMT, Hendrik Schreiber  
wrote:

> Trivial improvement.
> 
> Explicitly show how to create a `Cleaner` instance using `Cleaner.create()`.
> Repeat (again) in the code example that the `State` `Runnable `should be 
> implemented as static class and not reference the instance to be cleaned, to 
> make the point even more clear to those people who never read the javadoc 
> *prose*.
> 
> I have signed the OCA a while back as 
> [hschreiber](https://openjdk.java.net/census#hschreiber).

Submitted the RFE for it. Please change the PR title to "8276700: Improve 
java.lang.ref.Cleaner javadocs" to let bots hook it up.

-

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