Add JFR GCHeapSummary event implementation #6466

kazukg · 2023-04-21T00:09:14Z

TL;DR

This pull request allows native applications to emit jdk.GCHeapSummary which is a jfr-event.

Jdk.GCHeapSummary is a jfr-event that has not yet been implemented.
GCHeapSummary is useful for observing changes of usedHeapSize that occur before and after a garbage collection event.

The sample events

jdk.GCHeapSummary {
  startTime = 00:37:25.636
  gcId = 7
  when = "Before GC"
  heapSpace = {
    start = 0x00000000
    committedEnd = 0x00000000
    committedSize = 0 bytes
    reservedEnd = 0x00000000
    reservedSize = 0 bytes
  }
  heapUsed = 218.6 MB
}

jdk.GCHeapSummary {
  startTime = 00:37:25.639
  gcId = 7
  when = "After GC"
  heapSpace = {
    start = 0x00000000
    committedEnd = 0x00000000
    committedSize = 0 bytes
    reservedEnd = 0x00000000
    reservedSize = 0 bytes
  }
  heapUsed = 138.0 MB
}

We can visually monitor the heap usage of the application using VisualVM or JDK Mission Control.

Goals

Implement Start Time, gcId(gcEpoch), gcWhen, usedHeapSize in GCHeapSummary.

Non-Goals

Do not yet implement start, committedEnd, committedSize, reservedEnd, reservedSize in GCHeapSummary.

Because I've been struggling to find out where I can obtain those specific values.
If someone can offer suggestions on how to accomplish those tasks, I am willing to give it a shot.

Details

I developed it with reference to the implementation of jdk.GarbageCollection.

Add GCHeapSummaryEvent, GCHeapSummaryEventSupport.
Modified JfrEvent to add jdk.GCHeapSummary.
Modified GCImpl to call GCHeapSummaryEvent.

Added JfrGCWhen to manage strings like "Before GC" and "After GC".

Added JfrGCWhenSerializer and JfrGCWhens.
Modified JfrFeature and JfrType to add those classes.

Test Code

Add TestGCHeapSummaryEvent
Modified JfrFileParser and GCWhenConstantPoolParser to test JfrType.GCWhen.

I have verified that I can display graphs using VisualVM or JDK Mission Control.

fniephaus · 2023-04-21T21:15:31Z

Could you take a look at this please, @roberttoyonaga?

roberttoyonaga

Hi @kazukg! Thanks for contributing. I've added a few minor comments.

roberttoyonaga · 2023-04-24T16:09:46Z

...m.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/JfrGCHeapSummaryEvent.java

