Skip to content

isCompilable() fix for JITServer - #17943

Merged
mpirvu merged 1 commit into
eclipse-openj9:masterfrom
SajinaKandy:isCompilable
Aug 18, 2023
Merged

isCompilable() fix for JITServer#17943
mpirvu merged 1 commit into
eclipse-openj9:masterfrom
SajinaKandy:isCompilable

Conversation

@SajinaKandy

@SajinaKandy SajinaKandy commented Aug 14, 2023

Copy link
Copy Markdown
Contributor

Fixes the issue mentioned in 17931.TR_ResolvedJ9Method::isCompilable() accesses some fields from JavaVM and it's not safe to use as is in out-of-process compilations.
As described in the issue #17931, the deleted "doPrivileged" code is redundant since the base class method TR_ResolvedJ9MethodBase::isCompilable() already filters those methods out.

Closes: ##17931

@mpirvu mpirvu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code should be deleted rather than commented out

@mpirvu

mpirvu commented Aug 14, 2023

Copy link
Copy Markdown
Contributor

I think that the commit message should indicate that the deleted code is redundant since the base class TR_ResolvedJ9Method already filters those methods out

@mpirvu mpirvu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Please squash the two commits

@mpirvu mpirvu self-assigned this Aug 14, 2023
@mpirvu mpirvu added the comp:jitserver Artifacts related to JIT-as-a-Service project label Aug 14, 2023
TR_ResolvedJ9Method::isCompilable() accesses some fields from JavaVM and it's not safe to use as is in out-of-process compilations.
As described in the issue eclipse-openj9#17931, the deleted "doPrivileged" code is redundant since the base class method TR_ResolvedJ9MethodBase::isCompilable() already filters those methods out.

Closes: #eclipse-openj9#17931
Signed-off-by: Sajina Kandy <sajina.kandy@ibm.com>

@mpirvu mpirvu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@mpirvu

mpirvu commented Aug 16, 2023

Copy link
Copy Markdown
Contributor

jenkins test sanity all jdk21

@SajinaKandy

SajinaKandy commented Aug 16, 2023

Copy link
Copy Markdown
Contributor Author

Test results for sanity.functional, sanity.system, sanity.openjdk, sanity.jck: Test Results
The failure in few tests were infra related and/or timing related, and on re-run they passed too:Re-run_1, Re-run_2

@mpirvu

mpirvu commented Aug 16, 2023

Copy link
Copy Markdown
Contributor

Build on AIX failed due to infra:

15:12:37  Failed fetching commit message from git directory: /home/jenkins/workspace/Build_JDK21_ppc64_aix_Personal/.git
15:12:37  With the following error: /bin/sh: git:  not found

@mpirvu

mpirvu commented Aug 17, 2023

Copy link
Copy Markdown
Contributor

jenkins test sanity aix jdk21

@mpirvu
mpirvu marked this pull request as ready for review August 18, 2023 17:01
@mpirvu
mpirvu merged commit 7243f7a into eclipse-openj9:master Aug 18, 2023
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

None yet

Development

Successfully merging this pull request may close these issues.

2 participants