On Mon, 12 Apr 2021 18:53:07 GMT, Erik Gahlin <egah...@openjdk.org> wrote:
>> 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.
Right. So, for the initial drop I will just leave the container events
registered unconditionally.
> 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.
Ok. So what would be a good rule-of-thumb for when one should introduce a flag?
-------------
PR: https://git.openjdk.java.net/jdk/pull/3126
