From 4f13be937eda5ee18010b271474a91894348e3e4 Mon Sep 17 00:00:00 2001 From: Spottedleaf Date: Mon, 3 Jun 2024 13:28:31 -0700 Subject: [PATCH] Do not perform chunk existance check for I/O scheduling In order to check if a chunk exists, the RegionFile lock (if the RegionFile is opened) will be acquired. However, the RegionFile may be performing I/O operations, in which case will stall the acquire operation. To ensure that threads scheduling loads do not incur a stall, we can avoid this check entirely - the RegionFile I/O thread(s) will eventually perform the exist check itself. --- .../server/0976-Rewrite-chunk-system.patch | 62 ++----------------- 1 file changed, 4 insertions(+), 58 deletions(-) diff --git a/patches/server/0976-Rewrite-chunk-system.patch b/patches/server/0976-Rewrite-chunk-system.patch index d1e7cfff7e09..c2d476a2a653 100644 --- a/patches/server/0976-Rewrite-chunk-system.patch +++ b/patches/server/0976-Rewrite-chunk-system.patch @@ -3519,10 +3519,10 @@ index 0000000000000000000000000000000000000000..15ee41452992714108efe53b708b5a4e +} diff --git a/src/main/java/io/papermc/paper/chunk/system/io/RegionFileIOThread.java b/src/main/java/io/papermc/paper/chunk/system/io/RegionFileIOThread.java new file mode 100644 -index 0000000000000000000000000000000000000000..2934f0cf0ef09c84739312b00186c2ef0019a165 +index 0000000000000000000000000000000000000000..2096e57c025858519e7c46788993b9aac1ec60e8 --- /dev/null +++ b/src/main/java/io/papermc/paper/chunk/system/io/RegionFileIOThread.java -@@ -0,0 +1,1343 @@ +@@ -0,0 +1,1289 @@ +package io.papermc.paper.chunk.system.io; + +import ca.spottedleaf.concurrentutil.collection.MultiThreadedQueue; @@ -4337,34 +4337,6 @@ index 0000000000000000000000000000000000000000..2934f0cf0ef09c84739312b00186c2ef + return thread.loadDataAsyncInternal(world, chunkX, chunkZ, type, onComplete, intendingToBlock, priority); + } + -+ private static Boolean doesRegionFileExist(final int chunkX, final int chunkZ, final boolean intendingToBlock, -+ final ChunkDataController taskController) { -+ final ChunkPos chunkPos = new ChunkPos(chunkX, chunkZ); -+ if (intendingToBlock) { -+ return taskController.computeForRegionFile(chunkX, chunkZ, true, (final RegionFile file) -> { -+ if (file == null) { // null if no regionfile exists -+ return Boolean.FALSE; -+ } -+ -+ return file.hasChunk(chunkPos) ? Boolean.TRUE : Boolean.FALSE; -+ }); -+ } else { -+ // first check if the region file for sure does not exist -+ if (taskController.doesRegionFileNotExist(chunkX, chunkZ)) { -+ return Boolean.FALSE; -+ } // else: it either exists or is not known, fall back to checking the loaded region file -+ -+ return taskController.computeForRegionFileIfLoaded(chunkX, chunkZ, (final RegionFile file) -> { -+ if (file == null) { // null if not loaded -+ // not sure at this point, let the I/O thread figure it out -+ return Boolean.TRUE; -+ } -+ -+ return file.hasChunk(chunkPos) ? Boolean.TRUE : Boolean.FALSE; -+ }); -+ } -+ } -+ + Cancellable loadDataAsyncInternal(final ServerLevel world, final int chunkX, final int chunkZ, + final RegionFileType type, final BiConsumer onComplete, + final boolean intendingToBlock, final PrioritisedExecutor.Priority priority) { @@ -4377,20 +4349,6 @@ index 0000000000000000000000000000000000000000..2934f0cf0ef09c84739312b00186c2ef + if (running == null) { + // not scheduled + -+ if (callbackInfo.regionFileCalculation == null) { -+ // caller will compute this outside of compute(), to avoid holding the bin lock -+ callbackInfo.needsRegionFileTest = true; -+ return null; -+ } -+ -+ if (callbackInfo.regionFileCalculation == Boolean.FALSE) { -+ // not on disk -+ callbackInfo.data = null; -+ callbackInfo.throwable = null; -+ callbackInfo.completeNow = true; -+ return null; -+ } -+ + // set up task + final ChunkDataTask newTask = new ChunkDataTask( + world, chunkX, chunkZ, taskController, RegionFileIOThread.this, priority @@ -4422,17 +4380,7 @@ index 0000000000000000000000000000000000000000..2934f0cf0ef09c84739312b00186c2ef + return running; + }; + -+ ChunkDataTask curr = taskController.tasks.get(key); -+ if (curr == null) { -+ callbackInfo.regionFileCalculation = doesRegionFileExist(chunkX, chunkZ, intendingToBlock, taskController); -+ } -+ ChunkDataTask ret = taskController.tasks.compute(key, compute); -+ if (callbackInfo.needsRegionFileTest) { -+ // curr isn't null but when we went into compute() it was -+ callbackInfo.regionFileCalculation = doesRegionFileExist(chunkX, chunkZ, intendingToBlock, taskController); -+ // now it should be fine -+ ret = taskController.tasks.compute(key, compute); -+ } ++ final ChunkDataTask ret = taskController.tasks.compute(key, compute); + + // needs to be scheduled + if (callbackInfo.tasksNeedsScheduling) { @@ -4491,8 +4439,6 @@ index 0000000000000000000000000000000000000000..2934f0cf0ef09c84739312b00186c2ef + public Throwable throwable; + public boolean completeNow; + public boolean tasksNeedsScheduling; -+ public boolean needsRegionFileTest; -+ public Boolean regionFileCalculation; + + } + @@ -19500,7 +19446,7 @@ index 0382b6597a130d746f8954a93a756a9d1ac81d50..ffbb3bf9ff3fc968ef69d4f889b0baf7 } } diff --git a/src/main/java/net/minecraft/world/entity/Entity.java b/src/main/java/net/minecraft/world/entity/Entity.java -index 370d00afe8384556ee92e28d253c44ed6989efab..6324b875472fc2dbc581157306ff255ef1bf5db2 100644 +index 62363c09111aaa31220fb260940744c097af7b3c..ff497f0e80889508dd8c183b48cd33bc7831ba6c 100644 --- a/src/main/java/net/minecraft/world/entity/Entity.java +++ b/src/main/java/net/minecraft/world/entity/Entity.java @@ -481,6 +481,58 @@ public abstract class Entity implements SyncedDataHolder, Nameable, EntityAccess