Use JFR at build time if JFR is included in the native image

[roberttoyonaga]

This PR enforces JFR is used at build time when JFR support is going to be included in the native image.
This will add support for JFR events that are implemented via byte code instrumentation. These events include jdk.SocketRead, jdk.SocketWrite, jdk.FileRead, jdk.FileWrite, and jdk.FileForce.

[oubidar-Abderrahim]

Hi, Thank you for contributing,
@christianhaeubl @christianwimmer can you please review this PR? Thank you all.

[christianhaeubl]

@roberttoyonaga: thanks for working on that. The changes look good but I encounter the following problem during testing:

There was 1 failure:
1) com.oracle.svm.test.jfr.TestClassEvent#test
java.lang.AssertionError: Event: com.oracle.svm.test.jfr.events.ClassEvent not found in recording!
	at org.junit.Assert.fail(Assert.java:88)
	at com.oracle.svm.test.jfr.JfrTest.checkEvents(JfrTest.java:114)
	at com.oracle.svm.test.jfr.JfrTest.checkRecording(JfrTest.java:124)
	at com.oracle.svm.test.jfr.JfrTest.endRecording(JfrTest.java:80)

On JDK 20, I only see this issue when all the test cases are executed in a single image. A reproducer is below:

graal/substratevm$ mx build
graal/substratevm$ mx native-unittest --build-args '--features=com.oracle.svm.test.ImageInfoTest$TestFeature,com.oracle.svm.test.ServiceLoaderTest$TestFeature,com.oracle.svm.test.SecurityServiceTest$TestFeature' '-H:AdditionalSecurityProviders=com.oracle.svm.test.SecurityServiceTest$NoOpProvider' '-H:AdditionalSecurityServiceTypes=com.oracle.svm.test.SecurityServiceTest$JCACompliantNoOpService' --enable-monitoring=jfr

[roberttoyonaga]

I only see this issue when all the test cases are executed

Hi @christianhaeubl it seems like when the tests testEagerStream and testEagerIterator (but not testLazyStream) in AbstractServiceLoaderTest are run in the same execution as the JFR tests, the first JFR test to run will not emit any java level events. After the first JFR test is run, subsequent JFR tests will emit Java level events normally. Do you know why this might be?

I'm having trouble understanding how ServiceLoader.load(class) is related to JFR event emission or why only the first JFR test in the execution order is affected.

[christianhaeubl]

I debugged the broken test case: JFR events such as jdk.JavaExceptionThrow were emitted even if JFR recording was disabled. This broke some things and eventually resulted in the test case failure as a side effect. I was able to fix this particular problem but the JFR instrumentation for exceptions (jdk.JavaExceptionThrow and jdk.JavaErrorThrow) is still a major issue because it can break Native Image (e.g., Native Image may throw pre-allocated exceptions from uninterruptible or allocation-free code).

[roberttoyonaga]

Do you mean that the problem is that when a pre-allocated exception is thrown, the instrumentation code for exceptions is called which could violate the constraints of uninterruptible/allocation free-code?

[christianhaeubl]

@roberttoyonaga: yes, the instrumentation violates the constraints of uninterruptible/allocation free-code.

[roberttoyonaga]

Hi @christianhaeubl , I think that the exception instrumentation code might actually only be called when a new exception is constructed, not when it eventually gets thrown (ConstructorTracerWriter#visitMethod and ConstructorWriter ). That could mean there won't be violations when using pre-allocated exceptions. Additionally, there might be noise in the build time recording from events due to pre-allocated exceptions, but at least they won't add noise to runtime recordings. What do you think of this?

An alternative is to revisit using substitutions instead of using JFR at build time to add the instrumentation for file, socket, and exception events.

[christianhaeubl]

@roberttoyonaga : I looked into that last week but I forgot to add a comment here: yes, as you mentioned, the instrumentation only instruments the allocation of exceptions and not the exception throw. It seems that I was mislead because a static initializer got called and that initializer did the allocation (so, it looked like as if the throw in the uninterruptible code was instrumented). It should be possible to work around that, I will try to have a more detailed look next week.