Re: RFR: 8337219: AccessFlags factories do not require necessary arguments [v2]
On Mon, 29 Jul 2024 23:30:48 GMT, Chen Liang wrote: >> Removes 6 `AccessFlags` factories that do not take class-file versions as >> its arguments. >> >> `AccessFlags` is a wrapper around a bit mask to support modifier streaming >> in ClassFile API. It additionally supports advanced validation based on >> location. >> >> However, as class file versions evolve, we may also need a class file >> version argument to ensure that the flags are correctly constructed. For >> example, a pre-valhalla class modifier without `ACC_SUPER` should not be >> interpreted as a value class. The current factories cannot find good default >> class file versions, and if they always assume latest, they will fail in >> this scenario. >> >> As a result, we should remove these 6 factories; note that users can still >> set the flags via `XxxBuilder::withFlags` with either `int` or >> `AccessFlag...` flags. In contrast, these builder APIs can fetch the >> previously-passed class file versions, and correctly validate or interpret >> these flags. Same story goes for parsing, which can also construct the right >> flags with available information. >> >> This enables us to add methods to interpret the logical flags with >> version-specific information. If there's need, we can always add a new >> `AccessFlags.of(int, AccessFlag.Location, ClassFileFormatVersion)` factory, >> given the flexibility from this removal. > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains three commits: > > - Reapply import changes after merge > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/accessflags-factory > - 8337219: AccessFlags factories do not require necessary arguments Marked as reviewed by asotona (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/20341#pullrequestreview-2207034646
Re: RFR: 8336032: Enforce immutability of Lists used by ClassFile API
On Mon, 29 Jul 2024 19:36:31 GMT, Chen Liang wrote: > In #20241, it's noted that `ClassFile.verify` can return a mix-and-match of > mutable and immutable lists. > > Though the mutability of the list returned by `verify` has no particular > importance, as the mutable list is constructed on every call and does not > mutate the models, there are a few scenarios that they matter. Since > ClassFile API is designed with immutability in mind, we should change all > lists in ClassFile API to be immutable. > > Change summary: > - Critical scenarios where ClassFile API stores mutable objects: > - `ClassSignature.typeParameters` - keeps user mutable list > - `CompoundElement.elementList` - buffered models expose the underlying > mutable list > - `RuntimeInvisibleParameterAnnotationsAttribute`(and Visible) - keeps > users' nest mutable lists in an immutable list > - `StackMapFrameInfo.locals/stack` - keeps user mutable lists > - Optional scenarios to return immutable for good practice > - `ClassFile.verify` - mutable for full verified results > - `CompoundElement.elementList` - safe mutable for bound models > - Fail fast on user null `Attribute`s in `BoundAttribute.readAttributes` to > prevent unmodifiable list containing null > > I have also checked if we should stick with null-hostile `List.of` lists > instead of `Collections.unmodifiableList` lists; we are using > `unmodifiableList` (extensively in Signature parsing, for example) or other > ad-hoc immutable lists, so it is somewhat impractical and not meaningful to > enforce null-hostility. (See the list in JBS) > > These use sites are too sporadic so I made no unit tests. Nice cleanup, thank you. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20380#pullrequestreview-2207025233
Re: RFR: 8337225: Demote maxStack and maxLocals from CodeModel to CodeAttribute [v4]
On Mon, 29 Jul 2024 12:32:10 GMT, Chen Liang wrote: >> As discussed in offline meeting, the max stack and locals information are >> part of the code attribute and not meaningful for buffered code elements. >> Computation would be costly and these see no real usage during >> transformations. Thus, the proposed solution is to move these APIs to be >> CodeAttribute specific, as this is already how all these APIs' users are >> using. >> >> Also removed useless `Writable` on buffered models, and fixed >> `BufferedMethodBuilder::code` implementation. > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > Spacing style issue > > Co-authored-by: Andrey Turbanov Marked as reviewed by asotona (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/20338#pullrequestreview-2204820542
Re: RFR: 8337225: Demote maxStack and maxLocals from CodeModel to CodeAttribute [v3]
On Mon, 29 Jul 2024 02:22:11 GMT, Chen Liang wrote: >> As discussed in offline meeting, the max stack and locals information are >> part of the code attribute and not meaningful for buffered code elements. >> Computation would be costly and these see no real usage during >> transformations. Thus, the proposed solution is to move these APIs to be >> CodeAttribute specific, as this is already how all these APIs' users are >> using. >> >> Also removed useless `Writable` on buffered models, and fixed >> `BufferedMethodBuilder::code` implementation. > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > Fix typo, and add tests Looks good to me. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20338#pullrequestreview-2204161711
Re: RFR: 8337219: AccessFlags factories do not require necessary arguments
On Thu, 25 Jul 2024 23:11:15 GMT, Chen Liang wrote: > Removes 6 `AccessFlags` factories that do not take class-file versions as its > arguments. > > `AccessFlags` is a wrapper around a bit mask to support modifier streaming in > ClassFile API. It additionally supports advanced validation based on location. > > However, as class file versions evolve, we may also need a class file version > argument to ensure that the flags are correctly constructed. For example, a > pre-valhalla class modifier without `ACC_SUPER` should not be interpreted as > a value class. The current factories cannot find good default class file > versions, and if they always assume latest, they will fail in this scenario. > > As a result, we should remove these 6 factories; note that users can still > set the flags via `XxxBuilder::withFlags` with either `int` or > `AccessFlag...` flags. In contrast, these builder APIs can fetch the > previously-passed class file versions, and correctly validate or interpret > these flags. Same story goes for parsing, which can also construct the right > flags with available information. > > This enables us to add methods to interpret the logical flags with > version-specific information. If there's need, we can always add a new > `AccessFlags.of(int, AccessFlag.Location, ClassFileFormatVersion)` factory, > given the flexibility from this removal. It is a reasonable cleanup of risky factory methods, given the fact we have more safe `withFlags` builders methods. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20341#pullrequestreview-2201219552
Re: RFR: 8337225: Demote maxStack and maxLocals from CodeModel to CodeAttribute
On Thu, 25 Jul 2024 22:41:08 GMT, Chen Liang wrote: > As discussed in offline meeting, the max stack and locals information are > part of the code attribute and not meaningful for buffered code elements. > Computation would be costly and these see no real usage during > transformations. Thus, the proposed solution is to move these APIs to be > CodeAttribute specific, as this is already how all these APIs' users are > using. > > Also removed useless `Writable` on buffered models, and fixed > `BufferedMethodBuilder::code` implementation. src/java.base/share/classes/jdk/internal/classfile/impl/BufferedCodeBuilder.java line 67: > 65: this.maxLocals = Util.maxLocals(methodInfo.methodFlags(), > methodInfo.methodTypeSymbol()); > 66: if (original != null) > 67: this.maxLocals = Math.max(this.maxLocals, > original.maxLocals()); `original::maxLocals`set the counter for `CodeBuilder::allocateLocal` By restricting calculation of maxLocals to "origin instanceof CodeAttribute" may cause invalid locals allocations for chained code builders. The number might not be exposed in the API, however we need to know it internally. - PR Review Comment: https://git.openjdk.org/jdk/pull/20338#discussion_r1692669453
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation [v6]
On Wed, 24 Jul 2024 13:00:47 GMT, Chen Liang wrote: >> `TypeAnnotation` is not an annotation, as it should not be used in places >> like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an >> annotation at a given location instead of to be an annotation. >> >> Depends on #20205. > > Chen Liang has updated the pull request incrementally with three additional > commits since the last revision: > > - More refinements from alex > - Artifact -> construct > - More about Annotation, add equals note Marked as reviewed by asotona (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/20247#pullrequestreview-2196775087
Re: RFR: 8307818: Convert Indify tool to Classfile API [v16]
On Wed, 24 Jul 2024 07:58:31 GMT, ExE Boss wrote: >> This transformation is called only after the `classModel` is transformed: >> - at line 472 and 380 when the `transformToBytes()` method is called is >> after effectively transforming the `classModel` inside the `Logic` class. > > There isn’t currently any API for converting a `ClassModel` directly to a > `byte[]` without calling a `ClassFile::transformClass(…)` method. Repeated parsing (bytes -> model) and transforming back (model -> bytes) is an anti-pattern. Indify tool should operate as a single-pass transformation. Looping on a class model methods / instructions while replacing the class model itself (with something partially transformed) is not the right approach. - PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689346557
Re: RFR: 8336927: Missing equals and hashCode in java.lang.classfile.Annotation [v2]
On Tue, 23 Jul 2024 15:43:42 GMT, Chen Liang wrote: >> Convert `AnnotationImpl` to a record so it comes with proper `equals` and >> `hashCode` implementations. This also fixes the comparison for >> annotation-valued annotation element values. > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains two commits: > > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/anno-hash-equals > - 8336927: Missing equals and hashCode in java.lang.classfile.Annotation Marked as reviewed by asotona (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/20291#pullrequestreview-2195787862
Re: RFR: 8335939: Hide element writing across the ClassFile API [v5]
On Tue, 23 Jul 2024 05:40:05 GMT, Chen Liang wrote: >> `WritableElement` has always been one of the biggest peculiarities of >> ClassFile API: it exposes element writing yet has no corresponding reading >> ability exposed. Its existence creates a lot of API noise, increasing >> maintenance cost in the long run. (This is an iceberg whose tip was exposed >> in #14705) >> >> Removal details: >> - `LocalVariable/LocalVariableType.writeTo` >> - `WritableElement` >> - In `BufWriter`: >> - `writeList(List)` >> - `writeListIndices`: Hidden as we are not exposing >> `BoundAttribute.readEntryList` >> - `writeBytes(BufWriter other)` >> - `ClassReader.compare`: Avoid reading from `BufWriter` >> >> Future implementation cleanup out of scope of this patch: >> - Annotation writing can be upgraded and move away from `Util.Writable` >> - The writing of CP indices and attributes can move to their dedicated >> methods >> - reading of entry list can pair up with writing of entry list in >> ClassReader/BufWriter in future addition > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > Fix test compile errors Marked as reviewed by asotona (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/20205#pullrequestreview-2193140947
Re: RFR: 8336927: Missing equals and hashCode in java.lang.classfile.Annotation
On Tue, 23 Jul 2024 04:44:57 GMT, Chen Liang wrote: > Convert `AnnotationImpl` to a record so it comes with proper `equals` and > `hashCode` implementations. This also fixes the comparison for > annotation-valued annotation element values. Looks good to me, thanks for the fix. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20291#pullrequestreview-2193137329
Integrated: 8336833: Endless loop in Javap ClassWriter
On Fri, 19 Jul 2024 15:32:24 GMT, Adam Sotona wrote: > Artificially corrupted class with overflowing max - min values of > `tableswitch` instruction cause infinite loop in > `jdk.internal.classfile.impl.CodeImpl::inflateJumpTargets` > > This patch fixes the overflow and adds relevant test. > > Please review. > > Thank you, > Adam This pull request has now been integrated. Changeset: 0db6c15e Author:Adam Sotona URL: https://git.openjdk.org/jdk/commit/0db6c15efe255bd313fb2b827d2ee05171e62ae9 Stats: 24 lines in 2 files changed: 22 ins; 0 del; 2 mod 8336833: Endless loop in Javap ClassWriter Reviewed-by: liach - PR: https://git.openjdk.org/jdk/pull/20258
Re: RFR: 8336833: Endless loop in Javap ClassWriter
On Fri, 19 Jul 2024 15:41:07 GMT, Chen Liang wrote:
>> Artificially corrupted class with overflowing max - min values of
>> `tableswitch` instruction cause infinite loop in
>> `jdk.internal.classfile.impl.CodeImpl::inflateJumpTargets`
>>
>> This patch fixes the overflow and adds relevant test.
>>
>> Please review.
>>
>> Thank you,
>> Adam
>
> src/java.base/share/classes/jdk/internal/classfile/impl/AbstractInstruction.java
> line 320:
>
>> 318: int low = code.classReader.readInt(ap + 4);
>> 319: int high = code.classReader.readInt(ap + 8);
>> 320: if (high < low || (long)high - low > code.codeLength >> 2) {
>
> Maybe `Integer.toUnsignedLong(high - low)` might be clearer?
I think it is safer to convert to long beforehand.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20258#discussion_r1684570869
RFR: 8336833: Endless loop in Javap ClassWriter
Artificially corrupted class with overflowing max - min values of `tableswitch` instruction cause infinite loop in `jdk.internal.classfile.impl.CodeImpl::inflateJumpTargets` This patch fixes the overflow and adds relevant test. Please review. Thank you, Adam - Commit messages: - 8336833: Endless loop in Javap ClassWriter Changes: https://git.openjdk.org/jdk/pull/20258/files Webrev: https://webrevs.openjdk.org/?repo=jdk=20258=00 Issue: https://bugs.openjdk.org/browse/JDK-8336833 Stats: 24 lines in 2 files changed: 22 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/20258.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20258/head:pull/20258 PR: https://git.openjdk.org/jdk/pull/20258
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation
On Fri, 19 Jul 2024 01:55:57 GMT, Chen Liang wrote: > `TypeAnnotation` is not an annotation, as it should not be used in places > like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an > annotation at a given location instead of to be an annotation. > > Depends on #20205. Looks good to me. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20247#pullrequestreview-2188296784
Re: RFR: 8336754: Remodel TypeAnnotation to "has" instead of "be" an Annotation
On Fri, 19 Jul 2024 01:55:57 GMT, Chen Liang wrote:
> `TypeAnnotation` is not an annotation, as it should not be used in places
> like `AnnotationValue.ofAnnotation`. Thus it's remodeled to contain an
> annotation at a given location instead of to be an annotation.
>
> Depends on #20205.
src/java.base/share/classes/jdk/internal/classfile/impl/AnnotationReader.java
line 302:
> 300: }
> 301:
> 302: public static void writeTypeAnnotation(BufWriterImpl buf,
> TypeAnnotation ta) {
Is there any reason to move writeTypeAnnotation from UnboundAttribute?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20247#discussion_r1684390757
Re: RFR: 8336777: BufferedMethodBuilder not initialized with static flag
On Thu, 18 Jul 2024 22:21:08 GMT, Chen Liang wrote: > `BufferedMethodBuilder` accepts a static flag and passes it to the > `CodeBuilder` it creates; yet it does not prevent static flag tampering like > `DirectMethodBuilder` does. This patch makes their behaviors consistent. > > Note that the throwing of IAE is provisional; it is open to discussion later. Looks good to me. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20244#pullrequestreview-2188235046
Integrated: 8333812: ClassFile.verify() can throw exceptions instead of returning VerifyErrors
On Thu, 18 Jul 2024 16:23:44 GMT, Adam Sotona wrote: > `ClassFile.verify()` should always return list of verification errors and > never throw an exception, even for corrupted classes. > `BoundAttribute` initializations of `LocalVariableTable` and > `LocalVariableTypeTable` attributes do not expect invalid possible locations > and cause `ClassCastException`. > > This patch fixes `BoundAttribute` to throw `IllegalArgumentException` for > invalid `LocalVariableTable` and `LocalVariableTypeTable` attributes > locations. And makes `VerifierImpl` a bit more resilient to exceptions thrown > from the verifier initialization. > > Relevant test is added. > > Please review. > > Thanks, > Adam This pull request has now been integrated. Changeset: c25c4896 Author:Adam Sotona URL: https://git.openjdk.org/jdk/commit/c25c4896ad9ef031e3cddec493aef66ff87c48a7 Stats: 44 lines in 4 files changed: 37 ins; 0 del; 7 mod 8333812: ClassFile.verify() can throw exceptions instead of returning VerifyErrors Reviewed-by: liach - PR: https://git.openjdk.org/jdk/pull/20241
Re: RFR: 8333812: ClassFile.verify() can throw exceptions instead of returning VerifyErrors [v3]
On Fri, 19 Jul 2024 08:26:46 GMT, Adam Sotona wrote: >> `ClassFile.verify()` should always return list of verification errors and >> never throw an exception, even for corrupted classes. >> `BoundAttribute` initializations of `LocalVariableTable` and >> `LocalVariableTypeTable` attributes do not expect invalid possible locations >> and cause `ClassCastException`. >> >> This patch fixes `BoundAttribute` to throw `IllegalArgumentException` for >> invalid `LocalVariableTable` and `LocalVariableTypeTable` attributes >> locations. And makes `VerifierImpl` a bit more resilient to exceptions >> thrown from the verifier initialization. >> >> Relevant test is added. >> >> Please review. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > nit change Thank you. - PR Comment: https://git.openjdk.org/jdk/pull/20241#issuecomment-2239099479
Re: RFR: 8335939: Hide element writing across the ClassFile API [v2]
On Thu, 18 Jul 2024 22:56:10 GMT, Chen Liang wrote: >> `WritableElement` has always been one of the biggest peculiarities of >> ClassFile API: it exposes element writing yet has no corresponding reading >> ability exposed. Its existence creates a lot of API noise, increasing >> maintenance cost in the long run. (This is an iceberg whose tip was exposed >> in #14705) >> >> Removal details: >> - `LocalVariable/LocalVariableType.writeTo` >> - `WritableElement` >> - In `BufWriter`: >> - `writeList(List)` >> - `writeListIndices`: Hidden as we are not exposing >> `BoundAttribute.readEntryList` >> - `writeBytes(BufWriter other)` >> - `ClassReader.compare`: Avoid reading from `BufWriter` >> >> Future implementation cleanup out of scope of this patch: >> - Annotation writing can be upgraded and move away from `Util.Writable` >> - The writing of CP indices and attributes can move to their dedicated >> methods >> - reading of entry list can pair up with writing of entry list in >> ClassReader/BufWriter in future addition > > 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 ten additional commits since > the last revision: > > - Merge branch 'master' of https://github.com/openjdk/jdk into fix/cf-apis > - Merge BoundAttributeTest > - Merge branch 'master' of https://github.com/openjdk/jdk into fix/cf-apis > - Merge branch 'master' of https://github.com/openjdk/jdk into fix/cf-apis > - 2 test failures > - Web review cleanup > - Remove WritableElement and reduce Writable usage > >The rest of Writable in annotations can be removed in later cleanup > - Fix up usages of Util.write > - Hide writeTo from all class file elements > >To consider the fate of WritableElement later! Marked as reviewed by asotona (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/20205#pullrequestreview-2187499298
Re: RFR: 8333812: ClassFile.verify() can throw exceptions instead of returning VerifyErrors [v3]
> `ClassFile.verify()` should always return list of verification errors and > never throw an exception, even for corrupted classes. > `BoundAttribute` initializations of `LocalVariableTable` and > `LocalVariableTypeTable` attributes do not expect invalid possible locations > and cause `ClassCastException`. > > This patch fixes `BoundAttribute` to throw `IllegalArgumentException` for > invalid `LocalVariableTable` and `LocalVariableTypeTable` attributes > locations. And makes `VerifierImpl` a bit more resilient to exceptions thrown > from the verifier initialization. > > Relevant test is added. > > Please review. > > Thanks, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: nit change - Changes: - all: https://git.openjdk.org/jdk/pull/20241/files - new: https://git.openjdk.org/jdk/pull/20241/files/c5f6e018..af796a0d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=20241=02 - incr: https://webrevs.openjdk.org/?repo=jdk=20241=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/20241.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20241/head:pull/20241 PR: https://git.openjdk.org/jdk/pull/20241
Re: RFR: 8333812: ClassFile.verify() can throw exceptions instead of returning VerifyErrors [v2]
> `ClassFile.verify()` should always return list of verification errors and > never throw an exception, even for corrupted classes. > `BoundAttribute` initializations of `LocalVariableTable` and > `LocalVariableTypeTable` attributes do not expect invalid possible locations > and cause `ClassCastException`. > > This patch fixes `BoundAttribute` to throw `IllegalArgumentException` for > invalid `LocalVariableTable` and `LocalVariableTypeTable` attributes > locations. And makes `VerifierImpl` a bit more resilient to exceptions thrown > from the verifier initialization. > > Relevant test is added. > > Please review. > > Thanks, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: wrapped ClassFile::verify(Method) + added test - Changes: - all: https://git.openjdk.org/jdk/pull/20241/files - new: https://git.openjdk.org/jdk/pull/20241/files/32476edf..c5f6e018 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=20241=01 - incr: https://webrevs.openjdk.org/?repo=jdk=20241=00-01 Stats: 13 lines in 2 files changed: 12 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/20241.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20241/head:pull/20241 PR: https://git.openjdk.org/jdk/pull/20241
Re: RFR: 8333812: ClassFile.verify() can throw exceptions instead of returning VerifyErrors
On Thu, 18 Jul 2024 21:57:52 GMT, Chen Liang wrote:
>> src/java.base/share/classes/jdk/internal/classfile/impl/verifier/VerifierImpl.java
>> line 117:
>>
>>> 115:
>>> 116: public static List verify(ClassModel classModel,
>>> ClassHierarchyResolver classHierarchyResolver, Consumer logger) {
>>> 117: String clsName = classModel.thisClass().asInternalName();
>>
>> This can still throw `ConstantPoolException` if this_class points to a
>> non-Class entry. This entry is lazily read by `ClassReader`, so you can
>> create a `ClassModel` for such a bad class.
>
> Alternatively, a malformed Class constant can point to a non-utf8, so the
> `asInternalName` can fail too.
Right, it will still throw if you pass a broken model to
`ClassFile::verify(ClassModel)`.
I'll catch this top-level exception in both `ClassFile::verify` methods.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20241#discussion_r1683981268
RFR: 8333812: ClassFile.verify() can throw exceptions instead of returning VerifyErrors
`ClassFile.verify()` should always return list of verification errors and never throw an exception, even for corrupted classes. `BoundAttribute` initializations of `LocalVariableTable` and `LocalVariableTypeTable` attributes do not expect invalid possible locations and cause `ClassCastException`. This patch fixes `BoundAttribute` to throw `IllegalArgumentException` for invalid `LocalVariableTable` and `LocalVariableTypeTable` attributes locations. And makes `VerifierImpl` a bit more resilient to exceptions thrown from the verifier initialization. Relevant test is added. Please review. Thanks, Adam - Commit messages: - 8333812: ClassFile.verify() can throw exceptions instead of returning VerifyErrors Changes: https://git.openjdk.org/jdk/pull/20241/files Webrev: https://webrevs.openjdk.org/?repo=jdk=20241=00 Issue: https://bugs.openjdk.org/browse/JDK-8333812 Stats: 31 lines in 3 files changed: 25 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/20241.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20241/head:pull/20241 PR: https://git.openjdk.org/jdk/pull/20241
Re: RFR: 8334771: [TESTBUG] Run TestDockerMemoryMetrics.java with -Xcomp fails exitValue = 137
On Mon, 24 Jun 2024 16:16:29 GMT, SendaoYan wrote: > Hi all, > After [JDK-8294960](https://bugs.openjdk.org/browse/JDK-8294960), the > footprint memory usage increased significantly when run the testcase with > -Xcomp jvm options, then cause the testcase was killed by docker by OOM. > Maybe the footprint memory usage increased was inevitable, so I think we > should increase the smallest memory limite for this testcase. > Only change the testcase, the change has been verified, no risk. I can confirm [JDK-8294960](https://bugs.openjdk.org/browse/JDK-8294960) had effect on JDK bootstrap (benchmarked and discussed in https://github.com/openjdk/jdk/pull/17108 ). And there might be measurable differences in various benchmarks sensitive to JDK bootstrap. However I cannot confirm the numbers above, simply because I do not know what they represent. - PR Comment: https://git.openjdk.org/jdk/pull/19864#issuecomment-2236630582
Re: RFR: 8334771: [TESTBUG] Run TestDockerMemoryMetrics.java with -Xcomp fails exitValue = 137
On Mon, 24 Jun 2024 16:16:29 GMT, SendaoYan wrote: > Hi all, > After [JDK-8294960](https://bugs.openjdk.org/browse/JDK-8294960), the > footprint memory usage increased significantly when run the testcase with > -Xcomp jvm options, then cause the testcase was killed by docker by OOM. > Maybe the footprint memory usage increased was inevitable, so I think we > should increase the smallest memory limite for this testcase. > Only change the testcase, the change has been verified, no risk. Unfortunately I'm not familiar with these tests. - PR Comment: https://git.openjdk.org/jdk/pull/19864#issuecomment-2236018675
Re: RFR: 8336588: Ensure Transform downstream receives upstream start items only after downstream started
On Wed, 17 Jul 2024 21:31:51 GMT, Chen Liang wrote: > There's another bug in ClassFile transform composition where the downstream > transform receives items from upstream transform's chained builders before > the downstream transform itself starts. This is a simple fix, and a test case > against `ClassTransform` is added. Good catch and fix, thanks. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20227#pullrequestreview-2184970902
Re: RFR: 8336585: BoundAttribute.readEntryList not type-safe
On Wed, 17 Jul 2024 20:51:32 GMT, Chen Liang wrote: > Qualify the reading of entry lists with the anticipated types up-front, so we > throw the correct `ConstantPoolException` instead of `ClassCastException` > when we encounter malformed attribute lists. (`ClassModel.getInterfaces` > already behave correctly, in comparison) Looks good to me. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20225#pullrequestreview-2184963817
Re: RFR: 8334714: Class-File API leaves preview [v2]
> 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 Adam Sotona has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits: - Merge branch 'master' into JDK-8334714-final - 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=01 Stats: 715 lines in 166 files changed: 0 ins; 477 del; 238 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: RFR: 8335939: Hide element writing across the ClassFile API
On Tue, 16 Jul 2024 23:50:17 GMT, Chen Liang wrote:
> `WritableElement` has always been one of the biggest peculiarities of
> ClassFile API: it exposes element writing yet has no corresponding reading
> ability exposed. Its existence creates a lot of API noise, increasing
> maintenance cost in the long run. (This is an iceberg whose tip was exposed
> in #14705)
>
> Removal details:
> - `LocalVariable/LocalVariableType.writeTo`
> - `WritableElement`
> - In `BufWriter`:
> - `writeList(List)`
> - `writeListIndices`: Hidden as we are not exposing
> `BoundAttribute.readEntryList`
> - `writeBytes(BufWriter other)`
> - `ClassReader.compare`: Avoid reading from `BufWriter`
>
> Future implementation cleanup out of scope of this patch:
> - Annotation writing can be upgraded and move away from `Util.Writable`
> - The writing of CP indices and attributes can move to their dedicated methods
> - reading of entry list can pair up with writing of entry list in
> ClassReader/BufWriter in future addition
src/java.base/share/classes/jdk/internal/classfile/impl/AnnotationReader.java
line 287:
> 285: public static void writeAnnotation(BufWriterImpl buf, Annotation
> annotation) {
> 286: // handles annotations and type annotations
> 287: // TODO annotation cleanup later
Do you have any specific annotation cleanup in mind?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20205#discussion_r1680657521
Re: RFR: 8335939: Hide element writing across the ClassFile API
On Tue, 16 Jul 2024 23:50:17 GMT, Chen Liang wrote: > `WritableElement` has always been one of the biggest peculiarities of > ClassFile API: it exposes element writing yet has no corresponding reading > ability exposed. Its existence creates a lot of API noise, increasing > maintenance cost in the long run. (This is an iceberg whose tip was exposed > in #14705) > > Removal details: > - `LocalVariable/LocalVariableType.writeTo` > - `WritableElement` > - In `BufWriter`: > - `writeList(List)` > - `writeListIndices`: Hidden as we are not exposing > `BoundAttribute.readEntryList` > - `writeBytes(BufWriter other)` > - `ClassReader.compare`: Avoid reading from `BufWriter` > > Future implementation cleanup out of scope of this patch: > - Annotation writing can be upgraded and move away from `Util.Writable` > - The writing of CP indices and attributes can move to their dedicated methods > - reading of entry list can pair up with writing of entry list in > ClassReader/BufWriter in future addition Great job! - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20205#pullrequestreview-2182288974
Re: RFR: 8335938: Review XxxBuilder.original and XxxModel.parent [v2]
On Mon, 15 Jul 2024 15:16:23 GMT, Chen Liang wrote: >> Remove unused `Class/Field/Method/CodeBuilder.original()`, and make >> `Field/Method/CodeModel.parent()` return present only if it's bound (i.e. >> not buffered transformed). See the CSR for details. > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains two commits: > > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/builder-original > - 8335938: Review XxxBuilder.original and XxxModel.parent Looks good to me, thanks. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20177#pullrequestreview-2182208563
[jdk23] Integrated: 8335820: java/lang/invoke/LFCaching/LFSingleThreadCachingTest.java fails due to IllegalArgumentException: hash must be nonzero
On Mon, 15 Jul 2024 05:53:12 GMT, Adam Sotona wrote: > 8335820: java/lang/invoke/LFCaching/LFSingleThreadCachingTest.java fails due > to IllegalArgumentException: hash must be nonzero This pull request has now been integrated. Changeset: 52cd9bb5 Author: Adam Sotona URL: https://git.openjdk.org/jdk/commit/52cd9bb5345b1dd80293426f4386d408361279e2 Stats: 57 lines in 4 files changed: 7 ins; 7 del; 43 mod 8335820: java/lang/invoke/LFCaching/LFSingleThreadCachingTest.java fails due to IllegalArgumentException: hash must be nonzero Reviewed-by: jpai Backport-of: 3f2636d9b71f5270c83d17dcf5d18cf907978475 - PR: https://git.openjdk.org/jdk/pull/20180
Re: [jdk23] RFR: 8335820: java/lang/invoke/LFCaching/LFSingleThreadCachingTest.java fails due to IllegalArgumentException: hash must be nonzero
On Mon, 15 Jul 2024 05:53:12 GMT, Adam Sotona wrote: > 8335820: java/lang/invoke/LFCaching/LFSingleThreadCachingTest.java fails due > to IllegalArgumentException: hash must be nonzero Thank you. - PR Comment: https://git.openjdk.org/jdk/pull/20180#issuecomment-2228021104
[jdk23] RFR: 8335820: java/lang/invoke/LFCaching/LFSingleThreadCachingTest.java fails due to IllegalArgumentException: hash must be nonzero
8335820: java/lang/invoke/LFCaching/LFSingleThreadCachingTest.java fails due to IllegalArgumentException: hash must be nonzero - Commit messages: - Backport 3f2636d9b71f5270c83d17dcf5d18cf907978475 Changes: https://git.openjdk.org/jdk/pull/20180/files Webrev: https://webrevs.openjdk.org/?repo=jdk=20180=00 Issue: https://bugs.openjdk.org/browse/JDK-8335820 Stats: 57 lines in 4 files changed: 7 ins; 7 del; 43 mod Patch: https://git.openjdk.org/jdk/pull/20180.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20180/head:pull/20180 PR: https://git.openjdk.org/jdk/pull/20180
Re: RFR: 8335642: Hide Transform implementation for Class-File API [v3]
On Sun, 14 Jul 2024 13:49:09 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 with a new target base due to a merge > or a rebase. The pull request now contains three commits: > > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/hide-transform > - return tag required > - 8335642: Hide Transform implementation for Class-File API Marked as reviewed by asotona (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/20102#pullrequestreview-2176891171
Integrated: 8335820: java/lang/invoke/LFCaching/LFSingleThreadCachingTest.java fails due to IllegalArgumentException: hash must be nonzero
On Mon, 8 Jul 2024 13:09:50 GMT, Adam Sotona wrote: > 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 This pull request has now been integrated. Changeset: 3f2636d9 Author:Adam Sotona URL: https://git.openjdk.org/jdk/commit/3f2636d9b71f5270c83d17dcf5d18cf907978475 Stats: 57 lines in 4 files changed: 7 ins; 7 del; 43 mod 8335820: java/lang/invoke/LFCaching/LFSingleThreadCachingTest.java fails due to IllegalArgumentException: hash must be nonzero Reviewed-by: liach - PR: https://git.openjdk.org/jdk/pull/20074
Re: RFR: 8335905: CompoundElement API cleanup [v2]
On Tue, 9 Jul 2024 23:29:28 GMT, Chen Liang wrote: >> `CompoundElement` already inherits `Iterable` and its `forEach(Consumer)`, >> thus we can remove `Iterable elements()` and `forEachElements(Consumer)`. > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > Two usages in tier3 Nice cleanup. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20103#pullrequestreview-2174161566
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
