Re: RFR: 8307818: Convert Indify tool to Classfile API [v16]
On Tue, 25 Jun 2024 13:49:31 GMT, Adam Sotona wrote: >> 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? 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. - PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1652868906
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: 8307818: Convert Indify tool to Classfile API [v16]
On Tue, 25 Jun 2024 13:17:13 GMT, Adam Sotona wrote:
>> 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?
Transforming the classModel back to the bytes array in order to write the
transformed classfile
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1652854265
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: 8307818: Convert Indify tool to Classfile API [v16]
> 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: remove: removed unnecessary logging - Changes: - all: https://git.openjdk.org/jdk/pull/18841/files - new: https://git.openjdk.org/jdk/pull/18841/files/c5a08302..02347415 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18841=15 - incr: https://webrevs.openjdk.org/?repo=jdk=18841=14-15 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18841.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18841/head:pull/18841 PR: https://git.openjdk.org/jdk/pull/18841
