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

Use the appropriate JIT API when enabling field watches #4361

Merged
merged 1 commit into from
Jan 21, 2019

Conversation

gacholio
Copy link
Contributor

@gacholio gacholio commented Jan 21, 2019

Use the old jitClassesRedefined API to flush the compilation queue if
inline field watches are not enabled. If they are enabled, use the new
jitFlushCompilationQueue API.

[ci skip]

Signed-off-by: Graham Chapman graham_chapman@ca.ibm.com

@gacholio
Copy link
Contributor Author

FYI @cathyzhyi

@gacholio gacholio requested a review from DanHeidinga January 21, 2019 15:46
@DanHeidinga
Copy link
Member

Should the other block of code checking inlineWatches be rolled into this one?

		if (inlineWatches) {
			/* Make all future compilations inline the field watch code */
			jitConfig->inlineFieldWatches = TRUE;
		}

Use the old jitClassesRedefined API to flush the compilation queue if
inline field watches are not enabled. If they are enabled, use the new
jitFlushCompilationQueue API.

[ci skip]

Signed-off-by: Graham Chapman <graham_chapman@ca.ibm.com>
@gacholio
Copy link
Contributor Author

Suggested change made.

@gacholio
Copy link
Contributor Author

jenkins test sanity zlinux jdk8

Copy link
Member

@DanHeidinga DanHeidinga left a comment

Choose a reason for hiding this comment

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

lgtm as GAC confirmed we have exclusive at this point so there is no ordering concerns with dumping the queue vs setting the inline fieldWatch

@DanHeidinga DanHeidinga self-assigned this Jan 21, 2019
@DanHeidinga DanHeidinga merged commit 176961a into eclipse-openj9:master Jan 21, 2019
@gacholio gacholio deleted the inline branch January 21, 2019 18:10
@gacholio
Copy link
Contributor Author

This should go into the current releases.

@pshipton
Copy link
Member

As it is merged, it will be in the 0.12 release.

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.

3 participants