From e00d8b4b346c5f08d184faa1bb78c6dd53794d61 Mon Sep 17 00:00:00 2001 From: Nowko Date: Sun, 11 Oct 2026 20:49:22 +0200 Subject: [PATCH] perf: only write unsaved studio projects on disconnect and shutdown Cached projects already match their files except for a final take that is still saving or whose write failed. Disconnects and shutdown now retry only those, instead of rewriting every cached project and risking the shutdown timeout. A disconnect evicts a current project from memory. "Take saved" is announced only for finished takes, not for these saves. Co-Authored-By: Claude Opus 5.5 --- .../studio/StudioService.java | 28 ++++++++---- .../studio/StudioServiceTest.java | 45 +++++++++++++++++++ 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java index 16b9f7f..eec83b9 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java @@ -49,6 +49,9 @@ public final class StudioService { private final Map previews = new HashMap<>(); private final Map> privateSounds = new HashMap<>(); private final Map pendingSaves = new HashMap<>(); + // Owners whose cached project is newer than its files: a final take still saving or whose write failed. + // Other cached projects match the disk, so disconnects and shutdown need not write them again. + private final Set unsaved = new HashSet<>(); private final Map jukeboxes = new HashMap<>(); private final Map editions = new LinkedHashMap<>(16, 0.75f, true) { @Override @@ -169,6 +172,7 @@ public void deleteProject(Player player, Project expected) throws IOException { store.delete(player.getUniqueId(), id); if (current != null && current.song().id().equals(id)) { projects.remove(player.getUniqueId()); + unsaved.remove(player.getUniqueId()); stopPreview(player); } player.sendMessage("Deleted '" + expected.song().title() + "'. Published discs keep their music."); @@ -200,6 +204,7 @@ public void save(Player player, Project project) throws IOException { requireIdle(player); store.save(project); // Persist successfully before replacing the in-memory version. projects.put(player.getUniqueId(), project); + unsaved.remove(player.getUniqueId()); } public void edit(Player player, Project project) throws IOException { @@ -632,18 +637,19 @@ private void finish(Player player, Capture capture, String reason) { Project result = capture.snapshot((int) Math.max(1, Math.min(settings.durationTicks(), clock - capture.start + 1))); // Retain the take in memory even if the disk fails, so /music keep can retry. projects.put(player.getUniqueId(), result); - queueSave(result); + unsaved.add(player.getUniqueId()); + queueSave(result, true); player.sendMessage(reason + ". Saving take; you can review it when the save finishes."); } - private void queueSave(Project project) { + private void queueSave(Project project, boolean take) { // The shared FIFO worker puts the final snapshot after every older checkpoint. // Main-thread edits are guarded until this write completes, so they cannot be overwritten. CompletableFuture future = CompletableFuture.runAsync(() -> { try { store.save(project); } catch (IOException ex) { throw new CompletionException(ex); } }, checkpointWriter); - pendingSaves.put(project.song().owner(), new PendingSave(future)); + pendingSaves.put(project.song().owner(), new PendingSave(future, take)); } private void completeSave(UUID owner, PendingSave pending) { @@ -652,8 +658,10 @@ private void completeSave(UUID owner, PendingSave pending) { try { // Called only after isDone(): this never waits on disk from the musical clock. pending.future().join(); + // Edits wait for this save, so the cached project now matches its files. + unsaved.remove(owner); if (player == null) projects.remove(owner); - else player.sendMessage("Take saved: /music preview take, /music keep, or /music discard."); + else if (pending.take()) player.sendMessage("Take saved: /music preview take, /music keep, or /music discard."); } catch (CompletionException ex) { plugin.getLogger().log(Level.SEVERE, "Could not save studio project for " + owner, ex.getCause()); if (player != null) player.sendMessage("Could not read/write the recording. Your take is retained in memory; try /music keep again."); @@ -672,8 +680,10 @@ public void quit(Player player) { finish(player, capture, "Recording stopped on disconnect"); } stopPreview(player); - Project project = projects.get(player.getUniqueId()); - if (project != null && !pendingSaves.containsKey(player.getUniqueId())) queueSave(project); + UUID owner = player.getUniqueId(); + if (pendingSaves.containsKey(owner)) return; // completeSave evicts the project once it is written. + if (unsaved.contains(owner)) queueSave(projects.get(owner), false); // Retry a take whose write failed. + else projects.remove(owner); // Its files are current; the next visit reloads it. } public void close() { @@ -688,8 +698,8 @@ public void close() { finish(player, entry.getValue(), "Recording stopped on shutdown"); } } - for (Project project : projects.values()) { - if (!pendingSaves.containsKey(project.song().owner())) queueSave(project); + for (var entry : projects.entrySet()) { + if (unsaved.contains(entry.getKey()) && !pendingSaves.containsKey(entry.getKey())) queueSave(entry.getValue(), false); } for (UUID owner : new ArrayList<>(privateSounds.keySet())) { Player player = plugin.getServer().getPlayer(owner); @@ -716,7 +726,7 @@ private void stopPrivateSounds(Player player) { if (sounds != null) for (String sound : sounds) player.stopSound(sound, SoundCategory.PLAYERS); } - private record PendingSave(CompletableFuture future) {} + private record PendingSave(CompletableFuture future, boolean take) {} public void failure(Player player, IOException ex) { plugin.getLogger().log(Level.SEVERE, "Studio storage operation failed for " + player.getUniqueId(), ex); diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioServiceTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioServiceTest.java index 2afd3d6..d8f57e4 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioServiceTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioServiceTest.java @@ -189,6 +189,51 @@ void shutdownRetriesAFailedFinalSaveWhenStorageHasRecovered() throws Exception { studio.close(); assertEquals("recovered", new StudioStore(directory).project(owner).orElseThrow() .pending().notes().getFirst().sound()); + verify(player, never()).sendMessage(startsWith("Take saved")); + } + + @Test + void disconnectAndShutdownDoNotRewriteProjectsAlreadyOnDisk() throws Exception { + studio.save(player, new Project(song(track(1, "cached")), null, 100, false)); + clearInvocations(store); + studio.quit(player); + studio.close(); + verify(store, never()).save(any()); + verify(player, never()).sendMessage(startsWith("Take saved")); + } + + @Test + void disconnectEvictsACurrentProjectSoTheNextVisitReadsItsFiles() throws Exception { + studio.save(player, new Project(song(track(1, "before")), null, 100, false)); + studio.quit(player); + new StudioStore(directory).save(new Project(song(track(1, "after")), null, 100, false)); + assertEquals("after", studio.requireProject(player).song().track(1).notes().getFirst().sound()); + } + + @Test + void disconnectRetriesATakeWhoseSaveFailedWhileOnline() throws Exception { + studio.create(player, "Song", false); + studio.record(player, 1); tick.run(); tick.run(); + studio.capture(player, "lute", "retry", 1, 1); + doThrow(new IOException("Temporary disk failure")).when(store).save(any()); + studio.stop(player); + awaitSaves(); + doCallRealMethod().when(store).save(any()); + studio.quit(player); + when(server.getPlayer(owner)).thenReturn(null); + awaitSaves(); + assertEquals("retry", new StudioStore(directory).project(owner).orElseThrow() + .pending().notes().getFirst().sound()); + } + + @Test + void onlyAFinishedTakeAnnouncesThatItWasSaved() throws Exception { + studio.create(player, "Song", false); + studio.record(player, 1); tick.run(); tick.run(); + studio.capture(player, "lute", "take", 1, 1); + studio.stop(player); + awaitSaves(); + verify(player).sendMessage("Take saved: /music preview take, /music keep, or /music discard."); } @ParameterizedTest