+
+    public static void emitJfrGCHeapSummaryEventBeforeGC(UnsignedWord gcEpoch,long start,long heapUsed) {
+        if (hasJfrSupport() ) {
+            jfrSupport().emitGCHeapSummaryEventBeforeGC(gcEpoch, start,heapUsed);


Can you run the eclipse formatter please? As well as mx checkstyle. I think that could help fix some of the PR gate issues.

Hi @roberttoyonaga !
Thank you for confirming. I'll do that.

I made changes to the source code and ran mx checkstyle on the substratevm folder, verifying that there are no errors.

roberttoyonaga · 2023-04-24T16:11:51Z

...e.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/JfrGCHeapSummaryEventSupport.java

+    }
+
+    @Uninterruptible(reason = "Accesses a JFR buffer.")
+    public void emitGCHeapSummaryEventAfterGC(UnsignedWord gcEpoch, long start, long heapUsed) {


If emitGCHeapSummaryEventAfterGC(UnsignedWord, long, long) and emitGCHeapSummaryEventBeforeGC(UnsignedWord, long, long) are basically the same, why not just add another method emitGCHeapSummaryEvent(UnsignedWord, long , long, JfrGCWhen) to reduce duplication?

I'll do that.

@roberttoyonaga
Could you review the changes I made to the source code?

roberttoyonaga · 2023-04-24T16:13:00Z

...e.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/JfrGCHeapSummaryEventSupport.java

+    public void beforeAnalysis(BeforeAnalysisAccess access) {
+        if (HasJfrSupport.get()) {
+
+            //ImageSingletons.add(JfrGCWhens.class, new JfrGCWhens());


I think you can remove this comment.

I'll do that.

I removed it.

roberttoyonaga · 2023-04-24T16:35:43Z

...e.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/JfrGCHeapSummaryEventSupport.java

+
+
+@AutomaticallyRegisteredFeature
+class JfrGCHeapSummaryEventFeature implements InternalFeature {


Hmm I think this should actually be similar to JfrGCEventFeature and only be included if the default SerialGC is being used. See isInConfiguration

I refered to the source code of JfrGCEventFeature, and added isInConfiguration.

roberttoyonaga · 2023-04-24T16:41:19Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrGCWhens.java

+        int id = whens.length;
+        JfrGCWhen result = new JfrGCWhen(id, when);
+
+        JfrGCWhen[] newArr = Arrays.copyOf(whens, id + 1);


Instead of having the logic this method provides, is there any reason you can't just hardcode initialize the array with "Before GC" and "After GC" JfrGCWhens? Is there a possibility of needing to add other JfrGCWhens in the future?

I don't believe that we will need to add another JfrGCWhen in the future.
I would modify the code to hardcode.

I updated the source code to hardcode the initialization of JfrGCWhens.whens, and added getBeforeGCWhen and getAfterGCWhen methods to allow JfrGCHeapSummaryEventSupport to obtain their values.

Removing unnecessary imports.

- Added `emitGCHeapSummaryEvent` method to reduce duplication. - Hardcoded initialization of `JfrGCWhens.whens`.

Added isInConfiguration to include JfrGCHeapSummaryEventFeature only when the default SerialGC is used.

roberttoyonaga

Thanks for making those changes. I added a couple more minor comments.

roberttoyonaga · 2023-05-03T19:27:39Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrType.java

@@ -44,6 +44,8 @@ public enum JfrType {
    FrameType("jdk.types.FrameType"),
    GCCause("jdk.types.GCCause"),
    GCName("jdk.types.GCName"),
+    GCWhen("jdk.types.GCWhen"),
+    VirtualSpace("jdk.types.VirtualSpace"),


Is this used anywhere?

Indeed, it does seem unnecessary. Initially, I added it without fully understanding it. I will remove it and verify that it does not affect the functionality.

I removed Definition of Virtual Space from JrfType.

roberttoyonaga · 2023-05-03T19:31:59Z

...e.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/JfrGCHeapSummaryEventSupport.java

+
+class JfrGCHeapSummaryEventSupport {
+
+    private final JfrGCWhen before;


Would it be simpler if you didn't cache these JfrGCWhens and instead just called JfrGCWhens.singleton().getBeforeGCWhen() and JfrGCWhens.singleton().getAfterGCWhen() from emitGCHeapSummaryEventBeforeGC and emitGCHeapSummaryEventAfterGC respectively?

I'll do that.

I have completed it.

roberttoyonaga · 2023-05-03T19:37:35Z

...e.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/JfrGCHeapSummaryEventSupport.java

+            // VirtualSpace
+            JfrNativeEventWriter.putLong(data, 0L); // start
+            JfrNativeEventWriter.putLong(data, 0L); // committedEnd : ulong
+            JfrNativeEventWriter.putLong(data, 0L); // committedSize : ulong


Just some thoughts: You could try using GCImpl.getPolicy().getCurrentHeapCapacity() to record the committedSize. I'm not certain, but reservedSize might not be applicable here, so maybe it makes the most sense to leave reservedEnd and reservedSize as 0 for now anyway. You listed these as non-goals so perhaps they can be done at a later time instead.

Thank you for the advice. I am not currently able to try it out but I will do so in a few days based on your recommendation.

I have added code to output the committed size, and I have verified that the committed size is recorded in jdk.GCHeapSummary.

jdk.GCHeapSummary { startTime = 07:17:28.397 gcId = 0 when = "Before GC" heapSpace = { start = 0x00000000 committedEnd = 0x00000000 committedSize = 120.0 MB reservedEnd = 0x00000000 reservedSize = 0 bytes } heapUsed = 48.1 MB } jdk.GCHeapSummary { startTime = 07:17:28.398 gcId = 0 when = "After GC" heapSpace = { start = 0x00000000 committedEnd = 0x00000000 committedSize = 105.0 MB reservedEnd = 0x00000000 reservedSize = 0 bytes } heapUsed = 599.8 kB }

- Changed not to cache - Improved code to emit committed Size.

roberttoyonaga · 2023-05-15T19:06:49Z

Thank you for making those changes. This seems good to me now. @christianhaeubl when you have time, can you have a look at this as well to see if it's ready for integration?

roberttoyonaga · 2023-05-15T19:20:59Z

...e.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/JfrGCHeapSummaryEventSupport.java

+
+class JfrGCHeapSummaryEventSupport {
+
+    public void emitGCHeapSummaryEventBeforeGC(UnsignedWord gcEpoch, long start, long committedSize, long heapUsed) {


nit: As a refactoring, you could get rid of emitGCHeapSummaryEventBeforeGC and emitGCHeapSummaryEventAfterGC and call emitGCHeapSummaryEvent(UnsignedWord , long , long , long , JfrGCWhen) directly from emitJfrGCHeapSummaryEventBeforeGC and emitJfrGCHeapSummaryEventAfterGC (by supplying JfrGCWhens.singleton().getBeforeGCWhen() or JfrGCWhens.singleton().getAfterGCWhen().)

I'll do that.

roberttoyonaga · 2023-05-15T19:21:06Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrGCWhens.java

+
+public class JfrGCWhens {
+
+    private JfrGCWhen[] whens = new JfrGCWhen[]{


nit: I think this can be final

christianhaeubl

Thanks for the PR, I added a few comments.

christianhaeubl · 2023-05-17T12:15:47Z

...m.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/JfrGCHeapSummaryEvent.java

@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved.


Please adjust the copyright headers of the files that you added so that they contain the correct year (it seems that you copy/pasted them from other files).

@christianhaeubl

Thank you for pointing that out.
I was copying and pasting the headers from other files.

I have made some modifications to several files, in addition to the ones I added. Do I need to update the copyright headers of those files as well?

Specifically, the files in question are JfrEvent, JfrFeature, JftType, and JftFileParser.

Regarding the existing files that you modified: you can update the copyright headers if you want to but you don't need to.

Thank you for reply.
I only modify the copyright headers of the files that I've added.

I have completed it.

christianhaeubl · 2023-05-17T12:21:56Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrGCWhen.java

+
+import com.oracle.svm.core.Uninterruptible;
+
+public class JfrGCWhen {


Please use an enum instead of a class. Then, you also don't need the class JfrGCWhens.

I'll do that.

I have completed it.

christianhaeubl · 2023-05-17T12:26:08Z

...e.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/JfrGCHeapSummaryEventSupport.java

+import com.oracle.svm.core.jfr.JfrNativeEventWriterData;
+import com.oracle.svm.core.jfr.JfrNativeEventWriterDataAccess;
+
+class JfrGCHeapSummaryEventSupport {


This class is stateless, so all its methods can be static. After doing that change, you can merge this class with JfrGCHeapSummaryEvent and you can remove the class JfrGCHeapSummaryEventFeature. In the end, your code should look similar to the class ThreadParkEvent.

Thank you for providing a specific example.
I will refer to the implementation of ThreadParkEvent.

I have modified JfrGCHeapSummaryEvent to be composed solely of static methods.

- Changed to be processed by static method - Replaced class with an enum for improved - Updated copyright headers to ensure accuracy

checksyle error

christianhaeubl · 2023-05-30T12:31:06Z

Thanks, I did further cleanups on top, see #6705. I also changed how the used/committed heap size is queried so that the logic is more consistent with other places where we are already querying that information. Let me know if you see any issues with those changes. This PR should get merged in the next few days.

Add GCHeapSummaryEvent

4b45892

oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Apr 21, 2023

kazukg changed the title ~~Add GCHeapSummaryEvent~~ Add JFR GCHeapSummary event implementation Apr 21, 2023

roberttoyonaga reviewed Apr 24, 2023

View reviewed changes

kazukg added 4 commits April 30, 2023 10:53

Modifications to avoid failing the build checks

6f50dae

Removing unnecessary imports.

improvements

6f628b4

- Added `emitGCHeapSummaryEvent` method to reduce duplication. - Hardcoded initialization of `JfrGCWhens.whens`.

fix checkstyle error

b27ff8d

Added isInConfiguration

dc171a2

Added isInConfiguration to include JfrGCHeapSummaryEventFeature only when the default SerialGC is used.

roberttoyonaga reviewed May 3, 2023

View reviewed changes

kazukg added 2 commits May 11, 2023 06:32

Remove unnecessary definition from JfrType

9971562

code improvements

9e99d5a

- Changed not to cache - Improved code to emit committed Size.

roberttoyonaga approved these changes May 15, 2023

View reviewed changes

roberttoyonaga reviewed May 15, 2023

View reviewed changes

christianhaeubl reviewed May 17, 2023

View reviewed changes

kazukg added 2 commits May 29, 2023 09:14

refactor

8adccb5

- Changed to be processed by static method - Replaced class with an enum for improved - Updated copyright headers to ensure accuracy

FIX

f16288b

checksyle error

graalvmbot merged commit a9b50f9 into oracle:master Jun 1, 2023



		@AutomaticallyRegisteredFeature
		class JfrGCHeapSummaryEventFeature implements InternalFeature {


		class JfrGCHeapSummaryEventSupport {

		private final JfrGCWhen before;


		class JfrGCHeapSummaryEventSupport {

		public void emitGCHeapSummaryEventBeforeGC(UnsignedWord gcEpoch, long start, long committedSize, long heapUsed) {


		public class JfrGCWhens {

		private JfrGCWhen[] whens = new JfrGCWhen[]{

		@@ -0,0 +1,54 @@
		/*
		* Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved.


		import com.oracle.svm.core.Uninterruptible;

		public class JfrGCWhen {

Add JFR GCHeapSummary event implementation #6466

Add JFR GCHeapSummary event implementation #6466

Conversation

kazukg commented Apr 21, 2023 • edited Loading

TL;DR

Goals

Non-Goals

Details

fniephaus commented Apr 21, 2023

roberttoyonaga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kazukg Apr 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roberttoyonaga Apr 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kazukg May 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roberttoyonaga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kazukg May 10, 2023 • edited Loading

Choose a reason for hiding this comment

roberttoyonaga commented May 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christianhaeubl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christianhaeubl commented May 30, 2023

kazukg commented Apr 21, 2023 •

edited

Loading

kazukg Apr 25, 2023 •

edited

Loading

roberttoyonaga Apr 24, 2023 •

edited

Loading

kazukg May 1, 2023 •

edited

Loading

kazukg May 10, 2023 •

edited

Loading