Skip to content

Fold MethodHandle.asType in Lambdaform-generated methods#23481

Open
matthewhall2 wants to merge 1 commit intoeclipse-openj9:masterfrom
matthewhall2:mh_fold_asType
Open

Fold MethodHandle.asType in Lambdaform-generated methods#23481
matthewhall2 wants to merge 1 commit intoeclipse-openj9:masterfrom
matthewhall2:mh_fold_asType

Conversation

@matthewhall2
Copy link
Copy Markdown
Contributor

@matthewhall2 matthewhall2 commented Mar 10, 2026

When calling originalMH.invoke(args), MH.asType is called at runtime to ensure the arguments are compatible with the original MH. A MethodType for the arguments is constructed and checked against the original MethodHandle's MethodType. This is essentially subtype compatibility.
When the types do not match exactly (but are compatible), the LambdaForm needs to be edited to perform typecasts before making the call to the target, resulting in a new MethodHandle. This can result in substantial runtime overhead.

Const refs allows us to create a reference to the resulting new MethodHandle when it is known to be constant at compile time, and to simply use that reference instead of having to call asType on each invocation of the MethodHandle.

This PR:

  • Adds recognized method for Invokers.checkGenericType (the main callsite for MH.asType).
  • Folds call to the asTypeCache if the types are compatible and the asTypeCache is non-null.
    • Adds const provenance edge from the original MethodHandle to the asTypeCache (the result of the asType) methodhandle.

(for calls like mh.invoke(args), mh is the original MethodHandle, and the result of the asType call is the target MethodHandle.)
Type checking each type in the MT in not needed. If the MT of the asTypeCache MH is the same as the MT of the target MH, we can just use the cached MH. If they are not the same, then we cannot fold since we have no way of obtaining the asType result at compile time. (we also should not even get here if they are not the same, since we shouldn't have a known-object for the target MT in that case).

@matthewhall2 matthewhall2 requested a review from dsouzai as a code owner March 10, 2026 19:53
@matthewhall2 matthewhall2 force-pushed the mh_fold_asType branch 3 times, most recently from e7b9e2f to 62f4605 Compare March 10, 2026 20:45
{ x(TR::java_lang_invoke_MethodHandle_asType_instance, "asType",
"(Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/MethodHandle;") },
{ x(TR::java_lang_invoke_Invokers_checkGenericType, "checkGenericType",
"(Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/MethodHandle;") },
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the right place to add this method. It is only for methods of class MethodHandle.

@matthewhall2 matthewhall2 marked this pull request as draft March 16, 2026 16:41
@matthewhall2 matthewhall2 force-pushed the mh_fold_asType branch 6 times, most recently from e63aa6b to c07d3c9 Compare March 16, 2026 20:21
@matthewhall2
Copy link
Copy Markdown
Contributor Author

thanks @nbhuiyan . I realized there is a much simpler way of checking the type compatibility. See PR description.

@matthewhall2 matthewhall2 marked this pull request as ready for review March 16, 2026 20:43
@matthewhall2 matthewhall2 requested a review from nbhuiyan March 16, 2026 20:43
*/
TR::KnownObjectTable *knot = comp->getKnownObjectTable();
if (!knot)
return false;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return 0 here similar to the return 0 at the end, as the return type of the function is uintptr_t.


/* We shouldn't need to check for anonymous classes or for same class loaders for the Hard Cache case.
* The java.lang.invoke infrastructure only sets this cache when it is safe to do so.
* TODO: if we find that the normal asTypeCache doesn't get many hits, add (careful) checks for the soft cache.å
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like some unintended character at the end of this line.

Also, I prefer not to add more TODO labels. The comment can be updated to describe the TODO as something to consider in the future.

return cachedMH;
}
return 0;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VMAccess ends here, with the function returning a raw pointer (if successful). The callers of this function then takes the resulting raw pointer, and then calls knot->getOrCreateIndex(result of this function), which is not GC safe. The VM access should be held all the way till the knot index is obtained, and the easiest way to do that would be to modify this function to return the knot index instead of the raw pointer, so that VM access is held throughout.

*/
void process_java_lang_invoke_Invokers_checkVarHandleGenericType(TR::TreeTop *tt, TR::Node *node);

void process_java_lang_invoke_MethodHandle_asType(TR::TreeTop *tt, TR::Node *node);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add doxygen documentation

#endif // TR_ALLOW_NON_CONST_KNOWN_OBJECTS
}

