You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by gt...@apache.org on 2021/04/29 14:12:04 UTC

[activemq-artemis] branch main updated: ARTEMIS-3271 removing not needed Field Updater and fixing reset

This is an automated email from the ASF dual-hosted git repository.

gtully pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git


The following commit(s) were added to refs/heads/main by this push:
     new a3295c9  ARTEMIS-3271 removing not needed Field Updater and fixing reset
a3295c9 is described below

commit a3295c9873943d6d0e8eaffe306b567e95b5b92f
Author: Clebert Suconic <cl...@apache.org>
AuthorDate: Thu Apr 29 08:28:04 2021 -0400

    ARTEMIS-3271 removing not needed Field Updater and fixing reset
---
 .../artemis/utils/critical/CriticalMeasure.java    | 23 +++++++++-------------
 .../utils/critical/CriticalMeasureTest.java        | 10 +++++++---
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalMeasure.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalMeasure.java
index 2088673..a8448bb 100644
--- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalMeasure.java
+++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalMeasure.java
@@ -17,7 +17,6 @@
 
 package org.apache.activemq.artemis.utils.critical;
 
-import java.util.concurrent.atomic.AtomicLongFieldUpdater;
 import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 
 import org.jboss.logging.Logger;
@@ -29,9 +28,6 @@ public class CriticalMeasure {
    // this is used on enterCritical, if the logger is in trace mode
    private volatile Exception traceEnter;
 
-   //uses updaters to avoid creates many AtomicLong instances
-   static final AtomicLongFieldUpdater<CriticalMeasure> TIME_ENTER_UPDATER = AtomicLongFieldUpdater.newUpdater(CriticalMeasure.class, "timeEnter");
-
    static final AtomicReferenceFieldUpdater<CriticalMeasure, Thread> CURRENT_THREAD_UDPATER = AtomicReferenceFieldUpdater.newUpdater(CriticalMeasure.class, Thread.class, "currentThread");
 
    // While resetting the leaveMethod, I want to make sure no enter call would reset the value.
@@ -39,7 +35,7 @@ public class CriticalMeasure {
    private static final Thread GHOST_THREAD = new Thread();
 
    private volatile Thread currentThread;
-   private volatile long timeEnter;
+   protected volatile long timeEnter;
 
    private final int id;
    private final CriticalComponent component;
@@ -47,7 +43,7 @@ public class CriticalMeasure {
    public CriticalMeasure(CriticalComponent component, int id) {
       this.id = id;
       this.component = component;
-      TIME_ENTER_UPDATER.set(this, 0);
+      this.timeEnter = 0;
    }
 
    public void enterCritical() {
@@ -55,8 +51,7 @@ public class CriticalMeasure {
       // a sampling of a single thread at a time will be sufficient for the analyser,
       // typically what causes one thread to stall will repeat on another
       if (CURRENT_THREAD_UDPATER.compareAndSet(this, null, Thread.currentThread())) {
-         //prefer lazySet in order to avoid heavy-weight full barriers on x86
-         TIME_ENTER_UPDATER.lazySet(this, System.nanoTime());
+         timeEnter = System.nanoTime();
 
          if (logger.isTraceEnabled()) {
             traceEnter = new Exception("entered");
@@ -83,7 +78,7 @@ public class CriticalMeasure {
             }
             traceEnter = null;
          }
-         TIME_ENTER_UPDATER.set(this, 0);
+         this.timeEnter = 0;
 
          // I am pretty sure this is single threaded by now.. I don't need compareAndSet here
          CURRENT_THREAD_UDPATER.set(this, null);
@@ -99,7 +94,7 @@ public class CriticalMeasure {
    }
 
    public boolean checkExpiration(long timeout, boolean reset) {
-      final long timeEnter = TIME_ENTER_UPDATER.get(this);
+      final long timeEnter = this.timeEnter;
       if (timeEnter != 0L) {
          long time = System.nanoTime();
          boolean expired = time - timeEnter > timeout;
@@ -113,13 +108,13 @@ public class CriticalMeasure {
                logger.warn("Component " + getComponentName() + " is expired on path " + id);
             }
 
+            if (reset) {
+               this.timeEnter = 0;
+            }
+
          }
          return expired;
       }
       return false;
    }
-
-   public long enterTime() {
-      return TIME_ENTER_UPDATER.get(this);
-   }
 }
\ No newline at end of file
diff --git a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/critical/CriticalMeasureTest.java b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/critical/CriticalMeasureTest.java
index 0e4d7ec..ca8e2b8 100644
--- a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/critical/CriticalMeasureTest.java
+++ b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/critical/CriticalMeasureTest.java
@@ -28,7 +28,7 @@ public class CriticalMeasureTest {
    public void testCriticalMeasure() throws Exception {
       CriticalMeasure measure = new CriticalMeasure(null, 1);
       long time = System.nanoTime();
-      CriticalMeasure.TIME_ENTER_UPDATER.set(measure, time - TimeUnit.SECONDS.toNanos(5));
+      measure.timeEnter = time - TimeUnit.SECONDS.toNanos(5);
       Assert.assertFalse(measure.checkExpiration(TimeUnit.SECONDS.toNanos(30), false));
    }
 
@@ -39,7 +39,7 @@ public class CriticalMeasureTest {
       CriticalMeasure measure = new CriticalMeasure(component, 1);
       long time = System.nanoTime();
       CriticalMeasure.CURRENT_THREAD_UDPATER.set(measure, Thread.currentThread());
-      CriticalMeasure.TIME_ENTER_UPDATER.set(measure, time - TimeUnit.MINUTES.toNanos(30));
+      measure.timeEnter = time - TimeUnit.MINUTES.toNanos(30);
       measure.leaveCritical();
       Assert.assertFalse(measure.checkExpiration(TimeUnit.SECONDS.toNanos(30), false));
    }
@@ -51,8 +51,12 @@ public class CriticalMeasureTest {
       CriticalMeasure measure = new CriticalMeasure(component, 1);
       long time = System.nanoTime();
       measure.enterCritical();
-      CriticalMeasure.TIME_ENTER_UPDATER.set(measure, time - TimeUnit.MINUTES.toNanos(5));
+      measure.timeEnter = time - TimeUnit.MINUTES.toNanos(5);
       Assert.assertTrue(measure.checkExpiration(TimeUnit.SECONDS.toNanos(30), false));
+      // subsequent call without reset should still fail
+      Assert.assertTrue(measure.checkExpiration(TimeUnit.SECONDS.toNanos(30), true));
+      // previous reset should have cleared it
+      Assert.assertFalse(measure.checkExpiration(TimeUnit.SECONDS.toNanos(30), false));
       measure.leaveCritical();
    }
 }