From dac6b4a2b06993764cbad8d85990a0a4b2f2da7a Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 31 Oct 2026 12:16:18 +0100 Subject: [PATCH 1/3] perf: read inventory holders without copying the block The pet and staff menu listeners run on every inventory click, drag and close. Each plain getHolder() call copied the whole block state when the inventory belonged to a chest or barrel. On Main StaffMenus.onClick and PetListener.onClick cost 3.3 s over 20 hours. The checks now use getHolder(false), which hands back the same holder without the copy, and the reload check reads the holder once instead of twice. Pet and staff menus have their own holders, 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/companionpets/PetsPlugin.java | 3 +- .../companionpets/gui/PetMenus.java | 6 +- .../companionpets/listen/PetListener.java | 6 +- .../ownership/CharacterLifecycle.java | 2 +- .../companionpets/staff/StaffMenus.java | 4 +- .../testutil/HolderLookupPatch.java | 62 +++++++++++++++++++ ....platform.launcher.LauncherSessionListener | 1 + 8 files changed, 80 insertions(+), 10 deletions(-) create mode 100644 src/test/java/net/tfminecraft/companionpets/testutil/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 d209470..c580a63 100644 --- a/pom.xml +++ b/pom.xml @@ -38,6 +38,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/companionpets/PetsPlugin.java b/src/main/java/net/tfminecraft/companionpets/PetsPlugin.java index f7b3dd7..159b2c3 100644 --- a/src/main/java/net/tfminecraft/companionpets/PetsPlugin.java +++ b/src/main/java/net/tfminecraft/companionpets/PetsPlugin.java @@ -252,7 +252,8 @@ private void reloadSettings(CommandSender sender) { return; } for (var player : Bukkit.getOnlinePlayers()) { - if (player.getOpenInventory().getTopInventory().getHolder() instanceof MenuHolder || player.getOpenInventory().getTopInventory().getHolder() instanceof net.tfminecraft.companionpets.staff.StaffMenuHolder) player.closeInventory(); + var holder = player.getOpenInventory().getTopInventory().getHolder(false); + if (holder instanceof MenuHolder || holder instanceof net.tfminecraft.companionpets.staff.StaffMenuHolder) player.closeInventory(); } actions.clearInteractions(); actions.holograms().clear(); diff --git a/src/main/java/net/tfminecraft/companionpets/gui/PetMenus.java b/src/main/java/net/tfminecraft/companionpets/gui/PetMenus.java index e3621a6..5306ac3 100644 --- a/src/main/java/net/tfminecraft/companionpets/gui/PetMenus.java +++ b/src/main/java/net/tfminecraft/companionpets/gui/PetMenus.java @@ -77,7 +77,7 @@ public void openCare(org.bukkit.entity.Player player, Pet pet, boolean petHouseB } public void refreshCare(org.bukkit.entity.Player player, Pet pet) { - if (!(player.getOpenInventory().getTopInventory().getHolder() instanceof MenuHolder holder) + if (!(player.getOpenInventory().getTopInventory().getHolder(false) instanceof MenuHolder holder) || holder.kind() != MenuHolder.Kind.CARE || !pet.id().equals(holder.petId())) { return; @@ -87,7 +87,7 @@ public void refreshCare(org.bukkit.entity.Player player, Pet pet) { private void fillCare(org.bukkit.entity.Player player, Inventory inventory, Pet pet) { fillProfile(inventory, pet, runtime.owns(pet, player), - inventory.getHolder() instanceof MenuHolder holder && holder.petHouseBack()); + inventory.getHolder(false) instanceof MenuHolder holder && holder.petHouseBack()); } public void fillInformation(Inventory inventory, Pet pet) { @@ -334,7 +334,7 @@ public void openTricks(org.bukkit.entity.Player player, Pet pet, String word, in private void inheritPetHouseNavigation(org.bukkit.entity.Player player, MenuHolder holder) { Inventory previousInventory = player.getOpenInventory().getTopInventory(); - if (previousInventory != null && previousInventory.getHolder() instanceof MenuHolder previous + if (previousInventory != null && previousInventory.getHolder(false) instanceof MenuHolder previous && holder.petId().equals(previous.petId())) { holder.petHouseBack(previous.petHouseBack()); holder.petHousePage(previous.petHousePage()); diff --git a/src/main/java/net/tfminecraft/companionpets/listen/PetListener.java b/src/main/java/net/tfminecraft/companionpets/listen/PetListener.java index 50d43f6..592fe4c 100644 --- a/src/main/java/net/tfminecraft/companionpets/listen/PetListener.java +++ b/src/main/java/net/tfminecraft/companionpets/listen/PetListener.java @@ -138,7 +138,7 @@ public void onChat(AsyncPlayerChatEvent event) { @EventHandler public void onClick(InventoryClickEvent event) { - InventoryHolder holder = event.getView().getTopInventory().getHolder(); + InventoryHolder holder = event.getView().getTopInventory().getHolder(false); if (!(holder instanceof MenuHolder menu) || !(event.getWhoClicked() instanceof Player player)) { return; } @@ -152,14 +152,14 @@ public void onClick(InventoryClickEvent event) { @EventHandler public void onDrag(InventoryDragEvent event) { - if (event.getView().getTopInventory().getHolder() instanceof MenuHolder) { + if (event.getView().getTopInventory().getHolder(false) instanceof MenuHolder) { event.setCancelled(true); } } @EventHandler public void onClose(InventoryCloseEvent event) { - if (event.getInventory().getHolder() instanceof MenuHolder menu + if (event.getInventory().getHolder(false) instanceof MenuHolder menu && menu.kind() == MenuHolder.Kind.TRICK && !menu.navigating() && event.getPlayer() instanceof Player player) { diff --git a/src/main/java/net/tfminecraft/companionpets/ownership/CharacterLifecycle.java b/src/main/java/net/tfminecraft/companionpets/ownership/CharacterLifecycle.java index 007b619..1358314 100644 --- a/src/main/java/net/tfminecraft/companionpets/ownership/CharacterLifecycle.java +++ b/src/main/java/net/tfminecraft/companionpets/ownership/CharacterLifecycle.java @@ -20,7 +20,7 @@ public void end(CharacterSessionEndEvent event) { Player player = Bukkit.getPlayer(event.getSession().ref().ownerId()); if (player != null) { actions.ownerSessionChanged(player); - if (player.getOpenInventory().getTopInventory().getHolder() instanceof MenuHolder) player.closeInventory(); + if (player.getOpenInventory().getTopInventory().getHolder(false) instanceof MenuHolder) player.closeInventory(); } runtime.sessions().clearPlayer(event.getSession().ref().ownerId()); } diff --git a/src/main/java/net/tfminecraft/companionpets/staff/StaffMenus.java b/src/main/java/net/tfminecraft/companionpets/staff/StaffMenus.java index c554d2b..ae3ac34 100644 --- a/src/main/java/net/tfminecraft/companionpets/staff/StaffMenus.java +++ b/src/main/java/net/tfminecraft/companionpets/staff/StaffMenus.java @@ -101,7 +101,7 @@ private static ItemStack named(ItemStack source, String title, String... lore) { } @EventHandler public void onClick(InventoryClickEvent event) { - if (!(event.getView().getTopInventory().getHolder() instanceof StaffMenuHolder holder)) return; + if (!(event.getView().getTopInventory().getHolder(false) instanceof StaffMenuHolder holder)) return; event.setCancelled(true); if (!(event.getWhoClicked() instanceof Player player) || !holder.viewer().equals(player.getUniqueId())) return; if (event.getClickedInventory() != event.getView().getTopInventory() || !(event.getClick().isLeftClick() || event.getClick().isRightClick()) @@ -109,6 +109,6 @@ private static ItemStack named(ItemStack source, String title, String... lore) { if (allowed(player, "list")) holder.click(event.getSlot(), event.getClick().isRightClick()); } @EventHandler public void onDrag(InventoryDragEvent event) { - if (event.getView().getTopInventory().getHolder() instanceof StaffMenuHolder) event.setCancelled(true); + if (event.getView().getTopInventory().getHolder(false) instanceof StaffMenuHolder) event.setCancelled(true); } } diff --git a/src/test/java/net/tfminecraft/companionpets/testutil/HolderLookupPatch.java b/src/test/java/net/tfminecraft/companionpets/testutil/HolderLookupPatch.java new file mode 100644 index 0000000..9b53129 --- /dev/null +++ b/src/test/java/net/tfminecraft/companionpets/testutil/HolderLookupPatch.java @@ -0,0 +1,62 @@ +package net.tfminecraft.companionpets.testutil; + +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..390a242 --- /dev/null +++ b/src/test/resources/META-INF/services/org.junit.platform.launcher.LauncherSessionListener @@ -0,0 +1 @@ +net.tfminecraft.companionpets.testutil.HolderLookupPatch From 23ec4ad318f1a2b4c72089a121f202743d2bcddd Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 31 Oct 2026 14:13:18 +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 --- .../testutil/HolderLookupPatch.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/test/java/net/tfminecraft/companionpets/testutil/HolderLookupPatch.java b/src/test/java/net/tfminecraft/companionpets/testutil/HolderLookupPatch.java index 9b53129..8862196 100644 --- a/src/test/java/net/tfminecraft/companionpets/testutil/HolderLookupPatch.java +++ b/src/test/java/net/tfminecraft/companionpets/testutil/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 7f224caf73c4ce279adef3857dd7c95d624a9c55 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 31 Oct 2026 14:45:37 +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 --- .../companionpets/testutil/HolderLookupPatch.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/test/java/net/tfminecraft/companionpets/testutil/HolderLookupPatch.java b/src/test/java/net/tfminecraft/companionpets/testutil/HolderLookupPatch.java index 8862196..52367a9 100644 --- a/src/test/java/net/tfminecraft/companionpets/testutil/HolderLookupPatch.java +++ b/src/test/java/net/tfminecraft/companionpets/testutil/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;