Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref [v32]

[Joe Darcy]

On Fri, 24 May 2024 21:39:33 GMT, Brent Christian  wrote:

>> Classes in the `java.lang.ref` package would benefit from an update to bring 
>> the spec in line with how the VM already behaves. The changes would focus on 
>> _happens-before_ edges at some key points during reference processing.
>> 
>> A couple key things we want to be able to say are:
>> - `Reference.reachabilityFence(x)` _happens-before_ reference processing 
>> occurs for 'x'.
>> - `Cleaner.register()` _happens-before_ the Cleaner thread runs the 
>> registered cleaning action.
>> 
>> This will bring Cleaner in line (or close) with the memory visibility 
>> guarantees made for finalizers in [JLS 
>> 17.4.5](https://docs.oracle.com/javase/specs/jls/se18/html/jls-17.html#jls-17.4.5):
>> _"There is a happens-before edge from the end of a constructor of an object 
>> to the start of a finalizer (§12.6) for that object."_
>
> Brent Christian has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 69 commits:
> 
>  - Merge branch 'master' into refDocs2
>  - add link to Thread.isAlive()
>  - small review tweaks; shorten MemoryConsistency links
>  - small grammar fixes
>  - new section for finalizer memviz
>  - add memviz bullet for finalization
>  - remove quotes from dequeue
>  - package spec updates, mostly about reference queues and dequeueing
>  - move reachability section before notification; update section header
>  - add details on use of reference queues; swap reachability/memviz sections
>  - ... and 59 more: https://git.openjdk.org/jdk/compare/7bf1989f...d7cbf0d3

src/java.base/share/classes/java/lang/ref/Reference.java line 556:

> 554:  *   int myIndex;
> 555:  *   Resource(...) {
> 556:  * myIndex = ...

Convert this code sample to a snippet?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1619935008

Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref [v32]

[Joe Darcy]

On Fri, 24 May 2024 21:39:33 GMT, Brent Christian  wrote:

>> Classes in the `java.lang.ref` package would benefit from an update to bring 
>> the spec in line with how the VM already behaves. The changes would focus on 
>> _happens-before_ edges at some key points during reference processing.
>> 
>> A couple key things we want to be able to say are:
>> - `Reference.reachabilityFence(x)` _happens-before_ reference processing 
>> occurs for 'x'.
>> - `Cleaner.register()` _happens-before_ the Cleaner thread runs the 
>> registered cleaning action.
>> 
>> This will bring Cleaner in line (or close) with the memory visibility 
>> guarantees made for finalizers in [JLS 
>> 17.4.5](https://docs.oracle.com/javase/specs/jls/se18/html/jls-17.html#jls-17.4.5):
>> _"There is a happens-before edge from the end of a constructor of an object 
>> to the start of a finalizer (§12.6) for that object."_
>
> Brent Christian has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 69 commits:
> 
>  - Merge branch 'master' into refDocs2
>  - add link to Thread.isAlive()
>  - small review tweaks; shorten MemoryConsistency links
>  - small grammar fixes
>  - new section for finalizer memviz
>  - add memviz bullet for finalization
>  - remove quotes from dequeue
>  - package spec updates, mostly about reference queues and dequeueing
>  - move reachability section before notification; update section header
>  - add details on use of reference queues; swap reachability/memviz sections
>  - ... and 59 more: https://git.openjdk.org/jdk/compare/7bf1989f...d7cbf0d3

src/java.base/share/classes/java/lang/ref/Reference.java line 491:

> 489:  * method is unsuccessful and returns false.
> 490:  *
> 491:  * Memory 
> consistency effects:

Note the `https://git.openjdk.org/jdk/pull/16644#discussion_r1619921063
PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1619923554

Re: RFR: 8333103: Re-examine the console provider loading

[Jaikiran Pai]

On Wed, 29 May 2024 19:51:36 GMT, Naoto Sato  wrote:

> There is an initialization code in `Console` class that searches for the 
> Console implementations. Refactoring the init code not to use lambda/stream 
> would reduce the (initial) number of loaded classes by about 100 for 
> java.base implementations. This would become relevant when the java.io.IO 
> (JEP 477) uses Console as the underlying framework.

The change to replace lambda with anonymous class looks OK to me.

-

Marked as reviewed by jpai (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19467#pullrequestreview-2087164330

RFR: 8330702: Update failure handler to don't generate Error message if cores actions are empty.

[Leonid Mesnik]

The message is generated if cores (or any other tools) section doesn't exist or 
is empty. However, there is no any tool for cores processing now defined. So 
ERROR message is generating, confusing users.
The fix is to don't print error for empty toolset which is the valid case. The 
message is still generate is tool is not defined to get error message in the 
case of miswriting.

-

Commit messages:
 - 8330702

Changes: https://git.openjdk.org/jdk/pull/19470/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=19470=00
  Issue: https://bugs.openjdk.org/browse/JDK-8330702
  Stats: 9 lines in 2 files changed: 5 ins; 0 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/19470.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19470/head:pull/19470

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

Re: RFR: 8320448: Accelerate IndexOf using AVX2 [v48]

[Vladimir Kozlov]

On Wed, 29 May 2024 22:20:31 GMT, Scott Gibbons  wrote:

>> Re-write the IndexOf code without the use of the pcmpestri instruction, only 
>> using AVX2 instructions.  This change accelerates String.IndexOf on average 
>> 1.3x for AVX2.  The benchmark numbers:
>> 
>> 
>> Benchmark   Score
>> Latest  
>> StringIndexOf.advancedWithMediumSub   343.573317.934 
>> 0.925375393x
>> StringIndexOf.advancedWithShortSub11039.081  1053.96 
>> 1.014319384x
>> StringIndexOf.advancedWithShortSub255.828110.541 
>> 1.980027943x
>> StringIndexOf.constantPattern9.361   11.906  
>> 1.271872663x
>> StringIndexOf.searchCharLongSuccess  4.216   4.218   
>> 1.000474383x
>> StringIndexOf.searchCharMediumSuccess3.133   3.216   
>> 1.02649218x
>> StringIndexOf.searchCharShortSuccess 3.763.761   
>> 1.000265957x
>> StringIndexOf.success9.186   
>> 9.713   1.057369911x
>> StringIndexOf.successBig   14.34146.343  
>> 3.231504079x
>> StringIndexOfChar.latin1_AVX2_String   6220.918  12154.52
>> 1.953814533x
>> StringIndexOfChar.latin1_AVX2_char 5503.556  5540.044
>> 1.006629895x
>> StringIndexOfChar.latin1_SSE4_String   6978.854  6818.689
>> 0.977049957x
>> StringIndexOfChar.latin1_SSE4_char 5657.499  5474.624
>> 0.967675646x
>> StringIndexOfChar.latin1_Short_String  7132.541  
>> 6863.3590.962260014x
>> StringIndexOfChar.latin1_Short_char  16013.389 16162.437 
>> 1.009307711x
>> StringIndexOfChar.latin1_mixed_String  7386.12314771.622 
>> 1.15517x
>> StringIndexOfChar.latin1_mixed_char9901.671  9782.245
>> 0.987938803
>
> Scott Gibbons has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove duplicate vm.compiler2.enabled

My testing passed.

-

Marked as reviewed by kvn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16753#pullrequestreview-2086978326

Re: Question on Lambda function

[Zhengyu Gu]

Hi Chen,

What is your usage pattern of these single-abstract-method implementations? 
Since it sounds like you are
creating a lot of them, are you storing them in collections?

Yes, we do have such usage patterns, e.g. stores methods as Function in hash 
table as handlers, etc.


If you are keeping a lot of them in collection (say, as event handlers), you 
may try to use `MethodHandleProxies.asInterfaceInstance` as a temporary 
workaround on JDK 22 and higher (older version uses Proxy, which has horrible 
invocation performance).

Thanks for the suggestion. We are currently at 17, I will investigate the 
library.


Best,

-Zhengyu


If you are on older versions from 15 to 21, unfortunately you might have to 
write a hidden class for the same purpose or use an existing library. One 
library that might be useful is https://github.com/LanternPowered/Lmbda that 
effectively generates unloadable hidden classes, but its 3.x builds are not 
maven central so you have to build yourself.

- Chen

On Wed, May 29, 2024 at 3:35 PM Zhengyu Gu 
mailto:zhengyu...@servicenow.com>> wrote:
Hi Chen,

Thanks for the insights.

We did refactor our code to avoid using LambdaMetaFactory,metafactory() 
directly.

With increasing use of Lambdas, in our applications and libraries, the 
metaspace impact becomes a concern. If current implementation (not able to 
unload unused Lambda classes) here to stay, we must come up with a coding 
guideline to avoid excessive creation of Lambda classes,  any pointers or 
suggestions would be greatly appreciated.

Best,

-Zhengyu

From: Chen Liang mailto:liangchenb...@gmail.com>>
Date: Wednesday, May 29, 2024 at 2:43 PM
To: Zhengyu Gu mailto:zhengyu...@servicenow.com>>
Cc: core-libs-dev@openjdk.org 
mailto:core-libs-dev@openjdk.org>>
Subject: Re: Question on Lambda function
[External Email]


Hi Gu,
CallSite is specific to each invokedynamic instruction instead of each 
InvokeDynamic constant pool entry: 
https://docs.oracle.com/javase/specs/jvms/se22/html/jvms-6.html#jvms-6.5.invokedynamic
And the linking is done by MethodHandleNatives.linkCallSite if you want to 
follow the Java implementation code.
For why the lambda in the loop is constant, it's a feature from 
InnerClassLambdaMetafactory: 
https://github.com/openjdk/jdk/blob/c8eea59f508158075382079316cf0990116ff98e/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java#L236
When the lambda is non-capturing, the bootstrap method 
LambdaMetafactory.metafactory will eagerly create a singleton instance and 
return this singleton in the indy instruction.

Also, your metaspace pressure might be caused by the fact that Lambda classes 
(not instances) are no longer eagerly unloaded; see 
https://github.com/openjdk/jdk/pull/12493 and 
https://bugs.openjdk.org/browse/JDK-8302154.
 You are recommended to create your own facility to create hidden classes in 
Java 17 instead of continue to use LambdaMetafactory explicitly in code.

Regards,
Chen Liang

On Wed, May 29, 2024 at 12:53 PM Zhengyu Gu 
mailto:zhengyu...@servicenow.com>> wrote:
Hello Lambda experts,

Since we upgraded JDK from 11 to 17, we’re experiencing metaspace pressure, 
largely due to Lambda class implementation changes.

There’s a scenario (see attached test case),  that is especially puzzled me, 
hopefully, you can share some insights.

In this test case, there is only one Lambda class is created inside the loop, 
but each one for the same functions outside loop.

Example output:

0: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called
1: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called
2: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called
3: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called
4: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called

….

Outside loop1, Func =  LambdaFunc$$Lambda/0x1f8c4c58@402f32ff
testMethod() called
Outside loop2 Func = LambdaFunc$$Lambda/0x1f8d1000@5ae9a829
testMethod() called
Outside loop3 Func = LambdaFunc$$Lambda/0x1f8d1238@548b7f67
testMethod() called

And jcmd also confirmed there were 4 Lambda classes created:

  49: CLD 0x6134cb50: "app" instance of 
jdk.internal.loader.ClassLoaders$AppClassLoader
  Loaded classes:
 1:LambdaFunc$$Lambda/0x1f8d1238
 2:LambdaFunc$$Lambda/0x1f8d1000
 3:LambdaFunc$$Lambda/0x1f8c4c58
 4:LambdaFunc$$Lambda/0x1f8c4a20
 5:LambdaFunc

Looking into bytecode, all four call sites have the same invokedynamic bytecode 
(invokedynamic #7,  0  // InvokeDynamic 
#0:run:()Ljava/lang/Runnable; ) and the first invokedynamic bytecode is inside 
the loop.

But 

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v10]

[Doug Lea]

> This set of changes address causes of poor utilization with small numbers of 
> cores due to overly aggressive contention avoidance. A number of further 
> adjustments were needed to still avoid most contention effects in deployments 
> with large numbers of cores

Doug Lea has updated the pull request incrementally with one additional commit 
since the last revision:

  unguard signal

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19131/files
  - new: https://git.openjdk.org/jdk/pull/19131/files/5e44de45..4564b92c

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19131=09
 - incr: https://webrevs.openjdk.org/?repo=jdk=19131=08-09

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

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

Re: RFR: 8333103: Re-examine the console provider loading

[Naoto Sato]

On Wed, 29 May 2024 21:39:37 GMT, Pavel Rappo  wrote:

>> src/java.base/share/classes/java/io/Console.java line 673:
>> 
>>> 671: return new ProxyingConsole(jc);
>>> 672: }
>>> 673: break;
>> 
>> Suggestion:
>> 
>> 
>> The original `findAny` is only after `filter(Objects::nonNull)` meaning we 
>> don't return `null` on a null `jcp.console` result.
>
> Yes, `break` guarantees that the search completes one way or another once the 
> module name has been matched. This is not how it used to be done.

Right. Since `findAny` is after the module name matching, there is at most 1 
match. In the case we didn't find any, the final `orElse(null)` eventually 
returns null. So the refactored code is semantically the same.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19467#discussion_r1619537359

Re: RFR: 8320448: Accelerate IndexOf using AVX2 [v47]

[Scott Gibbons]

On Wed, 29 May 2024 21:41:42 GMT, Vladimir Kozlov  wrote:

>> Scott Gibbons has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Move assert to where it's actually important.
>
> test/jdk/TEST.ROOT line 103:
> 
>> 101: vm.jvmti \
>> 102: vm.cpu.features \
>> 103: vm.compiler2.enabled \
> 
> `vm.compiler2.enabled ` already listed at line 91

Thanks!  Removed.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1619532884

Re: RFR: 8320448: Accelerate IndexOf using AVX2 [v48]

[Scott Gibbons]

> Re-write the IndexOf code without the use of the pcmpestri instruction, only 
> using AVX2 instructions.  This change accelerates String.IndexOf on average 
> 1.3x for AVX2.  The benchmark numbers:
> 
> 
> BenchmarkScore
> Latest  
> StringIndexOf.advancedWithMediumSub   343.573 317.934 
> 0.925375393x
> StringIndexOf.advancedWithShortSub1 1039.081  1053.96 
> 1.014319384x
> StringIndexOf.advancedWithShortSub2 55.828110.541 
> 1.980027943x
> StringIndexOf.constantPattern 9.361   11.906  
> 1.271872663x
> StringIndexOf.searchCharLongSuccess   4.216   4.218   
> 1.000474383x
> StringIndexOf.searchCharMediumSuccess 3.133   3.216   
> 1.02649218x
> StringIndexOf.searchCharShortSuccess  3.763.761   
> 1.000265957x
> StringIndexOf.success 9.186   9.713   
> 1.057369911x
> StringIndexOf.successBig14.34146.343  
> 3.231504079x
> StringIndexOfChar.latin1_AVX2_String6220.918  12154.52
> 1.953814533x
> StringIndexOfChar.latin1_AVX2_char  5503.556  5540.044
> 1.006629895x
> StringIndexOfChar.latin1_SSE4_String6978.854  6818.689
> 0.977049957x
> StringIndexOfChar.latin1_SSE4_char  5657.499  5474.624
> 0.967675646x
> StringIndexOfChar.latin1_Short_String   7132.541  6863.359
> 0.962260014x
> StringIndexOfChar.latin1_Short_char   16013.389 16162.437 
> 1.009307711x
> StringIndexOfChar.latin1_mixed_String   7386.12314771.622 
> 1.15517x
> StringIndexOfChar.latin1_mixed_char 9901.671  9782.245
> 0.987938803

Scott Gibbons has updated the pull request incrementally with one additional 
commit since the last revision:

  Remove duplicate vm.compiler2.enabled

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16753/files
  - new: https://git.openjdk.org/jdk/pull/16753/files/db0ab75a..ed06edd6

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=16753=47
 - incr: https://webrevs.openjdk.org/?repo=jdk=16753=46-47

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

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

Re: RFR: 8320448: Accelerate IndexOf using AVX2 [v47]

[Vladimir Kozlov]

On Tue, 28 May 2024 23:52:27 GMT, Scott Gibbons  wrote:

>> Re-write the IndexOf code without the use of the pcmpestri instruction, only 
>> using AVX2 instructions.  This change accelerates String.IndexOf on average 
>> 1.3x for AVX2.  The benchmark numbers:
>> 
>> 
>> Benchmark   Score
>> Latest  
>> StringIndexOf.advancedWithMediumSub   343.573317.934 
>> 0.925375393x
>> StringIndexOf.advancedWithShortSub11039.081  1053.96 
>> 1.014319384x
>> StringIndexOf.advancedWithShortSub255.828110.541 
>> 1.980027943x
>> StringIndexOf.constantPattern9.361   11.906  
>> 1.271872663x
>> StringIndexOf.searchCharLongSuccess  4.216   4.218   
>> 1.000474383x
>> StringIndexOf.searchCharMediumSuccess3.133   3.216   
>> 1.02649218x
>> StringIndexOf.searchCharShortSuccess 3.763.761   
>> 1.000265957x
>> StringIndexOf.success9.186   
>> 9.713   1.057369911x
>> StringIndexOf.successBig   14.34146.343  
>> 3.231504079x
>> StringIndexOfChar.latin1_AVX2_String   6220.918  12154.52
>> 1.953814533x
>> StringIndexOfChar.latin1_AVX2_char 5503.556  5540.044
>> 1.006629895x
>> StringIndexOfChar.latin1_SSE4_String   6978.854  6818.689
>> 0.977049957x
>> StringIndexOfChar.latin1_SSE4_char 5657.499  5474.624
>> 0.967675646x
>> StringIndexOfChar.latin1_Short_String  7132.541  
>> 6863.3590.962260014x
>> StringIndexOfChar.latin1_Short_char  16013.389 16162.437 
>> 1.009307711x
>> StringIndexOfChar.latin1_mixed_String  7386.12314771.622 
>> 1.15517x
>> StringIndexOfChar.latin1_mixed_char9901.671  9782.245
>> 0.987938803
>
> Scott Gibbons has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Move assert to where it's actually important.

test/jdk/TEST.ROOT line 103:

> 101: vm.jvmti \
> 102: vm.cpu.features \
> 103: vm.compiler2.enabled \

`vm.compiler2.enabled ` already listed at line 91

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1619506711

Re: RFR: 8333103: Re-examine the console provider loading

[Pavel Rappo]

On Wed, 29 May 2024 21:27:30 GMT, Chen Liang  wrote:

>> There is an initialization code in `Console` class that searches for the 
>> Console implementations. Refactoring the init code not to use lambda/stream 
>> would reduce the (initial) number of loaded classes by about 100 for 
>> java.base implementations. This would become relevant when the java.io.IO 
>> (JEP 477) uses Console as the underlying framework.
>
> src/java.base/share/classes/java/io/Console.java line 673:
> 
>> 671: return new ProxyingConsole(jc);
>> 672: }
>> 673: break;
> 
> Suggestion:
> 
> 
> The original `findAny` is only after `filter(Objects::nonNull)` meaning we 
> don't return `null` on a null `jcp.console` result.

Yes, `break` guarantees that the search completes one way or another once the 
module name has been matched. This is not how it used to be done.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19467#discussion_r1619504988

Re: RFR: 8320448: Accelerate IndexOf using AVX2 [v47]

[Vladimir Kozlov]

On Tue, 28 May 2024 23:52:27 GMT, Scott Gibbons  wrote:

>> Re-write the IndexOf code without the use of the pcmpestri instruction, only 
>> using AVX2 instructions.  This change accelerates String.IndexOf on average 
>> 1.3x for AVX2.  The benchmark numbers:
>> 
>> 
>> Benchmark   Score
>> Latest  
>> StringIndexOf.advancedWithMediumSub   343.573317.934 
>> 0.925375393x
>> StringIndexOf.advancedWithShortSub11039.081  1053.96 
>> 1.014319384x
>> StringIndexOf.advancedWithShortSub255.828110.541 
>> 1.980027943x
>> StringIndexOf.constantPattern9.361   11.906  
>> 1.271872663x
>> StringIndexOf.searchCharLongSuccess  4.216   4.218   
>> 1.000474383x
>> StringIndexOf.searchCharMediumSuccess3.133   3.216   
>> 1.02649218x
>> StringIndexOf.searchCharShortSuccess 3.763.761   
>> 1.000265957x
>> StringIndexOf.success9.186   
>> 9.713   1.057369911x
>> StringIndexOf.successBig   14.34146.343  
>> 3.231504079x
>> StringIndexOfChar.latin1_AVX2_String   6220.918  12154.52
>> 1.953814533x
>> StringIndexOfChar.latin1_AVX2_char 5503.556  5540.044
>> 1.006629895x
>> StringIndexOfChar.latin1_SSE4_String   6978.854  6818.689
>> 0.977049957x
>> StringIndexOfChar.latin1_SSE4_char 5657.499  5474.624
>> 0.967675646x
>> StringIndexOfChar.latin1_Short_String  7132.541  
>> 6863.3590.962260014x
>> StringIndexOfChar.latin1_Short_char  16013.389 16162.437 
>> 1.009307711x
>> StringIndexOfChar.latin1_mixed_String  7386.12314771.622 
>> 1.15517x
>> StringIndexOfChar.latin1_mixed_char9901.671  9782.245
>> 0.987938803
>
> Scott Gibbons has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Move assert to where it's actually important.

Let me test the latest version before integration.

-

PR Comment: https://git.openjdk.org/jdk/pull/16753#issuecomment-2138303300

Re: RFR: 8333103: Re-examine the console provider loading

[Chen Liang]

On Wed, 29 May 2024 19:51:36 GMT, Naoto Sato  wrote:

> There is an initialization code in `Console` class that searches for the 
> Console implementations. Refactoring the init code not to use lambda/stream 
> would reduce the (initial) number of loaded classes by about 100 for 
> java.base implementations. This would become relevant when the java.io.IO 
> (JEP 477) uses Console as the underlying framework.

src/java.base/share/classes/java/io/Console.java line 673:

> 671: return new ProxyingConsole(jc);
> 672: }
> 673: break;

Suggestion:


The original `findAny` is only after `filter(Objects::nonNull)` meaning we 
don't return `null` on a null `jcp.console` result.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19467#discussion_r1619495420

Re: RFR: 8333103: Re-examine the console provider loading

[Claes Redestad]

On Wed, 29 May 2024 19:51:36 GMT, Naoto Sato  wrote:

> There is an initialization code in `Console` class that searches for the 
> Console implementations. Refactoring the init code not to use lambda/stream 
> would reduce the (initial) number of loaded classes by about 100 for 
> java.base implementations. This would become relevant when the java.io.IO 
> (JEP 477) uses Console as the underlying framework.

Marked as reviewed by redestad (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/19467#pullrequestreview-2086537783

Re: Question on Lambda function

[Chen Liang]

Hi Zhengyu,
What is your usage pattern of these single-abstract-method implementations?
Since it sounds like you are creating a lot of them, are you storing them
in collections?
If you are keeping a lot of them in collection (say, as event handlers),
you may try to use `MethodHandleProxies.asInterfaceInstance` as a temporary
workaround on JDK 22 and higher (older version uses Proxy, which has
horrible invocation performance).
If you are on older versions from 15 to 21, unfortunately you might have to
write a hidden class for the same purpose or use an existing library. One
library that might be useful is https://github.com/LanternPowered/Lmbda
that effectively generates unloadable hidden classes, but its 3.x builds
are not maven central so you have to build yourself.

- Chen

On Wed, May 29, 2024 at 3:35 PM Zhengyu Gu 
wrote:

> Hi Chen,
>
>
>
> Thanks for the insights.
>
>
>
> We did refactor our code to avoid using LambdaMetaFactory,metafactory()
> directly.
>
>
>
> With increasing use of Lambdas, in our applications and libraries, the
> metaspace impact becomes a concern. If current implementation (not able to
> unload unused Lambda classes) here to stay, we must come up with a coding
> guideline to avoid excessive creation of Lambda classes,  any pointers or
> suggestions would be greatly appreciated.
>
>
>
> Best,
>
>
>
> -Zhengyu
>
>
>
> *From: *Chen Liang 
> *Date: *Wednesday, May 29, 2024 at 2:43 PM
> *To: *Zhengyu Gu 
> *Cc: *core-libs-dev@openjdk.org 
> *Subject: *Re: Question on Lambda function
> *[External Email]*
>
>
> --
>
> Hi Gu,
>
> CallSite is specific to each invokedynamic instruction instead of each
> InvokeDynamic constant pool entry:
> https://docs.oracle.com/javase/specs/jvms/se22/html/jvms-6.html#jvms-6.5.invokedynamic
>
> And the linking is done by MethodHandleNatives.linkCallSite if you want to
> follow the Java implementation code.
>
> For why the lambda in the loop is constant, it's a feature from
> InnerClassLambdaMetafactory:
> https://github.com/openjdk/jdk/blob/c8eea59f508158075382079316cf0990116ff98e/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java#L236
>
> When the lambda is non-capturing, the bootstrap method
> LambdaMetafactory.metafactory will eagerly create a singleton instance and
> return this singleton in the indy instruction.
>
>
>
> Also, your metaspace pressure might be caused by the fact that Lambda
> classes (not instances) are no longer eagerly unloaded; see
> https://github.com/openjdk/jdk/pull/12493 and
> https://bugs.openjdk.org/browse/JDK-8302154. You are recommended to
> create your own facility to create hidden classes in Java 17 instead of
> continue to use LambdaMetafactory explicitly in code.
>
>
>
> Regards,
>
> Chen Liang
>
>
>
> On Wed, May 29, 2024 at 12:53 PM Zhengyu Gu 
> wrote:
>
> Hello Lambda experts,
>
>
>
> Since we upgraded JDK from 11 to 17, we’re experiencing metaspace
> pressure, largely due to Lambda class implementation changes.
>
>
>
> There’s a scenario (see attached test case),  that is especially puzzled
> me, hopefully, you can share some insights.
>
>
>
> In this test case, there is only one Lambda class is created inside the
> loop, but each one for the same functions outside loop.
>
>
>
> Example output:
>
>
>
> 0: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
>
> testMethod() called
>
> 1: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
>
> testMethod() called
>
> 2: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
>
> testMethod() called
>
> 3: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
>
> testMethod() called
>
> 4: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
>
> testMethod() called
>
>
>
> ….
>
>
>
> Outside loop1, Func =  LambdaFunc$$Lambda/0x1f8c4c58@402f32ff
>
> testMethod() called
>
> Outside loop2 Func = LambdaFunc$$Lambda/0x1f8d1000@5ae9a829
>
> testMethod() called
>
> Outside loop3 Func = LambdaFunc$$Lambda/0x1f8d1238@548b7f67
>
> testMethod() called
>
>
>
> And jcmd also confirmed there were 4 Lambda classes created:
>
>
>
>   49: CLD 0x6134cb50: "app" instance of
> jdk.internal.loader.ClassLoaders$AppClassLoader
>
>   Loaded classes:
>
>  1:LambdaFunc$$Lambda/0x1f8d1238
>
>  2:LambdaFunc$$Lambda/0x1f8d1000
>
>  3:LambdaFunc$$Lambda/0x1f8c4c58
>
>  4:LambdaFunc$$Lambda/0x1f8c4a20
>
>  5:LambdaFunc
>
>
>
> Looking into bytecode, all four call sites have the same invokedynamic
> bytecode (invokedynamic #7,  0  // InvokeDynamic
> #0:run:()Ljava/lang/Runnable; ) and the first invokedynamic bytecode is
> inside the loop.
>
>
>
> But when I ran the program with -XX:+TraceBytecodes, it seems that the
> first invokedynamic was hoisted and result was used in the subsequence loop.
>
>
>
> Can anyone explain where this magic happens?  If the magic can apply to
> the instances outside the 

Re: Question on Lambda function

[Zhengyu Gu]

Hi Chen,

Thanks for the insights.

We did refactor our code to avoid using LambdaMetaFactory,metafactory() 
directly.

With increasing use of Lambdas, in our applications and libraries, the 
metaspace impact becomes a concern. If current implementation (not able to 
unload unused Lambda classes) here to stay, we must come up with a coding 
guideline to avoid excessive creation of Lambda classes,  any pointers or 
suggestions would be greatly appreciated.

Best,

-Zhengyu

From: Chen Liang 
Date: Wednesday, May 29, 2024 at 2:43 PM
To: Zhengyu Gu 
Cc: core-libs-dev@openjdk.org 
Subject: Re: Question on Lambda function
[External Email]


Hi Gu,
CallSite is specific to each invokedynamic instruction instead of each 
InvokeDynamic constant pool entry: 
https://docs.oracle.com/javase/specs/jvms/se22/html/jvms-6.html#jvms-6.5.invokedynamic
And the linking is done by MethodHandleNatives.linkCallSite if you want to 
follow the Java implementation code.
For why the lambda in the loop is constant, it's a feature from 
InnerClassLambdaMetafactory: 
https://github.com/openjdk/jdk/blob/c8eea59f508158075382079316cf0990116ff98e/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java#L236
When the lambda is non-capturing, the bootstrap method 
LambdaMetafactory.metafactory will eagerly create a singleton instance and 
return this singleton in the indy instruction.

Also, your metaspace pressure might be caused by the fact that Lambda classes 
(not instances) are no longer eagerly unloaded; see 
https://github.com/openjdk/jdk/pull/12493 and 
https://bugs.openjdk.org/browse/JDK-8302154.
 You are recommended to create your own facility to create hidden classes in 
Java 17 instead of continue to use LambdaMetafactory explicitly in code.

Regards,
Chen Liang

On Wed, May 29, 2024 at 12:53 PM Zhengyu Gu 
mailto:zhengyu...@servicenow.com>> wrote:
Hello Lambda experts,

Since we upgraded JDK from 11 to 17, we’re experiencing metaspace pressure, 
largely due to Lambda class implementation changes.

There’s a scenario (see attached test case),  that is especially puzzled me, 
hopefully, you can share some insights.

In this test case, there is only one Lambda class is created inside the loop, 
but each one for the same functions outside loop.

Example output:

0: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called
1: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called
2: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called
3: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called
4: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called

….

Outside loop1, Func =  LambdaFunc$$Lambda/0x1f8c4c58@402f32ff
testMethod() called
Outside loop2 Func = LambdaFunc$$Lambda/0x1f8d1000@5ae9a829
testMethod() called
Outside loop3 Func = LambdaFunc$$Lambda/0x1f8d1238@548b7f67
testMethod() called

And jcmd also confirmed there were 4 Lambda classes created:

  49: CLD 0x6134cb50: "app" instance of 
jdk.internal.loader.ClassLoaders$AppClassLoader
  Loaded classes:
 1:LambdaFunc$$Lambda/0x1f8d1238
 2:LambdaFunc$$Lambda/0x1f8d1000
 3:LambdaFunc$$Lambda/0x1f8c4c58
 4:LambdaFunc$$Lambda/0x1f8c4a20
 5:LambdaFunc

Looking into bytecode, all four call sites have the same invokedynamic bytecode 
(invokedynamic #7,  0  // InvokeDynamic 
#0:run:()Ljava/lang/Runnable; ) and the first invokedynamic bytecode is inside 
the loop.

But when I ran the program with -XX:+TraceBytecodes, it seems that the first 
invokedynamic was hoisted and result was used in the subsequence loop.

Can anyone explain where this magic happens?  If the magic can apply to the 
instances outside the loop, so that only one Lambda class is created?


Thank you for your time and expertise,

-Zhengyu

RFR: 8333103: Re-examine the console provider loading

[Naoto Sato]

There is an initialization code in `Console` class that searches for the 
Console implementations. Refactoring the init code not to use lambda/stream 
would reduce the (initial) number of loaded classes by about 100 for java.base 
implementations. This would become relevant when the java.io.IO (JEP 477) uses 
Console as the underlying framework.

-

Commit messages:
 - initial commit

Changes: https://git.openjdk.org/jdk/pull/19467/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=19467=00
  Issue: https://bugs.openjdk.org/browse/JDK-8333103
  Stats: 20 lines in 1 file changed: 4 ins; 0 del; 16 mod
  Patch: https://git.openjdk.org/jdk/pull/19467.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19467/head:pull/19467

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

Re: RFR: 8332614: Type-checked ConstantPool.entryByIndex and ClassReader.readEntryOrNull [v6]

[Chen Liang]

> I propose to add type-checked ConstantPool.entryByIndex and 
> ClassReader.readEntryOrNull taking an extra Class parameter, which throws 
> ConstantPoolException instead of ClassCastException on type mismatch, which 
> can happen to malformed ClassFiles.
> 
> Requesting a review from @asotona. Thanks!
> 
> Proposal on mailing list: 
> https://mail.openjdk.org/pipermail/classfile-api-dev/2024-May/000512.html

Chen Liang 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 10 additional commits since the 
last revision:

 - Add test to validate ClassReader behavior
 - Merge branch 'master' of https://github.com/openjdk/jdk into 
feature/entry-by-type
 - Null-check the entry class eagerly, avoid returning null or throwing IAE
 - Remove redundant import
 - Use switch
 - Merge branch 'master' of https://github.com/openjdk/jdk into 
feature/entry-by-type
 - Merge branch 'master' of https://github.com/openjdk/jdk into 
feature/entry-by-type
 - Merge branch 'master' of https://github.com/openjdk/jdk into 
feature/entry-by-type
 - Use constants, beautify code
 - 8332614: Type-checked ConstantPool.entryByIndex and 
ClassReader.readEntryOrNull

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19330/files
  - new: https://git.openjdk.org/jdk/pull/19330/files/ec56ce98..e9021f24

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19330=05
 - incr: https://webrevs.openjdk.org/?repo=jdk=19330=04-05

  Stats: 1876 lines in 41 files changed: 1109 ins; 498 del; 269 mod
  Patch: https://git.openjdk.org/jdk/pull/19330.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19330/head:pull/19330

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

Re: Question on Lambda function

[Chen Liang]

Hi Gu,
CallSite is specific to each invokedynamic instruction instead of each
InvokeDynamic constant pool entry:
https://docs.oracle.com/javase/specs/jvms/se22/html/jvms-6.html#jvms-6.5.invokedynamic
And the linking is done by MethodHandleNatives.linkCallSite if you want to
follow the Java implementation code.
For why the lambda in the loop is constant, it's a feature from
InnerClassLambdaMetafactory:
https://github.com/openjdk/jdk/blob/c8eea59f508158075382079316cf0990116ff98e/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java#L236
When the lambda is non-capturing, the bootstrap method
LambdaMetafactory.metafactory will eagerly create a singleton instance and
return this singleton in the indy instruction.

Also, your metaspace pressure might be caused by the fact that Lambda
classes (not instances) are no longer eagerly unloaded; see
https://github.com/openjdk/jdk/pull/12493 and
https://bugs.openjdk.org/browse/JDK-8302154. You are recommended to create
your own facility to create hidden classes in Java 17 instead of continue
to use LambdaMetafactory explicitly in code.

Regards,
Chen Liang

On Wed, May 29, 2024 at 12:53 PM Zhengyu Gu 
wrote:

> Hello Lambda experts,
>
>
>
> Since we upgraded JDK from 11 to 17, we’re experiencing metaspace
> pressure, largely due to Lambda class implementation changes.
>
>
>
> There’s a scenario (see attached test case),  that is especially puzzled
> me, hopefully, you can share some insights.
>
>
>
> In this test case, there is only one Lambda class is created inside the
> loop, but each one for the same functions outside loop.
>
>
>
> Example output:
>
>
>
> 0: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
>
> testMethod() called
>
> 1: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
>
> testMethod() called
>
> 2: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
>
> testMethod() called
>
> 3: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
>
> testMethod() called
>
> 4: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
>
> testMethod() called
>
>
>
> ….
>
>
>
> Outside loop1, Func =  LambdaFunc$$Lambda/0x1f8c4c58@402f32ff
>
> testMethod() called
>
> Outside loop2 Func = LambdaFunc$$Lambda/0x1f8d1000@5ae9a829
>
> testMethod() called
>
> Outside loop3 Func = LambdaFunc$$Lambda/0x1f8d1238@548b7f67
>
> testMethod() called
>
>
>
> And jcmd also confirmed there were 4 Lambda classes created:
>
>
>
>   49: CLD 0x6134cb50: "app" instance of
> jdk.internal.loader.ClassLoaders$AppClassLoader
>
>   Loaded classes:
>
>  1:LambdaFunc$$Lambda/0x1f8d1238
>
>  2:LambdaFunc$$Lambda/0x1f8d1000
>
>  3:LambdaFunc$$Lambda/0x1f8c4c58
>
>  4:LambdaFunc$$Lambda/0x1f8c4a20
>
>  5:LambdaFunc
>
>
>
> Looking into bytecode, all four call sites have the same invokedynamic
> bytecode (invokedynamic #7,  0  // InvokeDynamic
> #0:run:()Ljava/lang/Runnable; ) and the first invokedynamic bytecode is
> inside the loop.
>
>
>
> But when I ran the program with -XX:+TraceBytecodes, it seems that the
> first invokedynamic was hoisted and result was used in the subsequence loop.
>
>
>
> Can anyone explain where this magic happens?  If the magic can apply to
> the instances outside the loop, so that only one Lambda class is created?
>
>
>
>
>
> Thank you for your time and expertise,
>
>
>
> -Zhengyu
>
>
>
>
>
>
>

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v9]

[Doug Lea]

> This set of changes address causes of poor utilization with small numbers of 
> cores due to overly aggressive contention avoidance. A number of further 
> adjustments were needed to still avoid most contention effects in deployments 
> with large numbers of cores

Doug Lea 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 43 additional commits since the 
last revision:

 - Merge branch 'openjdk:master' into JDK-8322732
 - Address review comments
 - Merge branch 'openjdk:master' into JDK-8322732
 - Add test for utilization with interdependent tasks
 - Un-misplace onSpinWait call
 - Adjust control flow
 - Reduce memory stalls
 - Merge branch 'openjdk:master' into JDK-8322732
 - More performance tradoffs
 - Address review comments
 - ... and 33 more: https://git.openjdk.org/jdk/compare/281b7e15...5e44de45

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19131/files
  - new: https://git.openjdk.org/jdk/pull/19131/files/e5c4a556..5e44de45

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19131=08
 - incr: https://webrevs.openjdk.org/?repo=jdk=19131=07-08

  Stats: 617 lines in 10 files changed: 419 ins; 182 del; 16 mod
  Patch: https://git.openjdk.org/jdk/pull/19131.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19131/head:pull/19131

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

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v7]

[Doug Lea]

On Wed, 29 May 2024 14:09:51 GMT, Viktor Klang  wrote:

>> Doug Lea 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 41 additional commits 
>> since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into JDK-8322732
>>  - Add test for utilization with interdependent tasks
>>  - Un-misplace onSpinWait call
>>  - Adjust control flow
>>  - Reduce memory stalls
>>  - Merge branch 'openjdk:master' into JDK-8322732
>>  - More performance tradoffs
>>  - Address review comments
>>  - Merge branch 'openjdk:master' into JDK-8322732
>>  - Repack some fields; adjust control flow
>>  - ... and 31 more: https://git.openjdk.org/jdk/compare/ae4752d2...cf5fe55c
>
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1969:
> 
>> 1967: else if ((e & SHUTDOWN) == 0)
>> 1968: return 0;
>> 1969: else if (compareAndSetCtl(c, c) && casRunState(e, e | 
>> STOP))
> 
> is the `compareAndSetCtl(c, c)` needed for the read+write fence if ctl == c?

yes.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19131#discussion_r1619209374

Re: RFR: 8320448: Accelerate IndexOf using AVX2 [v47]

[Scott Gibbons]

On Tue, 28 May 2024 23:52:27 GMT, Scott Gibbons  wrote:

>> Re-write the IndexOf code without the use of the pcmpestri instruction, only 
>> using AVX2 instructions.  This change accelerates String.IndexOf on average 
>> 1.3x for AVX2.  The benchmark numbers:
>> 
>> 
>> Benchmark   Score
>> Latest  
>> StringIndexOf.advancedWithMediumSub   343.573317.934 
>> 0.925375393x
>> StringIndexOf.advancedWithShortSub11039.081  1053.96 
>> 1.014319384x
>> StringIndexOf.advancedWithShortSub255.828110.541 
>> 1.980027943x
>> StringIndexOf.constantPattern9.361   11.906  
>> 1.271872663x
>> StringIndexOf.searchCharLongSuccess  4.216   4.218   
>> 1.000474383x
>> StringIndexOf.searchCharMediumSuccess3.133   3.216   
>> 1.02649218x
>> StringIndexOf.searchCharShortSuccess 3.763.761   
>> 1.000265957x
>> StringIndexOf.success9.186   
>> 9.713   1.057369911x
>> StringIndexOf.successBig   14.34146.343  
>> 3.231504079x
>> StringIndexOfChar.latin1_AVX2_String   6220.918  12154.52
>> 1.953814533x
>> StringIndexOfChar.latin1_AVX2_char 5503.556  5540.044
>> 1.006629895x
>> StringIndexOfChar.latin1_SSE4_String   6978.854  6818.689
>> 0.977049957x
>> StringIndexOfChar.latin1_SSE4_char 5657.499  5474.624
>> 0.967675646x
>> StringIndexOfChar.latin1_Short_String  7132.541  
>> 6863.3590.962260014x
>> StringIndexOfChar.latin1_Short_char  16013.389 16162.437 
>> 1.009307711x
>> StringIndexOfChar.latin1_mixed_String  7386.12314771.622 
>> 1.15517x
>> StringIndexOfChar.latin1_mixed_char9901.671  9782.245
>> 0.987938803
>
> Scott Gibbons has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Move assert to where it's actually important.

Thank you all for the comments.  If there are no objections, I'll integrate 
these fixes tomorrow morning.

I've run tier1-3 tests with the appropriate options on my machine with no 
errors, so my confidence is high.

-

PR Comment: https://git.openjdk.org/jdk/pull/16753#issuecomment-2137861781

Question on Lambda function

[Zhengyu Gu]

Hello Lambda experts,

Since we upgraded JDK from 11 to 17, we’re experiencing metaspace pressure, 
largely due to Lambda class implementation changes.

There’s a scenario (see attached test case),  that is especially puzzled me, 
hopefully, you can share some insights.

In this test case, there is only one Lambda class is created inside the loop, 
but each one for the same functions outside loop.

Example output:

0: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called
1: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called
2: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called
3: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called
4: Func =  LambdaFunc$$Lambda/0x1f8c4a20@4de8b406
testMethod() called

….

Outside loop1, Func =  LambdaFunc$$Lambda/0x1f8c4c58@402f32ff
testMethod() called
Outside loop2 Func = LambdaFunc$$Lambda/0x1f8d1000@5ae9a829
testMethod() called
Outside loop3 Func = LambdaFunc$$Lambda/0x1f8d1238@548b7f67
testMethod() called

And jcmd also confirmed there were 4 Lambda classes created:

  49: CLD 0x6134cb50: "app" instance of 
jdk.internal.loader.ClassLoaders$AppClassLoader
  Loaded classes:
 1:LambdaFunc$$Lambda/0x1f8d1238
 2:LambdaFunc$$Lambda/0x1f8d1000
 3:LambdaFunc$$Lambda/0x1f8c4c58
 4:LambdaFunc$$Lambda/0x1f8c4a20
 5:LambdaFunc

Looking into bytecode, all four call sites have the same invokedynamic bytecode 
(invokedynamic #7,  0  // InvokeDynamic 
#0:run:()Ljava/lang/Runnable; ) and the first invokedynamic bytecode is inside 
the loop.

But when I ran the program with -XX:+TraceBytecodes, it seems that the first 
invokedynamic was hoisted and result was used in the subsequence loop.

Can anyone explain where this magic happens?  If the magic can apply to the 
instances outside the loop, so that only one Lambda class is created?


Thank you for your time and expertise,

-Zhengyu





LambdaFunc.java
Description: LambdaFunc.java

Re: RFR: 8332826: Make hashCode methods in ArraysSupport friendlier [v2]

[Pavel Rappo]

On Wed, 29 May 2024 15:50:05 GMT, Pavel Rappo  wrote:

>> @cl4es, here are some results from my machine (macosx-aarch64):
>> 
>> Name   (size) Cnt BaseError  TestError  
>> Unit  Change
>> ArraysHashCode.bytes1  150.715 ±  0.004 0.725 ±  0.029 
>> ns/op   0.99x (p = 0.182 )
>> ArraysHashCode.bytes   10  153.753 ±  0.024 3.747 ±  0.011 
>> ns/op   1.00x (p = 0.322 )
>> ArraysHashCode.bytes  100  15   69.731 ±  0.15769.737 ±  0.092 
>> ns/op   1.00x (p = 0.891 )
>> ArraysHashCode.bytes1  15 9369.386 ±  1.449  9372.008 ±  6.678 
>> ns/op   1.00x (p = 0.133 )
>> ArraysHashCode.chars1  150.719 ±  0.024 0.734 ±  0.024 
>> ns/op   0.98x (p = 0.076 )
>> ArraysHashCode.chars   10  153.744 ±  0.005 3.746 ±  0.004 
>> ns/op   1.00x (p = 0.308 )
>> ArraysHashCode.chars  100  15   69.741 ±  0.11269.714 ±  0.044 
>> ns/op   1.00x (p = 0.365 )
>> ArraysHashCode.chars1  15 9367.123 ±  5.320  9371.325 ±  6.407 
>> ns/op   1.00x (p = 0.046 )
>> ArraysHashCode.ints 1  150.711 ±  0.013 0.706 ±  0.006 
>> ns/op   1.01x (p = 0.137 )
>> ArraysHashCode.ints10  153.750 ±  0.002 3.752 ±  0.004 
>> ns/op   1.00x (p = 0.283 )
>> ArraysHashCode.ints   100  15   69.753 ±  0.08669.711 ±  0.016 
>> ns/op   1.00x (p = 0.065 )
>> ArraysHashCode.ints 1  15 9376.225 ±  5.845  9376.218 ± 12.181 
>> ns/op   1.00x (p = 0.999 )
>> ArraysHashCode.multibytes   1  150.741 ±  0.001 0.740 ±  0.001 
>> ns/op   1.00x (p = 0.038 )
>> ArraysHashCode.multibytes  10  152.737 ±  0.001 2.826 ±  0.136 
>> ns/op   0.97x (p = 0.017 )
>> ArraysHashCode.multibytes 100  15   32.202 ±  0.05932.153 ±  0.006 
>> ns/op   1.00x (p = 0.004*)
>> ArraysHashCode.multibytes   1  15 4922.740 ± 25.590  4921.468 ±  7.372 
>> ns/op   1.00x (p = 0.846 )
>> ArraysHashCode.multichars   1  150.740 ±  0.005 0.740 ±  0.000 
>> ns/op   1.00x (p = 0.996 )
>> ArraysHashCode.multichars  10  152.732 ±  0.002 2.737 ±  0.003 
>> ns/op   1.00x (p = 0.000*)
>> ArraysHashCode.multichars 100  15   32.109 ±  0.01732.182 ±  0.028 
>> ns/op   1.00x (p = 0.000*)
>> ArraysHashCode.multichars   1  15 4925.750 ± 46.366  4930.684 ± 26.001 
>> ns/op   1.00x (p = 0.704 )
>> ArraysHashCode.multiints1  150.740 ±  0.000 0.739 ±  0.000 
>> ns/op   1.00x (p = 0.000*)
>> ArraysHashCode.multiints   10  152.919 ±  0.002 2.953 ±  0.059 
>> ns/op   0.99x (p = 0.033 )
>> ArraysHashCode.multiints  100  15   32.140 ±  ...
>
> Just to clarify, the above comparison is between master and 
> https://github.com/openjdk/jdk/pull/19414/commits/534ff367bc50ec4150e4b206ce7203c7ff9f5cad.

> For clarity, if you think it helps:

I did that and a little bit more in 
https://github.com/openjdk/jdk/pull/19414/commits/534ff367bc50ec4150e4b206ce7203c7ff9f5cad.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19414#discussion_r1619130669

Re: RFR: 8332826: Make hashCode methods in ArraysSupport friendlier [v2]

[Pavel Rappo]

On Wed, 29 May 2024 12:53:42 GMT, Pavel Rappo  wrote:

>> src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 252:
>> 
>>> 250: return switch (length) {
>>> 251: case 0 -> initialValue;
>>> 252: case 1 -> 31 * initialValue + (int) a[fromIndex];
>> 
>> Suggestion:
>> 
>> case 1 -> 31 * initialValue + (int) a[fromIndex]; // sign 
>> extension
>
> To be honest, I don't think that this cast is needed. A better solution than 
> to add a comment would be to delete all `(int)` casts from new `hashCode*` 
> methods of `ArraysSupport`.
> 
> Those `(int)` casts migrated from `hashCode` methods of `Arrays` where there 
> were used if neither of two `+` operands were of type `int`. But in 
> `ArraysSupport` it's no longer the case: `31 * initialValue` is always `int` 
> because `initialValue` is. So, `a[fromIndex]` is promoted to `int` by the 
> virtue of 
> https://docs.oracle.com/javase/specs/jls/se22/html/jls-5.html#jls-5.6.
> 
> For more confidence, consider that the `private static int hashCode` methods 
> (implementation) of `ArraysSupport` do not have those casts.

Removed casts in 
https://github.com/openjdk/jdk/pull/19414/commits/d8dabc68c4264520fd6c57c949049f2ab5c8e0ec.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19414#discussion_r1619127224

Re: RFR: 8332826: Make hashCode methods in ArraysSupport friendlier [v2]

[Pavel Rappo]

On Wed, 29 May 2024 12:44:45 GMT, Pavel Rappo  wrote:

>> I don't care as long as microbenchmarks don't get a hiccup.
>
> @cl4es, here are some results from my machine (macosx-aarch64):
> 
> Name   (size) Cnt BaseError  TestError  
> Unit  Change
> ArraysHashCode.bytes1  150.715 ±  0.004 0.725 ±  0.029 
> ns/op   0.99x (p = 0.182 )
> ArraysHashCode.bytes   10  153.753 ±  0.024 3.747 ±  0.011 
> ns/op   1.00x (p = 0.322 )
> ArraysHashCode.bytes  100  15   69.731 ±  0.15769.737 ±  0.092 
> ns/op   1.00x (p = 0.891 )
> ArraysHashCode.bytes1  15 9369.386 ±  1.449  9372.008 ±  6.678 
> ns/op   1.00x (p = 0.133 )
> ArraysHashCode.chars1  150.719 ±  0.024 0.734 ±  0.024 
> ns/op   0.98x (p = 0.076 )
> ArraysHashCode.chars   10  153.744 ±  0.005 3.746 ±  0.004 
> ns/op   1.00x (p = 0.308 )
> ArraysHashCode.chars  100  15   69.741 ±  0.11269.714 ±  0.044 
> ns/op   1.00x (p = 0.365 )
> ArraysHashCode.chars1  15 9367.123 ±  5.320  9371.325 ±  6.407 
> ns/op   1.00x (p = 0.046 )
> ArraysHashCode.ints 1  150.711 ±  0.013 0.706 ±  0.006 
> ns/op   1.01x (p = 0.137 )
> ArraysHashCode.ints10  153.750 ±  0.002 3.752 ±  0.004 
> ns/op   1.00x (p = 0.283 )
> ArraysHashCode.ints   100  15   69.753 ±  0.08669.711 ±  0.016 
> ns/op   1.00x (p = 0.065 )
> ArraysHashCode.ints 1  15 9376.225 ±  5.845  9376.218 ± 12.181 
> ns/op   1.00x (p = 0.999 )
> ArraysHashCode.multibytes   1  150.741 ±  0.001 0.740 ±  0.001 
> ns/op   1.00x (p = 0.038 )
> ArraysHashCode.multibytes  10  152.737 ±  0.001 2.826 ±  0.136 
> ns/op   0.97x (p = 0.017 )
> ArraysHashCode.multibytes 100  15   32.202 ±  0.05932.153 ±  0.006 
> ns/op   1.00x (p = 0.004*)
> ArraysHashCode.multibytes   1  15 4922.740 ± 25.590  4921.468 ±  7.372 
> ns/op   1.00x (p = 0.846 )
> ArraysHashCode.multichars   1  150.740 ±  0.005 0.740 ±  0.000 
> ns/op   1.00x (p = 0.996 )
> ArraysHashCode.multichars  10  152.732 ±  0.002 2.737 ±  0.003 
> ns/op   1.00x (p = 0.000*)
> ArraysHashCode.multichars 100  15   32.109 ±  0.01732.182 ±  0.028 
> ns/op   1.00x (p = 0.000*)
> ArraysHashCode.multichars   1  15 4925.750 ± 46.366  4930.684 ± 26.001 
> ns/op   1.00x (p = 0.704 )
> ArraysHashCode.multiints1  150.740 ±  0.000 0.739 ±  0.000 
> ns/op   1.00x (p = 0.000*)
> ArraysHashCode.multiints   10  152.919 ±  0.002 2.953 ±  0.059 
> ns/op   0.99x (p = 0.033 )
> ArraysHashCode.multiints  100  15   32.140 ±  0.01132.094 ±  0.004 
> ns/op   1.00x (p = 0.000*)
> ...

Just to clarify, the above comparison is between master and 
https://github.com/openjdk/jdk/pull/19414/commits/534ff367bc50ec4150e4b206ce7203c7ff9f5cad.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19414#discussion_r1619125150

Re: RFR: 8332826: Make hashCode methods in ArraysSupport friendlier [v3]

[Pavel Rappo]

> Please review this PR, which supersedes a now withdrawn 
> https://github.com/openjdk/jdk/pull/14831.
> 
> This PR replaces `ArraysSupport.vectorizedHashCode` with a set of more 
> user-friendly methods. Here's a summary:
> 
> - Made the operand constants (i.e. `T_BOOLEAN` and friends) and the 
> `vectorizedHashCode` method private
> 
> - Made the `vectorizedHashCode` method private, but didn't rename it. 
> Renaming would dramatically increase this PR review cost, because that 
> method's name is used by a lot of VM code. On a bright side, since the method 
> is now private, it's no longer callable by clients of `ArraysSupport`, thus a 
> problem of an inaccurate name is less severe.
> 
> - Made the `ArraysSupport.utf16HashCode` method private
> 
> - Moved tiny cases (i.e. 0, 1, 2) to `ArraysSupport`

Pavel Rappo has updated the pull request incrementally with three additional 
commits since the last revision:

 - Update copyright years
   
   Note: any commit hashes below might be outdated due to subsequent
   history rewriting (e.g. git rebase).
   
+ update src/java.base/share/classes/java/lang/CharacterName.java due to 
4ed451d691c
+ update src/java.base/share/classes/java/lang/StringLatin1.java due to 
4ed451d691c
+ update src/java.base/share/classes/java/nio/Heap-X-Buffer.java.template 
due to 4ed451d691c
+ update 
src/java.base/share/classes/jdk/internal/classfile/impl/AbstractPoolEntry.java 
due to 4ed451d691c
+ update src/java.base/share/classes/sun/security/util/DerValue.java due to 
4ed451d691c
+ update src/java.base/unix/classes/sun/nio/fs/UnixPath.java due to 
4ed451d691c
+ update test/hotspot/jtreg/compiler/intrinsics/TestArraysHashCode.java due 
to 4ed451d691c
 - Drop redundant (int) cast
 - Use Byte.toUnsignedInt not & 0xff

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19414/files
  - new: https://git.openjdk.org/jdk/pull/19414/files/adc7557d..53d4ed09

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19414=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=19414=01-02

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

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

Re: RFR: 8331865: Consolidate size and alignment checks in LayoutPath

[Maurizio Cimadamore]

On Thu, 16 May 2024 14:11:47 GMT, Maurizio Cimadamore  
wrote:

> > > Do we have any performance tests available to see if there are any 
> > > potential impacts?
> > 
> > 
> > I've run all micro benchmarks whose name match `LoopOver*`. No regression 
> > was found. Few benchmarks seems a tad faster, but the percentages are so 
> > tiny that I'm sure it has nothing to do with the patch. I compared results 
> > before/after using [JMH visualizer](https://jmh.morethan.io/).
> 
> Results here: https://cr.openjdk.org/~mcimadamore/jdk/8331865/ Unfortunately 
> I can't seem to be able to upload the results in the visualizer (which would 
> be handy, so I could share a link to the results). Not sure if it's an issue 
> in the visualizer, or in the cr server itself. But, it is possible to at 
> least download the above results locally, and then upload them to the 
> visualizer tool.

Here's a link to the results:
https://jmh.morethan.io/?sources=https://corsproxy.io/?https://cr.openjdk.org/~mcimadamore/jdk/8331865/loop_over_00_baseline.json,https://corsproxy.io/?https://cr.openjdk.org/~mcimadamore/jdk/8331865/loop_over_01_8331865.json

-

PR Comment: https://git.openjdk.org/jdk/pull/19251#issuecomment-2137713233

Re: RFR: 8331189: Implementation of Scoped Values (Third Preview) [v2]

[Alan Bateman]

> JEP 481 proposes Scoped Values to continue to preview in JDK 23 with one 
> change. The type of the operation parameter of the callWhere method is 
> changed to a new functional interface to avoid having the API throw 
> Exception. With that change, the getWhere (and the corresponding method on 
> Carrier) are no longer needed. The functional interface is not proposed for 
> j.u.function at this time.

Alan Bateman 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:

 - Merge
 - Merge
 - Merge
 - Set JEP number
 - Sync up from loom repo
 - Merge
 - Initial commit

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19136/files
  - new: https://git.openjdk.org/jdk/pull/19136/files/744e8665..b2166e66

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19136=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=19136=00-01

  Stats: 25898 lines in 731 files changed: 16263 ins; 5627 del; 4008 mod
  Patch: https://git.openjdk.org/jdk/pull/19136.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19136/head:pull/19136

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

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v8]

[Doug Lea]

> This set of changes address causes of poor utilization with small numbers of 
> cores due to overly aggressive contention avoidance. A number of further 
> adjustments were needed to still avoid most contention effects in deployments 
> with large numbers of cores

Doug Lea has updated the pull request incrementally with one additional commit 
since the last revision:

  Address review comments

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19131/files
  - new: https://git.openjdk.org/jdk/pull/19131/files/cf5fe55c..e5c4a556

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19131=07
 - incr: https://webrevs.openjdk.org/?repo=jdk=19131=06-07

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

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

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v7]

[Doug Lea]

On Wed, 29 May 2024 14:19:52 GMT, Viktor Klang  wrote:

>> Doug Lea 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 41 additional commits 
>> since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into JDK-8322732
>>  - Add test for utilization with interdependent tasks
>>  - Un-misplace onSpinWait call
>>  - Adjust control flow
>>  - Reduce memory stalls
>>  - Merge branch 'openjdk:master' into JDK-8322732
>>  - More performance tradoffs
>>  - Address review comments
>>  - Merge branch 'openjdk:master' into JDK-8322732
>>  - Repack some fields; adjust control flow
>>  - ... and 31 more: https://git.openjdk.org/jdk/compare/2d88272f...cf5fe55c
>
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2094:
> 
>> 2092: if (((p = w.phase) & IDLE) != 0)
>> 2093: p = awaitWork(w, delay); // block, drop, 
>> or exit
>> 2094: }
> 
> I'm presuming the code below would be equivalent but avoid calculating the 
> delay unless w.phase is IDLE?
> 
> Suggestion:
> 
> if ((p & IDLE) != 0 && ((p = w.phase) & IDLE) != 0) {
> long delay = (((qc & RC_MASK) > 0L) ? 0L :
>   (w.source != INVALID_ID) ? keepAlive :
>   TIMEOUT_SLOP); // minimal delay if 
> cascade
> p = awaitWork(w, delay); // block, drop, or exit
> }

Seems slightly better not to recheck until call, since previous loop would 
usually have caught this if there was actually any work.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19131#discussion_r1619034460

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v7]

[Doug Lea]

On Wed, 29 May 2024 13:26:10 GMT, Viktor Klang  wrote:

>> Doug Lea 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 41 additional commits 
>> since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into JDK-8322732
>>  - Add test for utilization with interdependent tasks
>>  - Un-misplace onSpinWait call
>>  - Adjust control flow
>>  - Reduce memory stalls
>>  - Merge branch 'openjdk:master' into JDK-8322732
>>  - More performance tradoffs
>>  - Address review comments
>>  - Merge branch 'openjdk:master' into JDK-8322732
>>  - Repack some fields; adjust control flow
>>  - ... and 31 more: https://git.openjdk.org/jdk/compare/c056aa9c...cf5fe55c
>
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 912:
> 
>> 910:  * enough to avoid resizing in most tree-structured tasks, but
>> 911:  * larger for external queues where both false-sharing problems
>> 912:  * and the need for resizing are more common..  (Maintenance note:
> 
> Suggestion:
> 
>  * and the need for resizing are more common.  (Maintenance note:

thanks, done.

> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1857:
> 
>> 1855:(LMASK & c);
>> 1856: }
>> 1857: if ((tryTerminate(false, false) & STOP) == 0 && w != null) {
> 
> Suggestion:
> 
> if ((tryTerminate(false, false) & STOP) == 0L && w != null) {

yes.

> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2061:
> 
>> 2059: w.stackPred = (int)pc;   // set ctl stack link
>> 2060: qc = (active & LMASK) | ((pc - RC_UNIT) & UMASK);
>> 2061: if (pc == (pc = compareAndExchangeCtl(pc, qc)))
> 
> If I'm reading this right you might want to add a comment :)
> 
> Suggestion:
> 
> if (pc == (pc = compareAndExchangeCtl(pc, qc))) // consistent

thanks, done.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19131#discussion_r1619023295
PR Review Comment: https://git.openjdk.org/jdk/pull/19131#discussion_r1619024417
PR Review Comment: https://git.openjdk.org/jdk/pull/19131#discussion_r1619025818

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v7]

[Viktor Klang]

On Wed, 29 May 2024 11:33:40 GMT, Doug Lea  wrote:

>> This set of changes address causes of poor utilization with small numbers of 
>> cores due to overly aggressive contention avoidance. A number of further 
>> adjustments were needed to still avoid most contention effects in 
>> deployments with large numbers of cores
>
> Doug Lea 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 41 additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - Add test for utilization with interdependent tasks
>  - Un-misplace onSpinWait call
>  - Adjust control flow
>  - Reduce memory stalls
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - More performance tradoffs
>  - Address review comments
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - Repack some fields; adjust control flow
>  - ... and 31 more: https://git.openjdk.org/jdk/compare/1dc80514...cf5fe55c

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2094:

> 2092: if (((p = w.phase) & IDLE) != 0)
> 2093: p = awaitWork(w, delay); // block, drop, or 
> exit
> 2094: }

I'm presuming the code below would be equivalent but avoid calculating the 
delay unless w.phase is IDLE?

Suggestion:

if ((p & IDLE) != 0 && ((p = w.phase) & IDLE) != 0) {
long delay = (((qc & RC_MASK) > 0L) ? 0L :
  (w.source != INVALID_ID) ? keepAlive :
  TIMEOUT_SLOP); // minimal delay if cascade
p = awaitWork(w, delay); // block, drop, or exit
}

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19131#discussion_r1618980694

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v7]

[Viktor Klang]

On Wed, 29 May 2024 11:33:40 GMT, Doug Lea  wrote:

>> This set of changes address causes of poor utilization with small numbers of 
>> cores due to overly aggressive contention avoidance. A number of further 
>> adjustments were needed to still avoid most contention effects in 
>> deployments with large numbers of cores
>
> Doug Lea 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 41 additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - Add test for utilization with interdependent tasks
>  - Un-misplace onSpinWait call
>  - Adjust control flow
>  - Reduce memory stalls
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - More performance tradoffs
>  - Address review comments
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - Repack some fields; adjust control flow
>  - ... and 31 more: https://git.openjdk.org/jdk/compare/3d109313...cf5fe55c

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2061:

> 2059: w.stackPred = (int)pc;   // set ctl stack link
> 2060: qc = (active & LMASK) | ((pc - RC_UNIT) & UMASK);
> 2061: if (pc == (pc = compareAndExchangeCtl(pc, qc)))

If I'm reading this right you might want to add a comment :)

Suggestion:

if (pc == (pc = compareAndExchangeCtl(pc, qc))) // consistent

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19131#discussion_r1618974704

Re: RFR: 8332161: Test restoring echo in the Console implementation (java.base) [v6]

[Pavel Rappo]

On Tue, 28 May 2024 16:56:18 GMT, Naoto Sato  wrote:

>> This test intends to verify the behavior of JdkConsole for the java.base 
>> module, wrt restoring the echo. The test assumes the internal methods that 
>> sets/gets the echo status of the platform.
>
> Naoto Sato has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 23 commits:
> 
>  - incorporated the same fix from 8332922
>  - Merge branch 'master' into JDK-8332161-restoreEcho-Test
>  - Merge branch 'master' into JDK-8332161-restoreEcho-Test
>  - Merge branch 'JDK-8332084-restoreEcho-readLock' into 
> JDK-8332161-restoreEcho-Test
>  - Added comment for restoreEchoLock
>  - fixing typo
>  - Merge branch 'JDK-8332084-restoreEcho-readLock' into 
> JDK-8332161-restoreEcho-Test
>  - Merge branch 'master' into JDK-8332084-restoreEcho-readLock
>  - leftover typo
>  - get/setEcho() -> echo()
>  - ... and 13 more: https://git.openjdk.org/jdk/compare/b8f2ec90...2477adf4

I might be mistaken, but this looks like a very well written unit test, which 
tests that an implementation flag retains its initial value. This does not look 
like an end-to-end test, which we usually write.

I wonder if we can use `expect` to run two end-to-end tests as follows. The 
first test would set up a console with echo on and then verify that after the 
spawned command completes the echo is still on. The second test would do the 
same for echo off.

The difference between the above approach and the test that this PR has 
proposed is that with the above approach we won't rely on an implementation 
detail, whose relevance to an objectively observed state is non-obvious or 
unknown. Instead, we'll use an OS utility, which makes for a relevant, 
straightforward, and maintainable test.

-

PR Comment: https://git.openjdk.org/jdk/pull/19315#issuecomment-2137515849

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v7]

[Viktor Klang]

On Wed, 29 May 2024 11:33:40 GMT, Doug Lea  wrote:

>> This set of changes address causes of poor utilization with small numbers of 
>> cores due to overly aggressive contention avoidance. A number of further 
>> adjustments were needed to still avoid most contention effects in 
>> deployments with large numbers of cores
>
> Doug Lea 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 41 additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - Add test for utilization with interdependent tasks
>  - Un-misplace onSpinWait call
>  - Adjust control flow
>  - Reduce memory stalls
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - More performance tradoffs
>  - Address review comments
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - Repack some fields; adjust control flow
>  - ... and 31 more: https://git.openjdk.org/jdk/compare/01d11057...cf5fe55c

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1969:

> 1967: else if ((e & SHUTDOWN) == 0)
> 1968: return 0;
> 1969: else if (compareAndSetCtl(c, c) && casRunState(e, e | 
> STOP))

is the `compareAndSetCtl(c, c)` needed for the read+write fence if ctl == c?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19131#discussion_r1618963723

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v7]

[Viktor Klang]

On Wed, 29 May 2024 11:33:40 GMT, Doug Lea  wrote:

>> This set of changes address causes of poor utilization with small numbers of 
>> cores due to overly aggressive contention avoidance. A number of further 
>> adjustments were needed to still avoid most contention effects in 
>> deployments with large numbers of cores
>
> Doug Lea 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 41 additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - Add test for utilization with interdependent tasks
>  - Un-misplace onSpinWait call
>  - Adjust control flow
>  - Reduce memory stalls
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - More performance tradoffs
>  - Address review comments
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - Repack some fields; adjust control flow
>  - ... and 31 more: https://git.openjdk.org/jdk/compare/f715b3e6...cf5fe55c

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1857:

> 1855:(LMASK & c);
> 1856: }
> 1857: if ((tryTerminate(false, false) & STOP) == 0 && w != null) {

Suggestion:

if ((tryTerminate(false, false) & STOP) == 0L && w != null) {

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19131#discussion_r1618956269

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v7]

[Viktor Klang]

On Wed, 29 May 2024 11:33:40 GMT, Doug Lea  wrote:

>> This set of changes address causes of poor utilization with small numbers of 
>> cores due to overly aggressive contention avoidance. A number of further 
>> adjustments were needed to still avoid most contention effects in 
>> deployments with large numbers of cores
>
> Doug Lea 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 41 additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - Add test for utilization with interdependent tasks
>  - Un-misplace onSpinWait call
>  - Adjust control flow
>  - Reduce memory stalls
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - More performance tradoffs
>  - Address review comments
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - Repack some fields; adjust control flow
>  - ... and 31 more: https://git.openjdk.org/jdk/compare/2d55c424...cf5fe55c

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1452:

> 1450:  * Runs the given task, as well as remaining local tasks.
> 1451:  */
> 1452: final void topLevelExec(ForkJoinTask task, int cfg) {

Looks real clean, nice work @DougLea

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19131#discussion_r1618948755

Re: RFR: 8331879: Clean up non-standard use of /// comments in `java.base` [v2]

[Daniel Fuchs]

On Tue, 28 May 2024 22:31:15 GMT, Jonathan Gibbons  wrote:

>> With the advent of JEP 467, `///` comments may be treated as documentation 
>> comments, and may be subject to the recently new `javac` warning about 
>> "dangling doc comments" in unexpected places.
>> 
>> In keeping with the policy to keep the `java.base` module free of all 
>> `javac` warnings, this patch proposes edits to existing uses of `///`.
>> 
>> There are two dominant policies in the proposed changes. 
>> 1. A long horizontal line of `/` is replaced by `//-`
>> 2. A long vertical series of lines beginning `///` is replaced by lines 
>> beginning `//|`.
>> 
>> As with all style changes, I have also tried to honor local usage, for 
>> consistency.
>> 
>> In one place, a pair of comments appeared to contain directives 
>> (`CLOVER:ON`, `CLOVER:OFF`).  I investigated the use of such comments to 
>> determine that the exact form of the comment prefix was not significant. 
>> (Phew!)
>> 
>> 
>> (This PR is informally blocked by JEP 467).
>
> Jonathan Gibbons has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   incorporate review comments

I like `//---` ; +1!

-

PR Comment: https://git.openjdk.org/jdk/pull/19130#issuecomment-2137452920

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v7]

[Viktor Klang]

On Wed, 29 May 2024 11:33:40 GMT, Doug Lea  wrote:

>> This set of changes address causes of poor utilization with small numbers of 
>> cores due to overly aggressive contention avoidance. A number of further 
>> adjustments were needed to still avoid most contention effects in 
>> deployments with large numbers of cores
>
> Doug Lea 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 41 additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - Add test for utilization with interdependent tasks
>  - Un-misplace onSpinWait call
>  - Adjust control flow
>  - Reduce memory stalls
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - More performance tradoffs
>  - Address review comments
>  - Merge branch 'openjdk:master' into JDK-8322732
>  - Repack some fields; adjust control flow
>  - ... and 31 more: https://git.openjdk.org/jdk/compare/9388a872...cf5fe55c

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 912:

> 910:  * enough to avoid resizing in most tree-structured tasks, but
> 911:  * larger for external queues where both false-sharing problems
> 912:  * and the need for resizing are more common..  (Maintenance note:

Suggestion:

 * and the need for resizing are more common.  (Maintenance note:

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19131#discussion_r1618884462

Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v12]

[Chen Liang]

On Wed, 29 May 2024 07:17:38 GMT, Adam Sotona  wrote:

>> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use 
>> classfile API for reflection proxy-generation. Actual implementation of 
>> `ProxyGenerator` is focused on performance, however it causes JDK bootstrap 
>> regressions. `ProxyGenerator.TEMPLATE` class model is statically created and 
>> each proxy class is transformed from the template.
>> 
>> This patch is intended to examine plain proxy generation impact on 
>> performance and JDK bootstrap (vs proxy transformation from template).
>> 
>> The generated proxy is migrated from static initialization to CONDY 
>> bootstrap.
>> 
>> Please review.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 13 additional 
> commits since the last revision:
> 
>  - obsolete import
>  - Merge branch 'master' into JDK-8332457-proxy-startup
>  - missing bracket
>  - Update src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
>
>Co-authored-by: liach <7806504+li...@users.noreply.github.com>
>  - removed obsolete entry
>  - MTD_ cleanup
>  - fixed javadoc
>  - CONDY implementation of ProxyGenerator
>  - simplification of the proxy class loading
>  - more improvements
>  - ... and 3 more: https://git.openjdk.org/jdk/compare/7a251c2c...942342d5

Shouldn't condy mainly benefit the cases where only some of the methods are 
called so the proxy doesn't have to initialize all methods with reflection, or 
the case where a proxy is created but none of its methods is called? Does it 
show a regression in those cases as well?

-

PR Comment: https://git.openjdk.org/jdk/pull/19410#issuecomment-2137366357

Re: RFR: 8332826: Make hashCode methods in ArraysSupport friendlier [v2]

[Pavel Rappo]

On Tue, 28 May 2024 19:13:50 GMT, Jorn Vernee  wrote:

>> Pavel Rappo has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Fix incorrect utf16 hashCode adaptation
>
> src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 252:
> 
>> 250: return switch (length) {
>> 251: case 0 -> initialValue;
>> 252: case 1 -> 31 * initialValue + (int) a[fromIndex];
> 
> Suggestion:
> 
> case 1 -> 31 * initialValue + (int) a[fromIndex]; // sign 
> extension

To be honest, I don't think that this cast is needed. A better solution than to 
add a comment would be to delete all `(int)` casts from new `hashCode*` methods 
of `ArraysSupport`.

Those `(int)` casts migrated from `hashCode` methods of `Arrays` where there 
were used if neither of two `+` operands were of type `int`. But in 
`ArraysSupport` it's no longer the case: `31 * initialValue` is always `int` 
because `initialValue` is. So, `a[fromIndex]` is promoted to `int` by the 
virtue of https://docs.oracle.com/javase/specs/jls/se22/html/jls-5.html#jls-5.6.

For more confidence, consider that the `private static int hashCode` methods 
(implementation) of `ArraysSupport` do not have those casts.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19414#discussion_r1618835934

Re: RFR: 8332826: Make hashCode methods in ArraysSupport friendlier [v2]

[Pavel Rappo]

On Tue, 28 May 2024 20:40:30 GMT, Claes Redestad  wrote:

>> src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 275:
>> 
>>> 273: return switch (length) {
>>> 274: case 0 -> initialValue;
>>> 275: case 1 -> 31 * initialValue + (a[fromIndex] & 0xff);
>> 
>> For clarity, if you think it helps:
>> Suggestion:
>> 
>> case 1 -> 31 * initialValue + Byte.toUnsignedInt(a[fromIndex]);
>
> I don't care as long as microbenchmarks don't get a hiccup.

@cl4es, here are some results from my machine (macosx-aarch64):

Name   (size) Cnt BaseError  TestError  
Unit  Change
ArraysHashCode.bytes1  150.715 ±  0.004 0.725 ±  0.029 
ns/op   0.99x (p = 0.182 )
ArraysHashCode.bytes   10  153.753 ±  0.024 3.747 ±  0.011 
ns/op   1.00x (p = 0.322 )
ArraysHashCode.bytes  100  15   69.731 ±  0.15769.737 ±  0.092 
ns/op   1.00x (p = 0.891 )
ArraysHashCode.bytes1  15 9369.386 ±  1.449  9372.008 ±  6.678 
ns/op   1.00x (p = 0.133 )
ArraysHashCode.chars1  150.719 ±  0.024 0.734 ±  0.024 
ns/op   0.98x (p = 0.076 )
ArraysHashCode.chars   10  153.744 ±  0.005 3.746 ±  0.004 
ns/op   1.00x (p = 0.308 )
ArraysHashCode.chars  100  15   69.741 ±  0.11269.714 ±  0.044 
ns/op   1.00x (p = 0.365 )
ArraysHashCode.chars1  15 9367.123 ±  5.320  9371.325 ±  6.407 
ns/op   1.00x (p = 0.046 )
ArraysHashCode.ints 1  150.711 ±  0.013 0.706 ±  0.006 
ns/op   1.01x (p = 0.137 )
ArraysHashCode.ints10  153.750 ±  0.002 3.752 ±  0.004 
ns/op   1.00x (p = 0.283 )
ArraysHashCode.ints   100  15   69.753 ±  0.08669.711 ±  0.016 
ns/op   1.00x (p = 0.065 )
ArraysHashCode.ints 1  15 9376.225 ±  5.845  9376.218 ± 12.181 
ns/op   1.00x (p = 0.999 )
ArraysHashCode.multibytes   1  150.741 ±  0.001 0.740 ±  0.001 
ns/op   1.00x (p = 0.038 )
ArraysHashCode.multibytes  10  152.737 ±  0.001 2.826 ±  0.136 
ns/op   0.97x (p = 0.017 )
ArraysHashCode.multibytes 100  15   32.202 ±  0.05932.153 ±  0.006 
ns/op   1.00x (p = 0.004*)
ArraysHashCode.multibytes   1  15 4922.740 ± 25.590  4921.468 ±  7.372 
ns/op   1.00x (p = 0.846 )
ArraysHashCode.multichars   1  150.740 ±  0.005 0.740 ±  0.000 
ns/op   1.00x (p = 0.996 )
ArraysHashCode.multichars  10  152.732 ±  0.002 2.737 ±  0.003 
ns/op   1.00x (p = 0.000*)
ArraysHashCode.multichars 100  15   32.109 ±  0.01732.182 ±  0.028 
ns/op   1.00x (p = 0.000*)
ArraysHashCode.multichars   1  15 4925.750 ± 46.366  4930.684 ± 26.001 
ns/op   1.00x (p = 0.704 )
ArraysHashCode.multiints1  150.740 ±  0.000 0.739 ±  0.000 
ns/op   1.00x (p = 0.000*)
ArraysHashCode.multiints   10  152.919 ±  0.002 2.953 ±  0.059 
ns/op   0.99x (p = 0.033 )
ArraysHashCode.multiints  100  15   32.140 ±  0.01132.094 ±  0.004 
ns/op   1.00x (p = 0.000*)
ArraysHashCode.multiints1  15 4918.911 ±  3.512  4913.884 ± 11.618 
ns/op   1.00x (p = 0.105 )
ArraysHashCode.multishorts  1  150.740 ±  0.001 0.739 ±  0.000 
ns/op   1.00x (p = 0.000*)
ArraysHashCode.multishorts 10  152.736 ±  0.002 2.733 ±  0.008 
ns/op   1.00x (p = 0.159 )
ArraysHashCode.multishorts100  15   32.162 ±  0.03332.105 ±  0.008 
ns/op   1.00x (p = 0.000*)
ArraysHashCode.multishorts  1  15 4916.984 ±  3.276  4912.000 ± 11.479 
ns/op   1.00x (p = 0.103 )
ArraysHashCode.shorts   1  150.711 ±  0.023 0.709 ±  0.016 
ns/op   1.00x (p = 0.818 )
ArraysHashCode.shorts  10  153.745 ±  0.003 3.739 ±  0.010 
ns/op   1.00x (p = 0.049 )
ArraysHashCode.shorts 100  15   69.725 ±  0.08269.620 ±  0.051 
ns/op   1.00x (p = 0.000*)
ArraysHashCode.shorts   1  15 9370.882 ±  8.306  9356.215 ±  3.996 
ns/op   1.00x (p = 0.000*)
  * = significant

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19414#discussion_r1618821363

Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v12]

[Claes Redestad]

On Wed, 29 May 2024 07:17:38 GMT, Adam Sotona  wrote:

>> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use 
>> classfile API for reflection proxy-generation. Actual implementation of 
>> `ProxyGenerator` is focused on performance, however it causes JDK bootstrap 
>> regressions. `ProxyGenerator.TEMPLATE` class model is statically created and 
>> each proxy class is transformed from the template.
>> 
>> This patch is intended to examine plain proxy generation impact on 
>> performance and JDK bootstrap (vs proxy transformation from template).
>> 
>> The generated proxy is migrated from static initialization to CONDY 
>> bootstrap.
>> 
>> Please review.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 13 additional 
> commits since the last revision:
> 
>  - obsolete import
>  - Merge branch 'master' into JDK-8332457-proxy-startup
>  - missing bracket
>  - Update src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
>
>Co-authored-by: liach <7806504+li...@users.noreply.github.com>
>  - removed obsolete entry
>  - MTD_ cleanup
>  - fixed javadoc
>  - CONDY implementation of ProxyGenerator
>  - simplification of the proxy class loading
>  - more improvements
>  - ... and 3 more: https://git.openjdk.org/jdk/compare/62348071...942342d5

The condy bootstrapping adds a little noise when running this through an 
affected startup benchmark. Overall the net impact is small or even negative. 
This is a nice cleanup, though, but perhaps we ought to keep JDK-8332457 open 
(or file a new bug to keep investigating overheads).

-

PR Comment: https://git.openjdk.org/jdk/pull/19410#issuecomment-2137310415

Re: RFR: 8332614: Type-checked ConstantPool.entryByIndex and ClassReader.readEntryOrNull [v5]

[Chen Liang]

> I propose to add type-checked ConstantPool.entryByIndex and 
> ClassReader.readEntryOrNull taking an extra Class parameter, which throws 
> ConstantPoolException instead of ClassCastException on type mismatch, which 
> can happen to malformed ClassFiles.
> 
> Requesting a review from @asotona. Thanks!
> 
> Proposal on mailing list: 
> https://mail.openjdk.org/pipermail/classfile-api-dev/2024-May/000512.html

Chen Liang has updated the pull request incrementally with one additional 
commit since the last revision:

  Remove redundant import

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19330/files
  - new: https://git.openjdk.org/jdk/pull/19330/files/b3b94639..ec56ce98

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19330=04
 - incr: https://webrevs.openjdk.org/?repo=jdk=19330=03-04

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

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

Re: RFR: 8332826: Make hashCode methods in ArraysSupport friendlier [v2]

[Chen Liang]

On Mon, 27 May 2024 20:55:29 GMT, Pavel Rappo  wrote:

>> Please review this PR, which supersedes a now withdrawn 
>> https://github.com/openjdk/jdk/pull/14831.
>> 
>> This PR replaces `ArraysSupport.vectorizedHashCode` with a set of more 
>> user-friendly methods. Here's a summary:
>> 
>> - Made the operand constants (i.e. `T_BOOLEAN` and friends) and the 
>> `vectorizedHashCode` method private
>> 
>> - Made the `vectorizedHashCode` method private, but didn't rename it. 
>> Renaming would dramatically increase this PR review cost, because that 
>> method's name is used by a lot of VM code. On a bright side, since the 
>> method is now private, it's no longer callable by clients of 
>> `ArraysSupport`, thus a problem of an inaccurate name is less severe.
>> 
>> - Made the `ArraysSupport.utf16HashCode` method private
>> 
>> - Moved tiny cases (i.e. 0, 1, 2) to `ArraysSupport`
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix incorrect utf16 hashCode adaptation

Marked as reviewed by liach (Author).

-

PR Review: https://git.openjdk.org/jdk/pull/19414#pullrequestreview-2085162416

Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v12]

[Chen Liang]

On Wed, 29 May 2024 07:17:38 GMT, Adam Sotona  wrote:

>> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use 
>> classfile API for reflection proxy-generation. Actual implementation of 
>> `ProxyGenerator` is focused on performance, however it causes JDK bootstrap 
>> regressions. `ProxyGenerator.TEMPLATE` class model is statically created and 
>> each proxy class is transformed from the template.
>> 
>> This patch is intended to examine plain proxy generation impact on 
>> performance and JDK bootstrap (vs proxy transformation from template).
>> 
>> The generated proxy is migrated from static initialization to CONDY 
>> bootstrap.
>> 
>> Please review.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 13 additional 
> commits since the last revision:
> 
>  - obsolete import
>  - Merge branch 'master' into JDK-8332457-proxy-startup
>  - missing bracket
>  - Update src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
>
>Co-authored-by: liach <7806504+li...@users.noreply.github.com>
>  - removed obsolete entry
>  - MTD_ cleanup
>  - fixed javadoc
>  - CONDY implementation of ProxyGenerator
>  - simplification of the proxy class loading
>  - more improvements
>  - ... and 3 more: https://git.openjdk.org/jdk/compare/19d52556...942342d5

Marked as reviewed by liach (Author).

-

PR Review: https://git.openjdk.org/jdk/pull/19410#pullrequestreview-2085154590

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v7]

[Doug Lea]

> This set of changes address causes of poor utilization with small numbers of 
> cores due to overly aggressive contention avoidance. A number of further 
> adjustments were needed to still avoid most contention effects in deployments 
> with large numbers of cores

Doug Lea 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 41 additional commits since the 
last revision:

 - Merge branch 'openjdk:master' into JDK-8322732
 - Add test for utilization with interdependent tasks
 - Un-misplace onSpinWait call
 - Adjust control flow
 - Reduce memory stalls
 - Merge branch 'openjdk:master' into JDK-8322732
 - More performance tradoffs
 - Address review comments
 - Merge branch 'openjdk:master' into JDK-8322732
 - Repack some fields; adjust control flow
 - ... and 31 more: https://git.openjdk.org/jdk/compare/cc3280e5...cf5fe55c

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19131/files
  - new: https://git.openjdk.org/jdk/pull/19131/files/a262f6c6..cf5fe55c

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19131=06
 - incr: https://webrevs.openjdk.org/?repo=jdk=19131=05-06

  Stats: 23145 lines in 634 files changed: 14865 ins; 4679 del; 3601 mod
  Patch: https://git.openjdk.org/jdk/pull/19131.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19131/head:pull/19131

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

Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v6]

[Doug Lea]

> This set of changes address causes of poor utilization with small numbers of 
> cores due to overly aggressive contention avoidance. A number of further 
> adjustments were needed to still avoid most contention effects in deployments 
> with large numbers of cores

Doug Lea has updated the pull request incrementally with two additional commits 
since the last revision:

 - Add test for utilization with interdependent tasks
 - Un-misplace onSpinWait call

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19131/files
  - new: https://git.openjdk.org/jdk/pull/19131/files/72224779..a262f6c6

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19131=05
 - incr: https://webrevs.openjdk.org/?repo=jdk=19131=04-05

  Stats: 52 lines in 2 files changed: 51 ins; 1 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/19131.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19131/head:pull/19131

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

Integrated: 8331865: Consolidate size and alignment checks in LayoutPath

[Maurizio Cimadamore]

On Wed, 15 May 2024 15:43:45 GMT, Maurizio Cimadamore  
wrote:

> When creating a nested memory access var handle, we ensure that the segment 
> is accessed at the correct alignment for the root layout being accessed. But 
> we do not ensure that the segment has at least the size of the accessed root 
> layout. Example:
> 
> 
> MemoryLayout LAYOUT = sequenceLayout(2, structLayout(JAVA_INT.withName("x"), 
> JAVA_INT.withName("y")));
> VarHandle X_HANDLE = LAYOUT.varHandle(PathElement.sequenceElement(), 
> PathElement.groupElement("x"));
> 
> 
> If I have a memory segment whose size is 12, I can successfully call X_HANDLE 
> on it with index 1, like so:
> 
> 
> MemorySegment segment = Arena.ofAuto().allocate(12);
> int x = (int)X_HANDLE.get(segment, 0, 1);
> 
> 
> This seems incorrect: after all, the provided segment doesn't have enough 
> space to fit _two_ elements of the nested structs. 
> 
> This PR adds checks to detect this kind of scenario earlier, thus improving 
> usability. To achieve this we could, in principle, just tweak 
> `LayoutPath::checkEnclosingLayout` and add the required size check.
> 
> But doing so will add yet another redundant check on top of the other already 
> added by [pull/19124](https://git.openjdk.org/jdk/pull/19124). Instead, this 
> PR rethinks how memory segment var handles are created, in order to eliminate 
> redundant checks.
> 
> The main observation is that size and alignment checks depend on an 
> _enclosing_ layout. This layout *might* be the very same value layout being 
> accessed (this is the case when e.g. using `JAVA_INT.varHandle()`). But, in 
> the general case, the accessed value layout and the enclosing layout might 
> differ (e.g. think about accessing a struct field).
> 
> Furthermore, the enclosing layout check depends on the _base_ offset at which 
> the segment is accessed, _prior_ to any index computation that occurs if the 
> accessed layout path has any open elements. It is important to notice that 
> this base offset is only available when looking at the var handle that is 
> returned to the user. For instance, an indexed var handle with coordinates:
> 
> 
> (MemorySegment, long /* base */, long /* index 1 */, long /* index 2 */, long 
> /* index 3 */)
> 
> 
> Is obtained through adaptation, by taking a more basic var handle of the form:
> 
> 
> (MemorySegment, long /* offset */)
> 
> 
> And then injecting the result of the index multiplication into `offset`. As 
> such, we can't add an enclosing layout check inside the var handle returned 
> by `VarHandles::memorySegmentViewHandle`, as doing so will end up seeing the 
> *wrong* offset (e.g. an offset in which the index part has already been mixed 
> in)...

This pull request has now been integrated.

Changeset: c003c120
Author:Maurizio Cimadamore 
URL:   
https://git.openjdk.org/jdk/commit/c003c1207fae07bcfe5a6f642a9c05e6c591e7a6
Stats: 205 lines in 12 files changed: 72 ins; 39 del; 94 mod

8331865: Consolidate size and alignment checks in LayoutPath

Reviewed-by: psandoz, jvernee

-

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

Re: RFR: 8331865: Consolidate size and alignment checks in LayoutPath [v3]

[Maurizio Cimadamore]

On Wed, 22 May 2024 21:03:57 GMT, Jorn Vernee  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix typo in javadoc
>
> src/java.base/share/classes/java/lang/invoke/X-VarHandleSegmentView.java.template
>  line 123:
> 
>> 121: static $type$ get(VarHandle ob, Object obb, long base) {
>> 122: VarHandleSegmentViewBase handle = (VarHandleSegmentViewBase)ob;
>> 123: AbstractMemorySegmentImpl bb = checkReadOnly(obb, true);
> 
> For getter methods, which pass a constant `true` here, `checkReadOnly` 
> essentially just does a null check and cast on the segment. Not sure if it's 
> worth simplifying... (I'm happy if you want to leave it like this as well)

I'll leave it for now. There's always a trade-off with these generated 
templates...

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19251#discussion_r1618694460

Can we improve the charset parameter Javadoc in PrintStream?

[Daniel Schmid]

Hi,

When viewing the Javadoc of PrintStream, I noticed that the Javadocs of 
the constructors involving charsets seems to be a bit lacking.
In most cases, these just mention something like "a charset" instead of 
saying what it's used for. While the class description specifies that, I 
think it could also be present in the parameter information of the 
constructors since PrintStream is a commonly used class.


- The second parameter of PrintStream(File, String) is named "csn" and 
described as "The name of a supported charset". I suggest naming the 
parameter "encoding" and changing the description to "The name of a 
supported character encoding to be used for be used for converting 
characters to bytes" (with the appropriate link). This would be 
consistent with the PrintStream(OutputStream, boolean, String) constructor.
- The second parameter of PrintStream(File, Charset) is described as "A 
charset". I think this could be changed to "The charset used for 
converting characters to bytes" or similar.
- The third parameter of PrintStream(OutputStream, boolean, String) is 
named "encoding" and described as "The name of a supported character 
encoding". This could be changed to "The name of a supported character 
encoding to be used for be used for converting characters to bytes".
- The third parameter of PrintStream(OutputStream, boolean, Charset) is 
described as "A charset". I suggest changing this to "The charset used 
for converting characters to bytes".


Instead of using "encoding" for the String parameter names, 
"charsetName" would be another option. In this case, I would suggest 
something like "The name of a supported charset to be used for be used 
for converting characters to bytes".


If wanted, I can write a pull request for this but as I don't have JBS 
access, I can't create the issue and CSR by myself.


Yours,
Daniel



smime.p7s
Description: S/MIME Cryptographic Signature

Re: RFR: 8332826: Make hashCode methods in ArraysSupport friendlier [v2]

[Pavel Rappo]

On Wed, 29 May 2024 03:21:27 GMT, Chen Liang  wrote:

>> Pavel Rappo has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Fix incorrect utf16 hashCode adaptation
>
> src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 320:
> 
>> 318:  * @return the calculated hash value
>> 319:  */
>> 320: public static int hashCode(Object[] a, int fromIndex, int length, 
>> int initialValue) {
> 
> Is the object variant necessary here? The object version is hard for JIT to 
> profile as it's quite polymorphic compared to other arrays, and the initial 
> value is always 1.

This is a cleanup/refactoring PR, so none of this is necessary. My motivation 
for the `Object[]` variant was to provide reusable functionality for methods 
like these:

 - 
https://github.com/openjdk/jdk/blob/0ef03f122866f010ebf50683097e9b92e41cdaad/src/java.base/share/classes/java/util/concurrent/CopyOnWriteArrayList.java#L1076-L1083
 - 
https://github.com/openjdk/jdk/blob/0ef03f122866f010ebf50683097e9b92e41cdaad/src/java.base/share/classes/java/util/ArrayList.java#L669-L680

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19414#discussion_r1618644177

Re: RFR: 8332826: Make hashCode methods in ArraysSupport friendlier [v2]

[Pavel Rappo]

On Tue, 28 May 2024 20:21:34 GMT, Claes Redestad  wrote:

>> test/hotspot/jtreg/compiler/intrinsics/TestArraysHashCode.java line 88:
>> 
>>> 86: private static int testIntrinsic(byte[] bytes, int type)
>>> 87: throws InvocationTargetException, IllegalAccessException {
>>> 88: return (int) vectorizedHashCode.invoke(null, bytes, 0, 256, 1, 
>>> type);
>> 
>> Better to just call `hashCodeOfUnsigned` here I think.
>> 
>> The test for the non-constant type could be dropped. That is no longer a 
>> part of the 'API' of `ArraySupport`. It looks like the intrinsic bails out 
>> when the basic type is not constant any ways: 
>> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/library_call.cpp#L6401-L6404
>
> The non-constant test was added because that very bailout caused a crash. The 
> other test is actually less interesting since it'll likely be covered 
> indirectly by regular use. But as we are hiding these away this gets ever 
> more obscure and perhaps the test could be dropped entirely.

@cl4es, do you want me to delete that test file altogether?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19414#discussion_r1618536122

Re: RFR: 8332826: Make hashCode methods in ArraysSupport friendlier [v2]

[Claes Redestad]

On Mon, 27 May 2024 20:55:29 GMT, Pavel Rappo  wrote:

>> Please review this PR, which supersedes a now withdrawn 
>> https://github.com/openjdk/jdk/pull/14831.
>> 
>> This PR replaces `ArraysSupport.vectorizedHashCode` with a set of more 
>> user-friendly methods. Here's a summary:
>> 
>> - Made the operand constants (i.e. `T_BOOLEAN` and friends) and the 
>> `vectorizedHashCode` method private
>> 
>> - Made the `vectorizedHashCode` method private, but didn't rename it. 
>> Renaming would dramatically increase this PR review cost, because that 
>> method's name is used by a lot of VM code. On a bright side, since the 
>> method is now private, it's no longer callable by clients of 
>> `ArraysSupport`, thus a problem of an inaccurate name is less severe.
>> 
>> - Made the `ArraysSupport.utf16HashCode` method private
>> 
>> - Moved tiny cases (i.e. 0, 1, 2) to `ArraysSupport`
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix incorrect utf16 hashCode adaptation

Marked as reviewed by redestad (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/19414#pullrequestreview-2084714609

Re: RFR: 8332826: Make hashCode methods in ArraysSupport friendlier [v2]

[Claes Redestad]

On Wed, 29 May 2024 03:16:02 GMT, Chen Liang  wrote:

>> In fact, if I change it to `2`, the following tests will fail:
>> 
>>   - `jdk/jdk/classfile/Utf8EntryTest.java`
>>   - `jdk/java/util/zip/ZipCoding.java`
>>   - `jdk/java/text/Format/MessageFormat/MessageRegression.java`
>
> Indeed, the actual length passed at call site is `value.length >> 1` instead 
> of `value.length`; this adjusted char-length carries on to 
> `vectorizedHashCode` call.

Ah, sneaky. Might affect scores for the zero and one-char cases since the shift 
now happens unconditionally, but probably doesn't matter in practice.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19414#discussion_r1618456668

Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v12]

[Adam Sotona]

> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use 
> classfile API for reflection proxy-generation. Actual implementation of 
> `ProxyGenerator` is focused on performance, however it causes JDK bootstrap 
> regressions. `ProxyGenerator.TEMPLATE` class model is statically created and 
> each proxy class is transformed from the template.
> 
> This patch is intended to examine plain proxy generation impact on 
> performance and JDK bootstrap (vs proxy transformation from template).
> 
> The generated proxy is migrated from static initialization to CONDY bootstrap.
> 
> Please review.
> 
> Thank you,
> Adam

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

 - obsolete import
 - Merge branch 'master' into JDK-8332457-proxy-startup
 - missing bracket
 - Update src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
   
   Co-authored-by: liach <7806504+li...@users.noreply.github.com>
 - removed obsolete entry
 - MTD_ cleanup
 - fixed javadoc
 - CONDY implementation of ProxyGenerator
 - simplification of the proxy class loading
 - more improvements
 - ... and 3 more: https://git.openjdk.org/jdk/compare/386e173f...942342d5

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19410/files
  - new: https://git.openjdk.org/jdk/pull/19410/files/24b60451..942342d5

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19410=11
 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=10-11

  Stats: 4440 lines in 143 files changed: 3169 ins; 738 del; 533 mod
  Patch: https://git.openjdk.org/jdk/pull/19410.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19410/head:pull/19410

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

Re: RFR: 8332614: Type-checked ConstantPool.entryByIndex and ClassReader.readEntryOrNull [v4]

[Adam Sotona]

On Wed, 29 May 2024 05:19:28 GMT, Chen Liang  wrote:

>> I propose to add type-checked ConstantPool.entryByIndex and 
>> ClassReader.readEntryOrNull taking an extra Class parameter, which throws 
>> ConstantPoolException instead of ClassCastException on type mismatch, which 
>> can happen to malformed ClassFiles.
>> 
>> Requesting a review from @asotona. Thanks!
>> 
>> Proposal on mailing list: 
>> https://mail.openjdk.org/pipermail/classfile-api-dev/2024-May/000512.html
>
> Chen Liang 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:
> 
>  - Use switch
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 
> feature/entry-by-type
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 
> feature/entry-by-type
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 
> feature/entry-by-type
>  - Use constants, beautify code
>  - 8332614: Type-checked ConstantPool.entryByIndex and 
> ClassReader.readEntryOrNull

Looks good to me, thanks!

-

Marked as reviewed by asotona (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19330#pullrequestreview-2084437069