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

Reduce unnecessary TLS usage in JITServer code (resubmission) #19858

Merged

Conversation

cjjdespres
Copy link
Contributor

Avoid using the thread-local variable TR::compInfoPT (mostly through TR::CompilationInfo::getStream()) in contexts where the compInfoPT or the Compilation is readily available. TLS accesses in dynamically loaded shared libraries (such as the JIT) can have non-negligible overhead.

A resubmission of #14931. That PR also avoided using TLS in the methods of the J9::KnownObjectTable by accessing that class's comp() object to retrieve a JITServer::ServerStream. However, that is (not at all obviously) unsafe to do in that context, because some methods of the J9::KnownObjectTable may be called during the initialization of that very comp() object.

Fixes: #19809

Avoid using the thread-local variable TR::compInfoPT (mostly through
TR::CompilationInfo::getStream()) in contexts where the compInfoPT or
the Compilation is readily available. TLS accesses in dynamically loaded
shared libraries (such as the JIT) can have non-negligible overhead.

A resubmission of eclipse-openj9#14931. That PR also avoided using TLS in the methods
of the J9::KnownObjectTable by accessing that class's comp() object to
retrieve a JITServer::ServerStream. However, that is (not at all
obviously) unsafe to do in that context, because some methods of the
J9::KnownObjectTable may be called during the initialization of that
very comp() object.

Fixes: eclipse-openj9#19809
Signed-off-by: Christian Despres <[email protected]>
@cjjdespres cjjdespres requested a review from dsouzai as a code owner July 15, 2024 17:42
@cjjdespres
Copy link
Contributor Author

Attn @mpirvu

Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

LGTM

@mpirvu
Copy link
Contributor

mpirvu commented Jul 15, 2024

jenkins test sanity,extended,sanity.system,extended.system zlinuxjit,xlinuxjit jdk17

@mpirvu mpirvu self-assigned this Jul 15, 2024
@mpirvu mpirvu added the comp:jitserver Artifacts related to JIT-as-a-Service project label Jul 15, 2024
@mpirvu
Copy link
Contributor

mpirvu commented Jul 16, 2024

The following test failed on zlinux.

15:41:38  ===============================================
15:41:38  Running test DaaLoadTest_daa1_5m_0 ...
15:41:38  ===============================================
15:41:38  DaaLoadTest_daa1_5m_0 Start Time: Mon Jul 15 19:41:37 2024 Epoch Time (ms): 1721072497928
15:41:38  variation: Mode110
15:41:38  JVM_OPTIONS: -XX:+UseJITServer -Xjit -Xgcpolicy:gencon -Xnocompressedrefs 

Given that this test uses -Xnocompressedrefs, I think that JITServer was not used.

@mpirvu mpirvu merged commit f98185a into eclipse-openj9:master Jul 16, 2024
11 of 13 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.

DaaLoadTest_all_special_5m_17 segfault during message handling at JITServer client
2 participants