Skip to content

Support ignoring the client's local SCC when compiling a JITServer AOT cache store #18937

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

Merged
merged 5 commits into from
Feb 14, 2024

Conversation

cjjdespres
Copy link
Contributor

A new _useServerOffsets field in VMInfo has been added. It will be set when a client wants its local SCC (whether or not it exists) to be ignored during JITServer AOT cache compilations. Currently the client always sets this field to false.

The methods of TR_J9JITServerSharedCache and the handling of the well-known classes object in the SVM and AOT relocation header initialization have been modified so that it we are compiling an AOT cache store and are ignoring the client's local SCC, we now consult the JITServer AOT cache and use the idAndType of its records in place of local SCC offsets.

Various client-server messages have been modified to accommodate the changes in server compilation. The client will either test for a local SCC before accessing it, or the server will send an explicit boolean parameter indicating that a local SCC offset is required.

@cjjdespres cjjdespres requested a review from dsouzai as a code owner February 12, 2024 16:59
@cjjdespres
Copy link
Contributor Author

Attn @mpirvu. Maybe @dsouzai would like to look as well. This shouldn't change any behaviour, except that in JITServerHelpers::packRemoteROMClassInfo() we now send the name identifying the loader even if we didn't get a class chain identifying the loader. Though now that I think about it, we're still not tracking names without chains, so even that won't change any behaviour right now.

This is the bulk of the work needed in #18301 to support AOT cache store compilations when the client doesn't have a local SCC. Once everything is ready, the client will be able to set vmInfo._useServerOffsets (among other things) and the server will use serialization record identifiers instead of local SCC offsets in relo records.

@cjjdespres
Copy link
Contributor Author

Force-pushed to bring in the ROM class packing changes that were just merged.

@mpirvu mpirvu self-assigned this Feb 12, 2024
@mpirvu mpirvu added the comp:jitserver Artifacts related to JIT-as-a-Service project label Feb 12, 2024
The _useServerOffsets field will be set when a client wants its
local SCC (whether or not it exists) to be ignored during JITServer AOT
cache compilations. Currently the client always sets this field to
false.

Signed-off-by: Christian Despres <[email protected]>
@cjjdespres
Copy link
Contributor Author

Force-pushed just to rebase onto master.

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.

I have some inline comments

The methods of TR_J9JITServerSharedCache and the handling of the
well-known classes object in the SVM and AOT relocation header
initialization have been modified so that it we are compiling an
AOT cache store and are ignoring the client's local SCC, we now consult
the JITServer AOT cache and use the idAndType of its records in place
of local SCC offsets.

Various client-server messages have been modified to accommodate the
changes in server compilation. The client will either test for a local
SCC before accessing it, or the server will send an explicit boolean
parameter indicating that a local SCC offset is required.

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

jenkins test sanity plinuxjit,xlinuxjit,zlinuxjit,alinux64jit jdk17

@mpirvu
Copy link
Contributor

mpirvu commented Feb 14, 2024

jenkins compile aix,win jdk21

@mpirvu
Copy link
Contributor

mpirvu commented Feb 14, 2024

The failure on aarch64 is the well known CRIU restore one, hence merging.

@mpirvu mpirvu merged commit 56d1303 into eclipse-openj9:master Feb 14, 2024
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.

One minor improvement to add to the list (see inline).

// This is an out-of-process compilation; check the cache in the client session first
// This is an out-of-process compilation.

// If we're ignoring the client's SCC, we can skip client consultation here
Copy link
Contributor

Choose a reason for hiding this comment

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

It's still beneficial (at least by avoiding the global lock) to cache the WKC record in the client session and check the cached version before calling JITServerAOTCache::getWellKnownClassesRecord().

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