Update JFR Native Image Support for compatibility with JDK 17 #3726

jiekang · 2021-08-26T01:50:21Z

jiekang · 2021-08-26T01:51:54Z

@christianhaeubl Hi Christian! I started this PR with the intent to let the changes be reviewed sooner rather than later. There is still some work to do for closing chunks properly in 17, but otherwise it compiles and runs without error.

graalvmbot · 2021-08-26T02:00:09Z

Hello Zhengyu Gu, thanks for contributing a PR to our project!

We use the Oracle Contributor Agreement to make the copyright of contributions clear. We don't have a record of you having signed this yet, based on your email address zgu -(at)- redhat -(dot)- com. You can sign it at that link.

If you think you've already signed it, please comment below and we'll check.

graalvmbot · 2021-08-26T05:30:12Z

Zhengyu Gu has signed the Oracle Contributor Agreement (based on email address zgu -(at)- redhat -(dot)- com) so can contribute to this repository.

jiekang · 2021-08-26T16:09:22Z

Just noting in a comment here: the current issue is that the jdk.jfr.internal.ShutdownHook to support dumponexit is not running in the image for 17, but is for 11. It's not touched directly by us though, so continuing to investigate.

jiekang · 2021-08-26T20:15:44Z

Just noting in a comment here: the current issue is that the jdk.jfr.internal.ShutdownHook to support dumponexit is not running in the image for 17, but is for 11. It's not touched directly by us though, so continuing to investigate.

Issue described here is resolved now.

jiekang · 2021-08-27T15:09:39Z

Note this still needs H:+PlatformInterfaceCompatibilityMode to compile. Not yet sure how to resolve that.

…event handlers

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/HiddenClassSupport.java

christianhaeubl

Thanks for working on the JFR support for JDK17. I added a few comments, suggestions, and questions.

substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/Target_jdk_jfr_internal_JVM.java

substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/SubstrateJVM.java

christianhaeubl · 2021-09-04T07:51:48Z

substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrManager.java

