From f136df1717264686c0eeac5a8738d948a484cdfc Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 31 Oct 2026 11:53:54 +0100 Subject: [PATCH] perf: read inventory holders without copying the block The gear, resonance and artifact menus were slow on Main because every getHolder() call on a container inventory copies the whole block state. The gear click handler did this up to four times per click, and the resonance refresh did it for every online player. These now use getHolder(false), which returns the same holder without the copy, and the gear click handler reads it once. The code only checks the holder's type, so behaviour does not change. No call site needed the copying form. MockBukkit does not implement getHolder(false), so the affected tests use a server mock whose inventories answer it like getHolder(). Co-Authored-By: Claude Opus 5.5 --- .../magic/artifact/ArtifactCarePlaces.java | 2 +- .../magic/gear/gui/GearInventoryManager.java | 10 +-- .../magic/listener/MagicSessionListener.java | 4 +- .../manager/ArtifactCreateGuiManager.java | 6 +- .../magic/manager/ResonanceGuiManager.java | 8 +-- .../net/tfminecraft/magic/AttunementTest.java | 4 +- .../magic/ChargeEdgeCoverageTest.java | 2 +- .../magic/GearCoverageSupport.java | 2 +- .../magic/HolderLookupServerMock.java | 61 +++++++++++++++++++ .../net/tfminecraft/magic/ListenerTest.java | 2 +- 10 files changed, 82 insertions(+), 19 deletions(-) create mode 100644 src/test/java/net/tfminecraft/magic/HolderLookupServerMock.java diff --git a/src/main/java/net/tfminecraft/magic/artifact/ArtifactCarePlaces.java b/src/main/java/net/tfminecraft/magic/artifact/ArtifactCarePlaces.java index c8ad919..0ca0501 100644 --- a/src/main/java/net/tfminecraft/magic/artifact/ArtifactCarePlaces.java +++ b/src/main/java/net/tfminecraft/magic/artifact/ArtifactCarePlaces.java @@ -19,7 +19,7 @@ public static boolean accepted(Inventory inventory, ItemStack stack) { if (ArtifactDisplayIndex.isDisplayed(ArtifactIds.read(stack))) { return true; } - InventoryHolder holder = inventory.getHolder(); + InventoryHolder holder = inventory.getHolder(false); return holder instanceof Player || holder instanceof ItemFrame; } } diff --git a/src/main/java/net/tfminecraft/magic/gear/gui/GearInventoryManager.java b/src/main/java/net/tfminecraft/magic/gear/gui/GearInventoryManager.java index 35a4122..aa685cd 100644 --- a/src/main/java/net/tfminecraft/magic/gear/gui/GearInventoryManager.java +++ b/src/main/java/net/tfminecraft/magic/gear/gui/GearInventoryManager.java @@ -13,6 +13,7 @@ import org.bukkit.event.Listener; import org.bukkit.event.inventory.InventoryClickEvent; import org.bukkit.inventory.Inventory; +import org.bukkit.inventory.InventoryHolder; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.meta.ItemMeta; import org.bukkit.persistence.PersistentDataType; @@ -177,10 +178,11 @@ public void onClick(InventoryClickEvent event) { if (!(event.getWhoClicked() instanceof Player player)) { return; } - if (event.getInventory().getHolder() instanceof GearMenuHolder holder && !holder.current(player)) { + InventoryHolder menu = event.getInventory().getHolder(false); + if (menu instanceof GearMenuHolder holder && !holder.current(player)) { event.setCancelled(true); return; } - if (event.getInventory().getHolder() instanceof AssemblyHolder) { + if (menu instanceof AssemblyHolder) { event.setCancelled(true); if (event.getClickedInventory() != event.getInventory()) return; ItemStack current = event.getCurrentItem(); @@ -211,7 +213,7 @@ public void onClick(InventoryClickEvent event) { } return; } - if (event.getInventory().getHolder() instanceof TypeSelectionHolder) { + if (menu instanceof TypeSelectionHolder) { event.setCancelled(true); if (event.getClickedInventory() != event.getInventory()) return; ItemStack clicked = event.getCurrentItem(); @@ -236,7 +238,7 @@ public void onClick(InventoryClickEvent event) { } return; } - if (event.getInventory().getHolder() instanceof PartSelectionHolder holder) { + if (menu instanceof PartSelectionHolder holder) { event.setCancelled(true); if (event.getClickedInventory() != event.getInventory()) return; ItemStack clicked = event.getCurrentItem(); diff --git a/src/main/java/net/tfminecraft/magic/listener/MagicSessionListener.java b/src/main/java/net/tfminecraft/magic/listener/MagicSessionListener.java index c7bcf7c..4ba18ff 100644 --- a/src/main/java/net/tfminecraft/magic/listener/MagicSessionListener.java +++ b/src/main/java/net/tfminecraft/magic/listener/MagicSessionListener.java @@ -62,7 +62,7 @@ private void clearActivities(Player player, net.tfminecraft.tlibs.character.Char clearActivities(player, outgoing, true); } private void clearActivities(Player player, net.tfminecraft.tlibs.character.CharacterSession outgoing, boolean clearGear) { - Object holder = player.getOpenInventory().getTopInventory().getHolder(); + Object holder = player.getOpenInventory().getTopInventory().getHolder(false); MeditationService.cancel(player); SacrificeRiteService.cancelForPlayer(player.getUniqueId()); var captured = outgoing == null ? null : net.tfminecraft.magic.gear.ownership.GearCharacterBridge.token(outgoing); @@ -88,7 +88,7 @@ private void clearActivities(Player player, net.tfminecraft.tlibs.character.Char } private void clearStaleGear(Player player) { GearOrbService.finishStaleForPlayer(player.getUniqueId()); GearChargeService.clearStale(player); - if (player.getOpenInventory().getTopInventory().getHolder() instanceof net.tfminecraft.magic.gear.gui.GearMenuHolder gear && !gear.current(player)) player.closeInventory(); + if (player.getOpenInventory().getTopInventory().getHolder(false) instanceof net.tfminecraft.magic.gear.gui.GearMenuHolder gear && !gear.current(player)) player.closeInventory(); if (OpenStationManager.get(player) == null) { OpenStationManager.clear(player); SelectedPartsManager.clear(player); TypeSelectionManager.clear(player); } diff --git a/src/main/java/net/tfminecraft/magic/manager/ArtifactCreateGuiManager.java b/src/main/java/net/tfminecraft/magic/manager/ArtifactCreateGuiManager.java index 7c8eae3..38aa8a3 100644 --- a/src/main/java/net/tfminecraft/magic/manager/ArtifactCreateGuiManager.java +++ b/src/main/java/net/tfminecraft/magic/manager/ArtifactCreateGuiManager.java @@ -58,7 +58,7 @@ public void onInventoryClick(InventoryClickEvent event) { if (!(event.getWhoClicked() instanceof Player player)) { return; } - if (!(event.getView().getTopInventory().getHolder() instanceof ArtifactCreateGuiHolder holder)) { + if (!(event.getView().getTopInventory().getHolder(false) instanceof ArtifactCreateGuiHolder holder)) { return; } event.setCancelled(true); @@ -152,7 +152,7 @@ private static void giveOrDrop(Player player, ItemStack stack) { @EventHandler public void onInventoryDrag(InventoryDragEvent event) { - if (!(event.getView().getTopInventory().getHolder() instanceof ArtifactCreateGuiHolder)) { + if (!(event.getView().getTopInventory().getHolder(false) instanceof ArtifactCreateGuiHolder)) { return; } event.setCancelled(true); @@ -163,7 +163,7 @@ public void onInventoryClose(InventoryCloseEvent event) { if (!(event.getPlayer() instanceof Player player)) { return; } - if (!(event.getView().getTopInventory().getHolder() instanceof ArtifactCreateGuiHolder holder)) { + if (!(event.getView().getTopInventory().getHolder(false) instanceof ArtifactCreateGuiHolder holder)) { return; } sessions.remove(player.getUniqueId(), holder.getSession()); diff --git a/src/main/java/net/tfminecraft/magic/manager/ResonanceGuiManager.java b/src/main/java/net/tfminecraft/magic/manager/ResonanceGuiManager.java index f33695a..ccc6279 100644 --- a/src/main/java/net/tfminecraft/magic/manager/ResonanceGuiManager.java +++ b/src/main/java/net/tfminecraft/magic/manager/ResonanceGuiManager.java @@ -55,7 +55,7 @@ public void open(Player player) { public void refreshOpenInventories() { for (Player player : Bukkit.getOnlinePlayers()) { Inventory top = player.getOpenInventory().getTopInventory(); - if (!(top.getHolder() instanceof ResonanceGuiHolder holder)) { + if (!(top.getHolder(false) instanceof ResonanceGuiHolder holder)) { continue; } ResonanceSession session = currentSession(player, holder); @@ -72,7 +72,7 @@ public void onInventoryClick(InventoryClickEvent event) { if (!(event.getWhoClicked() instanceof Player player)) { return; } - if (!(event.getView().getTopInventory().getHolder() instanceof ResonanceGuiHolder holder)) { + if (!(event.getView().getTopInventory().getHolder(false) instanceof ResonanceGuiHolder holder)) { return; } event.setCancelled(true); @@ -128,14 +128,14 @@ private static String castModeChatLabel(String modeId) { @EventHandler public void onInventoryDrag(InventoryDragEvent event) { - if (!(event.getView().getTopInventory().getHolder() instanceof ResonanceGuiHolder)) { + if (!(event.getView().getTopInventory().getHolder(false) instanceof ResonanceGuiHolder)) { return; } event.setCancelled(true); } public void close(Player player) { - if (player.getOpenInventory().getTopInventory().getHolder() instanceof ResonanceGuiHolder) { + if (player.getOpenInventory().getTopInventory().getHolder(false) instanceof ResonanceGuiHolder) { player.closeInventory(); } } diff --git a/src/test/java/net/tfminecraft/magic/AttunementTest.java b/src/test/java/net/tfminecraft/magic/AttunementTest.java index 3f6afb2..c5f28bb 100644 --- a/src/test/java/net/tfminecraft/magic/AttunementTest.java +++ b/src/test/java/net/tfminecraft/magic/AttunementTest.java @@ -24,7 +24,7 @@ class AttunementTest { @BeforeEach void setup() { - server = MockBukkit.mock(); + server = MockBukkit.mock(new HolderLookupServerMock()); server.addSimpleWorld("display"); Magic.plugin = mock(Magic.class); when(Magic.plugin.namespace()).thenReturn("magic"); @@ -207,7 +207,7 @@ void careAcceptsPlayerInventoryAndIndexedDisplaysButNotOrdinaryStorage() { var chest = server.createInventory(null, 9); assertFalse(ArtifactCarePlaces.accepted(null, item)); assertFalse(ArtifactCarePlaces.accepted(chest, item)); - assertTrue(ArtifactCarePlaces.accepted(player.getInventory(), item)); + assertTrue(ArtifactCarePlaces.accepted(HolderLookupServerMock.withHolderLookup(player.getInventory()), item)); ArtifactDisplayIndex.add(item); assertTrue(ArtifactCarePlaces.accepted(chest, item)); } diff --git a/src/test/java/net/tfminecraft/magic/ChargeEdgeCoverageTest.java b/src/test/java/net/tfminecraft/magic/ChargeEdgeCoverageTest.java index 3aa1041..d76f88f 100644 --- a/src/test/java/net/tfminecraft/magic/ChargeEdgeCoverageTest.java +++ b/src/test/java/net/tfminecraft/magic/ChargeEdgeCoverageTest.java @@ -69,7 +69,7 @@ void identifiersRejectEmptyMetadataMalformedIdsAndZeroTiers() { assertFalse(Artifact.create().isCharge()); assertFalse(Charge.fromItem(chargeItem()).isArtifact()); var inventory = mock(Inventory.class); - when(inventory.getHolder()) + when(inventory.getHolder(false)) .thenReturn(mock(InventoryHolder.class, withSettings().extraInterfaces(ItemFrame.class))); assertTrue(ArtifactCarePlaces.accepted(inventory, s)); ArtifactDisplayIndex.add(s); diff --git a/src/test/java/net/tfminecraft/magic/GearCoverageSupport.java b/src/test/java/net/tfminecraft/magic/GearCoverageSupport.java index 895c08c..1cc6c48 100644 --- a/src/test/java/net/tfminecraft/magic/GearCoverageSupport.java +++ b/src/test/java/net/tfminecraft/magic/GearCoverageSupport.java @@ -26,7 +26,7 @@ abstract class GearCoverageSupport { @BeforeEach void setupGear() throws Exception { - server = MockBukkit.mock(); + server = MockBukkit.mock(new HolderLookupServerMock()); Magic.plugin = mock(Magic.class, RETURNS_DEEP_STUBS); when(Magic.plugin.getName()).thenReturn("Magic"); when(Magic.plugin.namespace()).thenReturn("magic"); diff --git a/src/test/java/net/tfminecraft/magic/HolderLookupServerMock.java b/src/test/java/net/tfminecraft/magic/HolderLookupServerMock.java new file mode 100644 index 0000000..e23dda7 --- /dev/null +++ b/src/test/java/net/tfminecraft/magic/HolderLookupServerMock.java @@ -0,0 +1,61 @@ +package net.tfminecraft.magic; + +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mockingDetails; +import static org.mockito.Mockito.spy; + +import net.kyori.adventure.text.Component; +import org.bukkit.event.inventory.InventoryType; +import org.bukkit.inventory.Inventory; +import org.bukkit.inventory.InventoryHolder; +import org.mockbukkit.mockbukkit.ServerMock; +import org.mockbukkit.mockbukkit.inventory.InventoryMock; + +/** MockBukkit leaves Paper's non-copying getHolder(boolean) unimplemented; the plugin uses it on hot paths. */ +public class HolderLookupServerMock extends ServerMock { + + /** A spy of {@code inventory} whose getHolder(boolean) answers like getHolder(). */ + @SuppressWarnings("unchecked") + public static T withHolderLookup(T inventory) { + if (mockingDetails(inventory).isMock()) return inventory; + T spied = spy(inventory); + doAnswer(call -> ((Inventory) call.getMock()).getHolder()).when(spied).getHolder(anyBoolean()); + return spied; + } + + @Override + public InventoryMock createInventory(InventoryHolder owner, InventoryType type, String title, int size) { + return withHolderLookup(super.createInventory(owner, type, title, size)); + } + + @Override + public InventoryMock createInventory(InventoryHolder owner, InventoryType type) { + return withHolderLookup(super.createInventory(owner, type)); + } + + @Override + public InventoryMock createInventory(InventoryHolder owner, InventoryType type, Component title) { + return withHolderLookup(super.createInventory(owner, type, title)); + } + + @Override + public InventoryMock createInventory(InventoryHolder owner, InventoryType type, String title) { + return withHolderLookup(super.createInventory(owner, type, title)); + } + + @Override + public InventoryMock createInventory(InventoryHolder owner, int size) { + return withHolderLookup(super.createInventory(owner, size)); + } + + @Override + public InventoryMock createInventory(InventoryHolder owner, int size, Component title) { + return withHolderLookup(super.createInventory(owner, size, title)); + } + + @Override + public InventoryMock createInventory(InventoryHolder owner, int size, String title) { + return withHolderLookup(super.createInventory(owner, size, title)); + } +} diff --git a/src/test/java/net/tfminecraft/magic/ListenerTest.java b/src/test/java/net/tfminecraft/magic/ListenerTest.java index f6639b5..89813c9 100644 --- a/src/test/java/net/tfminecraft/magic/ListenerTest.java +++ b/src/test/java/net/tfminecraft/magic/ListenerTest.java @@ -28,7 +28,7 @@ class ListenerTest { @BeforeEach void setup() { - server = MockBukkit.mock(); + server = MockBukkit.mock(new HolderLookupServerMock()); Magic.plugin = mock(Magic.class, RETURNS_DEEP_STUBS); when(Magic.plugin.getServer()).thenReturn(server); when(Magic.plugin.isEnabled()).thenReturn(true);