From a651a4fd680963b7eeeabf3d5809f87ab0641344 Mon Sep 17 00:00:00 2001 From: XxFran10xX Date: Sat, 31 Oct 2026 12:18:57 +0100 Subject: [PATCH 1/3] perf: read inventory holders without copying the block The activity and referral menu listeners run on every inventory click and drag. Each plain getHolder() call copied the whole block state when the inventory belonged to a chest or barrel. On Main ReferralGui.onClick cost 1.5 s over 20 hours. The checks now use getHolder(false), which hands back the same holder without the copy. Both menus have their 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 --- .../activitytf/gui/ActivityGui.java | 4 +- .../activitytf/gui/ReferralGui.java | 4 +- .../activitytf/managers/ActivityManager.java | 2 +- .../testsupport/HolderLookupPatch.java | 62 +++++++++++++++++++ ....platform.launcher.LauncherSessionListener | 1 + 5 files changed, 68 insertions(+), 5 deletions(-) create mode 100644 src/test/java/net/tfminecraft/activitytf/testsupport/HolderLookupPatch.java diff --git a/src/main/java/net/tfminecraft/activitytf/gui/ActivityGui.java b/src/main/java/net/tfminecraft/activitytf/gui/ActivityGui.java index 98b98f1..bafa0dd 100644 --- a/src/main/java/net/tfminecraft/activitytf/gui/ActivityGui.java +++ b/src/main/java/net/tfminecraft/activitytf/gui/ActivityGui.java @@ -328,7 +328,7 @@ private ItemStack item(ItemStack stack, String name, List lore) { @EventHandler public void onClick(InventoryClickEvent event) { - if (!(event.getView().getTopInventory().getHolder() instanceof Marker)) { + if (!(event.getView().getTopInventory().getHolder(false) instanceof Marker)) { return; } event.setCancelled(true); @@ -475,7 +475,7 @@ private void claim(Player player, InventoryClickEvent event) { @EventHandler public void onDrag(InventoryDragEvent event) { - if (!(event.getView().getTopInventory().getHolder() instanceof Marker)) { + if (!(event.getView().getTopInventory().getHolder(false) instanceof Marker)) { return; } for (int rawSlot : event.getRawSlots()) { diff --git a/src/main/java/net/tfminecraft/activitytf/gui/ReferralGui.java b/src/main/java/net/tfminecraft/activitytf/gui/ReferralGui.java index 76a1b5a..c84cbf1 100644 --- a/src/main/java/net/tfminecraft/activitytf/gui/ReferralGui.java +++ b/src/main/java/net/tfminecraft/activitytf/gui/ReferralGui.java @@ -602,7 +602,7 @@ public void confirmTyped(Player player, String name) { @EventHandler public void onClick(InventoryClickEvent event) { - if (!(event.getView().getTopInventory().getHolder() instanceof Marker marker)) { + if (!(event.getView().getTopInventory().getHolder(false) instanceof Marker marker)) { return; } event.setCancelled(true); @@ -764,7 +764,7 @@ static String linkKey(Link link) { @EventHandler public void onDrag(InventoryDragEvent event) { - if (!(event.getView().getTopInventory().getHolder() instanceof Marker)) { + if (!(event.getView().getTopInventory().getHolder(false) instanceof Marker)) { return; } int size = event.getView().getTopInventory().getSize(); diff --git a/src/main/java/net/tfminecraft/activitytf/managers/ActivityManager.java b/src/main/java/net/tfminecraft/activitytf/managers/ActivityManager.java index fb8e14f..89f90ca 100644 --- a/src/main/java/net/tfminecraft/activitytf/managers/ActivityManager.java +++ b/src/main/java/net/tfminecraft/activitytf/managers/ActivityManager.java @@ -788,7 +788,7 @@ public boolean runClickCommands(Player player, int slot) { void runClickCommandsNow(Player player, List commands, String id, Predicate console) { InventoryView open = player.getOpenInventory(); - if (open != null && open.getTopInventory().getHolder() instanceof ActivityGui.Marker) { + if (open != null && open.getTopInventory().getHolder(false) instanceof ActivityGui.Marker) { player.closeInventory(); } diff --git a/src/test/java/net/tfminecraft/activitytf/testsupport/HolderLookupPatch.java b/src/test/java/net/tfminecraft/activitytf/testsupport/HolderLookupPatch.java new file mode 100644 index 0000000..152fe6e --- /dev/null +++ b/src/test/java/net/tfminecraft/activitytf/testsupport/HolderLookupPatch.java @@ -0,0 +1,62 @@ +package net.tfminecraft.activitytf.testsupport; + +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 index 7a2b3e9..a9f92f8 100644 --- a/src/test/resources/META-INF/services/org.junit.platform.launcher.LauncherSessionListener +++ b/src/test/resources/META-INF/services/org.junit.platform.launcher.LauncherSessionListener @@ -1 +1,2 @@ net.tfminecraft.activitytf.testsupport.RegistryBootstrap +net.tfminecraft.activitytf.testsupport.HolderLookupPatch From 331e62ab96f7a6aad76381943ed63d34bd47db70 Mon Sep 17 00:00:00 2001 From: XxFran10xX Date: Sat, 31 Oct 2026 14:13:29 +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). Co-Authored-By: Claude Opus 5.5 --- .../testsupport/HolderLookupPatch.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/test/java/net/tfminecraft/activitytf/testsupport/HolderLookupPatch.java b/src/test/java/net/tfminecraft/activitytf/testsupport/HolderLookupPatch.java index 152fe6e..8c2b00e 100644 --- a/src/test/java/net/tfminecraft/activitytf/testsupport/HolderLookupPatch.java +++ b/src/test/java/net/tfminecraft/activitytf/testsupport/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 2911ab3c4a1036af409d7e3e55671f06d9e323ea Mon Sep 17 00:00:00 2001 From: XxFran10xX Date: Sat, 31 Oct 2026 14:45:48 +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 --- .../activitytf/testsupport/HolderLookupPatch.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/test/java/net/tfminecraft/activitytf/testsupport/HolderLookupPatch.java b/src/test/java/net/tfminecraft/activitytf/testsupport/HolderLookupPatch.java index 8c2b00e..4b7ac68 100644 --- a/src/test/java/net/tfminecraft/activitytf/testsupport/HolderLookupPatch.java +++ b/src/test/java/net/tfminecraft/activitytf/testsupport/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;