On Sat, 22 Jun 2024 15:55:28 GMT, Oussama Louati <d...@openjdk.org> 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
Even more formatting nits:
test/jdk/java/lang/invoke/indify/Indify.java line 483:
> 481: final char[] poolMarks;
> 482: final Map<String, PoolEntry> constants = new HashMap<>();
> 483: Logic(ClassModel classModel){
Suggestion:
Logic(ClassModel classModel) {
test/jdk/java/lang/invoke/indify/Indify.java line 566:
> 564: )) b.with(e);
> 565: else{
> 566: if(!quietly) System.err.println("Removing
> pattern method: " + ((MethodModel) e).methodName());
Suggestion:
else {
if (!quietly) System.err.println("Removing pattern
method: " + ((MethodModel) e).methodName());
test/jdk/java/lang/invoke/indify/Indify.java line 640:
> 638: }
> 639:
> 640: char nameAndTypeMark(Utf8Entry name, Utf8Entry type){
Suggestion:
char nameAndTypeMark(Utf8Entry name, Utf8Entry type) {
test/jdk/java/lang/invoke/indify/Indify.java line 716:
> 714:
> 715: private PoolEntry scanPattern(MethodModel method, char
> patternMark) {
> 716: if(verbose) System.err.println("Scanning the method: " +
> method.methodName().stringValue() + "for the pattern mark: " + patternMark);
Suggestion:
if (verbose) System.err.println("Scanning the method: " +
method.methodName().stringValue() + "for the pattern mark: " + patternMark);
test/jdk/java/lang/invoke/indify/Indify.java line 731:
> 729: List<Object> bsmArgs = null; // args to invokeGeneric
> 730: decode:
> 731: for(Instruction instruction : instructions){
Suggestion:
for (Instruction instruction : instructions) {
test/jdk/java/lang/invoke/indify/Indify.java line 735:
> 733: int bc = instruction.opcode().bytecode();
> 734: String UNKNOWN_CON = "<unknown>";
> 735: switch (bc){
Suggestion:
switch (bc) {
test/jdk/java/lang/invoke/indify/Indify.java line 743:
> 741: case ANEWARRAY :
> 742: arg = jvm.pop();
> 743: if( !(arg instanceof Integer)) break decode;
Suggestion:
if (!(arg instanceof Integer)) break decode;
test/jdk/java/lang/invoke/indify/Indify.java line 859:
> 857:
> buf.append(argClass.descriptorString());
> 858: }else if (typeArg instanceof
> PoolEntry argCon) {
> 859: if(argCon.tag() == TAG_CLASS) {
Suggestion:
for (Object typeArg : args) {
if (typeArg instanceof Class<?> argClass) {
buf.append(argClass.descriptorString());
} else if (typeArg instanceof PoolEntry
argCon) {
if (argCon.tag() == TAG_CLASS) {
test/jdk/java/lang/invoke/indify/Indify.java line 888:
> 886: continue;
> 887: case "lookupClass":
> 888: if(args.equals(List.of("lookup"))) {
Suggestion:
if (args.equals(List.of("lookup"))) {
test/jdk/java/lang/invoke/indify/Indify.java line 907:
> 905: case "Double.valueOf":
> 906: removeEmptyJVMSlots(args);
> 907: if(args.size() == 1 ) {
Suggestion:
if (args.size() == 1) {
test/jdk/java/lang/invoke/indify/Indify.java line 923:
> 921: continue;
> 922: }
> 923: if(!hasReceiver && ownMethod != null) {
Suggestion:
if (!hasReceiver && ownMethod != null) {
test/jdk/java/lang/invoke/indify/Indify.java line 937:
> 935: {
> 936: ++branchCount;
> 937: if(bsmArgs != null){
Suggestion:
if (bsmArgs != null) {
test/jdk/java/lang/invoke/indify/Indify.java line 952:
> 950: break;
> 951: if((arg instanceof PoolEntry) && ((PoolEntry)
> arg).tag() == wantedTag)
> 952: return (PoolEntry) arg;
Suggestion:
if (branchCount == 2 && UNKNOWN_CON.equals(arg))
break;
if ((arg instanceof PoolEntry) && ((PoolEntry)
arg).tag() == wantedTag)
return (PoolEntry) arg;
test/jdk/java/lang/invoke/indify/Indify.java line 956:
> 954: }
> 955: default:
> 956:
> if(jvm.stackMotion(instruction.opcode().bytecode())) break;
Suggestion:
if (jvm.stackMotion(instruction.opcode().bytecode()))
break;
test/jdk/java/lang/invoke/indify/Indify.java line 1008:
> 1006:
> 1007: }
> 1008: }
Suggestion:
private PoolEntry makeMethodTypeCon(Object x) {
if (x instanceof StringEntry stringEntry) {
return poolBuilder.methodTypeEntry(stringEntry.utf8());
} else {
return
poolBuilder.methodTypeEntry(poolBuilder.utf8Entry((String) x));
}
}
test/jdk/java/lang/invoke/indify/Indify.java line 1044:
> 1042: }
> 1043: return poolBuilder.methodHandleEntry(refKind, ref);
> 1044: }
Suggestion:
private PoolEntry parseMemberLookup(byte refKind, List<Object> args) {
//E.g.: lookup().findStatic(Foo.class, "name", MethodType)
if (args.size() != 4) return null;
NameAndTypeEntry nameAndTypeEntry;
Utf8Entry name, type;
ClassEntry ownerClass;
if (!"lookup".equals(args.getFirst())) return null;
if (args.get(1) instanceof ClassEntry classEntry) ownerClass =
classEntry;
else return null;
if (args.get(2) instanceof StringEntry stringEntry) name =
stringEntry.utf8();
else return null;
if (args.get(3) instanceof MethodTypeEntry methodTypeEntry) type =
methodTypeEntry.descriptor();
else return null;
nameAndTypeEntry = poolBuilder.nameAndTypeEntry(name,type);
MemberRefEntry ref;
if (refKind <= (byte) STATIC_SETTER.refKind) {
ref = poolBuilder.fieldRefEntry(ownerClass, nameAndTypeEntry);
return poolBuilder.methodHandleEntry(refKind, ref);
}
else if (refKind == (byte) INTERFACE_VIRTUAL.refKind) {
ref = poolBuilder.interfaceMethodRefEntry(ownerClass,
nameAndTypeEntry);
return poolBuilder.methodHandleEntry(refKind, ref);
}
else {
ref = poolBuilder.methodRefEntry(ownerClass, nameAndTypeEntry);
}
return poolBuilder.methodHandleEntry(refKind, ref);
}
test/jdk/java/lang/invoke/indify/Indify.java line 1049:
> 1047: // E.g.: MH_bsm.invokeGeneric(lookup(), "name", MethodType,
> "extraArg")
> 1048: removeEmptyJVMSlots(args);
> 1049: if(args.size() < 4) return null;
Suggestion:
if (args.size() < 4) return null;
-------------
PR Review: https://git.openjdk.org/jdk/pull/18841#pullrequestreview-2195922618
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689323572
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689328799
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689323765
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689330738
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689323955
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689324110
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689330906
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689330144
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689331095
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689331287
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689331509
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689324306
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689331836
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689332014
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689325088
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689326268
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1689326428