@@ -107,15 +121,175 @@ private static void initRecording() {
        Boolean dumpOnExit = parseBoolean(args, JfrStartArgument.DumpOnExit);
        Boolean pathToGcRoots = parseBoolean(args, JfrStartArgument.PathToGCRoots);

-        Target_jdk_jfr_internal_dcmd_DCmdStart dcmdStart = new Target_jdk_jfr_internal_dcmd_DCmdStart();


What is the reason for copying the DCmdStart implementation here? Copying code like that typically creates major issues in the long run.

DCmdStart and surrounding functionality was heavily modified in OpenJDK here: openjdk/jdk@f716711#diff-437f27c74f0b6eb69196ae8b8a27d398a188da4809c6c755ec8eae771e3f4b94R64

I don't know how to support both method signatures of DcmdStart.execute from 11 and 17 so I decided to follow the execution manually instead.

Specifically

public String[] execute(String name, String[] settings, Long delay, Long duration, Boolean disk, String path, Long maxAge, Long maxSize, Long flush, Boolean dumpOnExit, Boolean pathToGcRoots) throws DCmdException {

to

@Override public void execute(ArgumentParser parser) throws DCmdException {

I can revisit this.

If you see any chance of simplifying that, please open a new PR. This PR was merged to master in the meanwhile.

@christianhaeubl Okay. I will create a new issue specifically for DcmdStart support and look at that. Thanks for reviewing and merging this.

I have an attempt similar to HiddenClassSupport but the changes are quite substantial to support the two quite different APIs :|

substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/logging/JfrLogConfiguration.java

substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrTypeRepository.java

.../com.oracle.svm.core.jdk15/src/com/oracle/svm/core/jdk15/HiddenClassSupportJDK15OrLater.java

christianhaeubl · 2021-09-04T08:39:27Z

@jiekang: what is the error message that you get without -H:+PlatformInterfaceCompatibilityMode?

jiekang · 2021-09-06T15:27:54Z

@jiekang: what is the error message that you get without -H:+PlatformInterfaceCompatibilityMode?

@christianhaeubl

[com.redhat.jfr.tests.event.testconcurrenteventsclean:83210]    classlist:   3,804.60 ms,  0.96 GB
[com.redhat.jfr.tests.event.testconcurrenteventsclean:83210]        (cap):     742.61 ms,  0.96 GB
[com.redhat.jfr.tests.event.testconcurrenteventsclean:83210]        setup:   3,868.15 ms,  0.96 GB
[com.redhat.jfr.tests.event.testconcurrenteventsclean:83210]     (clinit):     462.59 ms,  2.38 GB
[com.redhat.jfr.tests.event.testconcurrenteventsclean:83210]   (typeflow):   5,848.35 ms,  2.38 GB
[com.redhat.jfr.tests.event.testconcurrenteventsclean:83210]    (objects):  40,922.54 ms,  2.38 GB
[com.redhat.jfr.tests.event.testconcurrenteventsclean:83210]   (features):   1,174.98 ms,  2.38 GB
[com.redhat.jfr.tests.event.testconcurrenteventsclean:83210]     analysis:  49,469.98 ms,  2.38 GB
Error: The interface org.graalvm.nativeimage.hosted.Feature is not available in the current platform, but used by com.oracle.svm.jfr.JfrFeature. GraalVM before version 21.2 ignored such interfaces, but this was an oversight. The old behavior can be temporarily restored using the option -H:+PlatformInterfaceCompatibilityMode. This option will be removed in a future GraalVM version.
Detailed message:
Trace: Object was reached by 
	reading field java.lang.Class.nestHost of
		constant java.lang.Class@f973499 reached by 
	Hub

com.oracle.svm.core.util.UserError$UserException: The interface org.graalvm.nativeimage.hosted.Feature is not available in the current platform, but used by com.oracle.svm.jfr.JfrFeature. GraalVM before version 21.2 ignored such interfaces, but this was an oversight. The old behavior can be temporarily restored using the option -H:+PlatformInterfaceCompatibilityMode. This option will be removed in a future GraalVM version.
Detailed message:
Trace: Object was reached by 
	reading field java.lang.Class.nestHost of
		constant java.lang.Class@f973499 reached by 
	Hub

	at com.oracle.svm.core.util.UserError.abort(UserError.java:87)
	at com.oracle.svm.hosted.FallbackFeature.reportAsFallback(FallbackFeature.java:233)
	at com.oracle.svm.hosted.NativeImageGenerator.runPointsToAnalysis(NativeImageGenerator.java:764)
	at com.oracle.svm.hosted.NativeImageGenerator.doRun(NativeImageGenerator.java:534)
	at com.oracle.svm.hosted.NativeImageGenerator.run(NativeImageGenerator.java:493)
	at com.oracle.svm.hosted.NativeImageGeneratorRunner.buildImage(NativeImageGeneratorRunner.java:400)
	at com.oracle.svm.hosted.NativeImageGeneratorRunner.build(NativeImageGeneratorRunner.java:566)
	at com.oracle.svm.hosted.NativeImageGeneratorRunner.main(NativeImageGeneratorRunner.java:122)
	at com.oracle.svm.hosted.NativeImageGeneratorRunner$JDK9Plus.main(NativeImageGeneratorRunner.java:596)
Caused by: com.oracle.graal.pointsto.constraints.UnsupportedFeatureException: The interface org.graalvm.nativeimage.hosted.Feature is not available in the current platform, but used by com.oracle.svm.jfr.JfrFeature. GraalVM before version 21.2 ignored such interfaces, but this was an oversight. The old behavior can be temporarily restored using the option -H:+PlatformInterfaceCompatibilityMode. This option will be removed in a future GraalVM version.
Detailed message:
Trace: Object was reached by 
	reading field java.lang.Class.nestHost of
		constant java.lang.Class@f973499 reached by 
	Hub

	at com.oracle.graal.pointsto.constraints.UnsupportedFeatures.report(UnsupportedFeatures.java:126)
	at com.oracle.svm.hosted.NativeImageGenerator.runPointsToAnalysis(NativeImageGenerator.java:761)
	... 6 more
[com.redhat.jfr.tests.event.testconcurrenteventsclean:83210]      [total]:  57,542.99 ms,  2.38 GB

christianhaeubl · 2021-09-06T15:55:24Z

@jiekan: thanks for the error message, I will try to debug that tomorrow.

christianhaeubl · 2021-09-07T07:00:50Z

@jiekang: regarding the issue that -H:+PlatformInterfaceCompatibilityMode is needed: the JFRFeature is reachable at runtime because of lambdas that are used in JFRFeature (this happens because of existing code and is not caused by any code that you introduced in your PR). I implemented a fix.

Let me know if I should push the fix to your branch.

jiekang · 2021-09-07T12:17:45Z

@jiekang: regarding the issue that -H:+PlatformInterfaceCompatibilityMode is needed: the JFRFeature is reachable at runtime because of lambdas that are used in JFRFeature (this happens because of existing code and is not caused by any code that you introduced in your PR). I implemented a fix.

Let me know if I should push the fix to your branch.

@christianhaeubl Please feel free to push the fix to this branch. I also do not mind having it merged in a separate PR and I can rebase this one easily enough. Whichever is ideal for you works for me. Thank you for investigating and fixing this.

christianhaeubl · 2021-09-07T12:20:53Z

@jiekang : I pushed my fix to your branch.

jiekang requested a review from christianhaeubl August 26, 2021 01:50

jiekang changed the title ~~Update JFR Native Image Support for compatibility with JDK 17~~ [WIP] Update JFR Native Image Support for compatibility with JDK 17 Aug 26, 2021

graalvmbot added the oca-signed label Aug 26, 2021

jiekang and others added 12 commits August 27, 2021 11:54

Update JFR Native Image support for JDK 17

ee58378

Add more JFR substitutions for JDK 17

5d3ecbd

Drop DCmdStart use and create recording from startup flags manually

7698f2a

Fix substitution of JVM.isRecording

b918460

Ignore jdk.jfr.events.AbstractBufferStatisticsEvent when registering …

c6be8c6

…event handlers

v0

3ffae77

Fix recording name

13466d5

Added JDK17 support

4a1b7b2

Fix JfrLogConfiguration for JDK17

585c159

Added hidden class support

470c1e3

Temporarily add empty flush method

0128a65

Add more substitutions related to streaming in 17

b363a3d

zhengyu123 reviewed Aug 30, 2021

View reviewed changes

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/HiddenClassSupport.java Outdated Show resolved Hide resolved

Remove unused imports in HiddenClassSupport

0a06b94

jiekang force-pushed the jkang-3538 branch from b813735 to 0a06b94 Compare August 30, 2021 18:03

Fix formatting in HiddenClassSupport

b97953e

jiekang requested a review from teshull September 1, 2021 14:17

jiekang changed the title ~~[WIP] Update JFR Native Image Support for compatibility with JDK 17~~ Update JFR Native Image Support for compatibility with JDK 17 Sep 1, 2021

christianhaeubl reviewed Sep 4, 2021

View reviewed changes

jiekang and others added 4 commits September 6, 2021 12:00

Fixes for JDK 17 compatibility review comments

d09721a

Remove setHandler and extra trace code

8772b69

Add fixes for JDK 16 support

35da244

Fix issue that JFRFeature is reachable at runtime.

d9d2587

jiekang added 2 commits September 9, 2021 09:26

Update comment for recording field in SubstrateJVM

c240362

Fix imports for Target_jdk_jfr_internal_JVM

2c02590

christianhaeubl closed this Sep 13, 2021

Update JFR Native Image Support for compatibility with JDK 17 #3726

Update JFR Native Image Support for compatibility with JDK 17 #3726

Uh oh!

Conversation

jiekang commented Aug 26, 2021

Uh oh!

jiekang commented Aug 26, 2021

Uh oh!

graalvmbot commented Aug 26, 2021

Uh oh!

graalvmbot commented Aug 26, 2021

Uh oh!

jiekang commented Aug 26, 2021

Uh oh!

jiekang commented Aug 26, 2021

Uh oh!

jiekang commented Aug 27, 2021

Uh oh!

Uh oh!

christianhaeubl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

christianhaeubl Sep 4, 2021

Choose a reason for hiding this comment

Uh oh!

jiekang Sep 6, 2021

Choose a reason for hiding this comment

Uh oh!

christianhaeubl Sep 13, 2021

Choose a reason for hiding this comment

Uh oh!

jiekang Sep 13, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

christianhaeubl commented Sep 4, 2021

Uh oh!

jiekang commented Sep 6, 2021

Uh oh!

christianhaeubl commented Sep 6, 2021

Uh oh!

christianhaeubl commented Sep 7, 2021

Uh oh!

jiekang commented Sep 7, 2021

Uh oh!

christianhaeubl commented Sep 7, 2021

Uh oh!

Uh oh!