diff --git a/src/main/java/org/embeddedt/modernfix/ModernFixClient.java b/src/main/java/org/embeddedt/modernfix/ModernFixClient.java index 75a8b37f..6b3f799a 100644 --- a/src/main/java/org/embeddedt/modernfix/ModernFixClient.java +++ b/src/main/java/org/embeddedt/modernfix/ModernFixClient.java @@ -17,6 +17,7 @@ import net.minecraftforge.common.MinecraftForge; import net.minecraftforge.event.TagsUpdatedEvent; import net.minecraftforge.event.TickEvent; import net.minecraftforge.event.level.LevelEvent; +import net.minecraftforge.event.server.ServerStartingEvent; import net.minecraftforge.eventbus.api.EventPriority; import net.minecraftforge.eventbus.api.SubscribeEvent; import net.minecraftforge.fml.ModContainer; @@ -24,9 +25,11 @@ import net.minecraftforge.fml.ModList; import net.minecraftforge.fml.util.ObfuscationReflectionHelper; import net.minecraftforge.network.NetworkEvent; import org.embeddedt.modernfix.core.ModernFixMixinPlugin; +import org.embeddedt.modernfix.core.config.ModernFixConfig; import org.embeddedt.modernfix.load.LoadEvents; import org.embeddedt.modernfix.packet.EntityIDSyncPacket; import org.embeddedt.modernfix.screen.DeferredLevelLoadingScreen; +import org.embeddedt.modernfix.world.IntegratedWatchdog; import java.lang.management.ManagementFactory; import java.lang.reflect.Field; @@ -222,4 +225,13 @@ public class ModernFixClient { context.get().setPacketHandled(true); } + + @SubscribeEvent + public void onServerStarted(ServerStartingEvent event) { + if(ModernFixConfig.INTEGRATED_SERVER_WATCHDOG.get()) { + IntegratedWatchdog watchdog = new IntegratedWatchdog(event.getServer()); + watchdog.start(); + } + + } } diff --git a/src/main/java/org/embeddedt/modernfix/classloading/FastAccessTransformerList.java b/src/main/java/org/embeddedt/modernfix/classloading/FastAccessTransformerList.java new file mode 100644 index 00000000..8e83b325 --- /dev/null +++ b/src/main/java/org/embeddedt/modernfix/classloading/FastAccessTransformerList.java @@ -0,0 +1,132 @@ +package org.embeddedt.modernfix.classloading; + +import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; +import net.minecraftforge.accesstransformer.AccessTransformer; +import net.minecraftforge.accesstransformer.AccessTransformerEngine; +import net.minecraftforge.accesstransformer.INameHandler; +import net.minecraftforge.accesstransformer.Target; +import net.minecraftforge.accesstransformer.parser.AccessTransformerList; +import net.minecraftforge.fml.util.ObfuscationReflectionHelper; +import org.objectweb.asm.Type; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.lang.reflect.Field; +import java.util.Collection; +import java.util.Map; +import java.util.Set; + +public class FastAccessTransformerList extends AccessTransformerList { + private FastATMap accessTransformerMap; + + public static void attemptReplace() { + AccessTransformerList masterList; + FastAccessTransformerList myList = new FastAccessTransformerList(); + try { + Field master = AccessTransformerEngine.class.getDeclaredField("masterList"); + master.setAccessible(true); + masterList = (AccessTransformerList)master.get(AccessTransformerEngine.INSTANCE); + Field transfomersMap = AccessTransformerList.class.getDeclaredField("accessTransformers"); + transfomersMap.setAccessible(true); + Map, AccessTransformer> map = (Map, AccessTransformer>)transfomersMap.get(masterList); + INameHandler nameHandler = ObfuscationReflectionHelper.getPrivateValue(AccessTransformerList.class, masterList, "nameHandler"); + myList.setNameHandler(nameHandler); + myList.accessTransformerMap = new FastATMap(map); + ObfuscationReflectionHelper.setPrivateValue(AccessTransformerList.class, myList, myList.accessTransformerMap, "accessTransformers"); + master.set(AccessTransformerEngine.INSTANCE, myList); + } catch(ReflectiveOperationException e) { + e.printStackTrace(); + } + } + + @Override + public boolean containsClassTarget(Type type) { + return this.accessTransformerMap.containsType(type); + } + + private static class FastATMap implements Map, AccessTransformer> { + private final Map, AccessTransformer> delegate; + private final Set allContainedTypes; + + public FastATMap(Map, AccessTransformer> delegate) { + this.delegate = delegate; + this.allContainedTypes = new ObjectOpenHashSet<>(); + } + + @Override + public int size() { + return this.delegate.size(); + } + + @Override + public boolean isEmpty() { + return this.delegate.isEmpty(); + } + + @Override + public boolean containsKey(Object o) { + return this.delegate.containsKey(o); + } + + @Override + public boolean containsValue(Object o) { + return this.delegate.containsValue(o); + } + + @Override + public AccessTransformer get(Object o) { + return this.delegate.get(o); + } + + @Nullable + @Override + public AccessTransformer put(Target target, AccessTransformer accessTransformer) { + this.allContainedTypes.add(target.getASMType()); + return this.delegate.put(target, accessTransformer); + } + + @Override + public AccessTransformer remove(Object o) { + if(o instanceof Target) { + this.allContainedTypes.remove(((Target)o).getASMType()); + } + return this.delegate.remove(o); + } + + @Override + public void putAll(@NotNull Map, ? extends AccessTransformer> map) { + for(Target key : map.keySet()) { + this.allContainedTypes.add(key.getASMType()); + } + this.delegate.putAll(map); + } + + @Override + public void clear() { + this.allContainedTypes.clear(); + this.delegate.clear(); + } + + @NotNull + @Override + public Set> keySet() { + return this.delegate.keySet(); + } + + @NotNull + @Override + public Collection values() { + return this.delegate.values(); + } + + @NotNull + @Override + public Set, AccessTransformer>> entrySet() { + return this.delegate.entrySet(); + } + + public boolean containsType(Type type) { + return this.allContainedTypes.contains(type); + } + } +} diff --git a/src/main/java/org/embeddedt/modernfix/core/ModernFixMixinPlugin.java b/src/main/java/org/embeddedt/modernfix/core/ModernFixMixinPlugin.java index 03ff7003..8d027fd3 100644 --- a/src/main/java/org/embeddedt/modernfix/core/ModernFixMixinPlugin.java +++ b/src/main/java/org/embeddedt/modernfix/core/ModernFixMixinPlugin.java @@ -4,8 +4,10 @@ import cpw.mods.modlauncher.api.INameMappingService; import net.minecraftforge.fml.util.ObfuscationReflectionHelper; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.embeddedt.modernfix.classloading.FastAccessTransformerList; import org.embeddedt.modernfix.core.config.ModernFixEarlyConfig; import org.embeddedt.modernfix.core.config.Option; +import org.embeddedt.modernfix.load.ModWorkManagerQueue; import org.embeddedt.modernfix.util.DummyList; import org.objectweb.asm.Opcodes; import org.objectweb.asm.tree.*; @@ -38,6 +40,9 @@ public class ModernFixMixinPlugin implements IMixinConfigPlugin { this.logger.info("Loaded configuration file for ModernFix: {} options available, {} override(s) found", config.getOptionCount(), config.getOptionOverrideCount()); + FastAccessTransformerList.attemptReplace(); + ModWorkManagerQueue.replace(); + /* https://github.com/FabricMC/Mixin/pull/99 */ try { Field groupMembersField = InjectorGroupInfo.class.getDeclaredField("members"); @@ -154,6 +159,14 @@ public class ModernFixMixinPlugin implements IMixinConfigPlugin { } } } + } else if(mixinClassName.equals("org.embeddedt.modernfix.mixin.bugfix.chunk_deadlock.valhesia.BlockStateBaseMixin")) { + // We need to destroy Valhelsia's callback so it can never run getBlockState + for(MethodNode m : targetClass.methods) { + if(m.name.contains("valhelsia_placeDousedTorch")) { + m.instructions.clear(); + m.instructions.add(new InsnNode(Opcodes.RETURN)); + } + } } } } \ No newline at end of file diff --git a/src/main/java/org/embeddedt/modernfix/core/config/ModernFixConfig.java b/src/main/java/org/embeddedt/modernfix/core/config/ModernFixConfig.java index f505bdb5..799814fc 100644 --- a/src/main/java/org/embeddedt/modernfix/core/config/ModernFixConfig.java +++ b/src/main/java/org/embeddedt/modernfix/core/config/ModernFixConfig.java @@ -27,6 +27,7 @@ public class ModernFixConfig { public static ForgeConfigSpec.BooleanValue ENABLE_DEBUG_RELOADER; public static ForgeConfigSpec.BooleanValue REBUILD_BLOCKSTATES_ASYNC; + public static ForgeConfigSpec.BooleanValue INTEGRATED_SERVER_WATCHDOG; public static Set jeiPluginBlacklist; @@ -44,6 +45,9 @@ public class ModernFixConfig { REBUILD_BLOCKSTATES_ASYNC = COMMON_BUILDER .comment("Rebuild blockstate cache asynchronously. Should work with most mods, but can be disabled.") .define("rebuild_blockstate_cache_async", true); + INTEGRATED_SERVER_WATCHDOG = COMMON_BUILDER + .comment("Automatically output a thread dump if the integrated server spends too long on one tick") + .define("integrated_server_watchdog", true); } static { diff --git a/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java b/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java index 8b421bf4..581db6af 100644 --- a/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java +++ b/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java @@ -41,8 +41,8 @@ public class ModernFixEarlyConfig { this.addMixinRule("perf.compress_blockstate", false); this.addMixinRule("bugfix.concurrency", true); this.addMixinRule("bugfix.edge_chunk_not_saved", true); + this.addMixinRule("bugfix.chunk_deadlock", true); this.addMixinRule("perf.thread_priorities", true); - this.addMixinRule("perf.sync_executor_sleep", true); this.addMixinRule("perf.scan_cache", true); this.addMixinRule("perf.flatten_model_predicates", true); this.addMixinRule("perf.deduplicate_location", false); diff --git a/src/main/java/org/embeddedt/modernfix/load/ModWorkManagerQueue.java b/src/main/java/org/embeddedt/modernfix/load/ModWorkManagerQueue.java new file mode 100644 index 00000000..0ed1e2fe --- /dev/null +++ b/src/main/java/org/embeddedt/modernfix/load/ModWorkManagerQueue.java @@ -0,0 +1,60 @@ +package org.embeddedt.modernfix.load; + +import net.minecraftforge.fml.ModWorkManager; +import net.minecraftforge.fml.util.ObfuscationReflectionHelper; + +import java.util.concurrent.ConcurrentLinkedDeque; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.LockSupport; + +public class ModWorkManagerQueue extends ConcurrentLinkedDeque { + private static final long PARK_TIME = TimeUnit.MILLISECONDS.toNanos(25); + + private static final Runnable DUMMY_TASK = () -> {}; + + private boolean shouldReturnDummyTask = false; + + /** + * Sleep for a bit if there are no tasks. + */ + @Override + public Runnable pollFirst() { + Runnable r = super.pollFirst(); + if(r == null) { + LockSupport.parkNanos(PARK_TIME); + boolean isReturning = shouldReturnDummyTask; + shouldReturnDummyTask = !shouldReturnDummyTask; + /* + * We need to kick FML to redraw the loading screen periodically, + * but also allow actually exiting the executor loop, so that + * loading can complete if async work is done. + * + * This is accomplished by alternating between returning a dummy + * task and nothing. + */ + return isReturning ? DUMMY_TASK : null; + } else { + return r; + } + } + + @SuppressWarnings({"unchecked", "rawtypes"}) + public static void replace() { + try { + Class syncExecutorClass = Class.forName("net.minecraftforge.fml.ModWorkManager$SyncExecutor"); + ConcurrentLinkedDeque taskQueue = (ConcurrentLinkedDeque) ObfuscationReflectionHelper.getPrivateValue((Class)syncExecutorClass, (Object)ModWorkManager.syncExecutor(), "tasks"); + ModWorkManagerQueue q = new ModWorkManagerQueue(); + Runnable task; + do { + task = taskQueue.pollFirst(); + if(task != null) + q.push(task); + } while(task != null); + ObfuscationReflectionHelper.setPrivateValue((Class)syncExecutorClass, (Object)ModWorkManager.syncExecutor(), q, "tasks"); + } catch(ReflectiveOperationException e) { + e.printStackTrace(); + } + } +} diff --git a/src/main/java/org/embeddedt/modernfix/mixin/bugfix/chunk_deadlock/ServerChunkCacheMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/bugfix/chunk_deadlock/ServerChunkCacheMixin.java new file mode 100644 index 00000000..93806ffc --- /dev/null +++ b/src/main/java/org/embeddedt/modernfix/mixin/bugfix/chunk_deadlock/ServerChunkCacheMixin.java @@ -0,0 +1,73 @@ +package org.embeddedt.modernfix.mixin.bugfix.chunk_deadlock; + +import com.mojang.datafixers.util.Either; +import net.minecraft.core.Holder; +import net.minecraft.core.Registry; +import net.minecraft.server.level.ChunkHolder; +import net.minecraft.server.level.ServerChunkCache; +import net.minecraft.server.level.ServerLevel; +import net.minecraft.world.level.ChunkPos; +import net.minecraft.world.level.biome.Biome; +import net.minecraft.world.level.biome.Biomes; +import net.minecraft.world.level.chunk.ChunkAccess; +import net.minecraft.world.level.chunk.ChunkStatus; +import net.minecraft.world.level.chunk.EmptyLevelChunk; +import org.embeddedt.modernfix.ModernFix; +import org.spongepowered.asm.mixin.Final; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.Shadow; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Inject; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; +import java.util.concurrent.*; + +@Mixin(value = ServerChunkCache.class, priority = 1100) +public abstract class ServerChunkCacheMixin { + @Shadow @Final private Thread mainThread; + @Shadow @Final public ServerLevel level; + + @Shadow protected abstract CompletableFuture> getChunkFutureMainThread(int k, int l, ChunkStatus arg, boolean bl); + + @Shadow @Final private ServerChunkCache.MainThreadExecutor mainThreadProcessor; + private final boolean debugDeadServerAccess = Boolean.getBoolean("modernfix.debugBadChunkloading"); + + @Inject(method = "getChunk", at = @At("HEAD"), cancellable = true) + private void bailIfServerDead(int chunkX, int chunkZ, ChunkStatus requiredStatus, boolean load, CallbackInfoReturnable cir) { + if(!this.mainThread.isAlive()) { + ModernFix.LOGGER.fatal("A mod is accessing chunks from a stopped server (this will also cause memory leaks)"); + if(debugDeadServerAccess) { + new Exception().printStackTrace(); + } + Holder plains = this.level.registryAccess().registryOrThrow(Registry.BIOME_REGISTRY).getHolderOrThrow(Biomes.PLAINS); + cir.setReturnValue(new EmptyLevelChunk(this.level, new ChunkPos(chunkX, chunkZ), plains)); + } else if(Thread.currentThread() != this.mainThread) { + CompletableFuture> future = CompletableFuture.supplyAsync(() -> this.getChunkFutureMainThread(chunkX, chunkZ, requiredStatus, false), this.mainThreadProcessor).join(); + if(!future.isDone()) { + // Wait at least 500 milliseconds before printing anything + Either resultingChunk = null; + try { + resultingChunk = future.get(500, TimeUnit.MILLISECONDS); + } catch(InterruptedException | ExecutionException | TimeoutException ignored) { + } + if(resultingChunk != null && resultingChunk.left().isPresent()) { + cir.setReturnValue(resultingChunk.left().get()); + return; + } + if(debugDeadServerAccess) + ModernFix.LOGGER.warn("Async loading of a chunk was requested, this might not be desirable", new Exception()); + else + ModernFix.LOGGER.warn("Suspicious async chunkload, pass -Dmodernfix.debugBadChunkloading=true for more details"); + try { + resultingChunk = future.get(10, TimeUnit.SECONDS); + if(resultingChunk.left().isPresent()) { + cir.setReturnValue(resultingChunk.left().get()); + return; + } + } catch(InterruptedException | ExecutionException | TimeoutException e) { + ModernFix.LOGGER.error("Async chunk load took way too long, this needs to be reported to the appropriate mod.", e); + } + //cir.setReturnValue(new EmptyLevelChunk(this.level, new ChunkPos(chunkX, chunkZ))); + } + } + } +} diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/sync_executor_sleep/SyncExecutorMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/sync_executor_sleep/SyncExecutorMixin.java deleted file mode 100644 index 81d59b09..00000000 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/sync_executor_sleep/SyncExecutorMixin.java +++ /dev/null @@ -1,38 +0,0 @@ -package org.embeddedt.modernfix.mixin.perf.sync_executor_sleep; - -import net.minecraftforge.fml.ModWorkManager; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.Overwrite; -import org.spongepowered.asm.mixin.Shadow; -import org.spongepowered.asm.mixin.injection.At; -import org.spongepowered.asm.mixin.injection.Inject; -import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; - -import java.util.concurrent.ConcurrentLinkedDeque; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.LockSupport; - -@Mixin(targets = "net/minecraftforge/fml/ModWorkManager$SyncExecutor") -public abstract class SyncExecutorMixin { - @Shadow(remap = false) public abstract boolean driveOne(); - - private static final long PARK_TIME = TimeUnit.MILLISECONDS.toNanos(50); - - /** - * Currently FML spins in driveOne while waiting for the modloading workers. We can improve this - * by sleeping for 50ms at a time. - * - * Also, render FML splash screen regardless of task availability. - * @author embeddedt - * @reason improve CPU efficiency - */ - public void drive(Runnable ticker) { - int executions = 0; - do { - executions++; - ticker.run(); - } while(driveOne()); - if(executions < 2) - LockSupport.parkNanos(PARK_TIME); - } -} diff --git a/src/main/java/org/embeddedt/modernfix/world/IntegratedWatchdog.java b/src/main/java/org/embeddedt/modernfix/world/IntegratedWatchdog.java new file mode 100644 index 00000000..c0e8f5a9 --- /dev/null +++ b/src/main/java/org/embeddedt/modernfix/world/IntegratedWatchdog.java @@ -0,0 +1,62 @@ +package org.embeddedt.modernfix.world; + +import com.mojang.logging.LogUtils; +import net.minecraft.DefaultUncaughtExceptionHandlerWithName; +import net.minecraft.Util; +import net.minecraft.server.MinecraftServer; +import org.slf4j.Logger; + +import java.lang.management.ManagementFactory; +import java.lang.management.ThreadInfo; +import java.lang.management.ThreadMXBean; +import java.util.concurrent.TimeUnit; + +public class IntegratedWatchdog extends Thread { + private static final Logger LOGGER = LogUtils.getLogger(); + + private final MinecraftServer server; + + private static final long MAX_TICK_DELTA = 40*1000; + + public IntegratedWatchdog(MinecraftServer server) { + this.server = server; + this.setDaemon(true); + this.setUncaughtExceptionHandler(new DefaultUncaughtExceptionHandlerWithName(LOGGER)); + this.setName("ModernFix integrated server watchdog"); + } + + public void run() { + while(server.isRunning()) { + long nextTick = this.server.getNextTickTime(); + long curTime = Util.getMillis(); + long delta = curTime - nextTick; + if(delta > MAX_TICK_DELTA) { + LOGGER.error("A single server tick has taken {}, more than {} milliseconds", delta, MAX_TICK_DELTA); + ThreadMXBean threadmxbean = ManagementFactory.getThreadMXBean(); + ThreadInfo[] athreadinfo = threadmxbean.dumpAllThreads(true, true); + StringBuilder sb = new StringBuilder(); + sb.append("Thread Dump:\n"); + for(ThreadInfo threadinfo : athreadinfo) { + sb.append(threadinfo); + StackTraceElement[] elements = threadinfo.getStackTrace(); + if(elements.length > 8) { + sb.append("extended trace:\n"); + for(int i = 8; i < elements.length; i++) { + sb.append("\tat "); + sb.append(elements[i]); + sb.append('\n'); + } + } + sb.append('\n'); + } + LOGGER.error(sb.toString()); + nextTick = 0; + curTime = 0; + } + try { + Thread.sleep(nextTick + MAX_TICK_DELTA - curTime); + } catch(InterruptedException ignored) { + } + } + } +} diff --git a/src/main/resources/META-INF/accesstransformer.cfg b/src/main/resources/META-INF/accesstransformer.cfg index ceba8d1e..ab5b89aa 100644 --- a/src/main/resources/META-INF/accesstransformer.cfg +++ b/src/main/resources/META-INF/accesstransformer.cfg @@ -24,3 +24,4 @@ public net.minecraft.world.level.block.state.BlockBehaviour$Properties f_60901_ public net.minecraft.world.level.block.state.BlockBehaviour$Properties f_60902_ # emissiveRendering public net.minecraft.world.level.block.state.BlockBehaviour$Properties f_60889_ # requiresCorrectToolForDrops public net.minecraft.world.level.block.state.BlockBehaviour$Properties f_60888_ # destroyTime +public net.minecraft.server.level.ServerChunkCache$MainThreadExecutor diff --git a/src/main/resources/modernfix.mixins.json b/src/main/resources/modernfix.mixins.json index 42d043f2..1997abd5 100644 --- a/src/main/resources/modernfix.mixins.json +++ b/src/main/resources/modernfix.mixins.json @@ -9,6 +9,7 @@ "bugfix.edge_chunk_not_saved.ChunkManagerMixin", "perf.modern_resourcepacks.VanillaPackResourcesMixin", "perf.modern_resourcepacks.PathPackResourcesMixin", + "bugfix.chunk_deadlock.ServerChunkCacheMixin", "perf.remove_biome_temperature_cache.BiomeMixin", "perf.reduce_blockstate_cache_rebuilds.GameDataMixin", "perf.reduce_blockstate_cache_rebuilds.BlockCallbacksMixin", @@ -16,7 +17,6 @@ "perf.reduce_blockstate_cache_rebuilds.BlocksMixin", "perf.reduce_blockstate_cache_rebuilds.BlockStateBaseMixin", "perf.deduplicate_location.MixinResourceLocation", - "perf.sync_executor_sleep.SyncExecutorMixin", "perf.cache_blockstate_cache_arrays.AbstractBlockStateCacheMixin", "perf.datapack_reload_exceptions.LootTableManagerMixin", "perf.datapack_reload_exceptions.RecipeManagerMixin",