Skip to content

perf: only write unsaved studio projects on disconnect and shutdown #30

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Nowkoo merged 1 commit into from
Oct 11, 2026
Merged

perf: only write unsaved studio projects on disconnect and shutdown #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 19 additions & 9 deletions src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ public final class StudioService {
private final Map<UUID, Preview> previews = new HashMap<>();
private final Map<UUID, Set<String>> privateSounds = new HashMap<>();
private final Map<UUID, PendingSave> 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<UUID> unsaved = new HashSet<>();
private final Map<BlockKey, Playback> jukeboxes = new HashMap<>();
private final Map<UUID, Song> editions = new LinkedHashMap<>(16, 0.75f, true) {
@Override
Expand Down Expand Up @@ -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.");
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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<Void> 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) {
Expand All @@ -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.");
Expand All @@ -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() {
Expand All @@ -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);
Expand All @@ -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<Void> future) {}
private record PendingSave(CompletableFuture<Void> future, boolean take) {}

public void failure(Player player, IOException ex) {
plugin.getLogger().log(Level.SEVERE, "Studio storage operation failed for " + player.getUniqueId(), ex);
Expand Down
45 changes: 45 additions & 0 deletions src/test/java/net/tfminecraft/musicalinstruments/studio/StudioServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down