From 776f964f5fe1f79b6056f8c02b138c919cc396c5 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 31 Oct 2026 12:08:47 +0100 Subject: [PATCH 1/3] perf: read inventory holders without copying the block The surgery menu check runs on every inventory click, drag and close. Its plain getHolder() call copied the whole block state when the inventory belonged to a chest or barrel. On Main isSurgeryMenu cost 2.2 s over 20 hours. The checks now use getHolder(false), which hands back the same holder without the copy. The surgery menu has its own holder, so the result is the same object as before. No call site changes a block state, so none were left as they were. MockBukkit throws on getHolder(false), so a JUnit session listener patches its inventories to return their plain holder. Co-Authored-By: Claude Opus 5.5 --- pom.xml | 6 ++ .../tfminecraft/surgery/SurgeryPlugin.java | 2 +- .../managers/SurgeryCompletionHandler.java | 2 +- .../surgery/managers/SurgeryMenuManager.java | 4 +- .../surgery/HolderLookupPatch.java | 62 +++++++++++++++++++ ....platform.launcher.LauncherSessionListener | 1 + 6 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 src/test/java/net/tfminecraft/surgery/HolderLookupPatch.java create mode 100644 src/test/resources/META-INF/services/org.junit.platform.launcher.LauncherSessionListener diff --git a/pom.xml b/pom.xml index 3334243..0622b11 100644 --- a/pom.xml +++ b/pom.xml @@ -67,6 +67,12 @@ 5.14.0 test + + org.junit.platform + junit-platform-launcher + 1.14.0 + test + org.mockbukkit.mockbukkit mockbukkit-v1.21 diff --git a/src/main/java/net/tfminecraft/surgery/SurgeryPlugin.java b/src/main/java/net/tfminecraft/surgery/SurgeryPlugin.java index 76aeef4..c0bbaf5 100644 --- a/src/main/java/net/tfminecraft/surgery/SurgeryPlugin.java +++ b/src/main/java/net/tfminecraft/surgery/SurgeryPlugin.java @@ -72,7 +72,7 @@ public void onDisable() { // a new classloader, so the instanceof check in the listener no longer // matches and players could pull items out of stale menus for (Player player : getServer().getOnlinePlayers()) { - if (player.getOpenInventory().getTopInventory().getHolder() instanceof SurgeryMenuHolder) { + if (player.getOpenInventory().getTopInventory().getHolder(false) instanceof SurgeryMenuHolder) { player.closeInventory(); } } diff --git a/src/main/java/net/tfminecraft/surgery/managers/SurgeryCompletionHandler.java b/src/main/java/net/tfminecraft/surgery/managers/SurgeryCompletionHandler.java index 2e9ef20..1f341be 100644 --- a/src/main/java/net/tfminecraft/surgery/managers/SurgeryCompletionHandler.java +++ b/src/main/java/net/tfminecraft/surgery/managers/SurgeryCompletionHandler.java @@ -191,7 +191,7 @@ private void reportFailure(Player surgeon, String message, String extra) { private Inventory surgeryMenu(Player surgeon, Outcome outcome) { Inventory menu = surgeon.getOpenInventory().getTopInventory(); - return menu.getHolder() instanceof SurgeryMenuHolder holder && holder.matches(surgeon, outcome.operationId()) + return menu.getHolder(false) instanceof SurgeryMenuHolder holder && holder.matches(surgeon, outcome.operationId()) ? menu : null; } diff --git a/src/main/java/net/tfminecraft/surgery/managers/SurgeryMenuManager.java b/src/main/java/net/tfminecraft/surgery/managers/SurgeryMenuManager.java index 9c641f1..75e7c30 100644 --- a/src/main/java/net/tfminecraft/surgery/managers/SurgeryMenuManager.java +++ b/src/main/java/net/tfminecraft/surgery/managers/SurgeryMenuManager.java @@ -89,7 +89,7 @@ public void openSurgeryMenu(Player surgeon, Player patient, HealingInjury ailmen // Checks if an inventory is the surgery menu (by holder, not title) // ============================================== public boolean isSurgeryMenu(org.bukkit.inventory.Inventory inventory) { - return inventory != null && inventory.getHolder() instanceof SurgeryMenuHolder; + return inventory != null && inventory.getHolder(false) instanceof SurgeryMenuHolder; } // ============================================== @@ -103,7 +103,7 @@ public void handleItemClick(Player player, Inventory inventory, ItemStack clicke } private boolean ownsMenu(Player player, Inventory inventory) { - return inventory.getHolder() instanceof SurgeryMenuHolder holder + return inventory.getHolder(false) instanceof SurgeryMenuHolder holder && holder.matches(player, stateManager.getOperationId(player.getUniqueId())); } diff --git a/src/test/java/net/tfminecraft/surgery/HolderLookupPatch.java b/src/test/java/net/tfminecraft/surgery/HolderLookupPatch.java new file mode 100644 index 0000000..670c48f --- /dev/null +++ b/src/test/java/net/tfminecraft/surgery/HolderLookupPatch.java @@ -0,0 +1,62 @@ +package net.tfminecraft.surgery; + +import static net.bytebuddy.matcher.ElementMatchers.is; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import net.bytebuddy.agent.ByteBuddyAgent; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; +import org.bukkit.inventory.Inventory; +import org.bukkit.inventory.InventoryHolder; +import org.junit.platform.launcher.LauncherSession; +import org.junit.platform.launcher.LauncherSessionListener; +import org.mockbukkit.mockbukkit.inventory.InventoryMock; + +/** + * MockBukkit throws on Paper's {@code getHolder(false)}. Its inventories hold no block snapshot, + * so the patch answers with the plain holder, which is what Paper returns for plugin GUIs. + */ +public final class HolderLookupPatch implements LauncherSessionListener { + private static boolean installed; + + @Override + public void launcherSessionOpened(LauncherSession session) { + install(); + } + + /** Patches {@link InventoryMock#getHolder(boolean)} once per test JVM. */ + public static synchronized void install() { + if (installed) { + return; + } + new AgentBuilder.Default() + .disableClassFormatChanges() + .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION) + .with(AgentBuilder.InitializationStrategy.NoOp.INSTANCE) + .with(AgentBuilder.TypeStrategy.Default.REDEFINE) + .with(AgentBuilder.Listener.StreamWriting.toSystemError().withErrorsOnly()) + .type(is(InventoryMock.class)) + .transform((builder, type, loader, module, domain) -> builder.visit(Advice.to(PlainHolder.class) + .on(named("getHolder").and(takesArguments(boolean.class))))) + .installOn(ByteBuddyAgent.install()); + installed = true; + } + + /** Skips the throwing body and returns {@code getHolder()} instead. */ + static final class PlainHolder { + private PlainHolder() { + } + + @Advice.OnMethodEnter(skipOn = Advice.OnNonDefaultValue.class) + static boolean skip() { + return true; + } + + @Advice.OnMethodExit + static void exit(@Advice.This Inventory inventory, + @Advice.Return(readOnly = false) InventoryHolder holder) { + holder = inventory.getHolder(); + } + } +} diff --git a/src/test/resources/META-INF/services/org.junit.platform.launcher.LauncherSessionListener b/src/test/resources/META-INF/services/org.junit.platform.launcher.LauncherSessionListener new file mode 100644 index 0000000..bd70596 --- /dev/null +++ b/src/test/resources/META-INF/services/org.junit.platform.launcher.LauncherSessionListener @@ -0,0 +1 @@ +net.tfminecraft.surgery.HolderLookupPatch From b8c7836318b21d9613159eb875bd519f639ca4de Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 31 Oct 2026 14:12:47 +0100 Subject: [PATCH 2/3] test: fail loudly if the holder patch does not take Byte Buddy reports a failed retransformation only to its listeners and returns normally, so the patch counted as installed even when InventoryMock kept its throwing getHolder(boolean). It now marks itself installed only after InventoryMock was transformed, and stops the test session with a clear error otherwise (CodeRabbit). The Byte Buddy agent is also loaded with -javaagent next to Mockito's, so the patch never needs runtime agent attachment. Co-Authored-By: Claude Opus 5.5 --- pom.xml | 2 +- .../surgery/HolderLookupPatch.java | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 0622b11..2bde20c 100644 --- a/pom.xml +++ b/pom.xml @@ -119,7 +119,7 @@ maven-surefire-plugin 3.5.4 - @{argLine} -javaagent:${org.mockito:mockito-core:jar} -Xshare:off + @{argLine} -javaagent:${org.mockito:mockito-core:jar} -javaagent:${net.bytebuddy:byte-buddy-agent:jar} -Xshare:off true diff --git a/src/test/java/net/tfminecraft/surgery/HolderLookupPatch.java b/src/test/java/net/tfminecraft/surgery/HolderLookupPatch.java index 670c48f..d2fa021 100644 --- a/src/test/java/net/tfminecraft/surgery/HolderLookupPatch.java +++ b/src/test/java/net/tfminecraft/surgery/HolderLookupPatch.java @@ -4,9 +4,14 @@ import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; +import java.util.concurrent.atomic.AtomicBoolean; + import net.bytebuddy.agent.ByteBuddyAgent; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.dynamic.DynamicType; +import net.bytebuddy.utility.JavaModule; import org.bukkit.inventory.Inventory; import org.bukkit.inventory.InventoryHolder; import org.junit.platform.launcher.LauncherSession; @@ -30,16 +35,30 @@ public static synchronized void install() { if (installed) { return; } + AtomicBoolean patched = new AtomicBoolean(); new AgentBuilder.Default() .disableClassFormatChanges() .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION) .with(AgentBuilder.InitializationStrategy.NoOp.INSTANCE) .with(AgentBuilder.TypeStrategy.Default.REDEFINE) .with(AgentBuilder.Listener.StreamWriting.toSystemError().withErrorsOnly()) + .with(new AgentBuilder.Listener.Adapter() { + @Override + public void onTransformation(TypeDescription type, ClassLoader loader, JavaModule module, + boolean loaded, DynamicType dynamicType) { + if (type.getName().equals(InventoryMock.class.getName())) { + patched.set(true); + } + } + }) .type(is(InventoryMock.class)) .transform((builder, type, loader, module, domain) -> builder.visit(Advice.to(PlainHolder.class) .on(named("getHolder").and(takesArguments(boolean.class))))) .installOn(ByteBuddyAgent.install()); + // A failed retransformation only reaches the listeners, so check it took before marking it done. + if (!patched.get()) { + throw new IllegalStateException("Could not patch InventoryMock#getHolder(boolean)"); + } installed = true; } From d1cf35cc6e45283a5b99fbc24041f59b6cd29eec Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 31 Oct 2026 14:45:06 +0100 Subject: [PATCH 3/3] test: also fail when the JVM rejects the holder patch onTransformation fires before the JVM applies the new bytes, so a rejection in retransformClasses reaches only onError. The patch now fails the test session in that case too (Opus review). Co-Authored-By: Claude Opus 5.5 --- .../net/tfminecraft/surgery/HolderLookupPatch.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/test/java/net/tfminecraft/surgery/HolderLookupPatch.java b/src/test/java/net/tfminecraft/surgery/HolderLookupPatch.java index d2fa021..a6294e6 100644 --- a/src/test/java/net/tfminecraft/surgery/HolderLookupPatch.java +++ b/src/test/java/net/tfminecraft/surgery/HolderLookupPatch.java @@ -36,6 +36,7 @@ public static synchronized void install() { return; } AtomicBoolean patched = new AtomicBoolean(); + AtomicBoolean rejected = new AtomicBoolean(); new AgentBuilder.Default() .disableClassFormatChanges() .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION) @@ -50,13 +51,22 @@ public void onTransformation(TypeDescription type, ClassLoader loader, JavaModul patched.set(true); } } + + // The JVM can still reject the new bytes after onTransformation. + @Override + public void onError(String typeName, ClassLoader loader, JavaModule module, boolean loaded, + Throwable throwable) { + if (typeName.equals(InventoryMock.class.getName())) { + rejected.set(true); + } + } }) .type(is(InventoryMock.class)) .transform((builder, type, loader, module, domain) -> builder.visit(Advice.to(PlainHolder.class) .on(named("getHolder").and(takesArguments(boolean.class))))) .installOn(ByteBuddyAgent.install()); // A failed retransformation only reaches the listeners, so check it took before marking it done. - if (!patched.get()) { + if (!patched.get() || rejected.get()) { throw new IllegalStateException("Could not patch InventoryMock#getHolder(boolean)"); } installed = true;