[GR-47109] Implement JFR Event Throttling. #6899

roberttoyonaga · 2023-06-27T18:25:16Z

Summary of Changes

These changes introduce JFR event throttling infrastructure. Throttling allows for an emission rate to be specified for a given event. This will allow for enabling allocation profiling via thejdk.ObjectAllocationSample event by default. Without throttling, this allocation profiling could produce too much overhead to enable by default. The approach taken in this PR is very similar to how the jfrAdaptiveSampler works in the OpenJDK.

Related Issue #5729

More Details

The implementation uses an alternating window scheme where multiple windows make up a period ( which can be specified by the user).
Only one thread may rotate windows at a time.
Each event type that allows throttling will have it's own throttling instance. But, similar to OpenJDK, threads share the same throttler for any given event type.
There is a concept of sampling debt. If the sampler under-samples during a window, debt will accumulate, causing more samples to be taken in successive windows.
Sampling is done based on a "projected population size" for the next window. This is an exponential weighted moving average that depends on previous windows (the "lookback")
Throttler code is allocation free because it is along the allocation slow path.

Notes

I've opted to use VMMutex for synchronization because it allows for locking with a state transition and can be used in interuptible code. The only issue is that there is no tryLock() method that could be used to optimize contention for window rotations.

…test of distribution

fniephaus · 2023-07-07T08:46:26Z

Should we add a changelog entry for the new event?

roberttoyonaga · 2023-07-07T12:49:54Z

Should we add a changelog entry for the new event?

Yup that's true, I'll add a note to the change log.

christianhaeubl

Thanks for the PR, I added a couple of questions and comments.

christianhaeubl · 2023-09-19T07:55:05Z

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

