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

Delete unused cache of CodeCacheManager from PersistentInfo #2883

Merged
merged 1 commit into from
Sep 17, 2018

Conversation

0xdaryl
Copy link
Contributor

@0xdaryl 0xdaryl commented Sep 16, 2018

The only reference was in DebugExt, but its value was always NULL.

Signed-off-by: Daryl Maier maier@ca.ibm.com

The only reference was in DebugExt, but its value was always NULL.

Signed-off-by: Daryl Maier <maier@ca.ibm.com>
@0xdaryl
Copy link
Contributor Author

0xdaryl commented Sep 16, 2018

Note to reviewers:

This PR is to remove a CodeCacheManager dependence in PersistentInfo. Its only use is this debug extension, and it is always NULL, hence the affected function has not been doing anything useful since it couldn't get a CodeCacheManager.

The right fix for this is to use the codeCacheManager that is cached in the private JITConfig struct (available from javaVM->jitConfig->privateConfig->codeCacheManager like kca uses). However, the privateConfig will have to be materialized from the remote VM. It wasn't done when the local VM was materialized in dbgext/j9dbgext.c presumably because it doesn't have access to the TR_JitPrivateConfig struct. Materializing this in the JIT debug extension requires the remote VM which is not presently available.

Fixing this properly is far more work with questionable benefit (no real consumers of the debug extensions). Hence, I will leave this unimplemented with a comment in the code. The functionality after this PR is no different than what currently exists (i.e., none).

@0xdaryl 0xdaryl changed the title WIP: Delete unused cache of CodeCacheManager from PersistentInfo Delete unused cache of CodeCacheManager from PersistentInfo Sep 16, 2018
@0xdaryl
Copy link
Contributor Author

0xdaryl commented Sep 16, 2018

Jenkins test sanity

@fjeremic fjeremic self-assigned this Sep 17, 2018
@fjeremic fjeremic merged commit d338f69 into eclipse-openj9:master Sep 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants