Skip to content

Conversation

@zhengyu123
Copy link
Contributor

JFR functionality should be available in core for implementing VM specific events, such as GC events (see #3743 for details).

This patch splits current com.oracle.svm.jfr into com.oracle.svm.core.jfr and com.oracle.svm.hosted.jfr as @christianwimmer suggested.

@christianhaeubl
Copy link
Member

christianhaeubl commented Sep 1, 2021

The basic change looks good but you also need to change the suite.py file (the projects are defined there - the comment in #3743 describes pretty much in detail what needs to be done). Please also make sure that you use the latest sources as there seem to be conflicts between master and this PR.

"workingSets": "SVM",
},

"com.oracle.svm.core.jfr": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should there be an entry for com.oracle.svm.hosted.jfr as well? (maybe similar to the entry for com.oracle.svm.hosted.jdk11`)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure here. Adding com.oracle.svm.core.jfr entry here, because it has extra dependencies, etc.

I believe that com.oracle.svm.hosted.jfr is different from com.oracle.svm.hosted.jdk11. com.oracle.svm.hosted.jdk11 has its own directory. Also, I don't see entries for sub-packages of com.oracle.svm.hosted, e.g. com.oracle.svm.hosted.jdk, etc.

Honestly, I don't quite understand how suite.py works. I would appreciate if you can point me any doc/wiki, etc.

Copy link
Member

Choose a reason for hiding this comment

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

The documentation is located at https://github.com/graalvm/mx

You need to do the following steps:

  • Add a project com.oracle.svm.core.jfr (analogue to com.oracle.svm.jfr) but with only one dependency on com.oracle.svm.core
  • Add a project com.oracle.svm.hosted.jfr (analogue to com.oracle.svm.jfr) but with only one dependency on com.oracle.svm.hosted
  • Add com.oracle.svm.core.jfr as a dependency to com.oracle.svm.core.genscavenge
  • Add the projects that you added to the distribution SVM
  • Remove the project com.oracle.svm.jfr
  • Do mx clean and mx ideclean
  • Test everything with mx build and mx ideinit. It should still be possible to build everything with mx build and the IDE should be happy as well (once you import the new project files generated by mx ideinit).

Copy link
Contributor Author

@zhengyu123 zhengyu123 Sep 2, 2021

Choose a reason for hiding this comment

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

@christianhaeubl Thanks for the instructions.

Now, I am running into Java compliance issue, as com.oracle.svm.core.genscavenge has dependence on com.oracle.svm.core.jfr with higher Java compliance.

Should I move com.oracle.svm.core.jfr under com.oracle.svm.core.jdk11 and create an interface, e.g. JfrSupport to workaround jdk8?

Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

I see, then the only option is to create a proper dummy implementation for JDK8. We would need at least 4 JFR projects then:

  • com.oracle.svm.core.jfr (common infrastructure and interfaces)
  • com.oracle.svm.core.jfr.jdk8 (a dummy implementation that always acts as if JFR support is disabled)
  • com.oracle.svm.core.jfr.jdk11 (the current implementation from com.oracle.svm.jfr)
  • com.oracle.svm.hosted.jfr (assuming that there is nothing JDK-specific in those classes)

com.oracle.svm.core.genscavenge would then only have a dependency on the JDK-independent part, i.e., com.oracle.svm.core.jfr.

"com.oracle.svm.core.genscavenge",
"com.oracle.svm.core.containers",
"com.oracle.svm.jni",
"com.oracle.svm.jfr",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also here, should there be two additional entries for core.jfr and hosted.jfr?

@zhengyu123
Copy link
Contributor Author

Close in favor of PR#3770

@zhengyu123 zhengyu123 closed this Sep 9, 2021
@zhengyu123 zhengyu123 deleted the mv-jfr branch September 29, 2021 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants