From 4df4a1f8e238ebf49d4b0e1e102ccdc3cdb82de9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20Gr=C3=B6nlund?= <mgronlun@openjdk.org>
Date: Mon, 10 Oct 2022 12:40:58 +0000
Subject: [PATCH] 8287832: jdk/jfr/event/runtime/TestActiveSettingEvent.java
 failed with "Expected two batches of Active Setting events"

Reviewed-by: egahlin
---
 src/hotspot/share/jfr/jni/jfrJniMethod.cpp    |  4 +--
 .../periodic/sampling/jfrThreadSampler.cpp    | 12 +++++---
 .../share/jfr/recorder/service/jfrEvent.hpp   | 28 +++++++++----------
 test/jdk/ProblemList.txt                      |  1 -
 4 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/src/hotspot/share/jfr/jni/jfrJniMethod.cpp b/src/hotspot/share/jfr/jni/jfrJniMethod.cpp
index a8e1d4ec099..07d08f4699f 100644
--- a/src/hotspot/share/jfr/jni/jfrJniMethod.cpp
+++ b/src/hotspot/share/jfr/jni/jfrJniMethod.cpp
@@ -277,9 +277,7 @@ JVM_ENTRY_NO_ENV(void, jfr_set_method_sampling_period(JNIEnv* env, jobject jvm,
   }
   JfrEventId typed_event_id = (JfrEventId)type;
   assert(EventExecutionSample::eventId == typed_event_id || EventNativeMethodSample::eventId == typed_event_id, "invariant");
-  if (periodMillis > 0) {
-    JfrEventSetting::set_enabled(typed_event_id, true); // ensure sampling event is enabled
-  }
+  JfrEventSetting::set_enabled(typed_event_id, periodMillis > 0);
   if (EventExecutionSample::eventId == type) {
     JfrThreadSampling::set_java_sample_period(periodMillis);
   } else {
diff --git a/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp b/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp
index 2fbe32f76f3..35744714a8c 100644
--- a/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp
+++ b/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp
@@ -299,14 +299,18 @@ static const uint MAX_NR_OF_NATIVE_SAMPLES = 1;
 void JfrThreadSampleClosure::commit_events(JfrSampleType type) {
   if (JAVA_SAMPLE == type) {
     assert(_added_java > 0 && _added_java <= MAX_NR_OF_JAVA_SAMPLES, "invariant");
-    for (uint i = 0; i < _added_java; ++i) {
-      _events[i].commit();
+    if (EventExecutionSample::is_enabled()) {
+      for (uint i = 0; i < _added_java; ++i) {
+        _events[i].commit();
+      }
     }
   } else {
     assert(NATIVE_SAMPLE == type, "invariant");
     assert(_added_native > 0 && _added_native <= MAX_NR_OF_NATIVE_SAMPLES, "invariant");
-    for (uint i = 0; i < _added_native; ++i) {
-      _events_native[i].commit();
+    if (EventNativeMethodSample::is_enabled()) {
+      for (uint i = 0; i < _added_native; ++i) {
+        _events_native[i].commit();
+      }
     }
   }
 }
diff --git a/src/hotspot/share/jfr/recorder/service/jfrEvent.hpp b/src/hotspot/share/jfr/recorder/service/jfrEvent.hpp
index 60ac9ca9da2..d487820c690 100644
--- a/src/hotspot/share/jfr/recorder/service/jfrEvent.hpp
+++ b/src/hotspot/share/jfr/recorder/service/jfrEvent.hpp
@@ -63,24 +63,20 @@ class JfrEvent {
  private:
   jlong _start_time;
   jlong _end_time;
-  bool _started;
   bool _untimed;
   bool _should_commit;
   bool _evaluated;
 
  protected:
   JfrEvent(EventStartTime timing=TIMED) : _start_time(0), _end_time(0),
-                                          _started(false), _untimed(timing == UNTIMED),
+                                          _untimed(timing == UNTIMED),
                                           _should_commit(false), _evaluated(false)
 #ifdef ASSERT
   , _verifier()
 #endif
   {
-    if (T::is_enabled() && JfrThreadLocal::is_included(Thread::current())) {
-      _started = true;
-      if (TIMED == timing && !T::isInstant) {
-        set_starttime(JfrTicks::now());
-      }
+    if (!T::isInstant && !_untimed && is_enabled()) {
+      set_starttime(JfrTicks::now());
     }
   }
 
@@ -146,20 +142,17 @@ class JfrEvent {
     return T::hasStackTrace;
   }
 
-  bool is_started() const {
-    return _started;
+  bool is_started() {
+    return is_instant() || _start_time != 0 || _untimed;
   }
 
   bool should_commit() {
-    if (!_started) {
+    if (!is_enabled()) {
       return false;
     }
     if (_untimed) {
       return true;
     }
-    if (_evaluated) {
-      return _should_commit;
-    }
     _should_commit = evaluate();
     _evaluated = true;
     return _should_commit;
@@ -167,11 +160,16 @@ class JfrEvent {
 
  private:
   bool should_write() {
-    return _started && (_evaluated ? _should_commit : evaluate());
+    if (_evaluated) {
+      return _should_commit;
+    }
+    if (!is_enabled()) {
+      return false;
+    }
+    return evaluate() && JfrThreadLocal::is_included(Thread::current());
   }
 
   bool evaluate() {
-    assert(_started, "invariant");
     if (_start_time == 0) {
       set_starttime(JfrTicks::now());
     } else if (_end_time == 0) {
diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt
index e94c75ebdc4..df508909503 100644
--- a/test/jdk/ProblemList.txt
+++ b/test/jdk/ProblemList.txt
@@ -749,7 +749,6 @@ jdk/jfr/startupargs/TestStartName.java                          8214685 windows-
 jdk/jfr/startupargs/TestStartDuration.java                      8214685 windows-x64
 jdk/jfr/jvm/TestWaste.java                                      8282427 generic-all
 jdk/jfr/api/consumer/recordingstream/TestOnEvent.java           8255404 linux-x64
-jdk/jfr/event/runtime/TestActiveSettingEvent.java               8287832 generic-all
 jdk/jfr/api/consumer/TestRecordingFileWrite.java                8287699 linux-x64,macosx-x64
 
 ############################################################################