@@ -237,7 +237,7 @@ private static Object slowPathNewInstanceWithoutAllocating(DynamicHub hub) {
            AlignedHeader newTlab = HeapImpl.getChunkProvider().produceAlignedChunk();
            return allocateInstanceInNewTlab(hub, newTlab);
        } finally {
-            ObjectAllocationInNewTLABEvent.emit(startTicks, hub, LayoutEncoding.getPureInstanceAllocationSize(hub.getLayoutEncoding()), HeapParameters.getAlignedHeapChunkSize());
+            JfrAllocationEvents.emit(startTicks, DynamicHub.toClass(hub), LayoutEncoding.getPureInstanceAllocationSize(hub.getLayoutEncoding()), HeapParameters.getAlignedHeapChunkSize());


I think it is easier to do the conversion from DynamicHub to Class inside of JfrAllocationEvents.emit(...).

The size computation can be moved out of allocateInstanceInNewTlab, then there is no need to call LayoutEncoding.getPureInstanceAllocationSize(hub.getLayoutEncoding()) again.

Good idea! Done

christianhaeubl · 2023-09-19T07:56:29Z

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

+/**
+ * Each event that allows throttling should have its own throttler instance.
+ */
+public class JfrThrottler {


I suppose this class is based on HotSpot code? Please document the git revision, the JDK version, and the C++ source files that this is based on.

Please also do the same for JfrThrottlerWindow.

christianhaeubl · 2023-09-19T07:57:16Z

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

+ */
+public class JfrThrottler {
+    private static final long SECOND_IN_MS = 1000;
+    private static final long SECOND_IN_NS = 1000000 * SECOND_IN_MS;


There are already constants for that, see TimeUtils.

christianhaeubl · 2023-09-19T08:00:32Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/events/JfrAllocationEvents.java

+        if (HasJfrSupport.get()) {
+            emitObjectAllocationInNewTLAB(startTicks, clazz, allocationSize, tlabSize);
+
+            if (shouldEmitObjectAllocationSample() && SubstrateJVM.get().shouldCommit(JfrEvent.ObjectAllocationSample)) {


I think that JfrEvent.shouldEmit() should always call SubstrateJVM.get().shouldCommit(...). Otherwise, it is easy to forget that throttling is necessary for certain events.

That's true. I've taken your suggestion and also made all the throttler code uninterruptible. It would now have to be uninterruptible anyway due to locking without transition (see other responses).

christianhaeubl · 2023-09-19T08:02:07Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/events/JfrAllocationEvents.java

+    }
+
+    /**
+     * This method exists as a slight optimization to avoid entering the sampler code if


I don't think that any optimization is necessary in this case. This is only called by slow-path code and the overhead is just one extra method call. Note that it is also impossible to inline shouldEmitObjectAllocationSample() because it is (and also needs to be) uninterruptible, so there is already a method call anyways.

Optimizations like that are primarily necessary in situations where we would start a VM operation (because that is truly expensive).

christianhaeubl · 2023-09-19T13:12:23Z

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

+    /**
+     * A rotation of the active window could happen while in this method. If so, then this window
+     * will be updated as usual, although it is now the "next" window. This results in some wasted
+     * effort, but doesn't affect correctness because this window will be reset before it becomes


Doesn't this assume that the active window never changes more than once while this method is executed. I don't think that there is anything is in place to guarantee that. How is this solved on HotSpot?

It doesn't seem like hotspot does anything to prevent such a double rotation. If a double rotation happens while a thread is busy sampling the following could happen:

The sample gets counted towards the new window (increases the measured population). It could be argued that this is ok since the evaluation happened during the new window's time slice. Depends on when we define a sample as being "taken".

Sampling races with the rotating thread writing new window parameters. This can cause it to either sample when it would otherwise not have, or not sample when it otherwise would have. Under sampling should be fine in this edge case. Oversampling is bad.

To avoid those problems and any ambiguity about which window a sample belongs to, I've added a read-write lock. The writer's lock must be acquired for window rotation. The reader's lock must be acquired to sample.

There is locking without transition so I've made the appropriate code uninterruptible. This also required avoiding Math.Random and instead copying the simple PRNG that JFR uses in hotspot.

christianhaeubl · 2023-09-19T13:20:15Z

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

+    // The following are set to match the values in OpenJDK
+    private static final int WINDOW_DIVISOR = 5;
+    private static final int LOW_RATE_UPPER_BOUND = 9;
+    private static final int EVENT_THROTTLER_OFF = -2;


Probably better to use @Alias on jdk.jfr.internal.Utils.THROTTLE_OFF.

christianhaeubl · 2023-09-19T13:38:54Z

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

+        }
+
+        this.eventSampleSize = samplesPerPeriod;
+        this.periodNs = (long) periodMs * 1000000;


Please use TimeUtils.millisToNanos(...).

christianhaeubl · 2023-09-19T14:11:36Z

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

+    }
+
+    /** Visible for testing. */
+    public volatile boolean isTest = false;


I am not a big fan of whitebox tests (tight coupling with implementation, become useless pretty quickly if the implementation changes), especially if they add extra fields and logic to production code. If you really need some way to customize the behavior for the test cases, then I think it would be better to create a dedicated subclass that only exists in the test project and where you @Override certain methods.

I've taken your suggestion and moved the code needed for testing out of JfrThrottler and JfrThrottlerWindow classes and into dedicated subclasses in the testing code. Some private methods/fields I've had to make protected in order to @Override.

christianhaeubl · 2023-09-19T15:57:05Z

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

+            samplesPerWindow = eventSampleSize;
+            windowDurationNs = periodNs;
+        }
+        activeWindow.samplesPerWindow = samplesPerWindow;


Other threads may access the values of the active window, so isn't it always problematic when the values of the active window are changed (i.e., other threads may read inconsistent data)?

Yes, other threads busy sampling use the active window, but they never use the same fields we are changing here. The lock must be acquired before accessing those fields.

But either way, this shouldn't be a problem anymore because the read-write lock prevents samplers from working while we are meddling with the active window.

… fields by lock access. Restrict access in JfrThrottler.

minor clean up

clean up JFR random

style

roberttoyonaga · 2024-01-04T16:48:55Z

Hi @christianhaeubl, just commenting here to keep this PR on your radar.

christianhaeubl · 2024-01-31T16:16:27Z

@roberttoyonaga : I started integrating this PR. I will let you know if I run into any issues.

roberttoyonaga · 2024-02-02T23:17:08Z

@roberttoyonaga : I started integrating this PR. I will let you know if I run into any issues.

Thank you Christian!

roberttoyonaga added 13 commits June 16, 2023 13:50

rebase with master. basic. Not uninterruptible. No stack traces

f3f4d51

add weight

6aeef6b

evenly space out samples based on previous window

b72be8a

debt. tests. EWMA.

bdc75f2

normalize and set low rate. tests pass

59b19f4

minor updates, comments, text adjustment

26ab73c

improve tests

f4ce168

use VMMutex instead of spinlock. Minor tweaks

1d6f2f4

fix computeAccumulatedDebtCarryLimit. Add zeroRate test. Add partial …

0c1736d

…test of distribution

distribution tests and fix adjustedProjectedpop

929debd

cleanup. Fix EWMA test case

d1386ea

use geometric sampling distribution. Adjust debt testcase

dc8f2d0

Clean up. Group allocation events. Add testing back door static class.

ecc2da1

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

gate fixes

26b20bd

roberttoyonaga force-pushed the throttle branch from 839ca0b to 26b20bd Compare June 27, 2023 20:00

throttle

2f3ff73

roberttoyonaga added feature native-image redhat-interest labels Jun 30, 2023

clean up volatile vars

e9bb9ac

fniephaus changed the title ~~Implement JFR Event Throttling~~ [GR-47109] Implement JFR Event Throttling. Jul 7, 2023

fniephaus assigned roberttoyonaga Jul 7, 2023

fniephaus requested a review from christianhaeubl July 7, 2023 08:43

roberttoyonaga added 4 commits July 7, 2023 12:14

update changelog

9a20073

Merge branch 'master' into throttle

07dfc05

clean up missed comments

1990018

Merge branch 'throttle' of github.com:roberttoyonaga/graal into throttle

2d94e69

christianhaeubl reviewed Sep 19, 2023

View reviewed changes

roberttoyonaga added 10 commits September 19, 2023 16:01

merge master

db05846

minor review feedback resolution

f585631

remove JfrThrottlerSupport. Add field to JfrEvent. Order JfrThrottler…

1823f09

… fields by lock access. Restrict access in JfrThrottler.

make uninterruptible. JfrRandom

750cf4b

add read-write lock

f68763a

cleanup. comments. javadoc. error handling. reset lastAllocationSize.

34ceac8

dedicated test class in test code

d4eb401

minor clean up

merge master

1093ee6

gate fixes and cleaning

48ac440

gate fixes. Math.ceil and style

f5fa957

roberttoyonaga requested a review from christianhaeubl September 27, 2023 18:23

fix some concurrency issues

41811ee

clean up JFR random

roberttoyonaga force-pushed the throttle branch from 6ae531e to 41811ee Compare October 11, 2023 15:45

roberttoyonaga added 5 commits November 1, 2023 13:08

fix conflicts

47928ae

minor clean up. Use jdk.graal.compiler

7e2cb91

Merge branch 'master' of github.com:roberttoyonaga/graal into throttle

63c89ed

Merge branch 'master' into throttle

1c021bd

fix conflicts

b0960fe

style

roberttoyonaga force-pushed the throttle branch from e6dcd98 to b0960fe Compare November 13, 2023 19:14

roberttoyonaga added 3 commits December 4, 2023 12:30

Merge branch 'master' of github.com:roberttoyonaga/graal into throttle

4e77a8d

comment

8a64fd6

style

6db1df2

christianhaeubl mentioned this pull request Feb 5, 2024

[GR-47109] Support JFR event ObjectAllocationSample. #8316

Merged

graalvmbot merged commit 54aa15f into oracle:master Feb 10, 2024
12 checks passed

fniephaus mentioned this pull request Feb 12, 2024

[GR-47109] Support JFR event throttling and jdk.ObjectAllocationSample #5729

Closed

fniephaus added this to the GraalVM for JDK 23 (September 17, 2024) milestone May 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GR-47109] Implement JFR Event Throttling. #6899

[GR-47109] Implement JFR Event Throttling. #6899

roberttoyonaga commented Jun 27, 2023 •

edited

Loading

fniephaus commented Jul 7, 2023

roberttoyonaga commented Jul 7, 2023

christianhaeubl left a comment

christianhaeubl Sep 19, 2023

roberttoyonaga Sep 27, 2023

christianhaeubl Sep 19, 2023

christianhaeubl Sep 19, 2023

christianhaeubl Sep 19, 2023

roberttoyonaga Sep 27, 2023

christianhaeubl Sep 19, 2023

christianhaeubl Sep 19, 2023

roberttoyonaga Sep 27, 2023

christianhaeubl Sep 19, 2023

christianhaeubl Sep 19, 2023

christianhaeubl Sep 19, 2023

roberttoyonaga Sep 27, 2023

christianhaeubl Sep 19, 2023

roberttoyonaga Sep 27, 2023

roberttoyonaga commented Jan 4, 2024

christianhaeubl commented Jan 31, 2024

roberttoyonaga commented Feb 2, 2024

[GR-47109] Implement JFR Event Throttling. #6899

[GR-47109] Implement JFR Event Throttling. #6899

Conversation

roberttoyonaga commented Jun 27, 2023 • edited Loading

Summary of Changes

fniephaus commented Jul 7, 2023

roberttoyonaga commented Jul 7, 2023

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

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 commented Jan 4, 2024

christianhaeubl commented Jan 31, 2024

roberttoyonaga commented Feb 2, 2024

roberttoyonaga commented Jun 27, 2023 •

edited

Loading