From 8d1c4106466a7797c8d23e25c662cb7aafaf367a Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 26 Apr 2023 23:41:18 +0200 Subject: [PATCH] feature(ActionTimeout): add garbage collection time tracking to ActionTimeout class This commit adds a new variable gcTime to the ActionTimeout class and subtracts it from the total time when calculating the time difference between the start and stop timestamps. The gcTime variable is calculated by iterating over the list of Garbage Collector MXBeans and summing up the collection times. This change improves the accuracy of the time tracking by accounting for the time spent in garbage collection. --- .../sc/framework/plugins/ActionTimeout.java | 184 +++++++++--------- 1 file changed, 97 insertions(+), 87 deletions(-) diff --git a/sdk/src/main/server-api/sc/framework/plugins/ActionTimeout.java b/sdk/src/main/server-api/sc/framework/plugins/ActionTimeout.java index 263617f61..e6a53a04f 100644 --- a/sdk/src/main/server-api/sc/framework/plugins/ActionTimeout.java +++ b/sdk/src/main/server-api/sc/framework/plugins/ActionTimeout.java @@ -1,127 +1,137 @@ package sc.framework.plugins; +import java.lang.management.ManagementFactory; +import java.lang.management.GarbageCollectorMXBean; +import java.util.List; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; // TODO We can probably utilise an inbuilt class instead. /** Tracks timeouts in Milliseconds. */ public class ActionTimeout { - static final Logger logger = LoggerFactory.getLogger(ActionTimeout.class); - - private final long softTimeoutInMilliseconds; - - private final long hardTimeoutInMilliseconds; + static final Logger logger = LoggerFactory.getLogger(ActionTimeout.class); - private final boolean canTimeout; + private final long softTimeoutInMilliseconds; - private Thread timeoutThread; + private final long hardTimeoutInMilliseconds; - private Status status = Status.NEW; + private final boolean canTimeout; - private long startTimestamp = 0; - private long stopTimestamp = 0; + private Thread timeoutThread; - private static final int DEFAULT_HARD_TIMEOUT = 10000; - private static final int DEFAULT_SOFT_TIMEOUT = 5000; + private Status status = Status.NEW; - private enum Status { - NEW, STARTED, STOPPED - } + private long startTimestamp = 0; + private long stopTimestamp = 0; + private long gcTime = 0; - public ActionTimeout(boolean canTimeout) { - this(canTimeout, DEFAULT_HARD_TIMEOUT, DEFAULT_SOFT_TIMEOUT); - } + private static final int DEFAULT_HARD_TIMEOUT = 10000; + private static final int DEFAULT_SOFT_TIMEOUT = 5000; - public ActionTimeout(boolean canTimeout, int hardTimeoutInMilliseconds) { - this(canTimeout, hardTimeoutInMilliseconds, hardTimeoutInMilliseconds); - } + private enum Status { + NEW, STARTED, STOPPED + } - public ActionTimeout(boolean canTimeout, int hardTimeoutInMilliseconds, int softTimeoutInMilliseconds) { - if (hardTimeoutInMilliseconds < softTimeoutInMilliseconds) { - throw new IllegalArgumentException( - "HardTimeout must be greater or equal the SoftTimeout"); + public ActionTimeout(boolean canTimeout) { + this(canTimeout, DEFAULT_HARD_TIMEOUT, DEFAULT_SOFT_TIMEOUT); } - this.canTimeout = canTimeout; - this.softTimeoutInMilliseconds = softTimeoutInMilliseconds; - this.hardTimeoutInMilliseconds = hardTimeoutInMilliseconds; - } + public ActionTimeout(boolean canTimeout, int hardTimeoutInMilliseconds) { + this(canTimeout, hardTimeoutInMilliseconds, hardTimeoutInMilliseconds); + } - public long getHardTimeout() { - return this.hardTimeoutInMilliseconds; - } + public ActionTimeout(boolean canTimeout, int hardTimeoutInMilliseconds, int softTimeoutInMilliseconds) { + if (hardTimeoutInMilliseconds < softTimeoutInMilliseconds) { + throw new IllegalArgumentException( + "HardTimeout must be greater or equal the SoftTimeout"); + } - public long getSoftTimeout() { - return this.softTimeoutInMilliseconds; - } + this.canTimeout = canTimeout; + this.softTimeoutInMilliseconds = softTimeoutInMilliseconds; + this.hardTimeoutInMilliseconds = hardTimeoutInMilliseconds; + } - public boolean canTimeout() { - return this.canTimeout; - } + public long getHardTimeout() { + return this.hardTimeoutInMilliseconds; + } - public long getTimeDiff() { - if (this.status == Status.NEW) { - throw new IllegalStateException("Timeout was never started."); + public long getSoftTimeout() { + return this.softTimeoutInMilliseconds; } - if (this.status == Status.STARTED) { - throw new IllegalStateException("Timeout was not stopped."); + public boolean canTimeout() { + return this.canTimeout; } - return stopTimestamp - startTimestamp; - } + public long getTimeDiff() { + if (this.status == Status.NEW) { + throw new IllegalStateException("Timeout was never started."); + } - public synchronized boolean didTimeout() { - return this.canTimeout() && this.getTimeDiff() > this.softTimeoutInMilliseconds; - } + if (this.status == Status.STARTED) { + throw new IllegalStateException("Timeout was not stopped."); + } - public synchronized void stop() { - if (this.status == Status.NEW) { - throw new IllegalStateException("Timeout was never started."); + // Subtract the garbage collection time for an unaltered time + return stopTimestamp - startTimestamp - gcTime; } - if (this.status == Status.STOPPED) { - logger.warn("Redundant stop: Timeout was already stopped."); - return; + public synchronized boolean didTimeout() { + return this.canTimeout() && this.getTimeDiff() > this.softTimeoutInMilliseconds; } - this.stopTimestamp = System.currentTimeMillis(); - this.status = Status.STOPPED; + public synchronized void stop() { + if (this.status == Status.NEW) { + throw new IllegalStateException("Timeout was never started."); + } - if (this.timeoutThread != null) { - this.timeoutThread.interrupt(); - } - } + if (this.status == Status.STOPPED) { + logger.warn("Redundant stop: Timeout was already stopped."); + return; + } + + this.stopTimestamp = System.currentTimeMillis(); + this.status = Status.STOPPED; + + if (this.timeoutThread != null) { + this.timeoutThread.interrupt(); + } - public synchronized void start(final Runnable onTimeout) { - if (this.status != Status.NEW) { - throw new IllegalStateException("Redundant start: was already started!"); + // Get the garbage collection time + this.gcTime = ManagementFactory.getGarbageCollectorMXBeans() + .stream() + .mapToLong(GarbageCollectorMXBean::getCollectionTime) + .sum(); } - if (canTimeout()) { - this.timeoutThread = new Thread(() -> { - try { - Thread.sleep(getHardTimeout()); - stop(); - onTimeout.run(); - } catch (InterruptedException e) { - logger.info("HardTimout wasn't reached."); + public synchronized void start(final Runnable onTimeout) { + if (this.status != Status.NEW) { + throw new IllegalStateException("Redundant start: was already started!"); + } + + clearGCStats(); + + if (canTimeout()) { + this.timeoutThread = new Thread(() -> { + try { + Thread.sleep(getHardTimeout()); + stop(); + onTimeout.run(); + } catch (InterruptedException e) { + logger.info("HardTimout wasn't reached."); + } + }); + this.timeoutThread.start(); } - }); - this.timeoutThread.start(); + + this.startTimestamp = System.currentTimeMillis(); + this.status = Status.STARTED; } - this.startTimestamp = System.currentTimeMillis(); - this.status = Status.STARTED; - } - - @Override - public String toString() { - return "ActionTimeout{" + - "canTimeout=" + canTimeout + - ", status=" + status + - ", start=" + startTimestamp + - ", stop=" + stopTimestamp + - '}'; - } -} + private void clearGCStats() { + // With the call of `getCollectionTime` we'll clear the GarbageCollectorMXBean from older gcs. + List gcBeans = ManagementFactory.getGarbageCollectorMXBeans(); + gcBeans.forEach(GarbageCollectorMXBean::getCollectionTime); + } +} \ No newline at end of file