Significantly reduce the number of ChunkCoordIntPair allocations (#312)

* Significantly reduce the number of ChunkCoordIntPair allocations

* spotlessApply (#315)

* Use a LongChunkCoordIntPairSet instead

* Still need to bypass the original loop

* small tweaks

* Wrap the "NEW" operation instead of making our own loop -- requires a UnMixin bump 0.1.14+

* Document Unimixins version dep jump

* Switch to an opt in unsafe iterator (Reuse), default to the safe one (allocatiosn)

* spotlessApply (#316)

* Fix running in an obf env, also update toArray() functions

* Disable the WorldServer Mixin if Optifine is present since it messes with the field

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

README.md

@@ -1,6 +1,6 @@
 # A Hodgepodge of Fixes
 
-Requires UniMixins 0.1.5+ (https://github.com/LegacyModdingMC/UniMixins/) to work.
+Requires UniMixins 0.1.14+ (https://github.com/LegacyModdingMC/UniMixins/) to work.
 
 ## Running
 

build.gradle

@@ -651,7 +651,7 @@ repositories {
 
 def mixinProviderGroup = "io.github.legacymoddingmc"
 def mixinProviderModule = "unimixins"
-def mixinProviderVersion = "0.1.13"
+def mixinProviderVersion = "0.1.15"
 def mixinProviderSpecNoClassifer = "${mixinProviderGroup}:${mixinProviderModule}:${mixinProviderVersion}"
 def mixinProviderSpec = "${mixinProviderSpecNoClassifer}:dev"
 ext.mixinProviderSpec = mixinProviderSpec

gradle.properties

@@ -21,6 +21,10 @@ forgeVersion = 10.13.4.1614
 # Do you need to test how your custom blocks interacts with a player that is not the owner? -> leave name empty
 developmentEnvironmentUserName = Developer
 
+# Enables using modern java syntax (up to version 17) via Jabel, while still targetting JVM 8.
+# See https://github.com/bsideup/jabel for details on how this works.
+enableModernJavaSyntax = true
+
 # Generate a class with String fields for the mod id, name, version and group name named with the fields below
 generateGradleTokenClass = com.mitchej123.hodgepodge.Tags
 gradleTokenModId =

src/main/java/com/mitchej123/hodgepodge/Hodgepodge.java

@@ -19,7 +19,7 @@
         version = Hodgepodge.VERSION,
         name = Hodgepodge.NAME,
         acceptableRemoteVersions = "*",
-        dependencies = "required-after:gtnhmixins@[2.0.1,)",
+        dependencies = "required-after:gtnhmixins@[2.0.1,); " + "required-after:unimixins@[0.0.14,); ",
         guiFactory = "com.mitchej123.hodgepodge.config.gui.HodgepodgeGuiConfigFactory")
 public class Hodgepodge {
 

src/main/java/com/mitchej123/hodgepodge/config/FixesConfig.java

@@ -5,6 +5,10 @@
 @Config(modid = "hodgepodge", category = "fixes")
 public class FixesConfig {
 
+    @Config.Comment("Fix too many allocations from Chunk Coordinate Int Pair")
+    @Config.DefaultBoolean(true)
+    @Config.RequiresMcRestart
+    public static boolean fixTooManyAllocationsChunkPositionIntPair;
     @Config.Comment("Removes duplicate Fermenter and Squeezer recipes and flower registration")
     @Config.DefaultBoolean(true)
     @Config.RequiresMcRestart

src/main/java/com/mitchej123/hodgepodge/hax/LongChunkCoordIntPairSet.java

@@ -0,0 +1,149 @@
+package com.mitchej123.hodgepodge.hax;
+
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.Set;
+
+import net.minecraft.world.ChunkCoordIntPair;
+
+import org.jetbrains.annotations.NotNull;
+
+import com.mitchej123.hodgepodge.mixins.interfaces.MutableChunkCoordIntPair;
+import com.mitchej123.hodgepodge.util.ChunkPosUtil;
+
+import it.unimi.dsi.fastutil.longs.LongIterator;
+import it.unimi.dsi.fastutil.longs.LongOpenHashSet;
+import it.unimi.dsi.fastutil.longs.LongSet;
+
+public class LongChunkCoordIntPairSet implements Set<ChunkCoordIntPair> {
+
+    private final MutableChunkCoordIntPair reusableChunkCoordIntPair = (MutableChunkCoordIntPair) new ChunkCoordIntPair(
+            0,
+            0);
+
+    public ChunkCoordIntPair fromLongUnsafe(long pos) {
+        return (ChunkCoordIntPair) reusableChunkCoordIntPair
+                .setChunkPos(ChunkPosUtil.getPackedX(pos), ChunkPosUtil.getPackedZ(pos));
+    }
+
+    public ChunkCoordIntPair fromLongSafe(long pos) {
+        return new ChunkCoordIntPair(ChunkPosUtil.getPackedX(pos), ChunkPosUtil.getPackedZ(pos));
+    }
+
+    private LongSet longSet = new LongOpenHashSet();
+
+    @Override
+    public int size() {
+        return longSet.size();
+    }
+
+    @Override
+    public boolean isEmpty() {
+        return longSet.isEmpty();
+    }
+
+    @Override
+    public boolean contains(Object o) {
+        if (o instanceof ChunkCoordIntPair chunkCoordIntPair) {
+            return longSet.contains(ChunkPosUtil.toLong(chunkCoordIntPair.chunkXPos, chunkCoordIntPair.chunkZPos));
+        } else if (o instanceof Long l) {
+            return longSet.contains(l);
+        }
+        return false;
+    }
+
+    public boolean contains(long l) {
+        return longSet.contains(l);
+    }
+
+    @NotNull
+    @Override
+    public Iterator<ChunkCoordIntPair> iterator() {
+        return longSet.stream().map(this::fromLongSafe).iterator();
+    }
+
+    public Iterator<ChunkCoordIntPair> unsafeIterator() {
+        // Reuses the same ChunkCoordIntPair object for every iteration, use this when you know the code won't
+        // be storing the result anywhere
+        return longSet.stream().map(this::fromLongUnsafe).iterator();
+    }
+
+    public LongIterator longIterator() {
+        return longSet.iterator();
+    }
+
+    @NotNull
+    @Override
+    public Object[] toArray() {
+        return longSet.stream().map(this::fromLongSafe).toArray();
+    }
+
+    @NotNull
+    @Override
+    public <T> T[] toArray(@NotNull T[] a) {
+        throw new UnsupportedOperationException("Not implemented");
+    }
+
+    @Override
+    public boolean add(ChunkCoordIntPair chunkCoordIntPair) {
+        return this.longSet.add(ChunkPosUtil.toLong(chunkCoordIntPair.chunkXPos, chunkCoordIntPair.chunkZPos));
+    }
+
+    public boolean addLong(long l) {
+        return this.longSet.add(l);
+    }
+
+    @Override
+    public boolean remove(Object o) {
+        if (o instanceof ChunkCoordIntPair chunkCoordIntPair) {
+            return longSet.remove(ChunkPosUtil.toLong(chunkCoordIntPair.chunkXPos, chunkCoordIntPair.chunkZPos));
+        } else if (o instanceof Long l) {
+            return longSet.remove(l);
+        }
+        return false;
+    }
+
+    @Override
+    public boolean containsAll(@NotNull Collection<?> c) {
+        return longSet.containsAll(c);
+    }
+
+    @Override
+    public boolean addAll(@NotNull Collection<? extends ChunkCoordIntPair> c) {
+        boolean added = false;
+        for (ChunkCoordIntPair chunkCoordIntPair : c) {
+            added |= this.longSet.add(ChunkPosUtil.toLong(chunkCoordIntPair.chunkXPos, chunkCoordIntPair.chunkZPos));
+        }
+        return added;
+    }
+
+    @Override
+    public boolean retainAll(@NotNull Collection<?> c) {
+        boolean removed = false;
+        for (long l : longSet) {
+            if (!c.contains(fromLongUnsafe(l))) {
+                removed |= longSet.remove(l);
+            }
+        }
+        return removed;
+    }
+
+    @Override
+    public boolean removeAll(@NotNull Collection<?> c) {
+        boolean removed = false;
+        for (Object o : c) {
+            if (o instanceof ChunkCoordIntPair chunkCoordIntPair) {
+                removed |= longSet
+                        .remove(ChunkPosUtil.toLong(chunkCoordIntPair.chunkXPos, chunkCoordIntPair.chunkZPos));
+            } else if (o instanceof Long l) {
+                removed |= longSet.remove(l);
+            }
+        }
+        return removed;
+    }
+
+    @Override
+    public void clear() {
+        this.longSet.clear();
+    }
+}

src/main/java/com/mitchej123/hodgepodge/mixins/Mixins.java

@@ -24,6 +24,19 @@ public enum Mixins {
     CHANGE_CATEGORY_SPRINT_KEY(new Builder("Moves the sprint keybind to the movement category")
             .addTargetedMod(TargetedMod.VANILLA).setSide(Side.CLIENT).setPhase(Phase.EARLY)
             .addMixinClasses("minecraft.MixinGameSetttings").setApplyIf(() -> TweaksConfig.changeSprintCategory)),
+    FIX_TOO_MANY_ALLOCATIONS_CHUNK_POSITION_INT_PAIR(
+            new Builder("Stops MC from allocating too many ChunkPositionIntPair objects")
+                    .addTargetedMod(TargetedMod.VANILLA).setSide(Side.BOTH).setPhase(Phase.EARLY)
+                    .addMixinClasses(
+                            "minecraft.MixinChunkCoordIntPair",
+                            "minecraft.MixinWorld_FixAllocations",
+                            "minecraft.MixinWorldClient_FixAllocations")
+                    .setApplyIf(() -> FixesConfig.fixTooManyAllocationsChunkPositionIntPair)),
+    FIX_TOO_MANY_ALLOCATIONS_CHUNK_POSITION_INT_PAIR_OPTIFINE_INCOMPAT(
+            new Builder("Stops MC from allocating too many ChunkPositionIntPair objects")
+                    .addTargetedMod(TargetedMod.VANILLA).addExcludedMod(TargetedMod.OPTIFINE).setSide(Side.BOTH)
+                    .setPhase(Phase.EARLY).addMixinClasses("minecraft.MixinWorldServer_FixAllocations")
+                    .setApplyIf(() -> FixesConfig.fixTooManyAllocationsChunkPositionIntPair)),
     FIX_RESOURCEPACK_FOLDER_OPENING(new Builder("Fix resource pack folder sometimes not opening on windows")
             .setPhase(Phase.EARLY).addMixinClasses("minecraft.MixinGuiScreenResourcePacks").setSide(Side.CLIENT)
             .setApplyIf(() -> FixesConfig.fixResourcePackOpening).addTargetedMod(TargetedMod.VANILLA)),

src/main/java/com/mitchej123/hodgepodge/mixins/early/minecraft/MixinChunkCoordIntPair.java

@@ -0,0 +1,32 @@
+package com.mitchej123.hodgepodge.mixins.early.minecraft;
+
+import net.minecraft.world.ChunkCoordIntPair;
+
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Shadow;
+
+import com.mitchej123.hodgepodge.mixins.interfaces.MutableChunkCoordIntPair;
+
+@Mixin(ChunkCoordIntPair.class)
+public class MixinChunkCoordIntPair implements MutableChunkCoordIntPair {
+
+    @Shadow
+    public int chunkXPos;
+    @Shadow
+    public int chunkZPos;
+
+    public void setChunkXPos(int chunkXPos) {
+        this.chunkXPos = chunkXPos;
+    }
+
+    public void setChunkZPos(int chunkZPos) {
+        this.chunkZPos = chunkZPos;
+    }
+
+    public MutableChunkCoordIntPair setChunkPos(int chunkXPos, int chunkZPos) {
+        this.chunkXPos = chunkXPos;
+        this.chunkZPos = chunkZPos;
+        return this;
+    }
+
+}

src/main/java/com/mitchej123/hodgepodge/mixins/early/minecraft/MixinWorldClient_FixAllocations.java

@@ -0,0 +1,28 @@
+package com.mitchej123.hodgepodge.mixins.early.minecraft;
+
+import java.util.Iterator;
+import java.util.Set;
+
+import net.minecraft.client.multiplayer.WorldClient;
+
+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 com.mitchej123.hodgepodge.hax.LongChunkCoordIntPairSet;
+
+@Mixin(WorldClient.class)
+public abstract class MixinWorldClient_FixAllocations {
+
+    @Shadow
+    private final Set previousActiveChunkSet = new LongChunkCoordIntPairSet();
+
+    @Redirect(
+            method = "func_147456_g",
+            at = @At(value = "INVOKE", target = "Ljava/util/Set;iterator()Ljava/util/Iterator;"))
+    private Iterator<?> fixAllocations(Set instance) {
+        return ((LongChunkCoordIntPairSet) instance).unsafeIterator();
+    }
+
+}

src/main/java/com/mitchej123/hodgepodge/mixins/early/minecraft/MixinWorldServer_FixAllocations.java

@@ -0,0 +1,24 @@
+package com.mitchej123.hodgepodge.mixins.early.minecraft;
+
+import java.util.Iterator;
+import java.util.Set;
+
+import net.minecraft.world.WorldServer;
+
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Redirect;
+
+import com.mitchej123.hodgepodge.hax.LongChunkCoordIntPairSet;
+
+@Mixin(WorldServer.class)
+public abstract class MixinWorldServer_FixAllocations {
+
+    @Redirect(
+            method = "func_147456_g",
+            at = @At(value = "INVOKE", target = "Ljava/util/Set;iterator()Ljava/util/Iterator;"))
+    private Iterator<?> fixAllocations(Set instance) {
+        return ((LongChunkCoordIntPairSet) instance).unsafeIterator();
+    }
+
+}

src/main/java/com/mitchej123/hodgepodge/mixins/early/minecraft/MixinWorld_FixAllocations.java

@@ -0,0 +1,34 @@
+package com.mitchej123.hodgepodge.mixins.early.minecraft;
+
+import java.util.Set;
+
+import net.minecraft.world.ChunkCoordIntPair;
+import net.minecraft.world.World;
+
+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 com.llamalad7.mixinextras.injector.wrapoperation.Operation;
+import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation;
+import com.mitchej123.hodgepodge.hax.LongChunkCoordIntPairSet;
+import com.mitchej123.hodgepodge.mixins.interfaces.MutableChunkCoordIntPair;
+
+@Mixin(World.class)
+public abstract class MixinWorld_FixAllocations {
+
+    @Shadow
+    protected Set activeChunkSet = new LongChunkCoordIntPairSet();
+
+    @Unique
+    private final MutableChunkCoordIntPair reusableCCIP = (MutableChunkCoordIntPair) new ChunkCoordIntPair(0, 0);
+
+    @WrapOperation(
+            at = @At(value = "NEW", target = "Lnet/minecraft/world/ChunkCoordIntPair;"),
+            method = "setActivePlayerChunksAndCheckLight")
+    private ChunkCoordIntPair reuseMutableChunkCoordIntPair(int x, int z, Operation<ChunkCoordIntPair> original) {
+        return (ChunkCoordIntPair) reusableCCIP.setChunkPos(x, z);
+    }
+
+}

src/main/java/com/mitchej123/hodgepodge/mixins/interfaces/MutableChunkCoordIntPair.java

@@ -0,0 +1,10 @@
+package com.mitchej123.hodgepodge.mixins.interfaces;
+
+public interface MutableChunkCoordIntPair {
+
+    void setChunkXPos(int chunkXPos);
+
+    void setChunkZPos(int chunkZPos);
+
+    MutableChunkCoordIntPair setChunkPos(int chunkXPos, int chunkZPos);
+}

src/main/java/com/mitchej123/hodgepodge/util/ChunkPosUtil.java

@@ -0,0 +1,19 @@
+package com.mitchej123.hodgepodge.util;
+
+public class ChunkPosUtil {
+
+    public static long INT_MASK = (1L << Integer.SIZE) - 1;
+
+    public static int getPackedX(long pos) {
+        return (int) (pos & INT_MASK);
+    }
+
+    public static int getPackedZ(long pos) {
+        return (int) (pos >>> 32 & INT_MASK);
+    }
+
+    public static long toLong(int x, int z) {
+        return (long) x & 4294967295L | ((long) z & 4294967295L) << 32;
+    }
+
+}