From 1acab571f4a9e9bbce403ea1e0fa11f238597a36 Mon Sep 17 00:00:00 2001 From: Xander Date: Thu, 13 Feb 2025 00:43:37 +0800 Subject: [PATCH] Move SavedData.save to IO worker to avoid synchronous IO on main thread (#1796) --- .../server/level/ServerLevel.java.patch | 5 +++- .../util/worldupdate/WorldUpgrader.java.patch | 10 +++++++ .../level/saveddata/SavedData.java.patch | 14 +++++++-- .../neoforge/common/IOUtilities.java | 13 ++++++++ .../unittest/IOUtilitiesWorkerTest.java | 30 +++++++++++++++++++ 5 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 patches/net/minecraft/util/worldupdate/WorldUpgrader.java.patch create mode 100644 tests/src/junit/java/net/neoforged/neoforge/unittest/IOUtilitiesWorkerTest.java diff --git a/patches/net/minecraft/server/level/ServerLevel.java.patch b/patches/net/minecraft/server/level/ServerLevel.java.patch index 411052eadc..51b6932ad6 100644 --- a/patches/net/minecraft/server/level/ServerLevel.java.patch +++ b/patches/net/minecraft/server/level/ServerLevel.java.patch @@ -112,11 +112,14 @@ this.getProfiler().pop(); for (Entity entity : p_8648_.getPassengers()) { -@@ -807,6 +_,7 @@ +@@ -807,6 +_,10 @@ } else { this.entityManager.autoSave(); } + net.neoforged.neoforge.common.NeoForge.EVENT_BUS.post(new net.neoforged.neoforge.event.level.LevelEvent.Save(this)); ++ if (p_8645_) { ++ net.neoforged.neoforge.common.IOUtilities.waitUntilIOWorkerComplete(); ++ } } } diff --git a/patches/net/minecraft/util/worldupdate/WorldUpgrader.java.patch b/patches/net/minecraft/util/worldupdate/WorldUpgrader.java.patch new file mode 100644 index 0000000000..8057bb1050 --- /dev/null +++ b/patches/net/minecraft/util/worldupdate/WorldUpgrader.java.patch @@ -0,0 +1,10 @@ +--- a/net/minecraft/util/worldupdate/WorldUpgrader.java ++++ b/net/minecraft/util/worldupdate/WorldUpgrader.java +@@ -111,6 +_,7 @@ + LOGGER.info("Upgrading blocks"); + new WorldUpgrader.ChunkUpgrader().upgrade(); + this.overworldDataStorage.save(); ++ net.neoforged.neoforge.common.IOUtilities.waitUntilIOWorkerComplete(); + i = Util.getMillis() - i; + LOGGER.info("World optimizaton finished after {} seconds", i / 1000L); + this.finished = true; diff --git a/patches/net/minecraft/world/level/saveddata/SavedData.java.patch b/patches/net/minecraft/world/level/saveddata/SavedData.java.patch index ac130d73e8..8d1edf0946 100644 --- a/patches/net/minecraft/world/level/saveddata/SavedData.java.patch +++ b/patches/net/minecraft/world/level/saveddata/SavedData.java.patch @@ -1,15 +1,23 @@ --- a/net/minecraft/world/level/saveddata/SavedData.java +++ b/net/minecraft/world/level/saveddata/SavedData.java -@@ -37,7 +_,7 @@ +@@ -36,18 +_,26 @@ + compoundtag.put("data", this.save(new CompoundTag(), p_324088_)); NbtUtils.addCurrentDataVersion(compoundtag); ++ //copy the contents to handle mods that just append an existing Tag to the Compound ++ CompoundTag copied = compoundtag.copy(); ++ ++ net.neoforged.neoforge.common.IOUtilities.withIOWorker(()->{ try { - NbtIo.writeCompressed(compoundtag, p_77758_.toPath()); -+ net.neoforged.neoforge.common.IOUtilities.writeNbtCompressed(compoundtag, p_77758_.toPath()); ++ net.neoforged.neoforge.common.IOUtilities.writeNbtCompressed(copied, p_77758_.toPath()); } catch (IOException ioexception) { LOGGER.error("Could not save data {}", this, ioexception); } -@@ -47,7 +_,10 @@ ++ }); + + this.setDirty(false); + } } public static record Factory( diff --git a/src/main/java/net/neoforged/neoforge/common/IOUtilities.java b/src/main/java/net/neoforged/neoforge/common/IOUtilities.java index 8c2165a263..6935b94378 100644 --- a/src/main/java/net/neoforged/neoforge/common/IOUtilities.java +++ b/src/main/java/net/neoforged/neoforge/common/IOUtilities.java @@ -17,8 +17,10 @@ import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; import java.nio.file.attribute.BasicFileAttributes; +import java.util.concurrent.CompletableFuture; import java.util.function.BiPredicate; import java.util.function.Consumer; +import net.minecraft.Util; import net.minecraft.nbt.CompoundTag; import net.minecraft.nbt.NbtIo; import org.apache.commons.io.output.CloseShieldOutputStream; @@ -34,6 +36,8 @@ public final class IOUtilities { StandardOpenOption.TRUNCATE_EXISTING }; + private static CompletableFuture saveDataTasks = CompletableFuture.completedFuture(null); + private IOUtilities() {} /** @@ -152,6 +156,15 @@ public static void atomicWrite(Path targetPath, WriteCallback writeCallback) thr } } + public static void withIOWorker(Runnable task) { + saveDataTasks = saveDataTasks.thenRunAsync(task, Util.ioPool()); + } + + public static void waitUntilIOWorkerComplete() { + saveDataTasks.join(); + saveDataTasks = CompletableFuture.completedFuture(null); + } + /** * Declares an interface which is functionally equivalent to {@link Consumer}, * except supports the ability to throw IOExceptions that may occur. diff --git a/tests/src/junit/java/net/neoforged/neoforge/unittest/IOUtilitiesWorkerTest.java b/tests/src/junit/java/net/neoforged/neoforge/unittest/IOUtilitiesWorkerTest.java new file mode 100644 index 0000000000..6a636dc127 --- /dev/null +++ b/tests/src/junit/java/net/neoforged/neoforge/unittest/IOUtilitiesWorkerTest.java @@ -0,0 +1,30 @@ +/* + * Copyright (c) NeoForged and contributors + * SPDX-License-Identifier: LGPL-2.1-only + */ + +package net.neoforged.neoforge.unittest; + +import java.util.concurrent.atomic.AtomicBoolean; +import net.neoforged.neoforge.common.IOUtilities; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class IOUtilitiesWorkerTest { + /** + * Tests that waiting on the IO worker will complete remaining tasks + */ + @Test + void testWaitOnWorker() { + AtomicBoolean value = new AtomicBoolean(false); + IOUtilities.withIOWorker(() -> { + try { + Thread.sleep(2000); + } catch (InterruptedException ignored) {} + value.set(true); + }); + Assertions.assertFalse(value.get(), "Value should not be set yet"); + IOUtilities.waitUntilIOWorkerComplete(); + Assertions.assertTrue(value.get(), "Value should be set after waiting"); + } +}