graal icon indicating copy to clipboard operation
graal copied to clipboard

Add Java-level JFR event tests and annotate JfrAdaptiveSampler methods with BasedOnJDKFile

Open roberttoyonaga opened this issue 1 year ago • 4 comments

Summary:

  1. Add tests for Java-level JFR events.

These events we typically get "for free", but problems related to substitutions can result in them being emitted incorrectly, or not at all.

  1. Add @BasedOnJdkFile to throttler classes.

I'm not sure if its better to annotate each individual method, or simply the whole class. Will this annotation somehow alert us if the relevant code in the JDK changes, or is it just a form of documentation?

roberttoyonaga avatar Mar 08 '24 17:03 roberttoyonaga

I'm not sure if its better to annotate each individual method, or simply the whole class.

I guess it depends. Individual methods give us higher granularity, so if only one method changes this might be simpler to adopt. On the other hand, if all of the cpp file in the JDK is just related to the Java file in native image, a single one might be fine as well.

One thing we should keep in mind when adding these annotations: It should be clear what needs to be done if the referenced JDK code snippet changes. If the is not clear from the native image code alone, please add further comments.

Will this annotation somehow alert us if the relevant code in the JDK changes, or is it just a form of documentation?

There is no public utility yet that uses this information, but we are reviewing those annotation when updating to a new JDK version and take care that they stay up to date.

That said, this is a pretty new approach and we are still learning what works and what not. So with that in mind, I'd deal use this as computer readable documentation for now.

In that sense, your change is very much OK wrt to BasedOnJDKFile, thanks. Very much appreciated!

zapster avatar Mar 11 '24 10:03 zapster

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA). The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public.

Thank you for signing the OCA.

Thank you!

roberttoyonaga avatar May 03 '24 13:05 roberttoyonaga