Skip to content
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.

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

Already on GitHub? Sign in to your account

move SavedData.save to IO worker to avoid synchronous IO on main thread #1796

Open
wants to merge 3 commits into
base: 1.21.1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
5 changes: 4 additions & 1 deletion patches/net/minecraft/server/level/ServerLevel.java.patch
Original file line number Diff line number Diff line change
Expand Up @@ -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();
+ }
}
}

Expand Down
10 changes: 10 additions & 0 deletions patches/net/minecraft/util/worldupdate/WorldUpgrader.java.patch
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
--- a/net/minecraft/world/level/saveddata/SavedData.java
+++ b/net/minecraft/world/level/saveddata/SavedData.java
@@ -37,7 +_,7 @@
@@ -36,18 +_,23 @@
compoundtag.put("data", this.save(new CompoundTag(), p_324088_));
thiakil marked this conversation as resolved.
Show resolved Hide resolved
NbtUtils.addCurrentDataVersion(compoundtag);

+ net.neoforged.neoforge.common.IOUtilities.withIOWorker(()->{
thiakil marked this conversation as resolved.
Show resolved Hide resolved
try {
- NbtIo.writeCompressed(compoundtag, p_77758_.toPath());
+ net.neoforged.neoforge.common.IOUtilities.writeNbtCompressed(compoundtag, p_77758_.toPath());
} catch (IOException ioexception) {
LOGGER.error("Could not save data {}", this, ioexception);
}
@@ -47,7 +_,10 @@
+ });

this.setDirty(false);
}
}

public static record Factory<T extends SavedData>(
Expand Down
28 changes: 28 additions & 0 deletions src/main/java/net/neoforged/neoforge/common/IOUtilities.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,15 @@
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import java.util.function.BiPredicate;
import java.util.function.Consumer;
import net.minecraft.nbt.CompoundTag;
import net.minecraft.nbt.NbtIo;
import net.minecraft.util.thread.NamedThreadFactory;
import org.apache.commons.io.output.CloseShieldOutputStream;
import org.jetbrains.annotations.Nullable;

Expand All @@ -33,9 +38,15 @@ public final class IOUtilities {
StandardOpenOption.WRITE,
StandardOpenOption.TRUNCATE_EXISTING
};
private static final ThreadFactory THREAD_FACTORY = new NamedThreadFactory("NeoForge-IOUtilities");
private static ExecutorService ioExecutor = newIOExecutor();

private IOUtilities() {}

private static ExecutorService newIOExecutor() {
return Executors.newSingleThreadExecutor(THREAD_FACTORY);
}

/**
* Cleans up any temporary files that may have been left over from interrupted
* calls to {@link #atomicWrite(Path, WriteCallback)}.
Expand Down Expand Up @@ -152,6 +163,23 @@ public static void atomicWrite(Path targetPath, WriteCallback writeCallback) thr
}
}

public static void withIOWorker(Runnable task) {
ioExecutor.execute(task);
}

public static void waitUntilIOWorkerComplete() {
ExecutorService old = ioExecutor;
ioExecutor = newIOExecutor();
old.shutdown();
try {
if (!old.awaitTermination(1L, TimeUnit.HOURS)) {
throw new RuntimeException("Timed out waiting for IO worker to complete");
}
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
}

/**
* Declares an interface which is functionally equivalent to {@link Consumer},
* except supports the ability to throw IOExceptions that may occur.
Expand Down
Original file line number Diff line number Diff line change
@@ -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");
}
}
Loading