Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate VM_isClassLibraryClass messages for JITServer #19477

Merged
merged 1 commit into from
May 14, 2024

Conversation

mpirvu
Copy link
Contributor

@mpirvu mpirvu commented May 10, 2024

After work to improve constant field folding, the number of VM_isClassLibraryClass messages increased a lot affecting the CPU overhead of the client.
This commit reimplements the TR_J9ServerVM::isClassLibraryClass() frontend query taking advantage of the caches maintained by JITServer and eliminating said messages completely.

After work to improve constant field folding, the number of
VM_isClassLibraryClass messages increased a lot affecting the
CPU overhead of the client.
This commit reimplements the `TR_J9ServerVM::isClassLibraryClass()`
frontend query taking advantage of the caches maintained
by JITServer and eliminating said messages completely.

Signed-off-by: Marius Pirvu <mpirvu@ca.ibm.com>
@mpirvu mpirvu requested a review from dsouzai as a code owner May 10, 2024 02:07
@mpirvu mpirvu added the comp:jitserver Artifacts related to JIT-as-a-Service project label May 10, 2024
@mpirvu
Copy link
Contributor Author

mpirvu commented May 10, 2024

jenkins test sanity xlinuxjit,zlinuxjit,alinux64jit jdk17

@mpirvu
Copy link
Contributor Author

mpirvu commented May 10, 2024

@ymanton Could you please review/merge this PR? Thanks
@AlexeyKhrabrov Given your expertise on the subject, could you please review this PR as well. Thanks

Copy link
Contributor

@AlexeyKhrabrov AlexeyKhrabrov left a comment

Choose a reason for hiding this comment

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

LGTM, but I have a question/suggestion for a potential future improvement.

If the defining class of a method is not cached at the server, getClassFromMethodBlock() sends a client request that only returns the RAMClass address, and doesn't cache the class:

JITServer::ServerStream *stream = _compInfoPT->getMethodBeingCompiled()->_stream;
stream->write(JITServer::MessageType::VM_getClassFromMethodBlock, method);
return std::get<0>(stream->read<TR_OpaqueClassBlock *>());

This results in two client requests instead of one for classes that are not cached at this point. I'm wondering if this happens frequently enough to warrant including the class info with the VM_getClassFromMethodBlock client response.

@mpirvu
Copy link
Contributor Author

mpirvu commented May 10, 2024

I looked at the message stats and for AcmeAirEE8 there are 10-20 VM_getClassFromMethodBlock messages out of 200,000

Copy link
Member

@ymanton ymanton left a comment

Choose a reason for hiding this comment

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

LGTM.

@ymanton ymanton merged commit dd6f997 into eclipse-openj9:master May 14, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:jitserver Artifacts related to JIT-as-a-Service project
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants