From 7c455649794911289c838f366d89773cffaff2f1 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 23 May 2026 16:32:56 -0400 Subject: [PATCH 1/6] Fix potential stronghold cache corruption if player exits world too quickly --- .../ChunkGeneratorMixin.java | 19 ++++++++++++------- .../cache_strongholds/ServerLevelMixin.java | 2 +- .../modernfix/duck/IChunkGenerator.java | 4 ++-- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ChunkGeneratorMixin.java b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ChunkGeneratorMixin.java index 65d5b383..0b865124 100644 --- a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ChunkGeneratorMixin.java +++ b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ChunkGeneratorMixin.java @@ -4,9 +4,9 @@ import com.llamalad7.mixinextras.injector.wrapmethod.WrapMethod; import com.llamalad7.mixinextras.injector.wrapoperation.Operation; import net.minecraft.Util; import net.minecraft.core.Holder; -import net.minecraft.core.RegistryAccess; import net.minecraft.nbt.*; import net.minecraft.resources.RegistryOps; +import net.minecraft.server.MinecraftServer; import net.minecraft.world.level.ChunkPos; import net.minecraft.world.level.biome.BiomeSource; import net.minecraft.world.level.chunk.ChunkGeneratorStructureState; @@ -41,22 +41,23 @@ public class ChunkGeneratorMixin implements IChunkGenerator { private BiomeSource biomeSource; private Path mfix$dimensionPath; - private RegistryAccess.Frozen mfix$registryAccess; + private MinecraftServer mfix$server; + private SoftReference>> mfix$cachedPositions = new SoftReference<>(null); private static final String CACHE_FILENAME = "mfix_stronghold_cache_v2.nbt"; @Override - public void mfix$setStrongholdCachePath(Path cachePath, RegistryAccess.Frozen registryAccess) { + public void mfix$setStrongholdCachePath(Path cachePath, MinecraftServer server) { this.mfix$dimensionPath = cachePath; - this.mfix$registryAccess = registryAccess; + this.mfix$server = server; } @WrapMethod(method = "generateRingPositions") private CompletableFuture> modernfix$cacheRingPositions(Holder structureSet, ConcentricRingsStructurePlacement placement, Operation>> original) { - if (this.mfix$registryAccess == null || this.mfix$dimensionPath == null) { + if (this.mfix$server == null || this.mfix$dimensionPath == null) { return original.call(structureSet, placement); } @@ -69,14 +70,18 @@ public class ChunkGeneratorMixin implements IChunkGenerator { return CompletableFuture.completedFuture(List.copyOf(cached)); } + var server = this.mfix$server; return original.call(structureSet, placement).thenApplyAsync(positions -> { - mfix$writeToCache(cacheKey, positions); + // Skip write if server exited before we finished + if (server.isRunning()) { + mfix$writeToCache(cacheKey, positions); + } return positions; }, Util.ioPool()); } private String mfix$makeCacheKey(ConcentricRingsStructurePlacement placement) { - RegistryOps ops = RegistryOps.create(NbtOps.INSTANCE, this.mfix$registryAccess); + RegistryOps ops = RegistryOps.create(NbtOps.INSTANCE, this.mfix$server.registryAccess()); String placementKey = ConcentricRingsStructurePlacement.CODEC.encodeStart(ops, placement) .result().map(Tag::toString).orElse(null); String biomeSourceKey = BiomeSource.CODEC.encodeStart(ops, this.biomeSource) diff --git a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ServerLevelMixin.java b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ServerLevelMixin.java index 8505303e..dcd2b2f8 100644 --- a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ServerLevelMixin.java +++ b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ServerLevelMixin.java @@ -24,7 +24,7 @@ public class ServerLevelMixin { @Local(ordinal = 0, argsOnly = true) LevelStorageSource.LevelStorageAccess levelStorageAccess, @Local(ordinal = 0, argsOnly = true) ResourceKey dimension, @Local(ordinal = 0, argsOnly = true) MinecraftServer server) { - ((IChunkGenerator)instance).mfix$setStrongholdCachePath(levelStorageAccess.getDimensionPath(dimension), server.registryAccess()); + ((IChunkGenerator)instance).mfix$setStrongholdCachePath(levelStorageAccess.getDimensionPath(dimension), server); original.call(instance); } } diff --git a/src/main/java/org/embeddedt/modernfix/duck/IChunkGenerator.java b/src/main/java/org/embeddedt/modernfix/duck/IChunkGenerator.java index 3cf83acc..312c5b44 100644 --- a/src/main/java/org/embeddedt/modernfix/duck/IChunkGenerator.java +++ b/src/main/java/org/embeddedt/modernfix/duck/IChunkGenerator.java @@ -1,9 +1,9 @@ package org.embeddedt.modernfix.duck; -import net.minecraft.core.RegistryAccess; +import net.minecraft.server.MinecraftServer; import java.nio.file.Path; public interface IChunkGenerator { - void mfix$setStrongholdCachePath(Path cachePath, RegistryAccess.Frozen registryAccess); + void mfix$setStrongholdCachePath(Path cachePath, MinecraftServer server); } From b62eb1845b978200d3f51494d08c9fb3f10c4854 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 23 May 2026 16:43:56 -0400 Subject: [PATCH 2/6] Avoid blocking chunk generation on concentric rings calculation where possible --- ...oncentricRingsStructurePlacementMixin.java | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ConcentricRingsStructurePlacementMixin.java diff --git a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ConcentricRingsStructurePlacementMixin.java b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ConcentricRingsStructurePlacementMixin.java new file mode 100644 index 00000000..79aa608f --- /dev/null +++ b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ConcentricRingsStructurePlacementMixin.java @@ -0,0 +1,123 @@ +package org.embeddedt.modernfix.common.mixin.perf.cache_strongholds; + +import net.minecraft.world.level.chunk.ChunkGeneratorStructureState; +import net.minecraft.world.level.levelgen.structure.placement.ConcentricRingsStructurePlacement; +import org.embeddedt.modernfix.annotation.FeatureLevel; +import org.embeddedt.modernfix.annotation.RequiresFeatureLevel; +import org.spongepowered.asm.mixin.Final; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.Shadow; +import org.spongepowered.asm.mixin.Unique; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Inject; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; + +@Mixin(ConcentricRingsStructurePlacement.class) +@RequiresFeatureLevel(FeatureLevel.BETA) +public class ConcentricRingsStructurePlacementMixin { + + @Shadow @Final private int distance; + @Shadow @Final private int spread; + @Shadow @Final private int count; + + /** + * Maximum per-axis section displacement from the initial ring chunk after biome snapping. + * + * Vanilla calls findBiomeHorizontal with radius=112 blocks. In quart space this is ±28, + * and converting the selected quart back to section coordinates yields at most ±7 chunks + * per axis from the original (initialX, initialZ). + */ + @Unique private static final int MFIX_MAX_BIOME_SNAP_SECTIONS_PER_AXIS = 7; + /** + * Worst-case Euclidean error introduced by rounding: + * initialX/Z = round(cos(angle) * dist), round(sin(angle) * dist). + */ + @Unique private static final double MFIX_MAX_ROUNDING_ERROR = Math.sqrt(2.0) * 0.5; + /** + * Worst-case Euclidean biome-snap displacement when each axis can move by at most 7 chunks. + */ + @Unique private static final double MFIX_MAX_BIOME_SNAP_ERROR = MFIX_MAX_BIOME_SNAP_SECTIONS_PER_AXIS * Math.sqrt(2.0); + /** + * Total conservative positional slack (rounding + biome snap) applied to radial bounds. + */ + @Unique private static final double MFIX_MAX_POSITION_ERROR = MFIX_MAX_ROUNDING_ERROR + MFIX_MAX_BIOME_SNAP_ERROR; + + /** Squared chunk-distance below which no ring position can ever land. */ + @Unique private long mfix$innerRadiusSq; + /** Squared chunk-distance above which no ring position can ever land. */ + @Unique private long mfix$outerRadiusSq; + + /** + * Precomputes conservative radial bounds for vanilla's ring placement distance: + * {@code dist = 4*i + i*i1*6 + noise}, where {@code i=distance} and {@code i1=circle}. + * + * - Inner bound uses the minimum possible base term ({@code i1=0} => {@code 4*i}). + * - Outer bound uses the maximum reachable {@code i1} for this ({@code spread,count}) pair. + * + * Both bounds are expanded by {@link #MFIX_MAX_POSITION_ERROR} so we never reject a valid + * chunk produced by rounding and biome snapping. + */ + @Inject( + method = "(Lnet/minecraft/core/Vec3i;Lnet/minecraft/world/level/levelgen/structure/placement/StructurePlacement$FrequencyReductionMethod;FILjava/util/Optional;IIILnet/minecraft/core/HolderSet;)V", + at = @At("RETURN") + ) + private void mfix$computeRadiusBounds(CallbackInfo ci) { + double maxNoise = this.distance * 1.25; // (nextDouble() - 0.5) * (distance * 2.5) + + // min(dist): 4*i + i*0*6 - maxNoise + double minDist = 4.0 * this.distance - maxNoise; + double safeInnerRadius = minDist - MFIX_MAX_POSITION_ERROR; + this.mfix$innerRadiusSq = (long)Math.max(0.0, Math.floor(safeInnerRadius * safeInnerRadius)); + + if (this.spread == 0) { + // Vanilla behavior becomes non-finite here (angle += 2π / 0), so keep only inner rejection. + this.mfix$outerRadiusSq = Long.MAX_VALUE; + return; + } + + int maxCircle = this.mfix$computeMaxCircleIndex(); + // max(dist): 4*i + i*maxCircle*6 + maxNoise + double maxDist = 4.0 * this.distance + (double)this.distance * maxCircle * 6.0 + maxNoise; + double safeOuterRadius = maxDist + MFIX_MAX_POSITION_ERROR; + this.mfix$outerRadiusSq = (long)Math.ceil(safeOuterRadius * safeOuterRadius); + } + + /** + * Computes the highest ring index ({@code circle}) that vanilla can reach for this placement. + * + * This mirrors the spread/total update logic in + * {@link net.minecraft.world.level.chunk.ChunkGeneratorStructureState#generateRingPositions}, + * but only tracks deterministic loop state (no RNG). + */ + @Unique + private int mfix$computeMaxCircleIndex() { + int ringSpread = this.spread; + int total = 0; + int circle = 0; + + while (total + ringSpread < this.count) { + total += ringSpread; + circle++; + ringSpread += 2 * ringSpread / (circle + 1); + ringSpread = Math.min(ringSpread, this.count - total); + } + + return circle; + } + + /** + * @author embeddedt, GPT-5.3-Codex + * @reason Avoid calling getRingPositionsFor() when we know the current chunk lies outside the region where + * concentric placement can even happen. This is particularly helpful when creating new worlds, because we can + * avoid blocking on the slow noise computations within the spawn region around (0, 0). + */ + @Inject(method = "isPlacementChunk", at = @At("HEAD"), cancellable = true) + private void mfix$earlyRejectByRadius(ChunkGeneratorStructureState structureState, int x, int z, + CallbackInfoReturnable cir) { + long distSq = (long)x * x + (long)z * z; + if (distSq < this.mfix$innerRadiusSq || distSq > this.mfix$outerRadiusSq) { + cir.setReturnValue(false); + } + } +} From 538c52bc2a0011406bf4a0e1a89e2daf20c3faa2 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 23 May 2026 17:00:08 -0400 Subject: [PATCH 3/6] Run stronghold gen on dedicated thread pool --- .../ChunkGeneratorMixin.java | 42 +++++++++++++++---- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ChunkGeneratorMixin.java b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ChunkGeneratorMixin.java index 0b865124..de35aa17 100644 --- a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ChunkGeneratorMixin.java +++ b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/cache_strongholds/ChunkGeneratorMixin.java @@ -2,6 +2,8 @@ package org.embeddedt.modernfix.common.mixin.perf.cache_strongholds; import com.llamalad7.mixinextras.injector.wrapmethod.WrapMethod; import com.llamalad7.mixinextras.injector.wrapoperation.Operation; +import com.llamalad7.mixinextras.sugar.Share; +import com.llamalad7.mixinextras.sugar.ref.LocalRef; import net.minecraft.Util; import net.minecraft.core.Holder; import net.minecraft.nbt.*; @@ -17,6 +19,8 @@ import org.embeddedt.modernfix.duck.IChunkGenerator; 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.Redirect; import java.lang.ref.SoftReference; import java.nio.charset.StandardCharsets; @@ -29,6 +33,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; @Mixin(ChunkGeneratorStructureState.class) public class ChunkGeneratorMixin implements IChunkGenerator { @@ -55,8 +61,9 @@ public class ChunkGeneratorMixin implements IChunkGenerator { @WrapMethod(method = "generateRingPositions") private CompletableFuture> modernfix$cacheRingPositions(Holder structureSet, - ConcentricRingsStructurePlacement placement, - Operation>> original) { + ConcentricRingsStructurePlacement placement, + Operation>> original, + @Share("threadPool") LocalRef threadPoolRef) { if (this.mfix$server == null || this.mfix$dimensionPath == null) { return original.call(structureSet, placement); } @@ -71,13 +78,30 @@ public class ChunkGeneratorMixin implements IChunkGenerator { } var server = this.mfix$server; - return original.call(structureSet, placement).thenApplyAsync(positions -> { - // Skip write if server exited before we finished - if (server.isRunning()) { - mfix$writeToCache(cacheKey, positions); - } - return positions; - }, Util.ioPool()); + ExecutorService strongholdPool = Executors.newFixedThreadPool(Math.max(1, Runtime.getRuntime().availableProcessors() - 2)); + threadPoolRef.set(strongholdPool); + try { + return original.call(structureSet, placement).thenApplyAsync(positions -> { + // Skip write if server exited before we finished + if (server.isRunning()) { + mfix$writeToCache(cacheKey, positions); + } + return positions; + }, Util.ioPool()); + } finally { + strongholdPool.shutdown(); + } + } + + /** + * @author embeddedt + * @reason Ring position calculation is often not required for initial chunk generation, but the tasks still occupy + * CPU time on the main worker pool and prevent higher priority work from progressing. To fix this we use a + * dedicated pool. + */ + @Redirect(method = "generateRingPositions", at = @At(value = "INVOKE", target = "Lnet/minecraft/Util;backgroundExecutor()Ljava/util/concurrent/ExecutorService;")) + private ExecutorService useDedicatedService(@Share("threadPool") LocalRef threadPoolRef) { + return threadPoolRef.get(); } private String mfix$makeCacheKey(ConcentricRingsStructurePlacement placement) { From 62dbbea083d52adfb4ac194aa290ed46e310abb8 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 23 May 2026 21:23:26 -0400 Subject: [PATCH 4/6] Optimize ZIP resource packs significantly --- .../resourcepacks/FilePackResourcesMixin.java | 95 ++++++ .../modernfix/resources/ZipPackIndex.java | 311 ++++++++++++++++++ 2 files changed, 406 insertions(+) create mode 100644 src/main/java/org/embeddedt/modernfix/common/mixin/perf/resourcepacks/FilePackResourcesMixin.java create mode 100644 src/main/java/org/embeddedt/modernfix/resources/ZipPackIndex.java diff --git a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/resourcepacks/FilePackResourcesMixin.java b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/resourcepacks/FilePackResourcesMixin.java new file mode 100644 index 00000000..2153f6df --- /dev/null +++ b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/resourcepacks/FilePackResourcesMixin.java @@ -0,0 +1,95 @@ +package org.embeddedt.modernfix.common.mixin.perf.resourcepacks; + +import net.minecraft.server.packs.FilePackResources; +import net.minecraft.server.packs.PackResources; +import net.minecraft.server.packs.PackType; +import org.embeddedt.modernfix.ModernFix; +import org.embeddedt.modernfix.annotation.FeatureLevel; +import org.embeddedt.modernfix.annotation.RequiresFeatureLevel; +import org.embeddedt.modernfix.resources.ZipPackIndex; +import org.jetbrains.annotations.Nullable; +import org.spongepowered.asm.mixin.Final; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.Shadow; +import org.spongepowered.asm.mixin.Unique; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Inject; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; + +import java.io.File; +import java.io.IOException; +import java.util.Set; +import java.util.zip.ZipFile; + +@Mixin(FilePackResources.class) +@RequiresFeatureLevel(FeatureLevel.BETA) +public class FilePackResourcesMixin { + @Final + @Shadow private File file; + + @Shadow @Nullable private ZipFile getOrCreateZipFile() { return null; } + + @Unique + @Nullable + private volatile ZipPackIndex mf$packIndex; + + @Unique + @Nullable + private ZipPackIndex mf$getOrCreateIndex() { + var index = mf$packIndex; + if (index == null) { + synchronized (this) { + index = mf$packIndex; + if (index == null) { + // Ensure the ZipFile is open first; if it fails, getOrCreateZipFile returns null. + if (getOrCreateZipFile() == null) { + return null; + } + try { + mf$packIndex = index = new ZipPackIndex(file.toPath()); + } catch (IOException e) { + ModernFix.LOGGER.error("Failed to build zip index for {}", file, e); + } + } + } + } + return index; + } + + /** + * @author embeddedt + * @reason use the index instead of scanning the whole zip + */ + @Inject(method = "getNamespaces", at = @At("HEAD"), cancellable = true) + private void mf$getNamespaces(PackType type, CallbackInfoReturnable> cir) { + ZipPackIndex index = mf$getOrCreateIndex(); + if (index != null) { + cir.setReturnValue(index.getNamespaces(type)); + } + } + + /** + * @author embeddedt + * @reason use the index instead of scanning the whole zip + */ + @Inject(method = "listResources", at = @At("HEAD"), cancellable = true) + private void mf$listResources(PackType packType, String namespace, String path, + PackResources.ResourceOutput resourceOutput, CallbackInfo ci) { + ZipFile zf = getOrCreateZipFile(); + ZipPackIndex index = mf$getOrCreateIndex(); + if (index != null && zf != null) { + index.listResources(packType, namespace, path, zf, resourceOutput); + ci.cancel(); + } + } + + /** + * Drop the index when the pack is closed so it can be rebuilt cleanly if the + * pack is ever re-opened. + */ + @Inject(method = "close", at = @At("HEAD")) + private void mf$invalidateIndex(CallbackInfo ci) { + mf$packIndex = null; + } +} diff --git a/src/main/java/org/embeddedt/modernfix/resources/ZipPackIndex.java b/src/main/java/org/embeddedt/modernfix/resources/ZipPackIndex.java new file mode 100644 index 00000000..813d2d4f --- /dev/null +++ b/src/main/java/org/embeddedt/modernfix/resources/ZipPackIndex.java @@ -0,0 +1,311 @@ +package org.embeddedt.modernfix.resources; + +import net.minecraft.resources.ResourceLocation; +import net.minecraft.server.packs.PackResources; +import net.minecraft.server.packs.PackType; +import net.minecraft.server.packs.resources.IoSupplier; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.nio.MappedByteBuffer; +import java.nio.channels.FileChannel; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.*; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; + +/** + * An index over a zip file's central directory that allows efficient namespace listing + * and resource enumeration without iterating all entries on every call. + * + *

The index is built once at construction time by memory-mapping the zip's central + * directory and parsing it into a {@link DirNode} tree. All subsequent queries run in + * O(depth + k) time where k is the number of matching results. + * + *

The caller is responsible for opening and closing the {@link ZipFile}; this class + * only holds a read-only view of the zip's metadata via a mmap'd buffer. + */ +public class ZipPackIndex { + + // ------------------------------------------------------------------------- + // Zip structural constants (identical to EfficientZipFileSystem in blacksmith) + // ------------------------------------------------------------------------- + + private static final int EOCD_SIGNATURE = 0x06054b50; + private static final int EOCD_SIZE = 22; + private static final int EOCD_OFF_CD_SIZE = 12; + private static final int EOCD_OFF_CD_OFFSET = 16; + private static final int EOCD_MAX_COMMENT_LENGTH = 65535; + + private static final int CD_ENTRY_SIGNATURE = 0x02014b50; + private static final int CD_ENTRY_HEADER_SIZE = 46; + private static final int CD_OFF_FILENAME_LENGTH = 28; + private static final int CD_OFF_EXTRA_LENGTH = 30; + private static final int CD_OFF_COMMENT_LENGTH = 32; + + private static final int[] EMPTY_OFFSETS = new int[0]; + + // ------------------------------------------------------------------------- + // DirNode + // ------------------------------------------------------------------------- + + static final class DirNode { + Map childDirs; + int[] fileChildOffsets; // offsets into cdBuffer for each direct file child + + DirNode() { + childDirs = new HashMap<>(); + fileChildOffsets = EMPTY_OFFSETS; + } + + void freeze() { + childDirs = childDirs.isEmpty() ? Map.of() : Map.copyOf(childDirs); + for (DirNode child : childDirs.values()) { + child.freeze(); + } + } + } + + // ------------------------------------------------------------------------- + // Fields + // ------------------------------------------------------------------------- + + /** Memory-mapped central directory. May be null for empty/invalid zips. */ + private final MappedByteBuffer cdBuffer; + /** Root of the directory tree, always non-null (may be empty but frozen). */ + private final DirNode root; + + // ------------------------------------------------------------------------- + // Construction + // ------------------------------------------------------------------------- + + /** + * Build an index from the zip at the given path. Does not open a {@link ZipFile} + * and does not keep a reference to one; the caller owns all {@link ZipFile} lifecycle. + * + * @throws IOException if the file cannot be read or its central directory cannot be mapped + */ + public ZipPackIndex(Path zipPath) throws IOException { + this.cdBuffer = mmapCentralDirectory(zipPath); + this.root = buildTree(); + } + + private static MappedByteBuffer mmapCentralDirectory(Path filePath) throws IOException { + try (FileChannel channel = FileChannel.open(filePath, StandardOpenOption.READ)) { + long fileSize = channel.size(); + if (fileSize < EOCD_SIZE) return null; + + int tailSize = (int) Math.min(fileSize, (long) EOCD_SIZE + EOCD_MAX_COMMENT_LENGTH); + ByteBuffer tail = ByteBuffer.allocate(tailSize); + tail.order(ByteOrder.LITTLE_ENDIAN); + + long tailStart = fileSize - tailSize; + while (tail.hasRemaining()) { + int n = channel.read(tail, tailStart + tail.position()); + if (n < 0) { + break; + } + } + if (tail.hasRemaining()) { + throw new IOException("Failed to read ZIP tail"); + } + tail.flip(); + + // Scan backwards for the EOCD signature and validate comment length. + int eocdPos = -1; + for (int i = tailSize - EOCD_SIZE; i >= 0; i--) { + if (tail.getInt(i) == EOCD_SIGNATURE) { + int commentLen = Short.toUnsignedInt(tail.getShort(i + 20)); + if (i + EOCD_SIZE + commentLen == tailSize) { + eocdPos = i; + break; + } + } + } + if (eocdPos < 0) return null; + + long cdSize = Integer.toUnsignedLong(tail.getInt(eocdPos + EOCD_OFF_CD_SIZE)); + long cdOffset = Integer.toUnsignedLong(tail.getInt(eocdPos + EOCD_OFF_CD_OFFSET)); + if (cdSize == 0) return null; + if (cdSize == 0xFFFFFFFFL || cdOffset == 0xFFFFFFFFL) { + throw new IOException("ZIP64 not supported by ZipPackIndex"); + } + if (cdOffset > fileSize - cdSize) { + throw new IOException("Invalid central directory range"); + } + + try { + MappedByteBuffer buf = channel.map(FileChannel.MapMode.READ_ONLY, cdOffset, cdSize); + buf.order(ByteOrder.LITTLE_ENDIAN); + return buf; + } catch (RuntimeException e) { + throw new IOException("Failed to map central directory", e); + } + } + } + + private DirNode buildTree() throws IOException { + DirNode treeRoot = new DirNode(); + if (cdBuffer == null) { + treeRoot.freeze(); + return treeRoot; + } + + // Accumulate file offsets per DirNode before compacting to int[] + IdentityHashMap> fileOffsets = new IdentityHashMap<>(); + + int pos = 0; + int limit = cdBuffer.limit(); + while (pos + CD_ENTRY_HEADER_SIZE <= limit) { + if (cdBuffer.getInt(pos) != CD_ENTRY_SIGNATURE) break; + + int fileNameLen = Short.toUnsignedInt(cdBuffer.getShort(pos + CD_OFF_FILENAME_LENGTH)); + int extraLen = Short.toUnsignedInt(cdBuffer.getShort(pos + CD_OFF_EXTRA_LENGTH)); + int commentLen = Short.toUnsignedInt(cdBuffer.getShort(pos + CD_OFF_COMMENT_LENGTH)); + int recordLen = CD_ENTRY_HEADER_SIZE + fileNameLen + extraLen + commentLen; + if (pos + recordLen > limit) { + throw new IOException("Truncated central directory"); + } + + byte[] nameBytes = new byte[fileNameLen]; + cdBuffer.get(pos + CD_ENTRY_HEADER_SIZE, nameBytes); + String name = new String(nameBytes, StandardCharsets.UTF_8); + + boolean isDirectory = name.endsWith("/"); + if (isDirectory) name = name.substring(0, name.length() - 1); + + if (!name.isEmpty()) { + String[] parts = name.split("/"); + DirNode current = treeRoot; + int dirDepth = isDirectory ? parts.length : parts.length - 1; + for (int i = 0; i < dirDepth; i++) { + current = current.childDirs.computeIfAbsent(parts[i], k -> new DirNode()); + } + if (!isDirectory) { + fileOffsets.computeIfAbsent(current, k -> new ArrayList<>()).add(pos); + } + } + + pos += recordLen; + } + + // Compact to int[] arrays + fileOffsets.forEach((node, offsets) -> { + int[] arr = new int[offsets.size()]; + for (int i = 0; i < arr.length; i++) arr[i] = offsets.get(i); + node.fileChildOffsets = arr; + }); + + treeRoot.freeze(); + return treeRoot; + } + + // ------------------------------------------------------------------------- + // CD buffer reads — absolute-position gets are thread-safe on Java 13+ + // ------------------------------------------------------------------------- + + /** + * Extract the basename (the portion after the last '/') of the entry whose + * central-directory record starts at {@code cdOffset}. + */ + String readBasename(int cdOffset) { + int nameLen = Short.toUnsignedInt(cdBuffer.getShort(cdOffset + CD_OFF_FILENAME_LENGTH)); + byte[] nameBytes = new byte[nameLen]; + cdBuffer.get(cdOffset + CD_ENTRY_HEADER_SIZE, nameBytes); + int lastSlash = -1; + for (int i = nameBytes.length - 1; i >= 0; i--) { + if (nameBytes[i] == '/') { lastSlash = i; break; } + } + return new String(nameBytes, lastSlash + 1, nameLen - lastSlash - 1, StandardCharsets.UTF_8); + } + + // ------------------------------------------------------------------------- + // Public API + // ------------------------------------------------------------------------- + + /** + * Returns all namespaces present under the given pack type directory. + * + *

Equivalent to {@code FilePackResources.getNamespaces(type)} but reads from + * the pre-built tree rather than scanning all zip entries. + */ + public Set getNamespaces(PackType type) { + DirNode typeNode = root.childDirs.get(type.getDirectory()); + if (typeNode == null) return Set.of(); + Set result = new HashSet<>(); + for (String ns : typeNode.childDirs.keySet()) { + if (ns.equals(ns.toLowerCase(Locale.ROOT))) { + result.add(ns); + } + } + return result; + } + + /** + * Enumerate all resources under {@code type/namespace/path/} and deliver them + * to {@code output}. + * + *

Equivalent to {@code FilePackResources.listResources(type, namespace, path, output)} + * but uses the pre-built tree for O(k) traversal instead of a full zip scan. + * + * @param zipFile the open zip file, used only to supply {@link InputStream}s on demand; + * the caller retains ownership of its lifecycle + */ + public void listResources(PackType type, String namespace, String path, + ZipFile zipFile, PackResources.ResourceOutput output) { + DirNode node = root.childDirs.get(type.getDirectory()); + if (node == null) return; + node = node.childDirs.get(namespace); + if (node == null) return; + + // Walk to the requested sub-path + String rlSubPath; + if (!path.isEmpty()) { + for (String segment : path.split("/")) { + if (segment.isEmpty()) continue; + node = node.childDirs.get(segment); + if (node == null) return; + } + rlSubPath = path + "/"; + } else { + rlSubPath = ""; + } + + // entryPrefix = the part of the zip entry name before the ResourceLocation path + String entryPrefix = type.getDirectory() + "/" + namespace + "/"; + collectResources(node, entryPrefix, rlSubPath, zipFile, namespace, output); + } + + /** + * Recursively walk {@code node}, reconstructing zip entry names as we go and + * emitting each file to {@code output}. + * + * @param entryPrefix the constant prefix before the RL path, e.g. {@code "assets/minecraft/"} + * @param rlSubPath the RL-relative path accumulated so far, e.g. {@code "textures/block/"} + */ + private void collectResources(DirNode node, String entryPrefix, String rlSubPath, + ZipFile zipFile, String namespace, + PackResources.ResourceOutput output) { + // Emit direct file children of this node + for (int cdOffset : node.fileChildOffsets) { + String basename = readBasename(cdOffset); + String rlPathFull = rlSubPath + basename; + ResourceLocation rl = ResourceLocation.tryBuild(namespace, rlPathFull); + if (rl != null) { + ZipEntry entry = zipFile.getEntry(entryPrefix + rlPathFull); + if (entry != null) { + output.accept(rl, IoSupplier.create(zipFile, entry)); + } + } + } + // Recurse into subdirectories + for (Map.Entry child : node.childDirs.entrySet()) { + collectResources(child.getValue(), entryPrefix, + rlSubPath + child.getKey() + "/", zipFile, namespace, output); + } + } +} From 74f76f7305fee265858eac4d29ba4aae6322207e Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 23 May 2026 21:44:14 -0400 Subject: [PATCH 5/6] Improvements to ZipPackIndex - Allow it to work on channels that don't support mapping - Skip indexing folders that are not part of a pack type --- .../modernfix/resources/ZipPackIndex.java | 37 ++++++++++++++----- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/embeddedt/modernfix/resources/ZipPackIndex.java b/src/main/java/org/embeddedt/modernfix/resources/ZipPackIndex.java index 813d2d4f..d0a4eb92 100644 --- a/src/main/java/org/embeddedt/modernfix/resources/ZipPackIndex.java +++ b/src/main/java/org/embeddedt/modernfix/resources/ZipPackIndex.java @@ -9,7 +9,6 @@ import java.io.IOException; import java.io.InputStream; import java.nio.ByteBuffer; import java.nio.ByteOrder; -import java.nio.MappedByteBuffer; import java.nio.channels.FileChannel; import java.nio.charset.StandardCharsets; import java.nio.file.Path; @@ -74,8 +73,8 @@ public class ZipPackIndex { // Fields // ------------------------------------------------------------------------- - /** Memory-mapped central directory. May be null for empty/invalid zips. */ - private final MappedByteBuffer cdBuffer; + /** Central directory buffer (memory-mapped or heap-allocated fallback). May be null for empty/invalid zips. */ + private final ByteBuffer cdBuffer; /** Root of the directory tree, always non-null (may be empty but frozen). */ private final DirNode root; @@ -87,14 +86,14 @@ public class ZipPackIndex { * Build an index from the zip at the given path. Does not open a {@link ZipFile} * and does not keep a reference to one; the caller owns all {@link ZipFile} lifecycle. * - * @throws IOException if the file cannot be read or its central directory cannot be mapped + * @throws IOException if the file cannot be read or its central directory cannot be parsed */ public ZipPackIndex(Path zipPath) throws IOException { - this.cdBuffer = mmapCentralDirectory(zipPath); + this.cdBuffer = readCentralDirectory(zipPath); this.root = buildTree(); } - private static MappedByteBuffer mmapCentralDirectory(Path filePath) throws IOException { + private static ByteBuffer readCentralDirectory(Path filePath) throws IOException { try (FileChannel channel = FileChannel.open(filePath, StandardOpenOption.READ)) { long fileSize = channel.size(); if (fileSize < EOCD_SIZE) return null; @@ -138,13 +137,24 @@ public class ZipPackIndex { throw new IOException("Invalid central directory range"); } + // Try memory-mapping first; fall back to a heap copy if the OS refuses. try { - MappedByteBuffer buf = channel.map(FileChannel.MapMode.READ_ONLY, cdOffset, cdSize); + ByteBuffer buf = channel.map(FileChannel.MapMode.READ_ONLY, cdOffset, cdSize); buf.order(ByteOrder.LITTLE_ENDIAN); return buf; - } catch (RuntimeException e) { - throw new IOException("Failed to map central directory", e); + } catch (Exception ignored) { + // mmap unavailable (e.g. some Linux mount flags, container restrictions); + // read the central directory into a heap buffer instead. } + + ByteBuffer buf = ByteBuffer.allocate((int) cdSize); + buf.order(ByteOrder.LITTLE_ENDIAN); + while (buf.hasRemaining()) { + int n = channel.read(buf, cdOffset + buf.position()); + if (n < 0) throw new IOException("Truncated central directory during heap read"); + } + buf.flip(); + return buf; } } @@ -155,6 +165,11 @@ public class ZipPackIndex { return treeRoot; } + // Computed here (not statically) so that any loader-injected PackType values + // registered after class-load are included. + Set packTypeDirs = new HashSet<>(); + for (PackType type : PackType.values()) packTypeDirs.add(type.getDirectory()); + // Accumulate file offsets per DirNode before compacting to int[] IdentityHashMap> fileOffsets = new IdentityHashMap<>(); @@ -180,6 +195,10 @@ public class ZipPackIndex { if (!name.isEmpty()) { String[] parts = name.split("/"); + if (!packTypeDirs.contains(parts[0])) { + pos += recordLen; + continue; + } DirNode current = treeRoot; int dirDepth = isDirectory ? parts.length : parts.length - 1; for (int i = 0; i < dirDepth; i++) { From 494203ef5af2669230bee9129d97ba4ee4e57a87 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sun, 24 May 2026 19:45:24 -0400 Subject: [PATCH 6/6] Fix potential crash during worldgen with release_protochunks enabled The crash can occur if a protochunk next to a FULL chunk is dropped, and then later re-requested. If it was not persisted to disk for any reason, it starts regeneration from scratch. At FEATURES stage, it may try to place blocks into the adjacent LevelChunk already in the world. The fix is to prevent this situation from even happening by pinning protochunks directly next to FULL chunks, and preventing them from unloading. --- .../mixin/perf/release_protochunks/ChunkHolderMixin.java | 3 +-- .../mixin/perf/release_protochunks/ChunkMapMixin.java | 3 +-- .../duck/release_protochunks/IClearableChunkHolder.java | 8 ++++++++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/release_protochunks/ChunkHolderMixin.java b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/release_protochunks/ChunkHolderMixin.java index b66ff4dc..56cc5efb 100644 --- a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/release_protochunks/ChunkHolderMixin.java +++ b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/release_protochunks/ChunkHolderMixin.java @@ -4,7 +4,6 @@ import com.mojang.datafixers.util.Either; import net.minecraft.server.level.ChunkHolder; import net.minecraft.server.level.ChunkLevel; import net.minecraft.server.level.ChunkMap; -import net.minecraft.server.level.FullChunkStatus; import net.minecraft.world.level.ChunkPos; import net.minecraft.world.level.chunk.ChunkAccess; import org.embeddedt.modernfix.duck.release_protochunks.IClearableChunkHolder; @@ -85,7 +84,7 @@ public class ChunkHolderMixin implements IClearableChunkHolder { } private void mfix$markAsNeedingProtoChunkDrop() { - if (!ChunkLevel.fullStatus(this.ticketLevel).isOrAfter(FullChunkStatus.FULL) + if (this.ticketLevel >= LOWEST_DROPPABLE_TICKET_LEVEL && ChunkLevel.isLoaded(this.ticketLevel)) { // register for suspension check when chain completes var map = ((ISuspendedHolderTrackingChunkMap)this.playerProvider); diff --git a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/release_protochunks/ChunkMapMixin.java b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/release_protochunks/ChunkMapMixin.java index 916f7325..2ec12983 100644 --- a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/release_protochunks/ChunkMapMixin.java +++ b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/release_protochunks/ChunkMapMixin.java @@ -8,7 +8,6 @@ import it.unimi.dsi.fastutil.longs.Long2ObjectLinkedOpenHashMap; import net.minecraft.server.level.ChunkHolder; import net.minecraft.server.level.ChunkLevel; import net.minecraft.server.level.ChunkMap; -import net.minecraft.server.level.FullChunkStatus; import net.minecraft.util.thread.BlockableEventLoop; import net.minecraft.world.level.ChunkPos; import net.minecraft.world.level.chunk.ChunkAccess; @@ -68,7 +67,7 @@ public abstract class ChunkMapMixin implements ISuspendedHolderTrackingChunkMap long pos = entry.getLongKey(); ChunkHolder holder = this.updatingChunkMap.get(pos); if (holder == null // already removed - || ChunkLevel.fullStatus(holder.getTicketLevel()).isOrAfter(FullChunkStatus.FULL) // promoted to FULL + || holder.getTicketLevel() < IClearableChunkHolder.LOWEST_DROPPABLE_TICKET_LEVEL // promoted to FULL or adjacent to FULL chunk || !ChunkLevel.isLoaded(holder.getTicketLevel()) // is going to be dropped through normal code path ) { dropIterator.remove(); diff --git a/src/main/java/org/embeddedt/modernfix/duck/release_protochunks/IClearableChunkHolder.java b/src/main/java/org/embeddedt/modernfix/duck/release_protochunks/IClearableChunkHolder.java index b6910cf6..5ba3c06e 100644 --- a/src/main/java/org/embeddedt/modernfix/duck/release_protochunks/IClearableChunkHolder.java +++ b/src/main/java/org/embeddedt/modernfix/duck/release_protochunks/IClearableChunkHolder.java @@ -1,8 +1,16 @@ package org.embeddedt.modernfix.duck.release_protochunks; +import net.minecraft.server.level.ChunkLevel; +import net.minecraft.server.level.FullChunkStatus; + import java.util.concurrent.atomic.AtomicInteger; public interface IClearableChunkHolder { + /** + * We don't want to drop FULL chunks, or chunks immediately surrouding FULL. So + 2 is the minimum we can drop. + */ + int LOWEST_DROPPABLE_TICKET_LEVEL = ChunkLevel.byStatus(FullChunkStatus.FULL) + 2; + void mfix$resetProtoChunkFutures(); AtomicInteger mfix$getGenerationRefCount();