From 8f5e160cc45c8b055a20c9626e4fa6e78d31ec7f Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 31 Oct 2026 11:32:08 +0100 Subject: [PATCH 1/2] perf: read inventory holders without copying the block Every click, drag and close in any inventory ran the letter GUI checks, and each plain getHolder() call copied the whole block state when the inventory belonged to a chest or barrel. On Main the click listener alone cost 3.6 s over 20 hours. The GUI checks now use getHolder(false), which hands back the same holder without the copy, and each handler reads the holder once. The letter and picker GUIs 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. Tests stub getHolder(false) next to the existing getHolder() stubs. Co-Authored-By: Claude Opus 5.5 --- .../birdmessenger/BirdMessenger.java | 4 ++-- .../birdmessenger/listener/GuiListener.java | 18 +++++++++++------- .../session/SendSessionManager.java | 6 ++++-- .../birdmessenger/PickerOpenTest.java | 2 ++ .../gui/GuiPresentationCoverageTest.java | 1 + .../listener/GuiListenerCoverageTest.java | 6 ++++++ .../session/SendSessionManagerTest.java | 2 ++ 7 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java b/src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java index c4f4c31..deca362 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java +++ b/src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java @@ -140,7 +140,7 @@ public void openPicker(Player player, SendSession session) { if (!sessions.isCurrent(player, session) || session.getLetter() == null || session.isConfirmed()) { return; } - if (player.getOpenInventory().getTopInventory().getHolder() instanceof CharacterPickerGui) { + if (player.getOpenInventory().getTopInventory().getHolder(false) instanceof CharacterPickerGui) { return; } if (!Bukkit.getPluginManager().isPluginEnabled("RPCharacters")) { @@ -170,7 +170,7 @@ public void openPicker(Player player, SendSession session) { private void refreshPickerHeads(Player player, CharacterPickerGui picker) { if (!sessions.isCurrent(player, picker.session()) - || player.getOpenInventory().getTopInventory().getHolder() != picker) { + || player.getOpenInventory().getTopInventory().getHolder(false) != picker) { return; } SendSession session = picker.session(); diff --git a/src/main/java/net/tfminecraft/birdmessenger/listener/GuiListener.java b/src/main/java/net/tfminecraft/birdmessenger/listener/GuiListener.java index a65e32e..dad2ccb 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/listener/GuiListener.java +++ b/src/main/java/net/tfminecraft/birdmessenger/listener/GuiListener.java @@ -12,6 +12,7 @@ import org.bukkit.event.inventory.InventoryCloseEvent; import org.bukkit.event.inventory.InventoryDragEvent; import org.bukkit.inventory.Inventory; +import org.bukkit.inventory.InventoryHolder; import org.bukkit.inventory.ItemStack; import net.tfminecraft.birdmessenger.BirdMessenger; @@ -34,7 +35,8 @@ public void onClick(InventoryClickEvent event) { if (!(event.getWhoClicked() instanceof Player player)) { return; } - if (event.getView().getTopInventory().getHolder() instanceof LetterGui gui) { + InventoryHolder holder = event.getView().getTopInventory().getHolder(false); + if (holder instanceof LetterGui gui) { if (!plugin.sessions().isCurrent(player, gui.session())) { event.setCancelled(true); return; @@ -42,7 +44,7 @@ public void onClick(InventoryClickEvent event) { handleLetterClick(event, player); return; } - if (event.getView().getTopInventory().getHolder() instanceof CharacterPickerGui picker) { + if (holder instanceof CharacterPickerGui picker) { handlePickerClick(event, player, picker); } } @@ -53,7 +55,8 @@ public void onDrag(InventoryDragEvent event) { return; } Inventory top = event.getView().getTopInventory(); - if (top.getHolder() instanceof LetterGui gui) { + InventoryHolder holder = top.getHolder(false); + if (holder instanceof LetterGui gui) { if (!plugin.sessions().isCurrent(player, gui.session())) { event.setCancelled(true); return; @@ -80,7 +83,7 @@ public void onDrag(InventoryDragEvent event) { } return; } - if (top.getHolder() instanceof CharacterPickerGui) { + if (holder instanceof CharacterPickerGui) { event.setCancelled(true); } } @@ -90,7 +93,8 @@ public void onClose(InventoryCloseEvent event) { if (!(event.getPlayer() instanceof Player player)) { return; } - if (event.getInventory().getHolder() instanceof LetterGui gui) { + InventoryHolder holder = event.getInventory().getHolder(false); + if (holder instanceof LetterGui gui) { SendSession session = gui.session(); if (!plugin.sessions().isCurrent(player, session)) { plugin.sessions().returnLetter(player, session, false); @@ -106,7 +110,7 @@ public void onClose(InventoryCloseEvent event) { Bukkit.getScheduler().runTask(plugin, () -> plugin.openPicker(player, session)); return; } - if (event.getInventory().getHolder() instanceof CharacterPickerGui picker) { + if (holder instanceof CharacterPickerGui picker) { plugin.sessions().returnLetter(player, picker.session(), true); } } @@ -142,7 +146,7 @@ private void handleLetterClick(InventoryClickEvent event, Player player) { } Bukkit.getScheduler().runTaskLater(plugin, () -> { // The player may already have closed this GUI and moved on to another one. - if (plugin.sessions().isCurrent(player, ((LetterGui) top.getHolder()).session()) + if (plugin.sessions().isCurrent(player, ((LetterGui) top.getHolder(false)).session()) && player.getOpenInventory().getTopInventory() == top) { player.closeInventory(); } diff --git a/src/main/java/net/tfminecraft/birdmessenger/session/SendSessionManager.java b/src/main/java/net/tfminecraft/birdmessenger/session/SendSessionManager.java index 33fd700..3301e41 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/session/SendSessionManager.java +++ b/src/main/java/net/tfminecraft/birdmessenger/session/SendSessionManager.java @@ -12,6 +12,7 @@ import org.bukkit.entity.Player; import org.bukkit.event.entity.PlayerDeathEvent; import org.bukkit.inventory.Inventory; +import org.bukkit.inventory.InventoryHolder; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.meta.ItemMeta; @@ -121,8 +122,9 @@ private void finish(Player player, SendSession session, boolean cancelledMessage } } Inventory top = player.getOpenInventory().getTopInventory(); - if ((top.getHolder() instanceof LetterGui gui && gui.session() == session) - || (top.getHolder() instanceof CharacterPickerGui picker && picker.session() == session)) { + InventoryHolder holder = top.getHolder(false); + if ((holder instanceof LetterGui gui && gui.session() == session) + || (holder instanceof CharacterPickerGui picker && picker.session() == session)) { player.closeInventory(); } if (!returned.isEmpty() && canReturn(player, session, ending)) { diff --git a/src/test/java/net/tfminecraft/birdmessenger/PickerOpenTest.java b/src/test/java/net/tfminecraft/birdmessenger/PickerOpenTest.java index 9c5aef3..ac67c50 100644 --- a/src/test/java/net/tfminecraft/birdmessenger/PickerOpenTest.java +++ b/src/test/java/net/tfminecraft/birdmessenger/PickerOpenTest.java @@ -71,6 +71,7 @@ class PickerOpenTest { when(player.getOpenInventory()).thenReturn(view); when(view.getTopInventory()).thenReturn(top); when(top.getHolder()).thenAnswer(call -> openHolder); + when(top.getHolder(false)).thenAnswer(call -> openHolder); when(player.openInventory(any(Inventory.class))).thenAnswer(call -> { opens++; openHolder = ((Inventory) call.getArgument(0)).getHolder(); @@ -381,6 +382,7 @@ private final class Mocks implements AutoCloseable { Inventory inventory = mock(Inventory.class); InventoryHolder holder = call.getArgument(0); when(inventory.getHolder()).thenReturn(holder); + when(inventory.getHolder(false)).thenReturn(holder); return inventory; }); rpc.when(RPCharacters::listMailTargets).thenAnswer(call -> new ArrayList<>(directory)); diff --git a/src/test/java/net/tfminecraft/birdmessenger/gui/GuiPresentationCoverageTest.java b/src/test/java/net/tfminecraft/birdmessenger/gui/GuiPresentationCoverageTest.java index 9700ae8..a114023 100644 --- a/src/test/java/net/tfminecraft/birdmessenger/gui/GuiPresentationCoverageTest.java +++ b/src/test/java/net/tfminecraft/birdmessenger/gui/GuiPresentationCoverageTest.java @@ -203,6 +203,7 @@ private static final class Harness implements AutoCloseable { Map slots = new HashMap<>(); contents.put(inventory, slots); when(inventory.getHolder()).thenReturn(holder); + when(inventory.getHolder(false)).thenReturn(holder); when(inventory.getSize()).thenReturn(size); doAnswer( set -> { diff --git a/src/test/java/net/tfminecraft/birdmessenger/listener/GuiListenerCoverageTest.java b/src/test/java/net/tfminecraft/birdmessenger/listener/GuiListenerCoverageTest.java index 6dafcdb..2ac1c0f 100644 --- a/src/test/java/net/tfminecraft/birdmessenger/listener/GuiListenerCoverageTest.java +++ b/src/test/java/net/tfminecraft/birdmessenger/listener/GuiListenerCoverageTest.java @@ -111,6 +111,7 @@ void setup() { .setItem(anyInt(), any()); top = mock(Inventory.class); when(top.getHolder()).thenReturn(mock(LetterGui.class)); + when(top.getHolder(false)).thenReturn(mock(LetterGui.class)); when(top.getSize()).thenReturn(9); when(top.getMaxStackSize()).thenReturn(64); when(top.getItem(anyInt())).thenAnswer(call -> topItems[call.getArgument(0)]); @@ -152,6 +153,7 @@ void setup() { LetterGui letterGui = mock(LetterGui.class); when(letterGui.session()).thenReturn(initialSession); when(top.getHolder()).thenReturn(letterGui); + when(top.getHolder(false)).thenReturn(letterGui); letterItems = mockStatic(LetterItems.class); letterItems .when(() -> LetterItems.isLetter(eq(config), any())) @@ -331,6 +333,7 @@ void nonPlayersAndUnrelatedInventoriesAreIgnored() { assertFalse(click.isCancelled()); assertFalse(drag.isCancelled()); when(top.getHolder()).thenReturn(null); + when(top.getHolder(false)).thenReturn(null); listener.onClick(click(null, InventoryAction.PLACE_ALL, ClickType.LEFT)); listener.onDrag(drag(null, Map.of())); listener.onClose(closeEvent()); @@ -860,6 +863,7 @@ void confirmationDetachesBeforeReentrantMailAndCannotCloseOrClearAReplacementPic when(nextHolder.session()).thenReturn(next); Inventory nextInventory = mock(Inventory.class); when(nextInventory.getHolder()).thenReturn(nextHolder); + when(nextInventory.getHolder(false)).thenReturn(nextHolder); openTop.set(nextInventory); listener.onClose(closeEvent()); ItemGive.giveOrDrop(player, letter); @@ -901,6 +905,7 @@ void failedMailHandoffRetainsTheCapturedCharacterRefundWithoutReplacingANewerPic when(nextHolder.session()).thenReturn(next); Inventory nextInventory = mock(Inventory.class); when(nextInventory.getHolder()).thenReturn(nextHolder); + when(nextInventory.getHolder(false)).thenReturn(nextHolder); openTop.set(nextInventory); listener.onClose(closeEvent()); throw failure; @@ -980,6 +985,7 @@ private CharacterPickerGui usePicker() { when(picker.session()).thenReturn(session); when(picker.getInventory()).thenReturn(top); when(top.getHolder()).thenReturn(picker); + when(top.getHolder(false)).thenReturn(picker); when(top.getSize()).thenReturn(54); return picker; } diff --git a/src/test/java/net/tfminecraft/birdmessenger/session/SendSessionManagerTest.java b/src/test/java/net/tfminecraft/birdmessenger/session/SendSessionManagerTest.java index feebea1..d89360d 100644 --- a/src/test/java/net/tfminecraft/birdmessenger/session/SendSessionManagerTest.java +++ b/src/test/java/net/tfminecraft/birdmessenger/session/SendSessionManagerTest.java @@ -86,6 +86,7 @@ class SendSessionManagerTest { LetterGui gui = mock(LetterGui.class); when(gui.session()).thenReturn(session); when(f.top.getHolder()).thenReturn(gui); + when(f.top.getHolder(false)).thenReturn(gui); session.setLetterInventory(f.top); when(f.top.getItem(LetterGui.LETTER_SLOT)).thenReturn(f.letter); f.current.set(null); @@ -130,6 +131,7 @@ class SendSessionManagerTest { CharacterPickerGui picker = mock(CharacterPickerGui.class); when(picker.session()).thenReturn(session); when(f.top.getHolder()).thenReturn(picker); + when(f.top.getHolder(false)).thenReturn(picker); session.setLetter(f.letter); f.sessions.close(); verify(f.inventory).addItem(f.letter); From c0f37a1468b4c054be9e7fcbdea9c0921c2a7e0f Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 31 Oct 2026 14:12:38 +0100 Subject: [PATCH 2/2] test: check the picker reads its holder without a block copy Both stubs answered the same holder, so reverting openPicker or refreshPickerHeads to getHolder() kept the test passing. It now verifies getHolder(false) runs and getHolder() never does (CodeRabbit). Co-Authored-By: Claude Opus 5.5 --- .../java/net/tfminecraft/birdmessenger/PickerOpenTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/java/net/tfminecraft/birdmessenger/PickerOpenTest.java b/src/test/java/net/tfminecraft/birdmessenger/PickerOpenTest.java index ac67c50..e9aa6b3 100644 --- a/src/test/java/net/tfminecraft/birdmessenger/PickerOpenTest.java +++ b/src/test/java/net/tfminecraft/birdmessenger/PickerOpenTest.java @@ -93,6 +93,10 @@ class PickerOpenTest { refreshCallbacks.getFirst().run(); assertEquals(1, opens, "refresh re-renders the open picker instead of reopening it"); assertEquals(2, picker.targets().size()); + // Both lookups of the open picker read the holder without copying a block. + Inventory top = player.getOpenInventory().getTopInventory(); + verify(top, never()).getHolder(); + verify(top, atLeastOnce()).getHolder(false); } }