Use correct query to get RAM Method for remote compilations #6911
Conversation
| uintptrj_t ramMethod = (uintptr_t)methodSymbol->getMethodAddress(); | ||
| // For remote compiles we do direct calls through snippets. However the method we're calling might already be compiled. | ||
| // In such a case we can't use getMethodAddress() to get the RAM Method because it returns the compiled code's entry point instead of the RAM Method. | ||
| uintptrj_t ramMethod = comp->getPersistentInfo()->getRemoteCompilationMode() == JITServer::SERVER && !methodSymbol->isInterpreted() ? (uintptrj_t)methodSymRef->getSymbol()->castToResolvedMethodSymbol()->getResolvedMethod()->getPersistentIdentifier() : (uintptrj_t)methodSymbol->getMethodAddress(); |
There was a problem hiding this comment.
Would it be wrong if we don't check whether the method is interpreted and always use getPersistentIdentifier()?
Also, to identify server size compilations comp->isOutOfProcessCompilation() is preferable.
There was a problem hiding this comment.
Just to clarify, are you proposing the following change?
uintptrj_t ramMethod = ..getPersistentIdentifier();
Or something like this:
uintptrj_t ramMethod = comp->isOutOfProcessCompilation() ? ..getPersistentIdentifier() : (uintptrj_t)methodSymbol->getMethodAddress();
There was a problem hiding this comment.
I was suggesting the latter, but now that you mentioned it, why is the first variation wrong? If all we want is the j9method, then getPersistentIdentifier() will us that
There was a problem hiding this comment.
Yes, that's why I was clarifying. I think getPersistentIdentifier() by itself should be sufficient. However I'd like to test it on JITServer as well as master. I'll report back once I have the results. If good, then we can make this change directly in the master branch.
|
Currently testing this in the master branch as well as my jitserver branch. |
|
JITServer build looks good. Still waiting on the master build. |
This query is better than calling getMethodAddress as it will return the correct data even in remote compilations. Signed-off-by: Dhruv Chopra <Dhruv.C.Chopra@ibm.com>
dchopra001
force-pushed
the
ramMethodRemote
branch
from
9309bf6
to
8379a24
Compare
|
@mpirvu Both builds passed. I have repurposed this PR to target the master branch. This should be good to merge now. |
|
Jenkins test sanity zlinux jdk8,jdk11 |
|
Tests have passed, hence merging. |
Signed-off-by: Dhruv Chopra Dhruv.C.Chopra@ibm.com