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

Add explicit well-known classes SCC handling #18888

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

cjjdespres
Copy link
Contributor

A new storeWellKnownClasses method in J9SharedCache is now responsible for storing a well-known classes object in the SCC.

The J9::AheadOfTimeCompile::offsetInSharedCacheFromPointer has been renamed offsetInSharedCacheFromWellKnownClasses, as getting the offset of the well-known classes object stored in the SVM is currently its sole responsibility.

@cjjdespres
Copy link
Contributor Author

Attn @mpirvu.

@cjjdespres
Copy link
Contributor Author

In #18301 I think I will simply modify J9::AheadOfTimeCompile::offsetInSharedCacheFromWellKnownClasses (or its use site) to get the identifier of the cached well-known classes serialization record from the SVM if we're ignoring the client's SCC during the compilation, since that's the only place where we try to get the well-known classes offset during a compilation.

@cjjdespres
Copy link
Contributor Author

Rebased to bring in the latest merges. Shouldn't have changed anything.

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. I have some small comments inline.

runtime/compiler/codegen/J9AheadOfTimeCompile.cpp Outdated Show resolved Hide resolved
runtime/compiler/env/J9SharedCache.cpp Outdated Show resolved Hide resolved
@mpirvu mpirvu self-assigned this Feb 7, 2024
@cjjdespres
Copy link
Contributor Author

Force-pushed to address comments.

* (uintptr_t *classChainOffsets) value whose first element is the number of subsequent elements,
* with subsequent elements being offsets to class chains.
*
* \param[in] vmThread The buffer to fill with the SCC key
Copy link
Contributor

Choose a reason for hiding this comment

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

The definition of vmThread was copied from below and it's incorrect

* @brief Same circumstance as offsetinSharedCacheFromClass above.
*
* @param sharedCache pointer to the TR_SharedCache object
* @param ptr well-known classes pointer whose offset in the SCC is required
Copy link
Contributor

Choose a reason for hiding this comment

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

ptr --> wellKnownClassesPtr

A new storeWellKnownClasses method in J9SharedCache is now responsible
for storing a well-known classes object in the SCC.

The J9::AheadOfTimeCompile::offsetInSharedCacheFromPointer has been
renamed offsetInSharedCacheFromWellKnownClasses, as getting the offset
of the well-known classes object stored in the SVM is currently
its sole responsibility.

Signed-off-by: Christian Despres <[email protected]>
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 Feb 8, 2024

jenkins test sanity all jdk21

@mpirvu
Copy link
Contributor

mpirvu commented Feb 8, 2024

aix build failed due to infra:
With the following error: /bin/sh: git: not found

@mpirvu
Copy link
Contributor

mpirvu commented Feb 8, 2024

jenkins test sanity aix jdk21

@mpirvu mpirvu merged commit 00cca79 into eclipse-openj9:master Feb 8, 2024
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants