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

Restrict special handling of MemorySegment methods to only the getters and setters #20696

Merged
merged 2 commits into from
Nov 29, 2024

Conversation

nbhuiyan
Copy link
Member

This PR does the following:

  • Only set RecognizedMethod info for MemorySegment getters and setters, as other MemorySegment methods do not require special handling - the special handling being forced to iterate with state in InterpreterEmulator.
  • Restrict peeking of MemorySegment method callers to only the getters and setters.

Other MemorySegment methods do not require special handling.

Signed-off-by: Nazim Bhuiyan <[email protected]>
Peeking of MemorySegment method callers are necessary for obtaining
the known object info of the layout argument that is used for
inlining the VarHandle operation methods. This fix restricts the
peeking only to those methods that call the MemorySegment getter
and setter methods, and reduces unnecessary peeking.

Signed-off-by: Nazim Bhuiyan <[email protected]>
@nbhuiyan
Copy link
Member Author

nbhuiyan commented Nov 27, 2024

@jdmpapin I'd appreciate a review from you as this mainly affects Inliner behaviour.

@jdmpapin
Copy link
Contributor

Jenkins test sanity all jdk21

@jdmpapin
Copy link
Contributor

Jenkins test sanity win jdk21

@jdmpapin
Copy link
Contributor

AIX sanity.functional failed "OSCache Test" in shrtest_aix_1. Port layer error code -174 "invalid argument" has been observed in #16448. There was also an assertion failure later in the test, and a similar assertion failure has been observed in #10275. Based on the test output, I think it's related to the destruction of semaphores in response to shared memory errors. There were a few other error codes from the port library besides -174, but it's all related to shared memory and the port library, and potentially the machine setup. I'm confident those errors are unrelated to this change

x86-64 Mac sanity.functional didn't run because it failed to fetch the merge commit

x86-64 Mac sanity.openjdk didn't run because curl couldn't connect to artifactory

z Linux sanity.openjdk failed DeflateIn_InflateOut in jdk_util_1. This doesn't seem to be a known issue. Can you look into this one please, @nbhuiyan?

Jenkins test sanity xmac jdk21

@jdmpapin
Copy link
Contributor

There are no windows build machines. It looks like it's been that way for nearly two weeks

@nbhuiyan
Copy link
Member Author

nbhuiyan commented Nov 29, 2024

z Linux sanity.openjdk failed DeflateIn_InflateOut in jdk_util_1. This doesn't seem to be a known issue.

This was also mentioned in #20376 (comment) last month. I don't think this particular change could cause this failure. I still launched a 20X Grinder run for jdk_util_1 on z Linux. These tests tend to not be reproducible if run on their own, which is why I opted to run the entirety of jdk_util_1 in the Grinder job.

@jdmpapin
Copy link
Contributor

Oh, it is a known issue after all: #14948

For some reason searching the issues for "DeflateIn_InflateOut" returned no results, but the issue shows up if I instead search for "DeflateIn InflateOut" with space instead of underscore

@jdmpapin
Copy link
Contributor

jdmpapin commented Nov 29, 2024

So all test failures are known issues, and the windows build failure is due to a lack of windows build machines. Merging

@jdmpapin jdmpapin merged commit 3111f29 into eclipse-openj9:master Nov 29, 2024
21 of 25 checks passed
@nbhuiyan
Copy link
Member Author

I have aborted the Grinder job so that we do not keep a zlinux machine occupied for a known issue.

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