From a25b9bc89b5899a55db5a2334bc837c021960424 Mon Sep 17 00:00:00 2001 From: Erik Gahlin Date: Tue, 17 May 2022 20:23:53 +0000 Subject: [PATCH] 8286688: JFR: Active Setting events should have the same timestamp Reviewed-by: mgronlun --- .../jdk/jfr/events/ActiveSettingEvent.java | 9 +++++++++ .../jdk/jfr/internal/EventControl.java | 10 ++++------ .../jdk/jfr/internal/MetadataRepository.java | 2 +- .../jdk/jfr/internal/PlatformRecorder.java | 3 ++- .../jdk/jfr/internal/SettingsManager.java | 7 ++++--- .../event/runtime/TestActiveSettingEvent.java | 19 +++++++++++++++++++ 6 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/jdk.jfr/share/classes/jdk/jfr/events/ActiveSettingEvent.java b/src/jdk.jfr/share/classes/jdk/jfr/events/ActiveSettingEvent.java index 7610ea07797..20a016b4918 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/events/ActiveSettingEvent.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/events/ActiveSettingEvent.java @@ -37,6 +37,11 @@ import jdk.jfr.internal.Type; @StackTrace(false) public final class ActiveSettingEvent extends AbstractJDKEvent { + public static final ActiveSettingEvent EVENT = new ActiveSettingEvent(); + + // The order of these fields must be the same as the parameters in + // commit(... , long, String, String) + @Label("Event Id") public long id; @@ -45,4 +50,8 @@ public final class ActiveSettingEvent extends AbstractJDKEvent { @Label("Setting Value") public String value; + + public static void commit(long startTime, long duration, long id, String name, String value) { + // Generated + } } diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/EventControl.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/EventControl.java index 562c0410f67..e1e421dcbe9 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/EventControl.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/EventControl.java @@ -279,7 +279,7 @@ public final class EventControl { } } - void writeActiveSettingEvent() { + void writeActiveSettingEvent(long timestamp) { if (!type.isRegistered()) { return; } @@ -289,11 +289,9 @@ public final class EventControl { if (value == null) { value = nc.control.getDefaultValue(); } - ActiveSettingEvent event = new ActiveSettingEvent(); - event.id = type.getId(); - event.name = nc.name; - event.value = value; - event.commit(); + if (ActiveSettingEvent.EVENT.isEnabled()) { + ActiveSettingEvent.commit(timestamp, 0L, type.getId(), nc.name(), value); + } } } } diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java index a578addf110..cda69b32bfa 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java @@ -158,7 +158,7 @@ public final class MetadataRepository { configuration.getPlatformEventType().setRegistered(true); typeLibrary.addType(configuration.getPlatformEventType()); if (jvm.isRecording()) { - settingsManager.setEventControl(configuration.getEventControl(), true); + settingsManager.setEventControl(configuration.getEventControl(), true, JVM.counterTime()); settingsManager.updateRetransform(Collections.singletonList((eventClass))); } setStaleMetadata(); diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecorder.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecorder.java index 6bf5587adab..8f2d150ff10 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecorder.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecorder.java @@ -483,8 +483,9 @@ public final class PlatformRecorder { } } if (activeSettingEvent.isEnabled()) { + long timestamp = JVM.counterTime(); for (EventControl ec : MetadataRepository.getInstance().getEventControls()) { - ec.writeActiveSettingEvent(); + ec.writeActiveSettingEvent(timestamp); } } } diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/SettingsManager.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/SettingsManager.java index 3d8ab50d4d1..ee92cb07316 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/SettingsManager.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/SettingsManager.java @@ -142,8 +142,9 @@ final class SettingsManager { if (Logger.shouldLog(LogTag.JFR_SETTING, LogLevel.INFO)) { eventControls.sort(Comparator.comparing(x -> x.getEventType().getName())); } + long timestamp = JVM.counterTime(); for (EventControl ec : eventControls) { - setEventControl(ec, writeSettingEvents); + setEventControl(ec, writeSettingEvents, timestamp); } } if (JVM.getJVM().getAllowedToDoEventRetransforms()) { @@ -211,7 +212,7 @@ final class SettingsManager { return internals.values(); } - void setEventControl(EventControl ec, boolean writeSettingEvents) { + void setEventControl(EventControl ec, boolean writeSettingEvents, long timestamp) { InternalSetting is = getInternalSetting(ec); boolean shouldLog = Logger.shouldLog(LogTag.JFR_SETTING, LogLevel.INFO); if (shouldLog) { @@ -251,7 +252,7 @@ final class SettingsManager { } } if (writeSettingEvents) { - ec.writeActiveSettingEvent(); + ec.writeActiveSettingEvent(timestamp); } if (shouldLog) { Logger.log(LogTag.JFR_SETTING, LogLevel.INFO, "}"); diff --git a/test/jdk/jdk/jfr/event/runtime/TestActiveSettingEvent.java b/test/jdk/jdk/jfr/event/runtime/TestActiveSettingEvent.java index 266520f53b2..f0e0e8b6252 100644 --- a/test/jdk/jdk/jfr/event/runtime/TestActiveSettingEvent.java +++ b/test/jdk/jdk/jfr/event/runtime/TestActiveSettingEvent.java @@ -22,6 +22,8 @@ */ package jdk.jfr.event.runtime; +import java.time.Duration; +import java.time.Instant; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -85,8 +87,18 @@ public final class TestActiveSettingEvent { r.stop(); Map settings = new HashMap<>(); List events = Events.fromRecording(r); + Instant timestamp = null; for (RecordedEvent e : events) { if (e.getEventType().getName().equals(ACTIVE_SETTING_EVENT_NAME)) { + if (!e.getDuration().equals(Duration.ZERO)) { + throw new Exception("Expected event to have zero duration"); + } + if (timestamp == null) { + timestamp = e.getStartTime(); + } + if (!e.getStartTime().equals(timestamp)) { + throw new Exception("Expected all events to have the same timestamp"); + } long id = e.getLong("id"); String name = e.getString("name"); String value = e.getString("value"); @@ -193,6 +205,13 @@ public final class TestActiveSettingEvent { assertSetting(events, type, "threshold", "0 ns"); // initial value assertSetting(events, type, "enabled", "true"); assertSetting(events, type, "threshold", "11 ns"); // changed value + Set timestamps = new HashSet<>(); + for (RecordedEvent e : events) { + timestamps.add(e.getStartTime()); + } + if (timestamps.size() != 2) { + throw new Exception("Expected two batches of Active Setting events, at Recording.start() and during Recording.setSetting(...)"); + } } }