Re: [jdk23] RFR: 8335935: Chained builders not sending transformed models to next transforms
On Wed, 10 Jul 2024 22:10:50 GMT, Chen Liang wrote: > Please review this non-clean backport of the bugfix in #20100 to release 23, > where ClassFile API chained builders does not emit certain elements through > downstream transforms and returns wrong builder for chaining. This backport > includes an additional change that rolls back a rename done in #19952 that > applies to the new test, intended for 24 only. Marked as reviewed by asotona (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/20127#pullrequestreview-2171073501
Re: RFR: 8335642: Hide Transform implementation for Class-File API [v2]
On Tue, 9 Jul 2024 19:00:45 GMT, Chen Liang wrote: >> Removes ClassFile API transformation implementation details accidentally >> leaked to public API. Users don't have access to classes necessary to >> correctly implement these transform resolutions. In addition, removed >> improper `canWriteDirect` and made `ClassFileBuilder::transform` chain >> returns. >> >> Replaces #19928. > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > return tag required Looks good to me. Nice cleanup job! - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20102#pullrequestreview-2168266528
Re: RFR: 8335935: Chained builders not sending transformed models to next transforms
On Tue, 9 Jul 2024 17:34:14 GMT, Chen Liang wrote: > Please review the fix for a major transformation bug within the ClassFile > API, where certain kinds of buffered elements produced by one transform is > not sent to the next, and the "chained" (`andThen` transformation chains) > builders returning the wrong builder for call chains. > > This is intended to be backported to JDK 23, as this has significant impact > on user experience with ClassFile API transformations. The backport won't be > clean as we already renamed `ClassFile.transform` to `transformClass`, which > will be reverted in the backport. It looks good to me. Nice job! - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20100#pullrequestreview-2168251509
Re: RFR: 8335820: java/lang/invoke/LFCaching/LFSingleThreadCachingTest.java fails due to IllegalArgumentException: hash must be nonzero [v2]
On Mon, 8 Jul 2024 13:57:20 GMT, Chen Liang wrote:
>> Adam Sotona has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fixed BootstrapMethodEntryImpl::computeHashCode
>
> src/java.base/share/classes/jdk/internal/classfile/impl/BootstrapMethodEntryImpl.java
> line 79:
>
>> 77: static int computeHashCode(MethodHandleEntryImpl handle,
>> 78:List
>> arguments) {
>> 79: return 31 * handle.hashCode() + arguments.hashCode();
>
> Should we update the algorithm here, as this algorithm processes hash codes
> of the 2nd last item of arguments and the handle the same, both just
> multiplied by 31?
>
> Also need a copyright year update.
Right, BSMEntries are also stored in EntryMap.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20074#discussion_r1668711691
Re: RFR: 8335820: java/lang/invoke/LFCaching/LFSingleThreadCachingTest.java fails due to IllegalArgumentException: hash must be nonzero [v2]
> Class-File API constant pool implementation requires non-zero entry hash code. > Unfortunately current implementation computes zero hash code for specific CP > entries. > > This patch removes invalid and obsolete `AbstractPoolEntry::phiMix` > calculation and assures all pool entries have non-zero hash. A regression > test of the actual zero-hash `IntegerEntry` has been added. > > All pre-computed hash codes in `BoundAttribute::standardAttribute` are > updated. > > The patch has no performance effect measurable by any of the actual > Class-File API benchmarks. > > Please review. > > Thanks, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: fixed BootstrapMethodEntryImpl::computeHashCode - Changes: - all: https://git.openjdk.org/jdk/pull/20074/files - new: https://git.openjdk.org/jdk/pull/20074/files/9d4a1a2a..d38b2ffb Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=20074=01 - incr: https://webrevs.openjdk.org/?repo=jdk=20074=00-01 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/20074.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20074/head:pull/20074 PR: https://git.openjdk.org/jdk/pull/20074
RFR: 8335820: java/lang/invoke/LFCaching/LFSingleThreadCachingTest.java fails due to IllegalArgumentException: hash must be nonzero
Class-File API constant pool implementation requires non-zero entry hash code. Unfortunately current implementation computes zero hash code for specific CP entries. This patch removes invalid and obsolete `AbstractPoolEntry::phiMix` calculation and assures all pool entries have non-zero hash. A regression test of the actual zero-hash `IntegerEntry` has been added. All pre-computed hash codes in `BoundAttribute::standardAttribute` are updated. The patch has no performance effect measurable by any of the actual Class-File API benchmarks. Please review. Thanks, Adam - Commit messages: - 8335820: java/lang/invoke/LFCaching/LFSingleThreadCachingTest.java fails due to IllegalArgumentException: hash must be nonzero Changes: https://git.openjdk.org/jdk/pull/20074/files Webrev: https://webrevs.openjdk.org/?repo=jdk=20074=00 Issue: https://bugs.openjdk.org/browse/JDK-8335820 Stats: 56 lines in 4 files changed: 7 ins; 7 del; 42 mod Patch: https://git.openjdk.org/jdk/pull/20074.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20074/head:pull/20074 PR: https://git.openjdk.org/jdk/pull/20074
Re: [jdk23] RFR: 8335475: ClassBuilder incorrectly calculates max_locals in some cases
On Wed, 3 Jul 2024 02:38:58 GMT, Chen Liang wrote: > Please review this clean backport of #19981 onto JDK 23, fixing > `StackMapGenerator` generating static methods with no declared local variable > a max local of 1. Marked as reviewed by asotona (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19993#pullrequestreview-2162951672
Re: RFR: 8335727: since-checker: Add `@since` tags to ClassFile::transformClass and CodeBuilder
On Thu, 4 Jul 2024 18:04:27 GMT, Nizar Benalla wrote: > Please review this simple doc only change. > Some methods in ClassFile API were renamed recently as part of > [JDK-8335290](https://bugs.openjdk.org/browse/JDK-8335290) and > [JDK-8335110](https://bugs.openjdk.org/browse/JDK-8335110) and need to have > `@since 24`, as they are essentially new methods. > > Thanks! Just FYI: move from preview to final will bump all `@since` tags and remove obsolete method-level `@since` tags (as they become equal to the class-level tag). see: #19826 - PR Comment: https://git.openjdk.org/jdk/pull/20041#issuecomment-2213355929
Re: RFR: 8335475: ClassBuilder incorrectly calculates max_locals in some cases
On Mon, 1 Jul 2024 22:54:16 GMT, Chen Liang wrote: > Trivial fix for the bug where `StackMapGenerator` is pre-allocating the > locals incorrectly, affecting static methods with 0 locals. `StackCounter` > was not affected. Looks good to me, thanks for the fix. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19981#pullrequestreview-2153129835
Re: RFR: 8335060: ClassCastException after JDK-8294960
On Wed, 26 Jun 2024 06:53:28 GMT, Adam Sotona wrote: > Conversion of `java.lang.invoke` package to Class-File API is failing to > execute method handles with specific type conversion requirements. Root cause > is in the new `TypeConvertingMethodAdapter::primitiveTypeKindFromClass` > implementation. Original code has been matching the types by hash code and it > mistakenly appeared to be matching the primitive types. > > This patch fixes `TypeConvertingMethodAdapter::primitiveTypeKindFromClass` > and adds tests to better cover `TypeConvertingMethodAdapter` functionality. > > Please review. > > Thanks, > Adam Thank you for the review. - PR Comment: https://git.openjdk.org/jdk/pull/19898#issuecomment-2199936345
Integrated: 8335060: ClassCastException after JDK-8294960
On Wed, 26 Jun 2024 06:53:28 GMT, Adam Sotona wrote: > Conversion of `java.lang.invoke` package to Class-File API is failing to > execute method handles with specific type conversion requirements. Root cause > is in the new `TypeConvertingMethodAdapter::primitiveTypeKindFromClass` > implementation. Original code has been matching the types by hash code and it > mistakenly appeared to be matching the primitive types. > > This patch fixes `TypeConvertingMethodAdapter::primitiveTypeKindFromClass` > and adds tests to better cover `TypeConvertingMethodAdapter` functionality. > > Please review. > > Thanks, > Adam This pull request has now been integrated. Changeset: 3ca2bcd4 Author:Adam Sotona URL: https://git.openjdk.org/jdk/commit/3ca2bcd402042791d7460dd79ee16a3f88436b3e Stats: 806 lines in 2 files changed: 798 ins; 0 del; 8 mod 8335060: ClassCastException after JDK-8294960 Reviewed-by: liach, jpai - PR: https://git.openjdk.org/jdk/pull/19898
Re: RFR: 8335060: ClassCastException after JDK-8294960
On Wed, 26 Jun 2024 06:53:28 GMT, Adam Sotona wrote: > Conversion of `java.lang.invoke` package to Class-File API is failing to > execute method handles with specific type conversion requirements. Root cause > is in the new `TypeConvertingMethodAdapter::primitiveTypeKindFromClass` > implementation. Original code has been matching the types by hash code and it > mistakenly appeared to be matching the primitive types. > > This patch fixes `TypeConvertingMethodAdapter::primitiveTypeKindFromClass` > and adds tests to better cover `TypeConvertingMethodAdapter` functionality. > > Please review. > > Thanks, > Adam Some volunteer to review this P2 fix? Thank you! - PR Comment: https://git.openjdk.org/jdk/pull/19898#issuecomment-2199449124
Re: RFR: 8335290: Rename ClassFile::transform to ClassFile::transformClass
On Fri, 28 Jun 2024 22:01:33 GMT, Chen Liang wrote: > `ClassFile::transform` was initially `ClassModel::transform`, which > transforms the receiver class model to a new class byte array. This > functionality was in parallel with `ClassBuilder::transform`, which accepts a > `ClassModel` and a `ClassTransform` and forwards the results to the receiver > builder. > > After the `ClassFile` context object introduction, `ClassModel::transform` > becomes `ClassFile::transform`; now, its role is more similar to > `ClassBuilder::transformMethod`, `ClassBuilder::transformField`, or > `MethodBuilder::transformCode` (transforming subtypes), and it is confusing > with `ClassFileBuilder::transform` (which accepts the same model type as the > built type). We should rename `ClassFile::transform` to > `ClassFile::transformClass` to make this method's role more clear. > > This is separate from #19928 as this has much more user impact. Looks good to me. It will require refresh before integration as the number of use cases across JDK is growing instantly. Thanks! - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19952#pullrequestreview-2150617276
Re: RFR: 8334437: De-duplicate ProxyMethod list creation
On Tue, 18 Jun 2024 07:41:26 GMT, Claes Redestad wrote: > Simple refactoring to extract identical, simple lambda expressions. Improve > code clarity and reduce classes generated. Looks good to me. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19760#pullrequestreview-2141591212
Re: RFR: 8335110: Fix instruction name and API spec inconsistencies in CodeBuilder [v2]
On Tue, 25 Jun 2024 21:22:34 GMT, Chen Liang wrote: >> This is a collection of fixes and improvements to CodeBuilder, plus 2 >> renames. >> >> Fixes include: >> 1. `CodeBuilder::receiverSlot` typo >> 2. `CodeAttribute::labelToBci` update spec >> 3. `CodeBuilder::exceptionCatch` implementation >> 4. `CodeBuilder::if_nonnull`/`if_null` -> `ifnonnull`/`ifnull` >> 5. Docs for what instructions factories emit, and to explain why some >> factories have name mismatch; also a section in summary. > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > Patch in csr but forgot to upload Looks good to me, thank you. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19889#pullrequestreview-2141230227
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960 [v3]
On Tue, 25 Jun 2024 13:47:39 GMT, Adam Sotona wrote: >> After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code >> generation and unfortunately it causes StackOverflow on BigEndian platforms. >> >> This patch converts all lambdas in ClassSpecializer into anonymous inner >> classes. >> >> Please review and test on a BigEndian platform. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > more de-lambda work on ClassSpecializer Thank you for confirmation. - PR Comment: https://git.openjdk.org/jdk/pull/19863#issuecomment-2191198224
Integrated: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960
On Mon, 24 Jun 2024 16:01:41 GMT, Adam Sotona wrote: > After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code > generation and unfortunately it causes StackOverflow on BigEndian platforms. > > This patch converts all lambdas in ClassSpecializer into anonymous inner > classes. > > Please review and test on a BigEndian platform. > > Thanks, > Adam This pull request has now been integrated. Changeset: 7f6804ce Author:Adam Sotona URL: https://git.openjdk.org/jdk/commit/7f6804ceb63568d72e825d45b02d08f314c9b0fc Stats: 290 lines in 1 file changed: 110 ins; 84 del; 96 mod 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960 Reviewed-by: redestad - PR: https://git.openjdk.org/jdk/pull/19863
RFR: 8335060: ClassCastException after JDK-8294960
Conversion of `java.lang.invoke` package to Class-File API is failing to execute method handles with specific type conversion requirements. Root cause is in the new `TypeConvertingMethodAdapter::primitiveTypeKindFromClass` implementation. Original code has been matching the types by hash code and it mistakenly appeared to be matching the primitive types. This patch fixes `TypeConvertingMethodAdapter::primitiveTypeKindFromClass` and adds tests to better cover `TypeConvertingMethodAdapter` functionality. Please review. Thanks, Adam - Commit messages: - 8335060: ClassCastException after JDK-8294960 Changes: https://git.openjdk.org/jdk/pull/19898/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19898=00 Issue: https://bugs.openjdk.org/browse/JDK-8335060 Stats: 806 lines in 2 files changed: 798 ins; 0 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/19898.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19898/head:pull/19898 PR: https://git.openjdk.org/jdk/pull/19898
Re: RFR: 8307818: Convert Indify tool to Classfile API [v16]
On Tue, 25 Jun 2024 13:45:17 GMT, Oussama Louati wrote:
>> test/jdk/java/lang/invoke/indify/Indify.java line 386:
>>
>>> 384:
>>> 385: byte[] transformToBytes(ClassModel classModel) {
>>> 386: return of().transform(classModel, ClassTransform.ACCEPT_ALL);
>>
>> What is the purpose of this transformation?
>
> Transforming the classModel back to the bytes array in order to write the
> transformed classfile
And what is the purpose of parsing a model from bytes and transforming it back
to bytes without a change?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1652861409
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960 [v2]
On Tue, 25 Jun 2024 09:36:37 GMT, Adam Sotona wrote: >> After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code >> generation and unfortunately it causes StackOverflow on BigEndian platforms. >> >> This patch converts all lambdas in ClassSpecializer into anonymous inner >> classes. >> >> Please review and test on a BigEndian platform. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > more lambdas conversions to fix bootstrap Thanks for the report and patience. Yes, CodeBuilder::withMethodBody is also internally using lambda. Another version of the patch is ready for test. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/19863#issuecomment-2189005614
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960 [v3]
> After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code > generation and unfortunately it causes StackOverflow on BigEndian platforms. > > This patch converts all lambdas in ClassSpecializer into anonymous inner > classes. > > Please review and test on a BigEndian platform. > > Thanks, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: more de-lambda work on ClassSpecializer - Changes: - all: https://git.openjdk.org/jdk/pull/19863/files - new: https://git.openjdk.org/jdk/pull/19863/files/357d36a0..1416d4d2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19863=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19863=01-02 Stats: 53 lines in 1 file changed: 4 ins; 0 del; 49 mod Patch: https://git.openjdk.org/jdk/pull/19863.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19863/head:pull/19863 PR: https://git.openjdk.org/jdk/pull/19863
Re: RFR: 8307818: Convert Indify tool to Classfile API [v16]
On Sat, 22 Jun 2024 15:55:28 GMT, Oussama Louati wrote:
>> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code
>> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle,
>> MethodType, and CallSite constants.
>> ### Purpose of Indify
>>
>> - **Transformation Tool**: `Indify` transforms existing class files to
>> leverage `invokedynamic` and other JSR 292 features. This transformation
>> allows for dynamic method calls.
>>
>> ### Key Functions
>>
>> - **Method Handle and Method Type Constants**:
>> - **MH_x and MT_x Methods**: These methods are used to generate
>> `MethodHandle` and `MethodType` constants. The tool transforms calls to
>> these methods into `CONSTANT_MethodHandle` and `CONSTANT_MethodType` "ldc"
>> (load constant) instructions.
>> - **Invokedynamic Call Sites**:
>> - **INDY_x Methods**: These methods generate `invokedynamic` call sites.
>> Each `INDY_x` method starts with a call to an `MH_x` method, which acts as
>> the bootstrap method. This is followed by an `invokeExact` call.
>> - **Transformation**: The tool transforms pairs of calls (to `INDY_x`
>> followed by `invokeExact`) into `invokedynamic` instructions. This mimics
>> the JVM's handling of `invokedynamic` instructions, creating dynamic call
>> sites that are linked at runtime.
>>
>> It currently uses ad-hoc code to process class files and intends to migrate
>> to ASM; but since we have the Classfile API, we can migrate to Classfile API
>> instead.
>
> Oussama Louati has updated the pull request incrementally with one additional
> commit since the last revision:
>
> remove: removed unnecessary logging
test/jdk/java/lang/invoke/indify/Indify.java line 386:
> 384:
> 385: byte[] transformToBytes(ClassModel classModel) {
> 386: return of().transform(classModel, ClassTransform.ACCEPT_ALL);
What is the purpose of this transformation?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1652808158
Re: RFR: 8307818: Convert Indify tool to Classfile API [v16]
On Sat, 22 Jun 2024 15:55:28 GMT, Oussama Louati wrote: >> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code >> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, >> MethodType, and CallSite constants. >> ### Purpose of Indify >> >> - **Transformation Tool**: `Indify` transforms existing class files to >> leverage `invokedynamic` and other JSR 292 features. This transformation >> allows for dynamic method calls. >> >> ### Key Functions >> >> - **Method Handle and Method Type Constants**: >> - **MH_x and MT_x Methods**: These methods are used to generate >> `MethodHandle` and `MethodType` constants. The tool transforms calls to >> these methods into `CONSTANT_MethodHandle` and `CONSTANT_MethodType` "ldc" >> (load constant) instructions. >> - **Invokedynamic Call Sites**: >> - **INDY_x Methods**: These methods generate `invokedynamic` call sites. >> Each `INDY_x` method starts with a call to an `MH_x` method, which acts as >> the bootstrap method. This is followed by an `invokeExact` call. >> - **Transformation**: The tool transforms pairs of calls (to `INDY_x` >> followed by `invokeExact`) into `invokedynamic` instructions. This mimics >> the JVM's handling of `invokedynamic` instructions, creating dynamic call >> sites that are linked at runtime. >> >> It currently uses ad-hoc code to process class files and intends to migrate >> to ASM; but since we have the Classfile API, we can migrate to Classfile API >> instead. > > Oussama Louati has updated the pull request incrementally with one additional > commit since the last revision: > > remove: removed unnecessary logging Unfortunately this pull request is not converging to an acceptable state. Core transformation should be designed from top to down, with understanding of the functionality and with respect to Class-File API architecture. Currently proposed implementation is confusing mix of the original code, glued together with fragments of Class-File API. The patch also contains many cosmetic changes and unrelated semantic changes making the patch very hard to read. The PR could not be finished just by requesting reviewers attention and partly reflecting their suggestions, it should be significantly re-designed. - Changes requested by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18841#pullrequestreview-2138609501
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960
On Mon, 24 Jun 2024 16:01:41 GMT, Adam Sotona wrote: > After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code > generation and unfortunately it causes StackOverflow on BigEndian platforms. > > This patch converts all lambdas in ClassSpecializer into anonymous inner > classes. > > Please review and test on a BigEndian platform. > > Thanks, > Adam More lambdas converted, hopefully it is the last round. @reinrich Please re-test. Thank you. - PR Comment: https://git.openjdk.org/jdk/pull/19863#issuecomment-2188431487
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960 [v2]
> After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code > generation and unfortunately it causes StackOverflow on BigEndian platforms. > > This patch converts all lambdas in ClassSpecializer into anonymous inner > classes. > > Please review and test on a BigEndian platform. > > Thanks, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: more lambdas conversions to fix bootstrap - Changes: - all: https://git.openjdk.org/jdk/pull/19863/files - new: https://git.openjdk.org/jdk/pull/19863/files/ac8fb2d5..357d36a0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19863=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19863=00-01 Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19863.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19863/head:pull/19863 PR: https://git.openjdk.org/jdk/pull/19863
Re: RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960
On Mon, 24 Jun 2024 16:01:41 GMT, Adam Sotona wrote: > After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code > generation and unfortunately it causes StackOverflow on BigEndian platforms. > > This patch converts all lambdas in ClassSpecializer into anonymous inner > classes. > > Please review and test on a BigEndian platform. > > Thanks, > Adam Yes, I'm working on it. Class-File API avoids lambdas on critical JDK paths only. Unfortunately we did not have the full map of the critical paths on all platforms. - PR Comment: https://git.openjdk.org/jdk/pull/19863#issuecomment-2188311520
RFR: 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960
After JDK-8294960 is java.lang.invoke.ClassSpecializer using lamdas for code generation and unfortunately it causes StackOverflow on BigEndian platforms. This patch converts all lambdas in ClassSpecializer into anonymous inner classes. Please review and test on a BigEndian platform. Thanks, Adam - Commit messages: - 8334872: BigEndian: java/lang/invoke/condy Tests failing since JDK-8294960 Changes: https://git.openjdk.org/jdk/pull/19863/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19863=00 Issue: https://bugs.openjdk.org/browse/JDK-8334872 Stats: 239 lines in 1 file changed: 65 ins; 45 del; 129 mod Patch: https://git.openjdk.org/jdk/pull/19863.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19863/head:pull/19863 PR: https://git.openjdk.org/jdk/pull/19863
Re: [jdk23] RFR: 8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS
On Sat, 22 Jun 2024 00:26:51 GMT, Chen Liang wrote: > Please review this clean backport of #19708, to make javap recover and > continue after encountering undefined access flag bits set while still > exiting with a code of error, allowing it to error against improper bits > while still providing as much output as possible. Marked as reviewed by asotona (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19839#pullrequestreview-2135714317
Re: RFR: 8334734: Remove specialized readXxxEntry methods from ClassReader
On Fri, 21 Jun 2024 15:52:44 GMT, Chen Liang wrote: > `ClassReader.readXxxEntry` were added before we had generic, type-aware > `readEntry` and `readEntryOrNull` APIs (#19330). We should remove these > specialized versions in favor of the generic version to reduce API bloating. Looks good to me, thanks. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19833#pullrequestreview-2134895664
Re: RFR: 8334726: Remove accidentally exposed individual methods from Class-File API
On Fri, 21 Jun 2024 14:38:44 GMT, Chen Liang wrote: > In preparation of Class-File API exiting review, we are housekeeping our API > surface. These 3 method removals are the most obvious and simple ones. > > This is separated from more throughout and (possibly controversial) changes > for the future, to make reviews (both code and CSR) easier. Looks good to me, thanks. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19832#pullrequestreview-2132887472
Re: RFR: 8334714: Class-File API leaves preview
On Fri, 21 Jun 2024 12:31:05 GMT, Alan Bateman wrote: > Looks like this has been accidentally created as a sub-task of the JEP issue, > I assume you'll fix that. Will there be another issue to update the tests and > drop `@enablePreview`? I thought the implementation is usually created as a subtask of JEP, however I can make it a standalone bug if neede. Yes, there will be follow up task to clean all the `@enablePreview` mess, I'll create it. - PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2182673557
Re: RFR: 8334714: Class-File API leaves preview
On Fri, 21 Jun 2024 11:56:37 GMT, Adam Sotona wrote: > Class-File API is leaving preview. > This is a removal of all `@PreviewFeature` annotations from Class-File API. > It also bumps all `@since` tags and removes > `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`. > > Please review. > > Thanks, > Adam Any potential API changes should be merged first, to avoid impression of changing a non-preview API. However any discussion and approval processes of API and documentation changes should not block the JEP process (and each other). Cleaning up preview-enabled tests has a lower priority and should follow. - PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-218256
RFR: 8334714: Class-File API leaves preview
Class-File API is leaving preview. This is a removal of all `@PreviewFeature` annotations from Class-File API. It also bumps all `@since` tags and removes `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`. Please review. Thanks, Adam - Commit messages: - bumped @since tag - 8334714: Class-File API leaves preview Changes: https://git.openjdk.org/jdk/pull/19826/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19826=00 Issue: https://bugs.openjdk.org/browse/JDK-8334714 Stats: 718 lines in 166 files changed: 0 ins; 479 del; 239 mod Patch: https://git.openjdk.org/jdk/pull/19826.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19826/head:pull/19826 PR: https://git.openjdk.org/jdk/pull/19826
Re: [jdk23] RFR: 8333854: IllegalAccessError with proxies after JDK-8332457
On Thu, 20 Jun 2024 20:11:06 GMT, Chen Liang wrote: > Please review this patch, which is a backport of the fix in #19615 to JDK 23. > > This is not a clean patch, because the old patch was done on JDK-8333479 > (#19585) which was absent in JDK 23; however, the conflicts were small, and > the only real changes were that `methodTypeDesc` and `classDesc` from > `ConstantUtils` were not available, so the original approaches were retained. > There is also import adjustments. > > Testing: tier 1-3 It looks good to me, thanks. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19815#pullrequestreview-2132482150
Integrated: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles
On Thu, 14 Dec 2023 12:39:52 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 This pull request has now been integrated. Changeset: 01ee4241 Author:Adam Sotona URL: https://git.openjdk.org/jdk/commit/01ee4241b76e78ca67803c4b083fcedecef1c96c Stats: 2175 lines in 11 files changed: 459 ins; 877 del; 839 mod 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles Co-authored-by: Claes Redestad Reviewed-by: redestad, liach - PR: https://git.openjdk.org/jdk/pull/17108
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v25]
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]
> 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
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v24]
> 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 one additional commit since the last revision: fixed sneaky completion typo - Changes: - all: https://git.openjdk.org/jdk/pull/17108/files - new: https://git.openjdk.org/jdk/pull/17108/files/ac20f1f8..257d66ee Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17108=23 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=22-23 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v23]
> 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 one additional commit since the last revision: Update src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> - Changes: - all: https://git.openjdk.org/jdk/pull/17108/files - new: https://git.openjdk.org/jdk/pull/17108/files/857b8821..ac20f1f8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17108=22 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=21-22 Stats: 1 line in 1 file changed: 0 ins; 1 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
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v22]
> 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 one additional commit since the last revision: Inlined condy construction directly into CP entries - Changes: - all: https://git.openjdk.org/jdk/pull/17108/files - new: https://git.openjdk.org/jdk/pull/17108/files/d3367487..857b8821 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17108=21 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=20-21 Stats: 9 lines in 1 file changed: 2 ins; 6 del; 1 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
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v21]
> 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 four additional commits since the last revision: - Merge pull request #8 from cl4es/serialization_hostile SerializationHostileMethod - Reduce gratuitous code movement - Inline Consumer into generateSer.. method, move seldom-used serialization support constants to new holder - SerializationHostileMethod - Changes: - all: https://git.openjdk.org/jdk/pull/17108/files - new: https://git.openjdk.org/jdk/pull/17108/files/3aaf246e..d3367487 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17108=20 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=19-20 Stats: 86 lines in 1 file changed: 43 ins; 38 del; 5 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
Re: RFR: 8333854: IllegalAccessError with proxies after JDK-8332457 [v5]
On Thu, 13 Jun 2024 14:11:48 GMT, Chen Liang wrote: >> Please review this patch that fixes a critical issue that breaks some Proxy >> usages. >> >> Since the problematic patch from before cannot be backed out, this patch >> aims to emulate the old behavior before. A diff between before the >> problematic patch and this patch is available at >> https://gist.github.com/7565b2091008f561eb0ada019bc5e517, generated by >> running `git diff 326dbb1b139dd1ec1b8605339b91697cdf49da9a -- >> src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java`. > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > methodField can be initialized eagerly It is unfortunate that LDC of a CP entry cannot substitute the original complex approach (and the critical cases were not covered by any tests). @liach great work on the restoration of the original approach, while keeping the other improvements Thanks! - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19615#pullrequestreview-2124720977
Re: RFR: 8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS [v4]
On Mon, 17 Jun 2024 17:38:56 GMT, Chen Liang wrote: >> Currently, javap crashes for class files that have set non-zero values for >> undefined access flag bits, as >> `java.lang.reflect.AccessFlag.maskToAccessFlag` and >> `java.lang.classfile.AccessFlags.flags` fail. In contrast, the JVMS, though >> asking for these bits to be set to 0, requires VM to proceed and ignore >> these bits. javap should emulate the VM behavior and proceed rendering, >> ignoring these undefined bits. >> >> In addition, a few bytecode generation utilities in the JDK set unused bits, >> such as in >> `java.lang.invoke.MethodHandleImpl.BindCaller#generateInvokerTemplate` and >> `java.lang.invoke.GenerateJLIClassesHelper#generateCodeBytesForLFs`. Those >> can be updated in a later cleanup. > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > Improved error message Now it looks good to me, thanks. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19708#pullrequestreview-2123504238
Re: RFR: 8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS [v3]
On Mon, 17 Jun 2024 13:55:41 GMT, Chen Liang wrote:
>> Currently, javap crashes for class files that have set non-zero values for
>> undefined access flag bits, as
>> `java.lang.reflect.AccessFlag.maskToAccessFlag` and
>> `java.lang.classfile.AccessFlags.flags` fail. In contrast, the JVMS, though
>> asking for these bits to be set to 0, requires VM to proceed and ignore
>> these bits. javap should emulate the VM behavior and proceed rendering,
>> ignoring these undefined bits.
>>
>> In addition, a few bytecode generation utilities in the JDK set unused bits,
>> such as in
>> `java.lang.invoke.MethodHandleImpl.BindCaller#generateInvokerTemplate` and
>> `java.lang.invoke.GenerateJLIClassesHelper#generateCodeBytesForLFs`. Those
>> can be updated in a later cleanup.
>
> Chen Liang has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Improve tests to check unmatched bit position and failure for
> non-inner-classes
> - Report error for flag problems
src/jdk.jdeps/share/classes/com/sun/tools/javap/BasicWriter.java line 84:
> 82: } catch (IllegalArgumentException ex) {
> 83: mask &= LOCATION_MASKS.get(location);
> 84: report(ex);
Unfortunately the original exception message is missing any info that it is
related to access flags and it is not very clear what "Error: Unmatched bit
position 0x2 for location CLASS" means.
I would add at least a message prefix, for example "Error: Access Flags:
Unmatched bit position 0x2 for location CLASS".
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19708#discussion_r1643041633
Re: RFR: 8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS [v3]
On Mon, 17 Jun 2024 13:55:41 GMT, Chen Liang wrote:
>> Currently, javap crashes for class files that have set non-zero values for
>> undefined access flag bits, as
>> `java.lang.reflect.AccessFlag.maskToAccessFlag` and
>> `java.lang.classfile.AccessFlags.flags` fail. In contrast, the JVMS, though
>> asking for these bits to be set to 0, requires VM to proceed and ignore
>> these bits. javap should emulate the VM behavior and proceed rendering,
>> ignoring these undefined bits.
>>
>> In addition, a few bytecode generation utilities in the JDK set unused bits,
>> such as in
>> `java.lang.invoke.MethodHandleImpl.BindCaller#generateInvokerTemplate` and
>> `java.lang.invoke.GenerateJLIClassesHelper#generateCodeBytesForLFs`. Those
>> can be updated in a later cleanup.
>
> Chen Liang has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Improve tests to check unmatched bit position and failure for
> non-inner-classes
> - Report error for flag problems
test/langtools/tools/javap/UndefinedAccessFlagTest.java line 105:
> 103: .writeAll()
> 104: .getOutputLines(Task.OutputKind.DIRECT);
> 105: assertTrue(lines.stream().anyMatch(l -> l.contains("Unmatched
> bit position")), () -> String.join("\n", lines));
I would add a check the "fatal" error does not occur or any other way to
confirm correct recovery.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19708#discussion_r1642972527
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v20]
> 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 with a new target base due to a merge or a rebase. The pull request now contains 32 commits: - Merge branch 'master' into JDK-8294960-invoke # Conflicts: #src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java # src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java - applied suggested changes - Update src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java Co-authored-by: Chen Liang - reverted static initialization of ConstantPoolBuilder and CP entries - fixed naming conventions - use of jdk.internal.constant to improve performance - fixed imports - Apply suggestions from code review There are new possibilities with decoupled constants implementation, thank you for the reminder. Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> - Merge branch 'master' into JDK-8294960-invoke # Conflicts: # src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java - Merge branch 'master' into JDK-8294960-invoke # Conflicts: #src/java.base/share/classes/java/lang/classfile/Attributes.java - ... and 22 more: https://git.openjdk.org/jdk/compare/cdf22b13...3aaf246e - Changes: https://git.openjdk.org/jdk/pull/17108/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17108=19 Stats: 2166 lines in 10 files changed: 462 ins; 881 del; 823 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
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v19]
> 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 one additional commit since the last revision: Update src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java Co-authored-by: Chen Liang - Changes: - all: https://git.openjdk.org/jdk/pull/17108/files - new: https://git.openjdk.org/jdk/pull/17108/files/f870a8df..9d105694 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17108=18 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=17-18 Stats: 1 line in 1 file changed: 0 ins; 1 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
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v18]
On Wed, 12 Jun 2024 20:27:05 GMT, Chen Liang wrote:
>> Adam Sotona has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - reverted static initialization of ConstantPoolBuilder and CP entries
>> - fixed naming conventions
>
> src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java
> line 1148:
>
>> 1146: }
>> 1147:
>> 1148: private void emitPopInsn(CodeBuilder cob, BasicType type) {
>
> We need a TypeKind-aware pop in CodeBuilder too :(
Yes, it would be helpful.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17108#discussion_r1642870775
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v18]
On Thu, 6 Jun 2024 13:17:20 GMT, Chen Liang wrote: >> Adam Sotona has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - reverted static initialization of ConstantPoolBuilder and CP entries >> - fixed naming conventions > > src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java > line 470: > >> 468: MethodVisitor mv = cw.visitMethod(ACC_PRIVATE + ACC_FINAL, >> 469: NAME_METHOD_WRITE_OBJECT, >> DESCR_METHOD_WRITE_OBJECT, >> 470: null, SER_HOSTILE_EXCEPTIONS); > > The exceptions attribute is now lost on the migrated methods. Is that ok? > It's private and only accessed by reflection so I think there's no real > impact. Good catch, they were lost in translation. I think we should keep them. - PR Review Comment: https://git.openjdk.org/jdk/pull/17108#discussion_r1642800017
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v18]
On Thu, 6 Jun 2024 12:22:41 GMT, Chen Liang wrote:
>> Adam Sotona has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - reverted static initialization of ConstantPoolBuilder and CP entries
>> - fixed naming conventions
>
> src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java
> line 96:
>
>> 94: }
>> 95: };
>> 96: record MethodBody(Consumer code) implements
>> Consumer {
>
> Why do we have these 2 instead of a noop record field builder consumer (flags
> is already set in withField, and MethodBody should just be direct usage of
> withMethodBody)
>
> Seems the problem is in CF implemetnation side. Then these should be part of
> CF implementation details.
There is no problem if you can rely on lambdas ;)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17108#discussion_r1642793485
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v18]
On Thu, 6 Jun 2024 12:17:14 GMT, Chen Liang wrote:
>> Adam Sotona has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - reverted static initialization of ConstantPoolBuilder and CP entries
>> - fixed naming conventions
>
> src/java.base/share/classes/java/lang/invoke/GenerateJLIClassesHelper.java
> line 564:
>
>> 562: clb.withFlags(ACC_PRIVATE | ACC_FINAL | ACC_SUPER)
>> 563:
>> .withSuperclass(InvokerBytecodeGenerator.INVOKER_SUPER_DESC)
>> 564:
>> .with(SourceFileAttribute.of(className.substring(className.lastIndexOf('/')
>> + 1)));
>
> Maybe we can keep the classDesc from ofInternalName and use its displayName.
I don't see any benefits, both ways are sub-stringing.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17108#discussion_r1642789436
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v18]
On Thu, 6 Jun 2024 12:13:44 GMT, Chen Liang wrote: >> Adam Sotona has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - reverted static initialization of ConstantPoolBuilder and CP entries >> - fixed naming conventions > > src/java.base/share/classes/java/lang/invoke/ClassSpecializer.java line 784: > >> 782: // mix them up and load them for the >> transform helper: >> 783: List helperArgs = >> speciesData.deriveTransformHelperArguments(transformMethods.get(whichtm), >> whichtm, targs, tfields); >> 784: List> helperTypes = new >> ArrayList<>(helperArgs.size()); > > Can we convert helperTypes here to List so we construct > invokeBasicType as MethodTypeDesc below? This part can be simplified to a directly used validated array of ClassDesc and many conversions can be skipped. - PR Review Comment: https://git.openjdk.org/jdk/pull/17108#discussion_r1642756354
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v18]
On Thu, 13 Jun 2024 09:27:44 GMT, Claes Redestad wrote:
>> src/java.base/share/classes/java/lang/invoke/ClassSpecializer.java line 616:
>>
>>> 614: final ClassDesc classDesc = ClassDesc.of(className0);
>>> 615: final ClassDesc superClassDesc =
>>> classDesc(speciesData.deriveSuperClass());
>>> 616: return ClassFile.of().build(classDesc, clb -> {
>>
>> We need a confirmation here that these BMH species are only initialized
>> after LMF is ready. @cl4es Is a dependency on LMF from BMH safe?
>
> LMF will BMH and `ClassSpecializer`, but does not call into this code
> normally as the simple bound method handle needed by LMF is statically
> defined (`BoundMethodHandle.Species_L`). To be perfectly safe - and to keep
> lambda/indy/condy bootstrap overheads to a minumum - I'll continue
> recommending the desugaring of lambdas in java.lang.invoke
It results in not very nice code, but makes sense.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17108#discussion_r1642653369
Re: RFR: 8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS [v2]
On Fri, 14 Jun 2024 17:02:40 GMT, Chen Liang wrote: >> Currently, javap crashes for class files that have set non-zero values for >> undefined access flag bits, as >> `java.lang.reflect.AccessFlag.maskToAccessFlag` and >> `java.lang.classfile.AccessFlags.flags` fail. In contrast, the JVMS, though >> asking for these bits to be set to 0, requires VM to proceed and ignore >> these bits. javap should emulate the VM behavior and proceed rendering, >> ignoring these undefined bits. >> >> In addition, a few bytecode generation utilities in the JDK set unused bits, >> such as in >> `java.lang.invoke.MethodHandleImpl.BindCaller#generateInvokerTemplate` and >> `java.lang.invoke.GenerateJLIClassesHelper#generateCodeBytesForLFs`. Those >> can be updated in a later cleanup. > > Chen Liang has updated the pull request incrementally with two additional > commits since the last revision: > > - I am a dumbass > - Retain strict flag for Method, even though it's obsolete `javap` should never silently ignore erroneous class file content. If the flag value violates JVMS - it should be reported as an error and `javap` should reflect that in the return value. On the other hand `javap` should handle such situations, provide relevant error message and continue with the report. It is similar case as `javap` detection of inappropriate CP entry types. - PR Comment: https://git.openjdk.org/jdk/pull/19708#issuecomment-2172842079
Re: RFR: 8307818: Convert Indify tool to Classfile API [v10]
On Fri, 24 May 2024 17:12:34 GMT, Oussama Louati wrote: >> test/jdk/java/lang/invoke/indify/Indify.java line 503: >> >>> 501: >>> 502: Iterator instructionIterator >>> =getInstructions(m).iterator(); >>> 503: final Stack shouldProceedAfterIndyAdded = new >>> Stack<>(); >> >> What this stack of booleans suppose to do? > > We need a boolean value to determine if we should proceed after replacing the > appropriate "pop" instruction with an "invokedynamic" instruction. However, > instead of using just a boolean field, we use a stack. The reason for this is > that within the lambda expression, we can only use final variables. By using > a stack, we can update its value as needed, which is why this approach is > chosen. I see here an iteration over instructions of a method, where the whole class is retransformed in certain situations and some status is passed back in a stack of booleans. The whole conversion should be implemented in a single transformation. Original code repeatedly replaced instructions inline (that is BTW reason why added nops below), however architecture of ClassFile API is different, you are transforming one class into completely new class (free to remove and add as many elements as you need). You can compose transformations into complex trees and you can also collect information before the transformation, however the class transformation should be executed only once. - PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1642418788
Re: RFR: 8307818: Convert Indify tool to Classfile API [v10]
On Tue, 28 May 2024 11:03:18 GMT, Oussama Louati wrote: >> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code >> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, >> MethodType, and CallSite constants. >> It currently uses ad-hoc code to process class files and intends to migrate >> to ASM; but since we have the Classfile API, we can migrate to Classfile API >> instead. > > Oussama Louati has updated the pull request incrementally with one additional > commit since the last revision: > > chore: changed pool marking initialization to be done in one pass test/jdk/java/lang/invoke/indify/Indify.java line 197: > 195: break; > 196: case "-v": case "--verbose": case "--verbose=": > 197: verbose = booleanOption(a2); // more output Why the comments have been removed? - PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1642379144
Re: RFR: 8307818: Convert Indify tool to Classfile API [v10]
On Fri, 24 May 2024 17:12:24 GMT, Oussama Louati wrote: >> test/jdk/java/lang/invoke/indify/Indify.java line 578: >> >>> 576: classTransform = >>> ClassTransform.transformingMethodBodies(filter, codeTransform); >>> 577: classModel = of().parse( >>> 578: of().transform(classModel, >>> classTransform)); >> >> What this transform (in the middle of instructions iteration) does? > > It updates the current classfile so when moving to a next action we avoid > this runtime error: > >> STATUS:Failed.`main' threw exception: java.lang.VerifyError: Bad type on >> operand stack Injected transform to avoid verify error, that seems to me conceptually wrong. What is the root cause of the verify error and how the transform suppose to fix it? - PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1642372748
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v18]
> 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: - reverted static initialization of ConstantPoolBuilder and CP entries - fixed naming conventions - Changes: - all: https://git.openjdk.org/jdk/pull/17108/files - new: https://git.openjdk.org/jdk/pull/17108/files/e814749a..f870a8df Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17108=17 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=16-17 Stats: 43 lines in 2 files changed: 7 ins; 13 del; 23 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
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v17]
> 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 one additional commit since the last revision: use of jdk.internal.constant to improve performance - Changes: - all: https://git.openjdk.org/jdk/pull/17108/files - new: https://git.openjdk.org/jdk/pull/17108/files/902b02ee..e814749a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17108=16 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=15-16 Stats: 113 lines in 6 files changed: 35 ins; 7 del; 71 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
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v16]
> 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 one additional commit since the last revision: fixed imports - Changes: - all: https://git.openjdk.org/jdk/pull/17108/files - new: https://git.openjdk.org/jdk/pull/17108/files/019633bd..902b02ee Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17108=15 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=14-15 Stats: 18 lines in 1 file changed: 6 ins; 7 del; 5 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
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v15]
> 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 one additional commit since the last revision: Apply suggestions from code review There are new possibilities with decoupled constants implementation, thank you for the reminder. Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> - Changes: - all: https://git.openjdk.org/jdk/pull/17108/files - new: https://git.openjdk.org/jdk/pull/17108/files/9360b0eb..019633bd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17108=14 - incr: https://webrevs.openjdk.org/?repo=jdk=17108=13-14 Stats: 10 lines in 1 file changed: 0 ins; 0 del; 10 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
Re: RFR: 8294960: Convert java.base/java.lang.invoke package to use the Classfile API to generate lambdas and method handles [v14]
> 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 with a new target base due to a merge or a rebase. The pull request now contains 24 commits: - Merge branch 'master' into JDK-8294960-invoke # Conflicts: # src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java - Merge branch 'master' into JDK-8294960-invoke # Conflicts: #src/java.base/share/classes/java/lang/classfile/Attributes.java - fixed CodeBuilder use in j.l.invoke - Merge branch 'master' into JDK-8294960-invoke - Merge pull request #4 from cl4es/boxunbox_holder Only create box/unbox MethodRefEntries on request - Only create box/unbox MethodRefEntries on request - Merge pull request #3 from cl4es/minor_init_improvements Reduce init overhead of InvokeBytecodeGenerator and StackMapGenerator - Remove stray MRE_LF_interpretWithArguments - Reduce init overhead of InvokeBytecodeGenerator and StackMapGenerator - Deferred initialization of attributes map by moving into a holder class Co-authored-by: Claes Redestad - ... and 14 more: https://git.openjdk.org/jdk/compare/f73922b2...9360b0eb - Changes: https://git.openjdk.org/jdk/pull/17108/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17108=13 Stats: 2113 lines in 10 files changed: 422 ins; 861 del; 830 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
Integrated: 8332457: Examine startup overheads from JDK-8294961
On Mon, 27 May 2024 09:01:36 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 This pull request has now been integrated. Changeset: d85b0ca5 Author:Adam Sotona URL: https://git.openjdk.org/jdk/commit/d85b0ca5cdc1820a886c46bf555b2051fed7f167 Stats: 500 lines in 9 files changed: 174 ins; 197 del; 129 mod 8332457: Examine startup overheads from JDK-8294961 8229959: Convert proxy class to use constant dynamic Reviewed-by: liach, redestad - PR: https://git.openjdk.org/jdk/pull/19410
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v20]
> [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 incrementally with one additional commit since the last revision: Update test/micro/org/openjdk/bench/java/lang/reflect/ProxyGenBench.java Co-authored-by: Chen Liang - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/54376fe8..a0822718 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=19 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=18-19 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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: 8332457: Examine startup overheads from JDK-8294961 [v19]
On Wed, 5 Jun 2024 12:00:25 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 incrementally with one additional > commit since the last revision: > > An assortment of potential improvements > > Co-authored-by: Claes Redestad Benchmark Mode Cnt Score Error Units ProxyGenBench.generate100Proxies(master)ss 10 15.295 ? 5.435 ms/op ProxyGenBench.generate100Proxies(#19410)ss 10 11.761 ? 3.323 ms/op Finished running test 'micro:.+ProxyGenBench.+' - PR Comment: https://git.openjdk.org/jdk/pull/19410#issuecomment-2149694078
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v19]
On Wed, 5 Jun 2024 12:00:25 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 incrementally with one additional > commit since the last revision: > > An assortment of potential improvements > > Co-authored-by: Claes Redestad Name Cnt Base Error Test Error Unit Change Perfstartup-JMH 2040.000 ± 0.000 40.000 ± 0.000 ms/op 1.00x (p = 0.000 ) :.cycles 236365197.300 ± 3630668.331 224263724.700 ± 4160144.635 cycles 0.95x (p = 0.000*) :.instructions 602001346.250 ± 3134647.039 574894340.900 ± 2297441.310 instructions 0.95x (p = 0.000*) :.taskclock 60.000 ± 0.000 58.500 ± 3.181 ms 0.98x (p = 0.083 ) * = significant - PR Comment: https://git.openjdk.org/jdk/pull/19410#issuecomment-2149681885
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v19]
> [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 incrementally with one additional commit since the last revision: An assortment of potential improvements Co-authored-by: Claes Redestad - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/6ca164bc..54376fe8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=18 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=17-18 Stats: 25 lines in 5 files changed: 15 ins; 0 del; 10 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: 8333086: Using Console.println is unnecessarily slow due to JLine initalization
On Thu, 30 May 2024 13:50:33 GMT, Jan Lahoda wrote:
> Consider these two programs:
>
>
> public class SystemPrint {
> public static void main(String... args) {
> System.err.println("Hello!");
> }
> }
>
> and:
>
> public class IOPrint {
> public static void main(String... args) {
> java.io.IO.println("Hello!");
> }
> }
>
>
> They do the same conceptual thing - write a text to the output. But,
> `IO.println` delegates to `Console.println`, which then delegates to a
> `Console` backend, and the default backend is currently based on JLine.
>
> The issues is that JLine takes a quite a long time to initialize, and in a
> program like this, JLine is not really needed - it is used to provide better
> editing experience when reading input, but there's no reading in these
> programs.
>
> For example, on my computer:
>
> $ time java -classpath /tmp SystemPrint
> Hello!
>
> real0m0,035s
> user0m0,019s
> sys 0m0,019s
>
> $ time java -classpath /tmp --enable-preview IOPrint
> Hello!
>
> real0m0,165s
> user0m0,324s
> sys 0m0,042s
>
>
> The proposal herein is to delegate to the simpler `Console` backend from
> `java.base` as long as the user only uses methods that print to output, and
> switch to the JLine delegate when other methods (typically input) is used.
> Note that while technically `writer()` is a method doing output, it will
> force JLine initialization to avoid possible problems if the client caches
> the writer and uses it after switching the delegates.
>
> With this patch, I can get timing like this:
>
> $ time java --enable-preview -classpath /tmp/ IOPrint
> Hello!
>
> real0m0,051s
> user0m0,038s
> sys 0m0,020s
>
>
> which seems much more acceptable.
>
> There is also #19467, which may reduce the time further.
>
> A future work might focus on making JLine initialize faster, but avoiding
> JLine initialization in case where we don't need it seems like a good step to
> me in any case.
It looks like a legit performance trick.
-
Marked as reviewed by asotona (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/19479#pullrequestreview-2096720909
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v18]
> [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 incrementally with one additional commit since the last revision: MethodTypeDescImpl::ofValidated changed to varargs - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/fb3cdcdd..6ca164bc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=17 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=16-17 Stats: 14 lines in 2 files changed: 0 ins; 0 del; 14 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: 8332457: Examine startup overheads from JDK-8294961 [v3]
On Mon, 27 May 2024 12:20:20 GMT, Chen Liang wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> performance improvements > > src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 76: > >> 74: >> 75: private static final MethodTypeDesc >> 76: MTD_boolean = MethodTypeDescImpl.ofValidated(CD_boolean, >> ConstantUtils.EMPTY_CLASSDESC), > > Maybe we can change the `MethodTypeDescImpl.ofValidated` to varargs so we > don't need explicit array initializations. Yes, I like that idea. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1624576109
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v17]
> [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 incrementally with one additional commit since the last revision: ClassFile context made static - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/29e269f5..fb3cdcdd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=16 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=15-16 Stats: 12 lines in 1 file changed: 3 ins; 8 del; 1 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: 8332457: Examine startup overheads from JDK-8294961 [v14]
On Mon, 3 Jun 2024 11:37:34 GMT, Claes Redestad wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> ProxyGenBench simplification > > test/micro/org/openjdk/bench/java/lang/reflect/Proxy/ProxyGenBench.java line > 23: > >> 21: * questions. >> 22: */ >> 23: package org.openjdk.bench.java.lang.reflect.Proxy; > > Package name needs to be lowercase. Not sure why the folder name is uppercase > Proxy, but the two pre-existing benchmarks both have lower case package > declarations. Uppercase letters in package names may subtly break a few tools Yes, I've moved it down to j/l/reflect. However the existing benchmarks probably need a separate treatment. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1624297815
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v16]
> [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 incrementally with one additional commit since the last revision: fixed package - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/09baa376..29e269f5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=15 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=14-15 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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: 8332457: Examine startup overheads from JDK-8294961 [v15]
> [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 incrementally with one additional commit since the last revision: ProxyGenBench moved - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/7b00967d..09baa376 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=14 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=13-14 Stats: 0 lines in 1 file changed: 0 ins; 0 del; 0 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: 8332457: Examine startup overheads from JDK-8294961 [v14]
> [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 incrementally with one additional commit since the last revision: ProxyGenBench simplification - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/834d65c5..7b00967d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=13 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=12-13 Stats: 44 lines in 1 file changed: 1 ins; 33 del; 10 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: 8332457: Examine startup overheads from JDK-8294961 [v13]
On Mon, 3 Jun 2024 10:30:03 GMT, Claes Redestad wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> added ProxyGenBench JMH micro benchmark > > test/micro/org/openjdk/bench/java/lang/reflect/Proxy/ProxyGenBench.java line > 66: > >> 64: ClassDesc tempDesc = >> ClassDesc.ofDescriptor(Interfaze.class.descriptorString()); >> 65: loader = new ClsLoader(); >> 66: clsMap = new HashMap<>(100); > > Suggestion: > > clsMap = HashMap.newHashMap(100); I've simplified it a lot. It uses a new `ClassLoader` instance for each proxy generation, instead of preparation of 100 different interfaces. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1624235034
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v13]
> [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 incrementally with one additional commit since the last revision: added ProxyGenBench JMH micro benchmark - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/942342d5..834d65c5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=12 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=11-12 Stats: 114 lines in 1 file changed: 114 ins; 0 del; 0 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: 8332457: Examine startup overheads from JDK-8294961 [v12]
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/2ed35129...942342d5 Added `ProxyGenBenchmark` measuring generation time of 100 proxies. Results for master branch: BenchmarkMode Cnt Score Error Units Proxy.ProxyGenBench.generateProxiesss 10 12.266 ? 2.571 ms/op Results for this PR: BenchmarkMode Cnt Score Error Units Proxy.ProxyGenBench.generateProxiesss 10 9.851 ? 2.424 ms/op - PR Comment: https://git.openjdk.org/jdk/pull/19410#issuecomment-2144806554
Re: RFR: 8333312: Incorrect since tags on new ClassReader and ConstantPool methods
On Fri, 31 May 2024 13:04:03 GMT, David M. Lloyd wrote: > The new method additions ClassReader.readEntryOrNull(int, Class) and > ConstantPool.entryByIndex(int, Class) have incorrect since tags; they should > be `@since 23`. Thank you for the fix. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19501#pullrequestreview-2090857569
Integrated: 8182774: Verify code in javap
On Thu, 4 Apr 2024 15:13:46 GMT, Adam Sotona wrote: > This patch adds `javap -verify` option to check the class and print obvious > verification errors found. > Implementation depends on extended Class-File API verification support, so PR > #16809 is important to precede. > The new `javap` option is mentioned in man pages and a release note will be > provided. > > Please review. > > Thank you, > Adam This pull request has now been integrated. Changeset: a7864af0 Author:Adam Sotona URL: https://git.openjdk.org/jdk/commit/a7864af08acbe63d09f770ca66780738260faac4 Stats: 101 lines in 6 files changed: 101 ins; 0 del; 0 mod 8182774: Verify code in javap Reviewed-by: mcimadamore - PR: https://git.openjdk.org/jdk/pull/18629
Re: RFR: 8182774: Verify code in javap [v3]
> This patch adds `javap -verify` option to check the class and print obvious > verification errors found. > Implementation depends on extended Class-File API verification support, so PR > #16809 is important to precede. > The new `javap` option is mentioned in man pages and a release note will be > provided. > > 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 pull request now contains 34 commits: - Merge branch 'master' into JDK-8182774-javap-verify # Conflicts: # src/java.base/share/classes/jdk/internal/classfile/impl/verifier/ParserVerifier.java # src/java.base/share/classes/jdk/internal/classfile/impl/verifier/VerifierImpl.java #test/jdk/jdk/classfile/VerifierSelfTest.java - fixed typo - added VerificationTest - Merge branch 'JDK-8320396-verifier-extension' into JDK-8182774-javap-verify - Merge branch 'master' into JDK-8320396-verifier-extension - added description to javap cmd help and man page - 8182774: Verify code in javap - added references to jvms - Merge remote-tracking branch 'openjdk/master' into JDK-8320396-verifier-extension - work in progress - ... and 24 more: https://git.openjdk.org/jdk/compare/22ef827e...2cbcb1b4 - Changes: https://git.openjdk.org/jdk/pull/18629/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18629=02 Stats: 101 lines in 6 files changed: 101 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18629.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18629/head:pull/18629 PR: https://git.openjdk.org/jdk/pull/18629
Integrated: 8320396: Class-File API ClassModel::verify should include checks from hotspot/share/classfile/classFileParser.cpp
On Fri, 24 Nov 2023 13:20:20 GMT, Adam Sotona wrote: > ClassFile API `jdk.internal.classfile.verifier.VerifierImpl` performed only > bytecode-level class verification. > This patch adds `jdk.internal.classfile.verifier.ParserVerifier` with > additional class checks inspired by > `hotspot/share/classfile/classFileParser.cpp`. > > Also new `VerifierSelfTest::testParserVerifier` has been added. > > Please review. > > Thanks, > Adam This pull request has now been integrated. Changeset: 22ef827e Author:Adam Sotona URL: https://git.openjdk.org/jdk/commit/22ef827e2cc2409f21ad5c26611cb13d39b5cb3e Stats: 869 lines in 6 files changed: 835 ins; 7 del; 27 mod 8320396: Class-File API ClassModel::verify should include checks from hotspot/share/classfile/classFileParser.cpp Reviewed-by: liach, mcimadamore - PR: https://git.openjdk.org/jdk/pull/16809
Integrated: 8332597: Remove redundant methods from j.l.classfile.ClassReader API
On Tue, 21 May 2024 09:20:36 GMT, Adam Sotona wrote: > j.l.classfile.ClassReader instance is exposed in the Class-File API through > j.l.classfile.AttributeMapper::readAttribute method only. > ClassReader only purpose is to serve as a tool for reading content of a > custom attribute in a user-provided AttribtueMapper. > It contains useful set of low-level class reading methods for user to > implement a custom attribute content parser. > > However methods ClassReader::thisClassPos and > ClassReader::skipAttributeHolder are not necessary for a custom attribute > content parsing and so redundant in the API. > Class-File API implementation internally use these methods, however they > should not be exposed in the API. > > This patch removes the methods from the API. > > Please review. > > Thanks, > Adam This pull request has now been integrated. Changeset: 579cf705 Author:Adam Sotona URL: https://git.openjdk.org/jdk/commit/579cf705ff74d1ebb56b605d9a7ca17d87c36d84 Stats: 23 lines in 5 files changed: 0 ins; 17 del; 6 mod 8332597: Remove redundant methods from j.l.classfile.ClassReader API Reviewed-by: liach, jlahoda - PR: https://git.openjdk.org/jdk/pull/19323
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v12]
> [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]
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
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v11]
> [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 incrementally with one additional commit since the last revision: missing bracket - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/9ce8377c..24b60451 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=10 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=09-10 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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: 8332457: Examine startup overheads from JDK-8294961 [v10]
> [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 incrementally with one additional commit since the last revision: Update src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java Co-authored-by: liach <7806504+li...@users.noreply.github.com> - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/b3f6be89..9ce8377c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=09 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=08-09 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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: 8332457: Examine startup overheads from JDK-8294961 [v9]
On Tue, 28 May 2024 15:11:39 GMT, Chen Liang wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> removed obsolete entry > > src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 676: > >> 674: toClassDesc(fromClass), >> 675: method.getName(), >> 676: >> MethodType.methodType(method.getReturnType(), >> method.getParameterTypes()).describeConstable().get())); > > Suggestion: > > desc); good catch :) - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1617486750
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v9]
> [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 incrementally with one additional commit since the last revision: removed obsolete entry - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/fe596465..b3f6be89 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=08 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=07-08 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 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: 8320396: Class-File API ClassModel::verify should include checks from hotspot/share/classfile/classFileParser.cpp [v9]
On Fri, 24 May 2024 16:17:41 GMT, Adam Sotona wrote: >> ClassFile API `jdk.internal.classfile.verifier.VerifierImpl` performed only >> bytecode-level class verification. >> This patch adds `jdk.internal.classfile.verifier.ParserVerifier` with >> additional class checks inspired by >> `hotspot/share/classfile/classFileParser.cpp`. >> >> Also new `VerifierSelfTest::testParserVerifier` has been added. >> >> Please review. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 37 commits: > > - fixed ParserVerifier and VerifierSelfTest > - Merge branch 'master' into JDK-8320396-verifier-extension > - added verification of TypeAnnotation attributes > - added verification of SMT attribute > - added verification of module-related attributes > - applied the suggested changes > - applied the suggested changes > - fixed error thrown by VerifierImpl > - applied suggested changes > - Merge branch 'master' into JDK-8320396-verifier-extension > - ... and 27 more: https://git.openjdk.org/jdk/compare/cfdc64fc...b352b794 Please review! This verifier extension is actually blocking new `javap` feature for 23, see: https://github.com/openjdk/jdk/pull/18629 Missing all of these class file verifications in `javap -verify` would be sad. Thank you! Adam - PR Comment: https://git.openjdk.org/jdk/pull/16809#issuecomment-2135209023
Integrated: 8332505: JEP 457: ClassRemapper forgets to remap bootstrap method references
On Mon, 20 May 2024 08:03:28 GMT, Adam Sotona wrote: > Class-File API `ClassRemapper` component suppose to remap all classes > referenced in a class file. > Actual implementation missed remapping of bootstrap methods referenced from > `invokedynamic` instructions. > This patch fixes the remapping and adds relevant test. > > Please review. > > Thanks, > Adam This pull request has now been integrated. Changeset: aa4c83a5 Author:Adam Sotona URL: https://git.openjdk.org/jdk/commit/aa4c83a5bfe146714a46fb454aafc7393d2d8453 Stats: 9 lines in 2 files changed: 4 ins; 0 del; 5 mod 8332505: JEP 457: ClassRemapper forgets to remap bootstrap method references Reviewed-by: jlahoda - PR: https://git.openjdk.org/jdk/pull/19299
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v8]
> [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). > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: MTD_ cleanup - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/22e87768..fe596465 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=07 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=06-07 Stats: 5 lines in 1 file changed: 3 ins; 0 del; 2 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: 8332457: Examine startup overheads from JDK-8294961 [v7]
> [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). > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: fixed javadoc - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/c407e40e..22e87768 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=06 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=05-06 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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: 8332457: Examine startup overheads from JDK-8294961 [v6]
> [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). > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: CONDY implementation of ProxyGenerator - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/3d975d28..c407e40e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=05 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=04-05 Stats: 69 lines in 1 file changed: 8 ins; 39 del; 22 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: 8332457: Examine startup overheads from JDK-8294961 [v3]
On Mon, 27 May 2024 12:24:31 GMT, Chen Liang wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> performance improvements > > src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 822: > >> 820:.iconst_0() // false >> 821:.aload(0)// classLoader >> 822:.invokestatic(CD_Class, "forName", >> MTD_Class_String_boolean_ClassLoader); > > We can probably replace this `forName(name, false, thisClassLoader)` with > loading a class constant to reduce load on symbols. I'm wondering why all the `Class.forName(... .getClassLoader())` is necessary? Simple `ldc ` seems to work well. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1616232095
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v5]
> [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). > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: simplification of the proxy class loading - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/eaf30201..3d975d28 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=04 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=03-04 Stats: 60 lines in 1 file changed: 0 ins; 50 del; 10 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: 8332457: Examine startup overheads from JDK-8294961 [v4]
> [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). > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: more improvements - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/29ca6a4e..eaf30201 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=03 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=02-03 Stats: 14 lines in 1 file changed: 3 ins; 0 del; 11 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: 8332457: Examine startup overheads from JDK-8294961 [v3]
> [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). > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: performance improvements - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/1d4edea5..29ca6a4e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=01-02 Stats: 77 lines in 2 files changed: 39 ins; 9 del; 29 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: 8332457: Examine startup overheads from JDK-8294961 [v2]
> [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). > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: manual stack maps - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/246efa11..1d4edea5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=00-01 Stats: 39 lines in 1 file changed: 18 ins; 0 del; 21 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
RFR: 8332457: Examine startup overheads from JDK-8294961
[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). Please review. Thank you, Adam - Commit messages: - 8332457: Examine startup overheads from JDK-8294961 Changes: https://git.openjdk.org/jdk/pull/19410/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19410=00 Issue: https://bugs.openjdk.org/browse/JDK-8332457 Stats: 233 lines in 1 file changed: 27 ins; 113 del; 93 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
