Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v25]

2024-06-19 Thread Chen Liang
On Wed, 19 Jun 2024 09:08:35 GMT, Adam Sotona  wrote:

>> java.base java.lang.invoke package heavily uses ASM to generate lambdas and 
>> method handles.
>> 
>> This patch converts ASM calls to Classfile API.
>> 
>> This PR is continuation of https://github.com/openjdk/jdk/pull/12945
>> 
>> Any comments and suggestions are welcome.
>> 
>> Please review.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - removed empty line
>  - problem-listed runtime/ClassInitErrors/TestStackOverflowDuringInit.java

The new changes since the last round of review is great. Also much thanks for 
fixing that sneaky condy typo in LMF.

-

Marked as reviewed by liach (Committer).

PR Review: https://git.openjdk.org/jdk/pull/17108#pullrequestreview-2128433285


Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v25]

2024-06-19 Thread Claes Redestad
On Wed, 19 Jun 2024 09:08:35 GMT, Adam Sotona  wrote:

>> java.base java.lang.invoke package heavily uses ASM to generate lambdas and 
>> method handles.
>> 
>> This patch converts ASM calls to Classfile API.
>> 
>> This PR is continuation of https://github.com/openjdk/jdk/pull/12945
>> 
>> Any comments and suggestions are welcome.
>> 
>> Please review.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - removed empty line
>  - problem-listed runtime/ClassInitErrors/TestStackOverflowDuringInit.java

No objections as long as there's an understanding that we'll need to work on 
some of the startup/warmup regressions this will cause.

We have preliminary data on a subset of benchmarks which suggests it's a 
combination of loading more classes and spreading the work across a larger set 
of methods, which means we need to warm and JIT more methods to reach peak 
performance. ASM is a smaller library with a more minimalist approach, so it 
loads faster and gets up and running a bit faster. We'll probably need to file 
regressions as they are detected for tracking purposes, but I expect we'll 
continue chipping away at and improving the classfile API in the months to come.

-

Marked as reviewed by redestad (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17108#pullrequestreview-2127764187


Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v25]

2024-06-19 Thread Adam Sotona
On Wed, 19 Jun 2024 09:08:35 GMT, Adam Sotona  wrote:

>> java.base java.lang.invoke package heavily uses ASM to generate lambdas and 
>> method handles.
>> 
>> This patch converts ASM calls to Classfile API.
>> 
>> This PR is continuation of https://github.com/openjdk/jdk/pull/12945
>> 
>> Any comments and suggestions are welcome.
>> 
>> Please review.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - removed empty line
>  - problem-listed runtime/ClassInitErrors/TestStackOverflowDuringInit.java

`runtime/ClassInitErrors/TestStackOverflowDuringInit.java` seems to be fragile 
and affected by this PR, so I created 
[JDK-8334545](https://bugs.openjdk.org/browse/JDK-8334545) and listed the test 
in `test/hotspot/jtreg/ProblemList.txt`

@mlchung @cl4es @liach @ExE-Boss  many thanks for significant contribution to 
this complex conversion!

Any final thoughts or objections to the integration?

Thank you!

-

PR Comment: https://git.openjdk.org/jdk/pull/17108#issuecomment-2178180439


Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v25]

2024-06-19 Thread Adam Sotona
> java.base java.lang.invoke package heavily uses ASM to generate lambdas and 
> method handles.
> 
> This patch converts ASM calls to Classfile API.
> 
> This PR is continuation of https://github.com/openjdk/jdk/pull/12945
> 
> Any comments and suggestions are welcome.
> 
> Please review.
> 
> Thank you,
> Adam

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

 - removed empty line
 - problem-listed runtime/ClassInitErrors/TestStackOverflowDuringInit.java

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17108/files
  - new: https://git.openjdk.org/jdk/pull/17108/files/257d66ee..6e851a50

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17108=24
 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=23-24

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

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