Skip to content

Commit 77e69e0

Browse files
committed
8358750: JFR: EventInstrumentation MASK_THROTTLE* constants should be computed in longs
Reviewed-by: mgronlun
1 parent dcc7254 commit 77e69e0

File tree

2 files changed

+56
-30
lines changed

2 files changed

+56
-30
lines changed

src/jdk.jfr/share/classes/jdk/jfr/internal/EventInstrumentation.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@
6161
*
6262
*/
6363
public final class EventInstrumentation {
64-
public static final long MASK_THROTTLE = 1 << 62;
65-
public static final long MASK_THROTTLE_CHECK = 1 << 63;
64+
public static final long MASK_THROTTLE = 1L << 62;
65+
public static final long MASK_THROTTLE_CHECK = 1L << 63;
6666
public static final long MASK_THROTTLE_BITS = MASK_THROTTLE | MASK_THROTTLE_CHECK;
6767
public static final long MASK_THROTTLE_CHECK_SUCCESS = MASK_THROTTLE_CHECK | MASK_THROTTLE;
6868
public static final long MASK_THROTTLE_CHECK_FAIL = MASK_THROTTLE_CHECK | 0;
@@ -482,6 +482,11 @@ private void updateInstanceCommit(BlockCodeBuilder blockCodeBuilder, Label end,
482482
blockCodeBuilder.ifne(durationEvent);
483483
invokestatic(blockCodeBuilder, TYPE_EVENT_CONFIGURATION, METHOD_TIME_STAMP);
484484
blockCodeBuilder.lstore(1);
485+
if (throttled) {
486+
blockCodeBuilder.aload(0);
487+
blockCodeBuilder.lload(1);
488+
putfield(blockCodeBuilder, eventClassDesc, ImplicitFields.FIELD_START_TIME);
489+
}
485490
Label commit = blockCodeBuilder.newLabel();
486491
blockCodeBuilder.goto_(commit);
487492
// if (duration == 0) {
@@ -491,7 +496,7 @@ private void updateInstanceCommit(BlockCodeBuilder blockCodeBuilder, Label end,
491496
blockCodeBuilder.labelBinding(durationEvent);
492497
blockCodeBuilder.aload(0);
493498
getfield(blockCodeBuilder, eventClassDesc, ImplicitFields.FIELD_DURATION);
494-
blockCodeBuilder.lconst_0();
499+
blockCodeBuilder.lconst_0(); // also blocks throttled event
495500
blockCodeBuilder.lcmp();
496501
blockCodeBuilder.ifne(commit);
497502
blockCodeBuilder.aload(0);
@@ -527,9 +532,7 @@ private void updateInstanceCommit(BlockCodeBuilder blockCodeBuilder, Label end,
527532
// write duration
528533
blockCodeBuilder.dup();
529534
// stack: [EW] [EW]
530-
blockCodeBuilder.aload(0);
531-
// stack: [EW] [EW] [this]
532-
getfield(blockCodeBuilder, eventClassDesc, ImplicitFields.FIELD_DURATION);
535+
getDuration(blockCodeBuilder);
533536
// stack: [EW] [EW] [long]
534537
invokevirtual(blockCodeBuilder, TYPE_EVENT_WRITER, EventWriterMethod.PUT_LONG.method());
535538
fieldIndex++;

test/jdk/jdk/jfr/api/metadata/annotations/TestThrottle.java

Lines changed: 47 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,38 +23,25 @@
2323

2424
package jdk.jfr.api.metadata.annotations;
2525

26-
import java.lang.annotation.Annotation;
27-
import java.lang.annotation.ElementType;
28-
import java.lang.annotation.Retention;
29-
import java.lang.annotation.RetentionPolicy;
30-
import java.lang.annotation.Target;
26+
import java.io.IOException;
3127
import java.lang.reflect.Constructor;
28+
import java.nio.file.Files;
29+
import java.nio.file.Path;
3230
import java.time.Duration;
33-
import java.util.HashSet;
34-
import java.util.List;
35-
import java.util.Map;
31+
import java.time.Instant;
3632
import java.util.Set;
3733
import java.util.concurrent.atomic.AtomicInteger;
38-
import java.util.concurrent.atomic.AtomicLong;
39-
import java.util.concurrent.atomic.AtomicReference;
4034

41-
import jdk.jfr.AnnotationElement;
42-
import jdk.jfr.Event;
43-
import jdk.jfr.EventType;
44-
import jdk.jfr.MetadataDefinition;
45-
import jdk.jfr.Name;
46-
import jdk.jfr.Threshold;
4735
import jdk.jfr.Enabled;
36+
import jdk.jfr.Event;
4837
import jdk.jfr.Recording;
38+
import jdk.jfr.SettingControl;
4939
import jdk.jfr.SettingDefinition;
50-
import jdk.jfr.SettingDescriptor;
40+
import jdk.jfr.Threshold;
5141
import jdk.jfr.Throttle;
52-
import jdk.jfr.ValueDescriptor;
5342
import jdk.jfr.consumer.RecordedEvent;
43+
import jdk.jfr.consumer.RecordingFile;
5444
import jdk.jfr.consumer.RecordingStream;
55-
import jdk.test.lib.Asserts;
56-
import jdk.test.lib.jfr.Events;
57-
import jdk.jfr.SettingControl;
5845

5946
/**
6047
* @test
@@ -65,6 +52,9 @@
6552
*/
6653
public class TestThrottle {
6754

55+
public static class UnthrottledEvent extends Event {
56+
}
57+
6858
@Throttle("off")
6959
@Enabled(false)
7060
public static class ThrottledDisabledEvent extends Event {
@@ -130,7 +120,11 @@ public static class ThrottledReuseEvent extends Event {
130120
public int index;
131121
}
132122

123+
private static Instant startTime;
124+
133125
public static void main(String[] args) throws Exception {
126+
startTime = determineMinimumTime();
127+
testUnthrottled(); // To ensure problem is specific to throttled events
134128
testThrottleDisabled();
135129
testThrottledOff();
136130
testThottleZeroRate();
@@ -140,6 +134,20 @@ public static void main(String[] args) throws Exception {
140134
testThrottleUserdefined();
141135
}
142136

137+
private static void testUnthrottled() throws Exception {
138+
testEvent(UnthrottledEvent.class, true);
139+
}
140+
141+
private static Instant determineMinimumTime() throws IOException {
142+
try (Recording r = new Recording()) {
143+
r.enable("jdk.JVMInformation");
144+
r.start();
145+
Path p = Path.of("start.jfr");
146+
r.dump(p);
147+
return RecordingFile.readAllEvents(p).get(0).getStartTime();
148+
}
149+
}
150+
143151
private static void testThrottleDisabled() throws Exception {
144152
testEvent(ThrottledDisabledEvent.class, false);
145153
}
@@ -220,8 +228,6 @@ private static void testThrottleUserdefined(String test, String throttle, boolea
220228
if (e5.shouldCommit()) {
221229
e5.commit();
222230
}
223-
224-
r.stop();
225231
assertEvents(r, eventName, emit ? 5 : 0);
226232
}
227233
}
@@ -272,14 +278,31 @@ private static void testEvent(Class<? extends Event> eventClass, boolean shouldC
272278

273279
private static void assertEvents(Recording r, String name, int expected) throws Exception {
274280
int count = 0;
275-
for (RecordedEvent event : Events.fromRecording(r)) {
281+
r.stop();
282+
Duration d = Duration.between(r.getStartTime(), r.getStopTime());
283+
Path file = Path.of("dump.jfr");
284+
r.dump(file);
285+
for (RecordedEvent event : RecordingFile.readAllEvents(file)) {
276286
if (event.getEventType().getName().equals(name)) {
277287
count++;
278288
}
289+
if (event.getDuration().isNegative()) {
290+
System.out.println(event);
291+
throw new Exception("Unexpected negative duration");
292+
}
293+
if (event.getStartTime().isBefore(startTime)) {
294+
System.out.println(event);
295+
throw new Exception("Unexpected early start time");
296+
}
297+
if (event.getDuration().toMillis() > 2 * d.toMillis()) {
298+
System.out.println(event);
299+
throw new Exception("Duration exceed twice the length of the recording");
300+
}
279301
}
280302
if (count != expected) {
281303
throw new Exception("Expected " + expected + " " + name + " events, but found " + count);
282304
}
305+
Files.delete(file);
283306
}
284307

285308
private static void assertShouldCommit(Event e, boolean expected) throws Exception {

0 commit comments

Comments
 (0)