From 4999c372ecbd4aa6e0277856afb37f630a3242f3 Mon Sep 17 00:00:00 2001 From: laforetbrut Date: Fri, 27 Mar 2026 14:15:29 +0100 Subject: [PATCH] perf: eliminate synchronous MySQL calls on server main thread Root cause of lag (TPS 9-16, MSPT spikes to 4846ms): PlayerEvent.SaveToFile triggered synchronous JDBC writes on the server main thread every Minecraft autosave cycle. With 35 players this caused hundreds of network round-trips to MySQL blocking the tick loop for up to 4846ms (97x the 50ms limit). Fixes applied: - onPlayerSaveToFile: now fully async. Entity state is snapshotted on the main thread (pure memory ops, <1ms), then ALL DB writes are submitted to the background executor. Main thread never blocks on MySQL again. - snapshotPlayerData: now captures ALL entity-dependent mod data (Curios, Accessories, CosmeticArmor, NeoForge attachments) on the main thread. Previously these were read from a background thread which is not thread-safe and could cause data corruption. - writeSnapshotToDB: single method that writes all player data in one background pass: player_data + curios + mod_player_data. - Auto-save background task: removed ModCompatSync.storeAll(player), storeSophisticatedBackpacks, storeSophisticatedStorageItems, storeRefinedStorageDisks from background thread. These all accessed entity state off-thread. Mod compat data is now in the main-thread snapshot; backpack/SS/RS2 contents are saved on logout/shutdown. - Added ModCompatSync snapshot API: snapshotAccessories(), snapshotCosmeticArmor(), snapshotAttachments(), writeModSnapshot() for clean separation of entity reads vs DB writes. --- .../fubuki/playersync/sync/VanillaSync.java | 151 ++++++++++++------ .../playersync/sync/addons/ModCompatSync.java | 109 ++++++++++++- 2 files changed, 210 insertions(+), 50 deletions(-) diff --git a/src/main/java/vip/fubuki/playersync/sync/VanillaSync.java b/src/main/java/vip/fubuki/playersync/sync/VanillaSync.java index 26f3e33..21ae501 100644 --- a/src/main/java/vip/fubuki/playersync/sync/VanillaSync.java +++ b/src/main/java/vip/fubuki/playersync/sync/VanillaSync.java @@ -686,21 +686,72 @@ public class VanillaSync { return "B64:" + Base64.getEncoder().encodeToString(object.getBytes(StandardCharsets.UTF_8)); } - public static void doPlayerSaveToFile(PlayerEvent.SaveToFile event) throws SQLException, IOException { - JDBCsetUp.executePreparedUpdate("UPDATE server_info SET last_update=? WHERE id=?", System.currentTimeMillis(), JdbcConfig.SERVER_ID.get()); - if (!event.getEntity().getTags().contains("player_synced")) return; - store(event.getEntity(), false); - } - - // FIX: SaveToFile already fires on the main thread. Running store() off-thread via - // executorService read player entity state (inventory, armor, effects) from a background - // thread, causing duplication/corruption. Run directly on the main thread. + /** + * FIX CRITICAL (performance): PlayerEvent.SaveToFile fires on the MAIN THREAD + * during Minecraft's own autosave cycle (every 6000 ticks) and on player logout. + * The previous implementation called store() synchronously, which includes: + * - Full inventory serialization + * - Multiple JDBC UPDATE/INSERT statements (each one a synchronous network round-trip + * to MySQL — 5ms to 4846ms depending on network latency) + * With 35 players this caused MSPT spikes of up to 4846ms (97× the 50ms limit). + * + * NEW APPROACH: + * 1. Update server heartbeat ASYNCHRONOUSLY (no main-thread DB call). + * 2. If the player has been synced, snapshot all entity state on the main thread + * (fast — pure memory serialization, no I/O). + * 3. Submit all DB writes to the background executor thread pool. + * 4. The main thread NEVER waits for MySQL — it returns immediately. + * + * Safety: backpack / SophisticatedStorage / RS2 contents are NOT saved here + * (they are saved completely on logout and shutdown, which is the correct moment). + * The snapshot covers inventory, effects, XP, curios, accessories, cosmetic armor, + * and NeoForge attachments — everything that changes frequently during gameplay. + */ @SubscribeEvent public static void onPlayerSaveToFile(PlayerEvent.SaveToFile event) { + // Always update server heartbeat — async, never blocks main thread + executorService.submit(() -> { + try { + JDBCsetUp.executePreparedUpdate("UPDATE server_info SET last_update=? WHERE id=?", + System.currentTimeMillis(), JdbcConfig.SERVER_ID.get()); + } catch (SQLException e) { + PlayerSync.LOGGER.error("Error updating server heartbeat on SaveToFile", e); + } + }); + + Player player = event.getEntity(); + String puuid = player.getUUID().toString(); + + if (!player.getTags().contains("player_synced")) return; + if (syncNotCompletedPlayer.contains(puuid)) return; + if (player.isDeadOrDying()) return; + + // Use tryLock: if a logout save or another SaveToFile save is already writing + // this player's data, skip — the other operation already has fresh data. + ReentrantLock lock = getPlayerLock(puuid); + if (!lock.tryLock()) return; + try { - doPlayerSaveToFile(event); + // === MAIN THREAD: snapshot all entity state (no DB I/O, pure memory ops) === + final PlayerDataSnapshot snapshot = snapshotPlayerData(player); + + // === BACKGROUND THREAD: all DB writes — main thread continues immediately === + executorService.submit(() -> { + ReentrantLock bgLock = getPlayerLock(puuid); + if (!bgLock.tryLock()) return; // another save started, skip + try { + writeSnapshotToDB(snapshot); + } catch (Exception e) { + PlayerSync.LOGGER.error("Error writing async SaveToFile snapshot for player {}", puuid, e); + } finally { + bgLock.unlock(); + } + }); + } catch (Exception e) { - PlayerSync.LOGGER.error("Error during player save-to-file", e); + PlayerSync.LOGGER.error("Error snapshotting player {} for SaveToFile", puuid, e); + } finally { + lock.unlock(); // main thread releases → background thread can now acquire } } @@ -1037,29 +1088,47 @@ public class VanillaSync { } } - // NOTE: Sophisticated Backpacks/Storage/RS2 saves are NOT done here anymore. - // They are done in the background thread (their entity reads are on SavedData which is thread-safe, - // and their DB writes should not block the main thread). + // Mod data snapshots — entity reads, MUST be on main thread. + // These are included in the snapshot so the background writer can persist them + // without touching the entity again. + String curiosData = ModList.get().isLoaded("curios") && !player.isDeadOrDying() + ? ModsSupport.snapshotCuriosData(player) : null; + String accessoriesData = ModCompatSync.snapshotAccessories(player); + String cosmeticArmorData = ModCompatSync.snapshotCosmeticArmor(player); + String attachmentsData = ModCompatSync.snapshotAttachments(player); + + // NOTE: Sophisticated Backpacks/Storage/RS2 saves are intentionally NOT in the + // periodic snapshot — their contents live in server-side SavedData and are + // always saved completely on logout / server shutdown. return new PlayerDataSnapshot( uuid, XP, score, foodLevel, health, leftHand, cursors, equipmentMap.toString(), inventoryMap.toString(), enderChestMap.toString(), effectMap.toString(), advancements, - null, null, null, null // Curios/Accessories/CosmeticArmor/Attachments handled by their own DB writes + curiosData, accessoriesData, cosmeticArmorData, attachmentsData ); } /** - * Writes a snapshot to the DB. Runs on BACKGROUND THREAD (no entity access). + * Writes a snapshot to the DB. Runs on BACKGROUND THREAD — no entity access. + * All data (basic + curios + mod compat) is written here in one pass. */ private static void writeSnapshotToDB(PlayerDataSnapshot s) throws Exception { + // Core player data JDBCsetUp.executePreparedUpdate( "UPDATE player_data SET inventory=?, armor=?, xp=?, effects=?, enderchest=?, score=?, food_level=?, health=?, advancements=?, left_hand=?, cursors=? WHERE uuid=?", s.inventory(), s.equipment(), s.xp(), s.effects(), s.enderChest(), s.score(), s.foodLevel(), s.health(), s.advancements(), s.leftHand(), s.cursors(), s.uuid()); - // Curios, Accessories, CosmeticArmor, Attachments are already written by their own store methods - // during the snapshot phase (they do their own DB writes internally) + // Curios (snapshotted on main thread, written here off-thread) + if (s.curiosData() != null) { + JDBCsetUp.executePreparedUpdate( + "REPLACE INTO curios (uuid, curios_item) VALUES (?, ?)", + s.uuid(), s.curiosData()); + } + + // Mod compat: Accessories + CosmeticArmor + NeoForge attachments + ModCompatSync.writeModSnapshot(s.uuid(), s.accessoriesData(), s.cosmeticArmorData(), s.attachmentsData()); } private static String getSyncWorldForServer() { @@ -1120,9 +1189,18 @@ public class VanillaSync { }); } - // Auto-save: Snapshot entity data on MAIN THREAD (fast), then write to DB on BACKGROUND THREAD. - // Previously, store() ran entirely on main thread including DB writes, blocking the tick loop - // for ~5ms per player per save (~5.66% server thread usage from Spark profiling). + // Auto-save: snapshot ALL entity data on MAIN THREAD (fast, no I/O), then write + // to DB on a BACKGROUND THREAD. + // + // FIX: Previously the background task called ModCompatSync.storeAll(player), + // storeSophisticatedBackpacks(player), etc. from off-thread — accessing entity + // state (inventory, Accessories API, CosmeticArmor, NeoForge attachments) in a + // non-thread-safe way. All entity reads are now done in snapshotPlayerData() + // on the main thread, and the background task only does DB writes. + // + // Backpack / SophisticatedStorage / RS2 contents live in server-side SavedData + // and are always saved completely on player logout + server shutdown — no need + // to include them in the periodic auto-save. if (autoSaveTickCounter >= AUTO_SAVE_INTERVAL_TICKS) { autoSaveTickCounter = 0; MinecraftServer server = ServerLifecycleHooks.getCurrentServer(); @@ -1135,40 +1213,17 @@ public class VanillaSync { ReentrantLock lock = getPlayerLock(puuid); if (!lock.tryLock()) continue; try { - // === MAIN THREAD: Snapshot ALL data (entity reads only, no DB I/O) === + // === MAIN THREAD: snapshot ALL entity state (no DB I/O) === + // snapshotPlayerData now includes curios, accessories, + // cosmeticarmor, and neoforge attachments. final PlayerDataSnapshot snapshot = snapshotPlayerData(player); - // Snapshot Curios data on main thread (entity read), DB write deferred - final String curiosSnapshot; - if (ModList.get().isLoaded("curios") && !player.isDeadOrDying()) { - curiosSnapshot = ModsSupport.snapshotCuriosData(player); - } else { - curiosSnapshot = null; - } - - // === BACKGROUND THREAD: ALL DB writes in one batch === + // === BACKGROUND THREAD: DB writes only (no entity access) === executorService.submit(() -> { ReentrantLock bgLock = getPlayerLock(puuid); if (!bgLock.tryLock()) return; try { writeSnapshotToDB(snapshot); - // Write curios data - if (curiosSnapshot != null) { - JDBCsetUp.executePreparedUpdate( - "REPLACE INTO curios (uuid, curios_item) VALUES (?, ?)", - puuid, curiosSnapshot); - } - // Mod compat + storage saves (all DB writes, off main thread) - ModCompatSync.storeAll(player); - if (ModList.get().isLoaded("sophisticatedbackpacks")) { - ModsSupport.storeSophisticatedBackpacks(player); - } - if (ModList.get().isLoaded("sophisticatedstorage")) { - ModsSupport.storeSophisticatedStorageItems(player); - } - if (ModList.get().isLoaded("refinedstorage")) { - ModsSupport.storeRefinedStorageDisks(player); - } } catch (Exception e) { PlayerSync.LOGGER.error("Error auto-saving player {}", puuid, e); } finally { diff --git a/src/main/java/vip/fubuki/playersync/sync/addons/ModCompatSync.java b/src/main/java/vip/fubuki/playersync/sync/addons/ModCompatSync.java index 3160b68..328d1f3 100644 --- a/src/main/java/vip/fubuki/playersync/sync/addons/ModCompatSync.java +++ b/src/main/java/vip/fubuki/playersync/sync/addons/ModCompatSync.java @@ -339,13 +339,118 @@ public class ModCompatSync { } } + // ============================ + // Snapshot methods (main thread - entity reads only, NO DB writes) + // These are used by auto-save and SaveToFile to capture entity state on the + // main thread, then the actual DB writes happen on a background thread. + // ============================ + + /** + * Captures Accessories slot data on the main thread. + * Returns serialized string or null if mod not loaded / no data. + */ + public static String snapshotAccessories(Player player) { + if (!ModList.get().isLoaded("accessories")) return null; + try { + io.wispforest.accessories.api.AccessoriesCapability cap = + io.wispforest.accessories.api.AccessoriesCapability.get(player); + if (cap == null) return null; + Map flatMap = new HashMap<>(); + for (Map.Entry entry : cap.getContainers().entrySet()) { + String slotType = entry.getKey(); + var accessories = entry.getValue().getAccessories(); + for (int i = 0; i < accessories.getContainerSize(); i++) { + ItemStack stack = accessories.getItem(i); + if (!stack.isEmpty()) { + flatMap.put(slotType + ":" + i, VanillaSync.getNbtForStorage(stack)); + } + } + } + return flatMap.isEmpty() ? null : flatMap.toString(); + } catch (Exception e) { + PlayerSync.LOGGER.error("Error snapshotting Accessories for player {}", player.getUUID(), e); + return null; + } + } + + /** + * Captures Cosmetic Armor slot data on the main thread. + * Returns serialized string or null if mod not loaded / no data. + */ + public static String snapshotCosmeticArmor(Player player) { + if (!ModList.get().isLoaded("cosmeticarmorreworked")) return null; + try { + lain.mods.cos.impl.inventory.InventoryCosArmor cosInv = + lain.mods.cos.impl.ModObjects.invMan.getCosArmorInventory(player.getUUID()); + if (cosInv == null) return null; + Map flatMap = new HashMap<>(); + for (int i = 0; i < cosInv.getContainerSize(); i++) { + ItemStack stack = cosInv.getItem(i); + if (!stack.isEmpty()) { + flatMap.put(i, VanillaSync.getNbtForStorage(stack)); + } + } + return flatMap.isEmpty() ? null : flatMap.toString(); + } catch (Exception e) { + PlayerSync.LOGGER.error("Error snapshotting CosmeticArmor for player {}", player.getUUID(), e); + return null; + } + } + + /** + * Captures NeoForge attachment data on the main thread via reflection. + * Returns BNBT-serialized string or null if no data. + */ + public static String snapshotAttachments(Player player) { + try { + if (!(player instanceof net.minecraft.server.level.ServerPlayer serverPlayer)) return null; + java.lang.reflect.Method serializeMethod = net.neoforged.neoforge.attachment.AttachmentHolder.class + .getDeclaredMethod("serializeAttachments", net.minecraft.core.HolderLookup.Provider.class); + serializeMethod.setAccessible(true); + net.minecraft.nbt.CompoundTag attachments = (net.minecraft.nbt.CompoundTag) + serializeMethod.invoke(player, serverPlayer.getServer().registryAccess()); + if (attachments == null || attachments.isEmpty()) return null; + return VanillaSync.serializeTagToBinaryBase64(attachments); + } catch (Exception e) { + PlayerSync.LOGGER.error("Error snapshotting NeoForge attachments for player {}", player.getUUID(), e); + return null; + } + } + + /** + * Writes pre-snapshotted mod data to the DB. + * NO entity access — safe to call from a background thread. + * + * @param uuid player UUID string + * @param accessoriesData serialized Accessories slots (may be null → skipped) + * @param cosmeticArmor serialized Cosmetic Armor slots (may be null → skipped) + * @param attachments serialized NeoForge attachments (may be null → skipped) + */ + public static void writeModSnapshot(String uuid, String accessoriesData, String cosmeticArmor, String attachments) throws SQLException { + if (accessoriesData != null) { + JDBCsetUp.executePreparedUpdate( + "REPLACE INTO mod_player_data (uuid, mod_id, data_value) VALUES (?, ?, ?)", + uuid, "accessories", accessoriesData); + } + if (cosmeticArmor != null) { + JDBCsetUp.executePreparedUpdate( + "REPLACE INTO mod_player_data (uuid, mod_id, data_value) VALUES (?, ?, ?)", + uuid, "cosmeticarmor", cosmeticArmor); + } + if (attachments != null) { + JDBCsetUp.executePreparedUpdate( + "REPLACE INTO mod_player_data (uuid, mod_id, data_value) VALUES (?, ?, ?)", + uuid, "neoforge_attachments", attachments); + } + } + // ============================ // Convenience methods // ============================ /** - * Saves all mod-specific data for a player. - * Called on logout and auto-save. + * Saves all mod-specific data for a player synchronously. + * Called on logout and server shutdown (main thread — entity reads are safe here). */ public static void storeAll(Player player) { storeAccessories(player);