On Wed, 21 Apr 2021 22:38:32 GMT, Erik Gahlin <egah...@openjdk.org> wrote:

>> Jaroslav Bachorik has refreshed the contents of this pull request, and 
>> previous commits have been removed. The incremental views will show 
>> differences compared to the previous content of the PR.
>
> I wonder if something similar to below could be added to 
> jdk.jfr.internal.Utils:
> 
>     private static Metrics[] metrics;
>     public static Metrics getMetrics() {
>         if (metrics == null) {
>             metrics = new Metrics[] { Metrics.systemMetrics() };
>         }
>         return metrics[0];
>     }
> 
>     public static boolean shouldSkipBytecode(String eventName, Class<?> 
> superClass) {
>         if (superClass != AbstractJDKEvent.class) {
>             return false;
>         }
>         if (!eventName.startsWith("jdk.Container")) {
>             return false;
>         }
>         return getMetrics() == null;
>     }
> 
> Then we could add checks to 
> jdk.jfr.internal.JVMUpcalls::bytesForEagerInstrumentation(...)
> 
>     eventName = ei.getEventName();
>     if (Utils.shouldSkipBytecode(eventName, superClass))) {
>         return oldBytes;
>     }
> 
> and jdk.jfr.internal.JVMUpcalls:onRetransform(...)
> 
>     if (jdk.internal.event.Event.class.isAssignableFrom(clazz) && 
> !Modifier.isAbstract(clazz.getModifiers())) {
>         if (Utils.shouldSkipBytecode(clazz.getName(), clazz.getSuperclass())) 
> {
>             return oldBytes;
>         }
> 
> This way we would not pay for generating bytecode for events in a 
> non-container environment. 
> 
> Not sure if it works, but could perhaps make startup faster? We would still 
> pay for generating the event handlers during registration, but it's much 
> trickier to avoid since we need to store the event type somewhere.

> @egahlin Unfortunately, I had to make one late change in the periodic event 
> hook registration.
> If the events are registered conditionally only when running in a container 
> the event metadata are not correct and `TestDefaultConfigurations.java` test 
> will fail. When I register the hooks unconditionally, the metadata is 
> correctly generated and the test passes.
> I will hold off integration until I hear back from you whether this is 
> acceptable or I should try to find an alternative solution.

I think we should always register the metadata, even if you can't get the 
event. 

That's how we handle different GCs. Users must be able to explore events even 
if they can't use them. For example, you must be able to configure container 
events in JMC (with correct labels/descriptions) without actually connecting to 
a JVM running in a Docker container.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3126

Reply via email to