void TR_MethodHandleTransformer::process_java_lang_invoke_MethodHandle_asType(TR::TreeTop *tt, TR::Node *node)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should check if const resfs are enabled, without which it should abort.

J9::ConstProvenanceGraph *cpg = comp()->constProvenanceGraph();
// cpg->addEdge(cpg->knownObject(idx), clazz);
auto knot = comp()->getKnownObjectTable();
bool transformed = false;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will trigger unused variable warnings.

logprintf(trace(), comp()->log(), "MethodHandle is obj%d\n", mhIndex);
logprintf(trace(), comp()->log(), "MethodType is obj%d\n", desiredMTIndex);
J9::ConstProvenanceGraph *cpg = comp()->constProvenanceGraph();
// cpg->addEdge(cpg->knownObject(idx), clazz);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not be adding commented out code.

uintptr_t convertedMH = fej9->getConvertedMethodhandle(comp(), mhIndex, desiredMTIndex);
if (0 != convertedMH) {
logprintf(trace(), comp()->log(), "Method types are the compatible%d\n");
TR::KnownObjectTable::Index convertedMHIndex = knot->getOrCreateIndex(convertedMH);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as in InterpreterEmulator. It is not safe to call getOrCreateIndex with a raw pointer without VM access being held for the whole duration.

void TR_MethodHandleTransformer::process_java_lang_invoke_MethodHandle_asType(TR::TreeTop *tt, TR::Node *node)
{
auto mhNode = node->getChild(0);
auto desiredMTNode = node->getChild(1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to use node->getArgument instead of node->getChild when trying to access specific args of a call node, as otherwise we run into the risk of unintentionally accessing the receiver instead of the first arg. In this case, using getChild is not incorrect, but should ideally use getArgument.

logprintf(trace(), comp()->log(), "Exact compatibilty check failed - checking subtype compatibility\n");
uintptr_t convertedMH = fej9->getConvertedMethodhandle(comp(), mhIndex, desiredMTIndex);
if (0 != convertedMH) {
logprintf(trace(), comp()->log(), "Method types are the compatible%d\n");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo, and a format specifier with no args to feed it.

uintptr_t TR_J9VMBase::getConvertedMethodhandle(TR::Compilation *comp, TR::KnownObjectTable::Index mhIndex,
TR::KnownObjectTable::Index desiredTypeIndex)
{
/* Indidivual type compatibilty checks on each type in the MT are not needed. Since we only fold
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sp: Individual

@matthewhall2
Copy link
Copy Markdown
Contributor Author

thanks @nbhuiyan , this is ready for another review

@matthewhall2 matthewhall2 requested a review from nbhuiyan March 24, 2026 14:17
Copy link
Copy Markdown
Member

@nbhuiyan nbhuiyan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to these review comments, please update JITServer MINOR_NUMBER in CommunicationStream.hpp


logprintf(trace(), comp()->log(), "Exact compatibilty check failed - checking subtype compatibility\n");
TR::KnownObjectTable::Index convertedMHIndex
= comp()->fej9()->getConvertedMethodhandle(comp(), mhIndex, desiredMTIndex);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fej9 is being accessed in 2 different ways in the same function.

&& !knot->isNull(desiredMTIndex)) {
J9::ConstProvenanceGraph *cpg = comp()->constProvenanceGraph();
logprintf(trace(), comp()->log(), "Checking exact compatibility\n");
if (fej9->isMethodHandleExpectedType(comp(), mhIndex, desiredMTIndex)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fej9 is being accessed in 2 different ways in the same function.

"VM_targetMethodFromInvokeCacheArrayMemberNameObj",
"VM_isLambdaFormGeneratedMethod",
"VM_getMemberNameMethodInfo",
"VM_isMethodHandleExpectedType",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

* Eliminates calls to Invokers.checkGeneric type when:
* 1. Both the MethodHandle and the MethodType are known objects, and
* 2. The MethodType exactly matches MethodType of the asTypeCache of the MethodHandle
*
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should also document the case where this is true: if (fej9->isMethodHandleExpectedType(comp(), mhIndex, desiredMTIndex)), where the asTypeCache is not involved but does result in transformation.


/**
* \brief
* Eliminates calls to Invokers.checkGeneric type when:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor typo: Invokers.checkGeneric type ---> Invokers.checkGenericType.

return;
}

logprintf(trace(), comp()->log(), "Exact compatibilty check failed - checking subtype compatibility\n");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: compatibility

* \param desiredTypeIndex known object index of java/lang/invoke/MethodType object
* \return the KOI index of the adapted method handle, which will be UNKNOWN if the MethodTypes do not match
*/
virtual OMR::KnownObjectTable::Index getConvertedMethodhandle(TR::Compilation *comp,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally should capitalize the H in handle for consistency. This same comment applies in other places too for this issue (such as VMJ9.cpp, VMJ9Server.hpp, etc).

OMR::KnownObjectTable::Index TR_J9VMBase::getConvertedMethodhandle(TR::Compilation *comp,
TR::KnownObjectTable::Index mhIndex, TR::KnownObjectTable::Index desiredTypeIndex)
{
/* Individual type compatibilty checks on each type in the MT are not needed. Since we only fold
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: compatibility

uintptr_t desiredMTObject = knot->getPointer(desiredTypeIndex);
// there is only one MT instance per unique signature, so this check is valid
if (desiredMTObject == cachedMT) {
logprintf(comp->getOption(TR_TraceILGen), comp->log(), "Hard cache match\n");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more appropriate logging condition is TR_TraceOptDetails here.

* \param node
* The call node representing the call to java/lang/invoke/Invokers.checkGenericType
*/
void process_java_lang_invoke_Invokers_MethodHandle_checkGenericType(TR::TreeTop *tt, TR::Node *node);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed this before, but the correct name for this function should be process_java_lang_invoke_Invokers_checkGenericType to be consistent with the existing pattern in use.


TR::KnownObjectTable::Index convertedMHIndex
= fe->getConvertedMethodHandle(comp, mhIndex, desiredTypeIndex);
client->write(response, convertedMHIndex, knot->getPointerLocation(mhIndex),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, only convertedMHIndex is being sent back to the server, not its pointer location, which is the only thing that's needed in the client response. See how case MessageType::VM_getMethodHandleTableEntryIndex: is implemented as an example you can follow to implement this handler.

auto recv = stream->read<TR::KnownObjectTable::Index, uintptr_t *, uintptr_t *>();

knot->updateKnownObjectTableAtServer(mhIndex, std::get<1>(recv));
knot->updateKnownObjectTableAtServer(desiredTypeIndex, std::get<2>(recv));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to update known object info for the objects that were passed to the client. Only the result (the converted MH, whether or not the converted MH lookup was successful or not) needs updating here. See TR_J9ServerVM::getMethodHandleTableEntryIndex as an example you could follow to implement this function.

TR_J9VMBase *fej9 = static_cast<TR_J9VMBase *>(comp()->fe());
if (knot && isKnownObject(mhIndex) && !knot->isNull(mhIndex) && isKnownObject(desiredMTIndex)
&& !knot->isNull(desiredMTIndex)) {
J9::ConstProvenanceGraph *cpg = comp()->constProvenanceGraph();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line could be moved closer to where cpg is actually used.

break;

Operand *targetMH = topn(1);
Operand *srcMT = topn(0);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe a better variable names here would be mhOperand desiredMTOperand or something similar, keeping it consistent with the MethodHandleTransformer's handler. It was not immediately clear to me what target/src was supposed to mean.

@matthewhall2 matthewhall2 force-pushed the mh_fold_asType branch 2 times, most recently from eb5c169 to 9775904 Compare April 2, 2026 15:35
@matthewhall2 matthewhall2 requested a review from nbhuiyan April 2, 2026 15:41
if (!comp()->useConstRefs())
break;

Operand *recieverMHOperand = topn(1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

stream->write(JITServer::MessageType::VM_getConvertedMethodHandle, mhIndex, desiredTypeIndex);
auto recv = stream->read<TR::KnownObjectTable::Index, uintptr_t *>();

TR::KnownObjectTable::Index mhIndex = std::get<0>(recv);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New declaration using same variable name as parameter here. I think you will get build errors with this. Please use a different name for this.

Adds recognized method for Invokers.checkGenericType (the callsite for
MH.asType).
Folds call to the asTypeCache if the MTs are exact same object.
- Type checking each type in the MT in not needed. If the MT of
  the asTypeCache MH is the same as the MT of the target MH, we can
just use the cache. If they are not the same, then we cannot fold as we
have no way of obtaining the asType result at compile time. (we also
should not even get here if they are not the same, since we shouldn't
have a known-object for the target MT in that case).

Signed-off-by: Matthew Hall <matthew.hall3@outlook.com>
@0xdaryl 0xdaryl self-assigned this Apr 3, 2026
@0xdaryl
Copy link
Copy Markdown
Contributor

0xdaryl commented Apr 3, 2026

Jenkins test sanity all jdk21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

3 participants