From 6d064a330b63a3b3f72e5a39789c21190e06cc6d Mon Sep 17 00:00:00 2001 From: MichaelR Date: Thu, 7 Nov 2024 17:59:20 +0000 Subject: [PATCH] Improve Simulation speed and Closing behavior on MacOS and Windows See merge request main/Sumatra!1880 sumatra-commit: f9b7ef1c3a4e43f94feeb355d9e0e45af70d82af --- .../java/edu/tigers/sumatra/AMainFrame.java | 8 +++-- .../edu/tigers/sumatra/AMainPresenter.java | 2 +- .../java/edu/tigers/sumatra/IMainFrame.java | 5 +++ .../tigers/sumatra/IMainFrameObserver.java | 2 +- .../edu/tigers/sumatra/clock/ThreadUtil.java | 7 +++-- .../tigers/sumatra/clock/ThreadUtilTest.java | 31 +++++++++++++++++-- .../sumatra/persistence/IRecordObserver.java | 12 ++----- .../sumatra/persistence/RecordManager.java | 30 ------------------ .../referee/SslGameControllerProcess.java | 5 +-- .../presenter/replay/ReplayPresenter.java | 5 ++- .../sumatra/view/replay/ReplayWindow.java | 6 ++-- .../sumatra/model/ModuliStateAdapter.java | 3 +- 12 files changed, 53 insertions(+), 63 deletions(-) diff --git a/modules/common-gui/src/main/java/edu/tigers/sumatra/AMainFrame.java b/modules/common-gui/src/main/java/edu/tigers/sumatra/AMainFrame.java index 7a39cef4..389f80e6 100644 --- a/modules/common-gui/src/main/java/edu/tigers/sumatra/AMainFrame.java +++ b/modules/common-gui/src/main/java/edu/tigers/sumatra/AMainFrame.java @@ -206,12 +206,14 @@ protected void startReplayCompressionThread(Path path) } - protected void exit() + @Override + public void onClose() { for (final IMainFrameObserver o : observers) { - o.onExit(); + o.onClose(); } + log.debug("Closed"); } @@ -636,7 +638,7 @@ public void actionPerformed(final ActionEvent e) public void windowClosing(final WindowEvent windowEvent) { super.windowClosing(windowEvent); - exit(); + onClose(); } } diff --git a/modules/common-gui/src/main/java/edu/tigers/sumatra/AMainPresenter.java b/modules/common-gui/src/main/java/edu/tigers/sumatra/AMainPresenter.java index 71a65790..a539a9e0 100644 --- a/modules/common-gui/src/main/java/edu/tigers/sumatra/AMainPresenter.java +++ b/modules/common-gui/src/main/java/edu/tigers/sumatra/AMainPresenter.java @@ -134,7 +134,7 @@ public void onLoadLayout(final String filename) @Override - public void onExit() + public void onClose() { GlobalShortcuts.removeAllForFrame(getMainFrame()); diff --git a/modules/common-gui/src/main/java/edu/tigers/sumatra/IMainFrame.java b/modules/common-gui/src/main/java/edu/tigers/sumatra/IMainFrame.java index f6accd3a..e834cd80 100644 --- a/modules/common-gui/src/main/java/edu/tigers/sumatra/IMainFrame.java +++ b/modules/common-gui/src/main/java/edu/tigers/sumatra/IMainFrame.java @@ -52,5 +52,10 @@ public interface IMainFrame * @param name */ void selectLayoutItem(String name); + + /** + * Hook to run some cleanup when the frame is closed. + */ + void onClose(); } diff --git a/modules/common-gui/src/main/java/edu/tigers/sumatra/IMainFrameObserver.java b/modules/common-gui/src/main/java/edu/tigers/sumatra/IMainFrameObserver.java index 364e356c..6d55fb55 100644 --- a/modules/common-gui/src/main/java/edu/tigers/sumatra/IMainFrameObserver.java +++ b/modules/common-gui/src/main/java/edu/tigers/sumatra/IMainFrameObserver.java @@ -34,7 +34,7 @@ public interface IMainFrameObserver /** * */ - void onExit(); + void onClose(); /** diff --git a/modules/common/src/main/java/edu/tigers/sumatra/clock/ThreadUtil.java b/modules/common/src/main/java/edu/tigers/sumatra/clock/ThreadUtil.java index 6d0f84fa..3700de79 100644 --- a/modules/common/src/main/java/edu/tigers/sumatra/clock/ThreadUtil.java +++ b/modules/common/src/main/java/edu/tigers/sumatra/clock/ThreadUtil.java @@ -29,11 +29,12 @@ public static void parkNanosSafe(final long sleepTotal) final long sleepStart = System.nanoTime(); long stillSleep = sleepTotal; - do + while (stillSleep > 1) { - LockSupport.parkNanos(stillSleep); + // On some OS, the threads sleep too long, so we apply an exponential strategy + LockSupport.parkNanos(stillSleep / 2); long timeSinceStart = System.nanoTime() - sleepStart; stillSleep = sleepTotal - timeSinceStart; - } while (stillSleep > 0); + } } } diff --git a/modules/common/src/test/java/edu/tigers/sumatra/clock/ThreadUtilTest.java b/modules/common/src/test/java/edu/tigers/sumatra/clock/ThreadUtilTest.java index 4943575f..32509932 100644 --- a/modules/common/src/test/java/edu/tigers/sumatra/clock/ThreadUtilTest.java +++ b/modules/common/src/test/java/edu/tigers/sumatra/clock/ThreadUtilTest.java @@ -5,12 +5,9 @@ import org.junit.Test; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.LockSupport; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; /** @@ -38,4 +35,32 @@ public void testParkNanosSafeLong() assertTrue("Not slept enough: " + duration + "ns < " + sleepFor + "ns!!!", duration > sleepFor); } } + + + private static void measureParkNanos() + { + long iterationCount = 100; + long[] sleepTimes = new long[] { 0, 1, 10_000, 100_000, 500_000, 1_000_000, 10_000_000 }; + + for (long sleepTime : sleepTimes) + { + long startNanos = System.nanoTime(); + for (long i = 0; i < iterationCount; i++) + { + LockSupport.parkNanos(sleepTime); + } + long durationNanos = System.nanoTime() - startNanos; + long microsPerIteration = durationNanos / iterationCount; + long diff = (microsPerIteration - sleepTime); + double rel = (double) diff / sleepTime; + + System.out.printf("%8d | %8d | %8d | %.1f %n", sleepTime, microsPerIteration, diff, rel * 100); + } + } + + + public static void main(String[] args) + { + measureParkNanos(); + } } diff --git a/modules/moduli-record/src/main/java/edu/tigers/sumatra/persistence/IRecordObserver.java b/modules/moduli-record/src/main/java/edu/tigers/sumatra/persistence/IRecordObserver.java index 30e5da7a..d9a04a78 100644 --- a/modules/moduli-record/src/main/java/edu/tigers/sumatra/persistence/IRecordObserver.java +++ b/modules/moduli-record/src/main/java/edu/tigers/sumatra/persistence/IRecordObserver.java @@ -15,14 +15,6 @@ public interface IRecordObserver * @param recording */ void onStartStopRecord(boolean recording); - - - /** - * @param persistence the open persistence reference - * @param startTime the initial time after opening the replay (timestamp in ns) - */ - default void onViewReplay(PersistenceDb persistence, long startTime) - { - } - + + } diff --git a/modules/moduli-record/src/main/java/edu/tigers/sumatra/persistence/RecordManager.java b/modules/moduli-record/src/main/java/edu/tigers/sumatra/persistence/RecordManager.java index 57b6372b..f3132f03 100644 --- a/modules/moduli-record/src/main/java/edu/tigers/sumatra/persistence/RecordManager.java +++ b/modules/moduli-record/src/main/java/edu/tigers/sumatra/persistence/RecordManager.java @@ -18,7 +18,6 @@ import org.apache.logging.log4j.Logger; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -333,35 +332,6 @@ protected void onNewPersistanceRecorder(PersistenceAsyncRecorder recorder) } - /** - * Notify UI to view a replay window - * - * @param startTime - */ - public void notifyViewReplay(long startTime) - { - String dbPath = getCurrentDbPath(); - if (dbPath.isEmpty()) - { - log.error("No open DB found."); - return; - } - PersistenceDb db; - try - { - db = newPersistenceDb(Paths.get(dbPath)); - } catch (Exception e) - { - log.error("Could not open DB", e); - return; - } - for (IRecordObserver observer : observers) - { - observer.onViewReplay(db, startTime); - } - } - - /** * @return */ diff --git a/modules/moduli-referee/src/main/java/edu/tigers/sumatra/referee/SslGameControllerProcess.java b/modules/moduli-referee/src/main/java/edu/tigers/sumatra/referee/SslGameControllerProcess.java index 0ff352db..91d5e049 100644 --- a/modules/moduli-referee/src/main/java/edu/tigers/sumatra/referee/SslGameControllerProcess.java +++ b/modules/moduli-referee/src/main/java/edu/tigers/sumatra/referee/SslGameControllerProcess.java @@ -44,7 +44,6 @@ public class SslGameControllerProcess implements Runnable private final int gcUiPort; private final String publishAddress; private final String timeAcquisitionMode; - private final Thread shutdownHook = new Thread(this::stop); @Setter private boolean useSystemBinary = false; @@ -82,8 +81,6 @@ public void run() return; } - Runtime.getRuntime().addShutdownHook(shutdownHook); - Path engineConfig = Path.of("config", "engine.yaml"); Path engineConfigDefault = Path.of("config", "engine-default.yaml"); if (!Files.exists(engineConfig) && Files.exists(engineConfigDefault)) @@ -137,7 +134,6 @@ public void run() log.warn("game-controller has returned a non-zero exit code: {}", p.exitValue()); } log.debug("game-controller process thread finished"); - Runtime.getRuntime().removeShutdownHook(shutdownHook); } @@ -264,5 +260,6 @@ public void stop() log.warn("Interrupted while waiting for the process to exit"); Thread.currentThread().interrupt(); } + log.debug("Process stopped"); } } diff --git a/modules/sumatra-gui-replay/src/main/java/edu/tigers/sumatra/presenter/replay/ReplayPresenter.java b/modules/sumatra-gui-replay/src/main/java/edu/tigers/sumatra/presenter/replay/ReplayPresenter.java index e706bc7c..c8bdaddc 100644 --- a/modules/sumatra-gui-replay/src/main/java/edu/tigers/sumatra/presenter/replay/ReplayPresenter.java +++ b/modules/sumatra-gui-replay/src/main/java/edu/tigers/sumatra/presenter/replay/ReplayPresenter.java @@ -172,11 +172,10 @@ private void addPositionObserver(final IReplayPositionObserver o) @Override - public void onExit() + public void onClose() { - super.onExit(); + super.onClose(); stop(); - getMainFrame().dispose(); } diff --git a/modules/sumatra-gui-replay/src/main/java/edu/tigers/sumatra/view/replay/ReplayWindow.java b/modules/sumatra-gui-replay/src/main/java/edu/tigers/sumatra/view/replay/ReplayWindow.java index 3555ab7d..6f9a6694 100644 --- a/modules/sumatra-gui-replay/src/main/java/edu/tigers/sumatra/view/replay/ReplayWindow.java +++ b/modules/sumatra-gui-replay/src/main/java/edu/tigers/sumatra/view/replay/ReplayWindow.java @@ -13,6 +13,7 @@ import javax.swing.JMenu; import javax.swing.JMenuItem; +import javax.swing.WindowConstants; import java.awt.event.KeyEvent; @@ -27,6 +28,7 @@ public class ReplayWindow extends AMainFrame public ReplayWindow() { setTitle("Replay"); + setDefaultCloseOperation(WindowConstants.DISPOSE_ON_CLOSE); addView(new LogView(false)); addView(new ReplayControlView()); @@ -42,10 +44,6 @@ public ReplayWindow() shortcutMenuItem.addActionListener(actionEvent -> new ShortcutsDialog(ReplayWindow.this)); replayMenu.add(shortcutMenuItem); - JMenuItem menuClose = new JMenuItem("Close"); - menuClose.addActionListener(e -> exit()); - replayMenu.add(menuClose); - getJMenuBar().add(replayMenu); addMenuItems(); } diff --git a/modules/sumatra-model/src/main/java/edu/tigers/sumatra/model/ModuliStateAdapter.java b/modules/sumatra-model/src/main/java/edu/tigers/sumatra/model/ModuliStateAdapter.java index 3558b327..02a1da03 100644 --- a/modules/sumatra-model/src/main/java/edu/tigers/sumatra/model/ModuliStateAdapter.java +++ b/modules/sumatra-model/src/main/java/edu/tigers/sumatra/model/ModuliStateAdapter.java @@ -9,6 +9,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import javax.swing.SwingUtilities; import java.beans.PropertyChangeEvent; import java.beans.PropertyChangeListener; import java.util.List; @@ -75,7 +76,7 @@ public void propertyChange(final PropertyChangeEvent evt) try { log.trace("Notify {}", o.getClass()); - o.onModuliStateChanged(newState); + SwingUtilities.invokeLater(() -> o.onModuliStateChanged(newState)); } catch (Exception err) { log.error("Exception while changing moduli state in class " + o.getClass().getName(), err);