Enable Field Watch Support for Power (doc)

[AlenBadel]

This commit enables support for field watch on Power, and proposes some minor clean up.

Signed-off-by: Alen Badel [email protected]

[AlenBadel]

Issue: #3422

Prerequisite: eclipse-omr/omr#4068

[AlenBadel]

FYI: @gita-omr @zl-wang @fjeremic @dchopra001

Still working on some final formatting. I've put this up for review for discussion.

[gita-omr]

Jenkins test sanity aix,plinux jdk8,jdk11

[gita-omr]

Jenkins test sanity plinux jdk8

[dchopra001]

Along with some questions/suggestions about the code, I've left comments suggesting some form and general cleanup. I'll take another pass once @AlenBadel gets a chance to address the current batch.

@gita-omr The changes made here affects X and Z codgens as well, should we run tests on those platforms as well?

[gita-omr]

@gita-omr The changes made here affects X and Z codgens as well, should we run tests on those platforms as well?

Yes, it was just a sniff test on pLinux. Thanks a lot for all the comments! I think I agree with most of them. I am not sure about the inlining though. I think even if function is called only once it makes the logic much more clear in the caller.

[fjeremic]

Approving assuming we'll do a final squash.

[dchopra001]

Quick question: are we going to be running the tests after #6257 has merged? Or will we be running the tests locally?

[gita-omr]

Quick question: are we going to be running the tests after this: #6257 has merged? Or will we be running the tests locally?

Both. While Jenkins is running on tests, @AlenBadel is working on running locally.

[fjeremic]

Jenkins test extended all jdk8,jdk11

[AlenBadel]

Thanks for the feedback during the review. Everything looks to be resolved, so I'm squashing the commits.

[pshipton]

Jenkins test sanity,extended all jdk8,jdk11

[AlenBadel]

Taking a look at the aix failure.

[gita-omr]

in runtime/codert_vm/pnathelp.m4:

SLOW_PATH_ONLY_HELPER_NO_RETURN_VALUE(jitReportInstanceFieldWrite,3)

after tracking backward it seems it eventually calls CALL_C_WITH_VMTHREAD, which is defined as:

define({CALL_C_WITH_VMTHREAD},{
mr r3,J9VMTHREAD
CALL_DIRECT($1)
})

@gacholio will this work with AIX linkage (function pointer has to be dereferenced) and why VMThread?

[gacholio]

Yes, all of this works on AIX (we build it every night). All of the helpers take only the vmThread as the argument, assuming the the register args have been spilled into the ELS save area.

[gita-omr]

@AlenBadel investigated and found that AIX test cases pass with IBM JDK but not with openj9 build. Also, he spoke with Joe, and Joe mentioned that OpenJ9 uses XL C 13.1 while IBM SDK uses 12.

I guess stepping through the debugger is the only way to figure it out.

[AlenBadel]

@gacholio
The JIT is successfully reading the fieldClass flags to determine if the field is being watched, and we only branch into into the helper when J9ClassHasWatchedFields is high.
However when entering into the helper on the VM side it's determining that no flags are set.

Inside
void* J9FASTCALL old_slow_jitReportStaticFieldRead(J9VMThread *currentThread)
When evaluating
if (J9_ARE_ANY_BITS_SET(fieldClass->classFlags, J9ClassHasWatchedFields))
The dataBlock address was correctly passed. The fieldClass address was correctly obtained. The flags not so much.
fieldClass->classFlags returns that J9ClassHasWatchedFields was not set.

[AlenBadel]

#6234 (comment) is no longer the case. Please ignore.

I've isolated the issue and will be pushing a fix sometime today.

[pshipton]

Jenkins test sanity,extended all jdk8,jdk11

[AlenBadel]

The sanity build results from #6234 (comment)
are https://ci.eclipse.org/openj9/job/PullRequest-OpenJ9/685/

All passed other than
Test TestAttachErrorHandling_SE80_0

On AIX Java11/Java8 Fails with:

 Caused by: java.net.SocketTimeoutException: Accept timed out
18:00:54      at java.net.AbstractPlainSocketImpl.accept(AbstractPlainSocketImpl.java:409)
18:00:54      at java.net.ServerSocket.implAccept(ServerSocket.java:545)
18:00:54      at java.net.ServerSocket.accept(ServerSocket.java:513)
18:00:54      at com.ibm.tools.attach.attacher.OpenJ9VirtualMachine.tryAttachTarget(OpenJ9VirtualMachine.java:478)

It's a known issue on AIX.
See #3081

So this PR is safe to merge.

[gita-omr]

@DanHeidinga @pshipton considering the PR is merged, do you think it would be too late to include into 0.15? The only reason really is that it would be nice to ship the same feature together with Z. We also wanted to write a blog about it.

[pshipton]

@gita-omr given this feature missed the feature complete (branch) date and milestone 1, and there doesn't seem to be a compelling reason to put it in this late, I think it can wait for the next release.

[DanHeidinga]

@gita-omr I think it's too late. This PR is large and was merged yesterday which doesn't give it a lot of time to for any hard to reproduce or intermittent issues to be found and resolved.

The 0.15.0 milestone 1 build is being prepared now so the window for larger changes has closed. Sorry.

We also wanted to write a blog about it.

This is a great idea regardless of which release it goes in. Looking forward to reading it.