From 45d308c6a47035843d165c06f21e083cfcd7ce25 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 13 May 2023 09:36:34 -0400 Subject: [PATCH 1/2] Fix incorrect logic in packet leak patch Related: #97 --- .../ClientPlayNetHandlerMixin.java | 33 ------------------- .../SCustomPayloadPlayPacketMixin.java | 12 ++----- .../modernfix/duck/IClientNetHandler.java | 7 ---- 3 files changed, 3 insertions(+), 49 deletions(-) delete mode 100644 common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/packet_leak/ClientPlayNetHandlerMixin.java delete mode 100644 common/src/main/java/org/embeddedt/modernfix/duck/IClientNetHandler.java diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/packet_leak/ClientPlayNetHandlerMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/packet_leak/ClientPlayNetHandlerMixin.java deleted file mode 100644 index 630007a8..00000000 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/packet_leak/ClientPlayNetHandlerMixin.java +++ /dev/null @@ -1,33 +0,0 @@ -package org.embeddedt.modernfix.common.mixin.bugfix.packet_leak; - -import net.minecraft.client.multiplayer.ClientPacketListener; -import net.minecraft.network.FriendlyByteBuf; -import net.minecraft.network.protocol.game.ClientboundCustomPayloadPacket; -import org.embeddedt.modernfix.annotation.ClientOnlyMixin; -import org.embeddedt.modernfix.duck.IClientNetHandler; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.injection.At; -import org.spongepowered.asm.mixin.injection.Redirect; - -@Mixin(ClientPacketListener.class) -@ClientOnlyMixin -public class ClientPlayNetHandlerMixin implements IClientNetHandler { - private FriendlyByteBuf savedCopy = null; - /** - * @author embeddedt - * @reason Release the packet buffer at the end. Needed in f - */ - @Redirect(method = "handleCustomPayload", at = @At(value = "INVOKE", target = "Lnet/minecraft/network/protocol/game/ClientboundCustomPayloadPacket;getData()Lnet/minecraft/network/FriendlyByteBuf;")) - private FriendlyByteBuf saveCopyForRelease(ClientboundCustomPayloadPacket instance) { - FriendlyByteBuf copy = instance.getData(); - savedCopy = copy; - return copy; - } - - @Override - public FriendlyByteBuf getCopiedCustomBuffer() { - FriendlyByteBuf copy = savedCopy; - savedCopy = null; - return copy; - } -} diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/packet_leak/SCustomPayloadPlayPacketMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/packet_leak/SCustomPayloadPlayPacketMixin.java index 083f0c0f..2862becd 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/packet_leak/SCustomPayloadPlayPacketMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/packet_leak/SCustomPayloadPlayPacketMixin.java @@ -5,7 +5,6 @@ import net.minecraft.network.FriendlyByteBuf; import net.minecraft.network.protocol.game.ClientboundCustomPayloadPacket; import net.minecraft.resources.ResourceLocation; import org.embeddedt.modernfix.annotation.ClientOnlyMixin; -import org.embeddedt.modernfix.duck.IClientNetHandler; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; import org.spongepowered.asm.mixin.injection.At; @@ -32,14 +31,9 @@ public class SCustomPayloadPlayPacketMixin { @Redirect(method = "handle(Lnet/minecraft/network/protocol/game/ClientGamePacketListener;)V", at = @At(value = "INVOKE", target = "Lnet/minecraft/network/protocol/game/ClientGamePacketListener;handleCustomPayload(Lnet/minecraft/network/protocol/game/ClientboundCustomPayloadPacket;)V")) private void handleAndFree(ClientGamePacketListener instance, ClientboundCustomPayloadPacket sCustomPayloadPlayPacket) { - try { - instance.handleCustomPayload(sCustomPayloadPlayPacket); - } finally { - FriendlyByteBuf copied = ((IClientNetHandler)instance).getCopiedCustomBuffer(); - if(copied != null) - copied.release(); - } + /* in 1.16, this method creates a copy inside it, but handles freeing correctly */ + instance.handleCustomPayload(sCustomPayloadPlayPacket); if(this.needsRelease) - this.data.release(); + this.data.release(); /* free our own copy of the data if needed */ } } diff --git a/common/src/main/java/org/embeddedt/modernfix/duck/IClientNetHandler.java b/common/src/main/java/org/embeddedt/modernfix/duck/IClientNetHandler.java deleted file mode 100644 index 71b1a0a6..00000000 --- a/common/src/main/java/org/embeddedt/modernfix/duck/IClientNetHandler.java +++ /dev/null @@ -1,7 +0,0 @@ -package org.embeddedt.modernfix.duck; - -import net.minecraft.network.FriendlyByteBuf; - -public interface IClientNetHandler { - FriendlyByteBuf getCopiedCustomBuffer(); -} From eb925bc3bafa513897a7c39143ed98073cd6f03d Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sun, 14 May 2023 19:23:36 -0400 Subject: [PATCH 2/2] Fix performance issue when loading large NBT maps Array map was not being changed to hash map until AFTER the insertions, which is bad --- .../org/embeddedt/modernfix/util/CanonizingStringMap.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java b/common/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java index e5bb2f2d..05bd219b 100644 --- a/common/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java +++ b/common/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java @@ -77,11 +77,15 @@ public class CanonizingStringMap implements Map { public void putAll(@NotNull Map map) { if(map.size() == 0) return; + // grow early if we know there are enough non-overlapping keys + if((map.size() - backingMap.size()) > GROWTH_THRESHOLD && !(backingMap instanceof Object2ObjectOpenHashMap)) { + backingMap = new Object2ObjectOpenHashMap<>(backingMap); + } map.forEach((String key, T val) -> { key = KEY_INTERNER.intern(key); backingMap.put(key, val); }); - // if it's too big to be an array, grow it + // if it's still an array, and now too big, grow it if(backingMap.size() > GROWTH_THRESHOLD && !(backingMap instanceof Object2ObjectOpenHashMap)) { backingMap = new Object2ObjectOpenHashMap<>(backingMap); }