Skip to content

Conversation

roberttoyonaga
Copy link
Collaborator

Related Issue: #9199

A recent JDK change has caused some JFR substitutions to go out-of-date. Native Image binaries with JFR currently will not build with the latest LabsJDK. This PR fixes those substitutions.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jun 26, 2024
@roberttoyonaga
Copy link
Collaborator Author

@zapster Can you please have a look at this one when you have time?

@roberttoyonaga
Copy link
Collaborator Author

roberttoyonaga commented Jun 26, 2024

Looks like the GHA gates are failing due to:

Internal exception: com.oracle.svm.core.util.UserError$UserException: Substitution target for com.oracle.svm.core.jfr.Target_jdk_jfr_internal_HiddenWait is not loaded. Use field `onlyWith` in the `TargetClass` annotation to make substitution only active when needed.

Maybe the GHAs are not using the latest LabsJDK?
Installing labsjdk-ce-latest-24+2-jvmci-b01 to jdk-dl/labsjdk-ce-latest-24+2-jvmci-b01...
It seems to work fine when I use labsjdk-ce-24+3.

@roberttoyonaga roberttoyonaga force-pushed the fix-jdk-compatibility branch from e9a943e to 484a90c Compare June 26, 2024 17:26
@zakkak zakkak added openjdk-updates Related to compatibility with upstream JDK changes redhat-interest labels Jun 26, 2024
@zapster
Copy link
Member

zapster commented Jun 27, 2024

Thanks @roberttoyonaga for your contribution! We have a similar adoption of this change in our JDK update PR #9181. In general, it always makes sense to look for such JDK update PRs when it comes to recent JDK change adoptions and potentially start discussions there.

In our adoption, we also ported the javaTimeSystemUTC method, because it has a higher resolution. (See also my inline comment.) From the only existing usage in jdk.jfr.internal.MetadataRepository#awaitEpochMilliShift, having only millisecond resolution seems fine, which makes me think we should rather use your approach. However, I'm worried that JVM.nanosNow() might be used by other methods in the future, which require sub-millisecond accuracy. Such new usages would be hard to detect. What do you think?

cc @christianhaeubl

@roberttoyonaga
Copy link
Collaborator Author

Closing this PR because the implementation is already done by another.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OCA Verified All contributors have signed the Oracle Contributor Agreement. openjdk-updates Related to compatibility with upstream JDK changes redhat-interest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants