On Fri, 2 Apr 2021 12:33:03 GMT, Jaroslav Bachorik <jbacho...@openjdk.org> 
wrote:

>> With this change it becomes possible to surface various cgroup level metrics 
>> (available via `jdk.internal.platform.Metrics`) as JFR events.
>> 
>> Only a subset of the metrics exposed by `jdk.internal.platform.Metrics` is 
>> turned into JFR events to start with.
>> * CPU related metrics
>> * Memory related metrics
>> * I/O related metrics
>> 
>> For each of those subsystems a configuration data will be emitted as well. 
>> The initial proposal is to emit the configuration data events at least once 
>> per chunk and the metrics values at 30 seconds interval. 
>> By using these values the emitted events seem to contain useful information 
>> without increasing overhead (the metrics values are read from `/proc` 
>> filesystem so that should not be done too frequently).
>
> Jaroslav Bachorik has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Remove trailing spaces

src/jdk.jfr/share/classes/jdk/jfr/internal/instrument/JDKEvents.java line 163:

> 161:     private static void initializeContainerEvents() {
> 162:         containerMetrics = Container.metrics();
> 163:         if (containerMetrics != null) {

I understand this will reduce startup time, but it's contrary to how we treat 
other events. 

We register events, even if they can't be used. We want users to see what 
events are available (and their metadata) and use JMC recording wizard or other 
means to configure a .jfc file without actually being connected to a 
containerized process. We want the same events to minimize (subtle) platform 
dependent bugs.

I think we should try to find other means to reduce the startup time. It's 
better to have consistent behaviour, but an initial implementation than isn't 
as performant, than inconsistent behavior and somewhat faster implementation.

At some point we will need to address the startup cost of registering Java 
events anyway. For example, we could generate metadata at build time in a 
binary format, similar to what we already do with native events. Could even be 
the same file. Then we can have hundreds of Java events without the cost of 
reflection and unnecessary class loading at startup. We could add a simple 
check so that bytecode for the container events (commit() etc) are not 
generated unless in a container environment. A couple of (cached) checks in 
JVMUpcalls may be sufficient to prevent instrumentation cost.

src/jdk.jfr/share/conf/jfr/default.jfc line 1051:

> 1049:       <flag name="class-loading-enabled" label="Class 
> Loading">false</flag>
> 1050: 
> 1051:       <flag name="container-events-enabled" label="Container 
> Events">true</flag>

I don't think we should create "flag" for "Container Events". Instead we should 
treat them like CPU and other OS events, always on. Since JFR can be used 
outside a container, it seems wrong to have this as an option.

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

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

Reply via email to