perf: write periodic station progress off the main thread

src/main/java/net/tfminecraft/vfbuilders/data/Database.java

@@ -16,38 +16,134 @@
 import java.lang.reflect.Type;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
+import java.time.Duration;
 import java.util.*;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
 
 public class Database {
 
-    private static final File file = new File(VFBuilders.plugin.getDataFolder(), "data/stations.json");
     private static final Gson gson = new GsonBuilder().setPrettyPrinting().create();
 
+    /** Held while a snapshot file is written; package-visible so tests can hold the writer. */
+    static final Object WRITE_LOCK = new Object();
+    private static final AtomicLong snapshots = new AtomicLong();
+    // Guarded by WRITE_LOCK: the newest snapshot that has started writing.
+    private static long newestWrite;
+    private static ExecutorService writer;
+
+    /** The stations file, under the plugin's current data folder. */
+    static Path path() {
+        return file().toPath();
+    }
+
+    private static File file() {
+        return new File(VFBuilders.plugin.getDataFolder(), "data/stations.json");
+    }
+
     public static void saveStations(Map<Location, ActiveStation> stations) {
         saveStationsNow(stations);
     }
 
+    /**
+     * Writes the stations durably before returning. Callers that must know the state is on disk
+     * (construction start, completion, cancellation, station removal, disable) use this.
+     */
     public static boolean saveStationsNow(Map<Location, ActiveStation> stations) {
         try {
-            List<StationData> dataList = new ArrayList<>();
-
-            for (ActiveStation station : stations.values()) {
-                StationData data = new StationData();
-                data.location = toSerializableLoc(station.getLocation());
-                data.stationId = station.getStation().getId(); // Save station identifier
-                data.spawnLocation = station.hasSpawnLocation() ? toSerializableLoc(station.getSpawnLocation()) : null;
-                data.blueprintId = station.hasBlueprint() ? station.getBlueprint().getId() : null;
-                data.timeLeft = station.getTimeLeft();
-                data.outputUuid = station.getOutputUuid();
-                if (station.hasBlueprint()) {
-                    data.constructorUuid = toText(station.getConstructorUuid());
-                    data.constructorCharacterId = toText(station.getConstructorCharacterId());
-                }
+            return write(snapshot(stations));
+        } catch (RuntimeException e) {
+            e.printStackTrace();
+            return false;
+        }
+    }
+
+    /**
+     * Snapshots the stations on the calling thread and writes them durably on the station writer
+     * thread. A snapshot taken earlier than one already written is dropped, so a later synchronous
+     * save is never overwritten by an older queued one.
+     */
+    public static CompletableFuture<Boolean> saveStationsLater(Map<Location, ActiveStation> stations) {
+        Snapshot snapshot;
+        try {
+            snapshot = snapshot(stations);
+        } catch (RuntimeException e) {
+            e.printStackTrace();
+            return CompletableFuture.completedFuture(false);
+        }
+        return CompletableFuture.supplyAsync(() -> write(snapshot), writer());
+    }
+
+    /** Finishes queued writes and stops the writer thread; the next background save starts a new one. */
+    public static void close() {
+        close(Duration.ofSeconds(30));
+    }
 
-                dataList.add(data);
+    static void close(Duration timeout) {
+        ExecutorService current;
+        synchronized (Database.class) {
+            current = writer;
+            writer = null;
+        }
+        if (current == null) return;
+        current.shutdown();
+        try {
+            if (!current.awaitTermination(timeout.toMillis(), TimeUnit.MILLISECONDS)) {
+                VFBuilders.plugin.getLogger().warning(
+                    "Station writer still busy after " + timeout.toSeconds() + "s; the last synchronous save stands.");
+            }
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+        }
+    }
+
+    private static synchronized ExecutorService writer() {
+        if (writer == null) {
+            writer = Executors.newSingleThreadExecutor(task -> {
+                Thread thread = new Thread(task, "VFBuilders station writer");
+                thread.setDaemon(true);
+                return thread;
+            });
+        }
+        return writer;
+    }
+
+    private static Snapshot snapshot(Map<Location, ActiveStation> stations) {
+        List<StationData> dataList = new ArrayList<>();
+
+        for (ActiveStation station : stations.values()) {
+            StationData data = new StationData();
+            data.location = toSerializableLoc(station.getLocation());
+            data.stationId = station.getStation().getId(); // Save station identifier
+            data.spawnLocation = station.hasSpawnLocation() ? toSerializableLoc(station.getSpawnLocation()) : null;
+            data.blueprintId = station.hasBlueprint() ? station.getBlueprint().getId() : null;
+            data.timeLeft = station.getTimeLeft();
+            data.outputUuid = station.getOutputUuid();
+            if (station.hasBlueprint()) {
+                data.constructorUuid = toText(station.getConstructorUuid());
+                data.constructorCharacterId = toText(station.getConstructorCharacterId());
             }
 
+            dataList.add(data);
+        }
+        return new Snapshot(snapshots.incrementAndGet(), file(), dataList);
+    }
+
+    private static boolean write(Snapshot snapshot) {
+        synchronized (WRITE_LOCK) {
+            if (snapshot.sequence() < newestWrite) return true; // A newer snapshot already wrote.
+            newestWrite = snapshot.sequence();
+            return writeFile(snapshot.target(), snapshot.stations());
+        }
+    }
+
+    private static boolean writeFile(File file, List<StationData> dataList) {
+        try {
             var directory = file.toPath().getParent();
             Files.createDirectories(directory);
             var temporary = Files.createTempFile(directory, "stations-", ".tmp");
@@ -70,9 +166,13 @@ public static boolean saveStationsNow(Map<Location, ActiveStation> stations) {
         }
     }
 
+    // The target is resolved on the calling thread, so the writer thread never reads plugin state.
+    private record Snapshot(long sequence, File target, List<StationData> stations) {}
+
     public static HashMap<Location, ActiveStation> loadStations() {
         HashMap<Location, ActiveStation> loaded = new HashMap<>();
 
+        File file = file();
         if (!file.exists()) return loaded;
 
         try (Reader reader = new FileReader(file)) {

src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java

@@ -82,7 +82,8 @@ public void start() {
             @Override
             public void run() {
                 if (stations.values().stream().anyMatch(ActiveStation::hasBlueprint)) {
-                    Database.saveStationsNow(stations);
+                    // Only countdown progress changes between the synchronous checkpoints.
+                    Database.saveStationsLater(stations);
                 }
             }
         }.runTaskTimer(VFBuilders.plugin, 600L, 600L);
@@ -93,6 +94,7 @@ public void stop() {
             station.removeDisplay();
         }
         Database.saveStations(stations);
+        Database.close();
     }
 
     public void rebindAfterReload(Runnable reloadDefinitions) {

src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java

@@ -4,8 +4,8 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 
-/** One data directory per JVM, matching Database's static path lifetime. */
-final class DatabaseFixture {
+/** One shared data directory per JVM for tests that save stations. */
+public final class DatabaseFixture {
   private static final Path ROOT;
 
   static {
@@ -16,7 +16,7 @@ final class DatabaseFixture {
     }
   }
 
-  static Path folder() throws IOException {
+  public static Path folder() throws IOException {
     return Files.createDirectories(ROOT);
   }
 }

src/test/java/net/tfminecraft/vfbuilders/SchedulerTest.java

@@ -75,15 +75,18 @@ void ticksPauseIncompleteWorkFinishReadyWorkAndRefreshOnlyLoadedChunks() throws
       verify(waiting).rebindDefinitions();
       db.clearInvocations();
       server.getScheduler().performTicks(579);
-      db.verify(() -> Database.saveStationsNow(map));
+      db.verify(() -> Database.saveStationsLater(map));
+      db.verify(() -> Database.saveStationsNow(map), never());
       when(ready.hasBlueprint()).thenReturn(false);
       when(waiting.hasBlueprint()).thenReturn(false);
       when(detached.hasBlueprint()).thenReturn(false);
       db.clearInvocations();
       server.getScheduler().performTicks(600);
-      db.verify(() -> Database.saveStationsNow(map), never());
+      db.verify(() -> Database.saveStationsLater(map), never());
       manager.stop();
-      db.verify(() -> Database.saveStations(map));
+      var order = inOrder(Database.class);
+      order.verify(db, () -> Database.saveStations(map));
+      order.verify(db, Database::close);
     } finally {
       MockBukkit.unmock();
       VFBuilders.plugin = null;

src/test/java/net/tfminecraft/vfbuilders/data/StationWriterTest.java

@@ -0,0 +1,125 @@
+package net.tfminecraft.vfbuilders.data;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+import java.nio.file.*;
+import java.time.Duration;
+import java.util.*;
+import java.util.concurrent.*;
+import java.util.logging.Logger;
+import net.tfminecraft.vfbuilders.DatabaseFixture;
+import net.tfminecraft.vfbuilders.VFBuilders;
+import net.tfminecraft.vfbuilders.core.*;
+import org.bukkit.*;
+import org.junit.jupiter.api.*;
+
+class StationWriterTest {
+  private Path file;
+
+  @BeforeEach
+  void plugin() throws Exception {
+    Path folder = DatabaseFixture.folder();
+    VFBuilders.plugin = mock(VFBuilders.class);
+    when(VFBuilders.plugin.getDataFolder()).thenReturn(folder.toFile());
+    file = Database.path();
+    Files.createDirectories(file.getParent());
+  }
+
+  @AfterEach
+  void cleanUp() throws Exception {
+    Database.close();
+    VFBuilders.plugin = null;
+    Files.deleteIfExists(file);
+  }
+
+  private static Map<Location, ActiveStation> stations(int timeLeft) {
+    var world = mock(World.class);
+    when(world.getName()).thenReturn("build-world");
+    var loc = new Location(world, 3, 64, 5);
+    var definition = mock(Station.class);
+    when(definition.getId()).thenReturn("dock");
+    var station = mock(ActiveStation.class);
+    when(station.getLocation()).thenReturn(loc);
+    when(station.getStation()).thenReturn(definition);
+    when(station.getTimeLeft()).thenReturn(timeLeft);
+    return Map.of(loc, station);
+  }
+
+  @Test
+  void backgroundSaveWritesTheSnapshotTakenOnTheCallingThread() throws Exception {
+    var stations = stations(41);
+    var station = stations.values().iterator().next();
+    var saved = Database.saveStationsLater(stations);
+    // Later changes are not part of the queued snapshot.
+    when(station.getTimeLeft()).thenReturn(7);
+    assertTrue(saved.get(5, TimeUnit.SECONDS));
+    assertTrue(Files.readString(file).contains("\"timeLeft\": 41"));
+    try (var paths = Files.list(file.getParent())) {
+      assertEquals(List.of(file), paths.toList());
+    }
+  }
+
+  @Test
+  void queuedOlderSnapshotNeverOverwritesALaterSynchronousSave() throws Exception {
+    CompletableFuture<Boolean> older;
+    synchronized (Database.WRITE_LOCK) {
+      older = Database.saveStationsLater(stations(30));
+      // The writer thread is now waiting for the lock; this synchronous save is newer.
+      assertTrue(Database.saveStationsNow(stations(12)));
+    }
+    assertTrue(older.get(5, TimeUnit.SECONDS));
+    assertTrue(Files.readString(file).contains("\"timeLeft\": 12"));
+    assertTrue(Database.saveStationsLater(stations(11)).get(5, TimeUnit.SECONDS));
+    assertTrue(Files.readString(file).contains("\"timeLeft\": 11"));
+  }
+
+  @Test
+  void snapshotFailureIsReportedWithoutQueueingAWrite() throws Exception {
+    Files.writeString(file, "[]");
+    var detached = mock(ActiveStation.class);
+    when(detached.getLocation()).thenReturn(new Location(null, 1, 2, 3));
+    assertFalse(Database.saveStationsLater(Map.of(new Location(null, 1, 2, 3), detached)).get());
+    assertEquals("[]", Files.readString(file));
+  }
+
+  @Test
+  void closeDrainsQueuedWritesAndTheWriterRestartsAfterwards() throws Exception {
+    Database.close(); // Nothing started yet.
+    CompletableFuture<Boolean> queued;
+    synchronized (Database.WRITE_LOCK) {
+      queued = Database.saveStationsLater(stations(9));
+    }
+    Database.close();
+    assertTrue(queued.isDone());
+    assertTrue(queued.get());
+    assertTrue(Files.readString(file).contains("\"timeLeft\": 9"));
+    assertTrue(Database.saveStationsLater(stations(8)).get(5, TimeUnit.SECONDS));
+    assertTrue(Files.readString(file).contains("\"timeLeft\": 8"));
+  }
+
+  @Test
+  void closeGivesUpWaitingForAStuckWriter() throws Exception {
+    var logger = mock(Logger.class);
+    when(VFBuilders.plugin.getLogger()).thenReturn(logger);
+    CompletableFuture<Boolean> stuck;
+    synchronized (Database.WRITE_LOCK) {
+      stuck = Database.saveStationsLater(stations(5));
+      Database.close(Duration.ofMillis(50));
+      verify(logger).warning(contains("Station writer still busy"));
+    }
+    assertTrue(stuck.get(5, TimeUnit.SECONDS));
+  }
+
+  @Test
+  void interruptedCloseKeepsTheInterruptFlag() throws Exception {
+    CompletableFuture<Boolean> pending;
+    synchronized (Database.WRITE_LOCK) {
+      pending = Database.saveStationsLater(stations(4));
+      Thread.currentThread().interrupt();
+      Database.close();
+      assertTrue(Thread.interrupted());
+    }
+    assertTrue(pending.get(5, TimeUnit.SECONDS));
+  }
+}