RFR: 8268398: 15% increase in JFR footprint in Noop-Base
Hi, Could I have review of a PR that skips bytecode instrumentation for container events and initialisation of Container.metrics() if the JVM is isn't running in a container, or to be precise, if container support is not available. If a user specifies -XX:-UseContainerSupport, events are not emitted. This was the case before this PR, and maybe it should be fixed, but it's an enhancement and outside the scope of this bug. Testing: - jdk/jdk/jfr - Manual testing in a container. I tried to run test/hotspot/jtreg/containers/docker/TestJFREvents.java, but the test is broken (image can't be built, so test is skipped). This fix may not be sufficient to reduce footprint introduced by the container events, but [JDK-8282420: JFR: Remove event handlers](https://bugs.openjdk.org/browse/JDK-8282420) also reduced number of loaded classes which should reduce footprint in JDK 19. There are an ongoing enhancement work to generate bytecode and metadata at build time, which will help, but it will be integrated in JDK 20. Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk19/pull/31/files Webrev: https://webrevs.openjdk.org/?repo=jdk19&pr=31&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8268398 Stats: 55 lines in 8 files changed: 40 ins; 5 del; 10 mod Patch: https://git.openjdk.org/jdk19/pull/31.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/31/head:pull/31 PR: https://git.openjdk.org/jdk19/pull/31
On Thu, 16 Jun 2022 21:41:44 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Hi,
Could I have a review of a PR that skips bytecode instrumentation for container events and initialisation of Container.metrics() if the JVM isn't running in a container, or to be precise, if container support is not available. If a user specifies -XX:-UseContainerSupport, events are not emitted. This was the case before this PR, and maybe it should be fixed, but it's an enhancement and outside the scope of this bug.
Testing: - jdk/jdk/jfr - Manual testing in a container.
I tried to run test/hotspot/jtreg/containers/docker/TestJFREvents.java, but the test is broken (image can't be built, so test is skipped).
This fix may not be sufficient to reduce footprint introduced by the container events, but [JDK-8282420: JFR: Remove event handlers](https://bugs.openjdk.org/browse/JDK-8282420) also reduced number of loaded classes which should reduce footprint in JDK 19. There are an ongoing enhancement work to generate bytecode and metadata at build time, which will help, but it will be integrated in JDK 20.
Thanks Erik
src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java line 221:
219: PlatformEventType pe = configuration.getPlatformEventType(); 220: pe.setRegistered(true); 221: if (jvm.isInstrumented(eventClass) || !Utils.shouldInstrument(pe.isJDK(), pe.getName())) {
This will call `setInstrumented()` even for events which are not really instrumented when they are not required to be instrumented, correct? ------------- PR: https://git.openjdk.org/jdk19/pull/31
On Fri, 17 Jun 2022 11:53:20 GMT, Jaroslav Bachorik <jbachorik@openjdk.org> wrote:
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
Update comment
src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java line 221:
219: PlatformEventType pe = configuration.getPlatformEventType(); 220: pe.setRegistered(true); 221: if (jvm.isInstrumented(eventClass) || !Utils.shouldInstrument(pe.isJDK(), pe.getName())) {
This will call `setInstrumented()` even for events which are not really instrumented when they are not required to be instrumented, correct?
Yes, I added comment to make it more clear. ------------- PR: https://git.openjdk.org/jdk19/pull/31
Hi,
Could I have a review of a PR that skips bytecode instrumentation for container events and initialisation of Container.metrics() if the JVM isn't running in a container, or to be precise, if container support is not available. If a user specifies -XX:-UseContainerSupport, events are not emitted. This was the case before this PR, and maybe it should be fixed, but it's an enhancement and outside the scope of this bug.
Testing: - jdk/jdk/jfr - Manual testing in a container.
I tried to run test/hotspot/jtreg/containers/docker/TestJFREvents.java, but the test is broken (image can't be built, so test is skipped).
This fix may not be sufficient to reduce footprint introduced by the container events, but [JDK-8282420: JFR: Remove event handlers](https://bugs.openjdk.org/browse/JDK-8282420) also reduced number of loaded classes which should reduce footprint in JDK 19. There are an ongoing enhancement work to generate bytecode and metadata at build time, which will help, but it will be integrated in JDK 20.
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision: Update comment ------------- Changes: - all: https://git.openjdk.org/jdk19/pull/31/files - new: https://git.openjdk.org/jdk19/pull/31/files/ac52ac51..66edea79 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk19&pr=31&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk19&pr=31&range=00-01 Stats: 2 lines in 2 files changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk19/pull/31.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/31/head:pull/31 PR: https://git.openjdk.org/jdk19/pull/31
On Fri, 17 Jun 2022 13:34:05 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Hi,
Could I have a review of a PR that skips bytecode instrumentation for container events and initialisation of Container.metrics() if the JVM isn't running in a container, or to be precise, if container support is not available. If a user specifies -XX:-UseContainerSupport, events are not emitted. This was the case before this PR, and maybe it should be fixed, but it's an enhancement and outside the scope of this bug.
Testing: - jdk/jdk/jfr - Manual testing in a container.
I tried to run test/hotspot/jtreg/containers/docker/TestJFREvents.java, but the test is broken (image can't be built, so test is skipped).
This fix may not be sufficient to reduce footprint introduced by the container events, but [JDK-8282420: JFR: Remove event handlers](https://bugs.openjdk.org/browse/JDK-8282420) also reduced number of loaded classes which should reduce footprint in JDK 19. There are an ongoing enhancement work to generate bytecode and metadata at build time, which will help, but it will be integrated in JDK 20.
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
Update comment
Marked as reviewed by jbachorik (Reviewer). ------------- PR: https://git.openjdk.org/jdk19/pull/31
On Fri, 17 Jun 2022 13:34:05 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Hi,
Could I have a review of a PR that skips bytecode instrumentation for container events and initialisation of Container.metrics() if the JVM isn't running in a container, or to be precise, if container support is not available. If a user specifies -XX:-UseContainerSupport, events are not emitted. This was the case before this PR, and maybe it should be fixed, but it's an enhancement and outside the scope of this bug.
Testing: - jdk/jdk/jfr - Manual testing in a container.
I tried to run test/hotspot/jtreg/containers/docker/TestJFREvents.java, but the test is broken (image can't be built, so test is skipped).
This fix may not be sufficient to reduce footprint introduced by the container events, but [JDK-8282420: JFR: Remove event handlers](https://bugs.openjdk.org/browse/JDK-8282420) also reduced number of loaded classes which should reduce footprint in JDK 19. There are an ongoing enhancement work to generate bytecode and metadata at build time, which will help, but it will be integrated in JDK 20.
Thanks Erik
Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
Update comment
src/jdk.jfr/share/classes/jdk/jfr/internal/JVM.java line 631:
629: * can't be emitted anyway. 630: */ 631: public native boolean isContainerized();
@egahlin I'm not sure this comment is correct, given https://bugs.openjdk.org/browse/JDK-8261242 and friends. It'll return true on almost all Linux systems (with cgroups pseudo filesystem available). ------------- PR: https://git.openjdk.org/jdk19/pull/31
On Thu, 16 Jun 2022 21:41:44 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
Hi,
Could I have a review of a PR that skips bytecode instrumentation for container events and initialisation of Container.metrics() if the JVM isn't running in a container, or to be precise, if container support is not available. If a user specifies -XX:-UseContainerSupport, events are not emitted. This was the case before this PR, and maybe it should be fixed, but it's an enhancement and outside the scope of this bug.
Testing: - jdk/jdk/jfr - Manual testing in a container.
I tried to run test/hotspot/jtreg/containers/docker/TestJFREvents.java, but the test is broken (image can't be built, so test is skipped).
This fix may not be sufficient to reduce footprint introduced by the container events, but [JDK-8282420: JFR: Remove event handlers](https://bugs.openjdk.org/browse/JDK-8282420) also reduced number of loaded classes which should reduce footprint in JDK 19. There are an ongoing enhancement work to generate bytecode and metadata at build time, which will help, but it will be integrated in JDK 20.
Thanks Erik
This pull request has now been integrated. Changeset: 97544be5 Author: Erik Gahlin <egahlin@openjdk.org> URL: https://git.openjdk.org/jdk19/commit/97544be5b68860bad0431ec88737ad7cdc28486... Stats: 56 lines in 8 files changed: 41 ins; 5 del; 10 mod 8268398: 15% increase in JFR footprint in Noop-Base Reviewed-by: jbachorik ------------- PR: https://git.openjdk.org/jdk19/pull/31
participants (3)
-
Erik Gahlin
-
Jaroslav Bachorik
-
Severin Gehwolf