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

JIT support for re-sizable SCC #10369

Merged
merged 13 commits into from
Aug 14, 2020
Merged

Conversation

dsouzai
Copy link
Contributor

@dsouzai dsouzai commented Aug 11, 2020

This had to be reverted in #10357 because of failures in JITServer tests. This PR is a cherry-pick of the commits in #9772 + one more commit to add support for JITServer

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
Duplicate the public TR_J9SharedCache APIs in order to faciliate later
commits which will update the various uses of these APIs to invoke the
appropriate (specific) APIs, rather than the general API currently being
used.

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
Add the APIs to wrap around the new APIs added to TR_J9SharedCache.

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
Update calls to pointerFromOffsetInSharedCache to invoke the correct
queries depending on the context, for example,
romClassFromOffsetInSharedCache if the offset is expected to be the
offset of a J9ROMClass.

Signed-of-by: Irwin D'Souzai <dsouza.gh@gmail.com>
Update calls to isOffsetInSharedCache to invoke the correct
queries depending on the context, for example,
isROMClassInSharedCache in order to check if a J9ROMClass is in the SCC.

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
Update calls to offsetInSharedCacheFromPointer to invoke the correct
queries depending on the context, for example,
offsetInSharedCacheFromROMClass in order to get the offset into the SCC
of a J9ROMClass.

Signed-off-by: Irwin D'Souza <dsouza.gh@gmail.com>
Update calls to isPointerInSharedCache to invoke the correct
queries depending on the context, for example,
isROMClassInSharedCache in order to check if a J9ROMClass is in the SCC.

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
TR_RelocationRecordValidateStaticField inherits from
TR_RelocationRecordValidateInstanceField (which inherits from
TR_RelocationRecordValidateClass). Unlike ValidateInstanceField and
ValidateClass, ValidateStaticField stores an offset to the J9ROMClass of
the class of the field, rather than an offset to the class chain of the
class of the field.

This commit adds a virtual method to these classes to ensure that the
right API is called.

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
This commit adds helper methods that will be used by the public APIs in
TR_J9SharedCache to convert between the various offsets and pointers.

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
The offsets for pointers into the ROMClass section of the SCC are
calculated using the start of the ROMClass section. The offsets for
pointers into the metadata section of the SCC are calculated using
the start of the metadata section (which grows backwards, towards the
ROMClass section). The offset is then left shifted one bit; the low
bit is used to determine whether the offset is calculated from the start
of the ROMClass section or the start of the metadatasection.

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
@dsouzai
Copy link
Contributor Author

dsouzai commented Aug 11, 2020

@mpirvu could you please review, specifically 9dee9ea. The rest of the commits are simply a cherry-pick from #9772, but feel free to review anyway if you'd like.

fyi @ymanton

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 Aug 11, 2020

Some sniff tests first
jenkins test sanity xlinuxjit jdk11

@mpirvu
Copy link
Contributor

mpirvu commented Aug 12, 2020

JITServer seems to be working.
Another remark: the name of TR_J9SharedCache::isPointerInMetadataSectionSectionInCache has "Section" twice. was that intentional?

@dsouzai
Copy link
Contributor Author

dsouzai commented Aug 12, 2020

Another remark: the name of TR_J9SharedCache::isPointerInMetadataSectionSectionInCache has "Section" twice. was that intentional?

Heh no, that was a typo; good catch. I fixed it in 7844bed

@mpirvu
Copy link
Contributor

mpirvu commented Aug 12, 2020

jenkins test sanity all jdk11

@dsouzai
Copy link
Contributor Author

dsouzai commented Aug 13, 2020

@mpirvu Should be ok to merge now.

@dsouzai
Copy link
Contributor Author

dsouzai commented Aug 13, 2020

I had a thought regarding JITServer; do we need to do anything to inform a JITServer instance that doesn't have this change that it shouldn't accept requests from a client that does have this change? Otherwise the offsets might be misinterpreted.

@mpirvu mpirvu self-assigned this Aug 13, 2020
@mpirvu
Copy link
Contributor

mpirvu commented Aug 13, 2020

I had a thought regarding JITServer; do we need to do anything to inform a JITServer instance that doesn't have this change that it shouldn't accept requests from a client that does have this change? Otherwise the offsets might be misinterpreted.

oops, yes in such cases we increment the minor version number:
https://github.com/eclipse/openj9/blob/master/runtime/compiler/net/CommunicationStream.hpp#L105

With this commit, the client will also send over the
romClassStartAddress and the metadataStartAddress for each cache (if
there is more than one layer), and updates the private helper methods in
TR_J9SharedCache to not do the extra checks of ensuring pointers/offsets
are within the appropriate sections in the SCC; this is ok because in
general the JITServer compilation knows what type of data it is
dealing with, and the check would've been done on the client side when
the information was requested.

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
@mpirvu
Copy link
Contributor

mpirvu commented Aug 14, 2020

jenkins test sanity all jdk11

@mpirvu mpirvu merged commit 8dd7d14 into eclipse-openj9:master Aug 14, 2020
@dsouzai dsouzai deleted the sccOffsets branch January 4, 2021 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants