Skip to content

Commit 7791a3e

Browse files
NichtStudioCodeNichtStudioCode
committed
Optimize CustomContainerMenu#sendChangesToRemote
- Only compare server and remote stacks when one has possibly changed - Avoid data component hash code calculations through identity-based layer 1 cache
1 parent f3c2c73 commit 7791a3e

File tree

7 files changed

+92
-37
lines changed

7 files changed

+92
-37
lines changed

‎invui/src/main/java/xyz/xenondevs/invui/internal/menu/CustomAnvilMenu.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ private void handleRename(ServerboundRenameItemPacket packet) {
6969
renameText = packet.getName();
7070
if (renameHandler != null)
7171
renameHandler.accept(renameText);
72-
remoteItems.set(2, DIRTY_MARKER);
72+
setRemoteItem(2, DIRTY_MARKER);
7373
remoteDataSlots[0] = ENCHANTMENT_COST_DIRTY_MARKER;
7474
}
7575

‎invui/src/main/java/xyz/xenondevs/invui/internal/menu/CustomCartographyMenu.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public void open(Component title) {
7474

7575
public void setView(CartographyWindow.View view) {
7676
this.view = view;
77-
setItem(1, CraftItemStack.asCraftMirror(items.get(1)));
77+
setItem(1, CraftItemStack.asCraftMirror(getItem(1)));
7878
}
7979

8080
public void setIcons(Collection<? extends MapIcon> icons, boolean sendUpdate) {
@@ -107,7 +107,7 @@ public void resetMap() {
107107
canvas = new byte[MAP_SIZE * MAP_SIZE];
108108
decorations.clear();
109109

110-
setItem(0, CraftItemStack.asCraftMirror(items.getFirst()));
110+
setItem(0, CraftItemStack.asCraftMirror(getItem(0)));
111111
}
112112

113113
private void sendMapUpdate(MapItemSavedData.@Nullable MapPatch patch, @Nullable Collection<MapDecoration> icons) {

‎invui/src/main/java/xyz/xenondevs/invui/internal/menu/CustomContainerMenu.java‎

Lines changed: 82 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.google.common.cache.CacheBuilder;
44
import com.google.common.cache.CacheLoader;
55
import com.google.common.cache.LoadingCache;
6+
import com.google.common.collect.MapMaker;
67
import com.google.common.hash.HashCode;
78
import com.mojang.serialization.DynamicOps;
89
import io.papermc.paper.adventure.PaperAdventure;
@@ -51,12 +52,10 @@
5152
import xyz.xenondevs.invui.window.Window;
5253

5354
import java.time.Duration;
54-
import java.util.ArrayList;
55-
import java.util.List;
56-
import java.util.Map;
57-
import java.util.Queue;
55+
import java.util.*;
5856
import java.util.concurrent.ConcurrentHashMap;
5957
import java.util.concurrent.ConcurrentLinkedQueue;
58+
import java.util.concurrent.ConcurrentMap;
6059

6160
/**
6261
* A packet-based container menu.
@@ -83,7 +82,12 @@ public abstract class CustomContainerMenu {
8382
*/
8483
private static final HashedPatchMap.HashGenerator HASH_GENERATOR = new HashedPatchMap.HashGenerator() {
8584

86-
private final LoadingCache<TypedDataComponent<?>, Integer> cache = CacheBuilder.newBuilder()
85+
// layer 1 cache uses identity hash code to avoid expensive component hash code calculations
86+
private final ConcurrentMap<Object, Integer> layer1 = new MapMaker()
87+
.weakKeys() // also enables identity-based lookup
88+
.makeMap();
89+
90+
private final LoadingCache<TypedDataComponent<?>, Integer> layer2 = CacheBuilder.newBuilder()
8791
.expireAfterAccess(Duration.ofMinutes(1))
8892
.build(new CacheLoader<>() {
8993

@@ -101,7 +105,10 @@ public Integer load(TypedDataComponent<?> key) {
101105

102106
@Override
103107
public Integer apply(TypedDataComponent<?> typedDataComponent) {
104-
return cache.getUnchecked(typedDataComponent);
108+
return layer1.computeIfAbsent(
109+
typedDataComponent.value(),
110+
_ -> layer2.getUnchecked(typedDataComponent)
111+
);
105112
}
106113

107114
};
@@ -123,8 +130,8 @@ public Integer apply(TypedDataComponent<?> typedDataComponent) {
123130
private @Nullable Window window;
124131
private final ContainerMenuProxy proxy;
125132

126-
protected final NonNullList<ItemStack> items;
127-
protected final NonNullList<HashedStack> remoteItems;
133+
private final NonNullList<ItemStack> items;
134+
private final NonNullList<HashedStack> remoteItems;
128135
private ItemStack carried = ItemStack.EMPTY;
129136
private HashedStack remoteCarried = HashedStack.EMPTY;
130137
private HashedStack remoteOffHand;
@@ -139,6 +146,13 @@ public Integer apply(TypedDataComponent<?> typedDataComponent) {
139146
protected final Queue<Packet<? super ServerGamePacketListener>> incoming = new ConcurrentLinkedQueue<>();
140147
private final Map<Integer, PingData> pendingPongs = new ConcurrentHashMap<>();
141148

149+
// Only items marked as dirty via the fields below will be compared with their remote counterparts.
150+
// Then, only if they differ, an update packet will be created for them.
151+
// The intention of these dirty fields is to avoid expensive comparisons between the remote and server items.
152+
private final BitSet dirtyItems;
153+
private boolean dirtyCarried;
154+
private boolean dirtyOffHand;
155+
142156
/**
143157
* Creates a new {@link CustomContainerMenu} for the specified player.
144158
*
@@ -155,6 +169,7 @@ protected CustomContainerMenu(MenuType<?> menuType, org.bukkit.entity.Player pla
155169
this.items = NonNullList.withSize(size, ItemStack.EMPTY);
156170
this.remoteItems = NonNullList.withSize(size, HashedStack.EMPTY);
157171
this.remoteOffHand = HashedStack.create(serverPlayer.getOffhandItem(), HASH_GENERATOR);
172+
this.dirtyItems = new BitSet(size);
158173

159174
int dataSize = InventoryUtils.getDataSlotCountOf(menuType);
160175
this.dataSlots = new int[dataSize];
@@ -174,6 +189,14 @@ public void setItem(int slot, org.bukkit.inventory.@Nullable ItemStack item) {
174189
throw new IllegalArgumentException("Slot out of bounds: " + slot);
175190

176191
items.set(slot, item == null ? ItemStack.EMPTY : CraftItemStack.unwrap(item));
192+
dirtyItems.set(slot);
193+
}
194+
195+
public ItemStack getItem(int slot) {
196+
if (slot < 0 || slot >= items.size())
197+
throw new IllegalArgumentException("Slot out of bounds: " + slot);
198+
199+
return items.get(slot);
177200
}
178201

179202
/**
@@ -183,6 +206,7 @@ public void setItem(int slot, org.bukkit.inventory.@Nullable ItemStack item) {
183206
*/
184207
public void setCursor(org.bukkit.inventory.@Nullable ItemStack item) {
185208
carried = item == null ? ItemStack.EMPTY : CraftItemStack.unwrap(item);
209+
dirtyCarried = true;
186210
}
187211

188212
/**
@@ -195,6 +219,23 @@ public org.bukkit.inventory.ItemStack getCursor() {
195219
}
196220

197221
//<editor-fold desc="synchronization">
222+
protected void setRemoteItem(int slot, HashedStack item) {
223+
if (slot < 0 || slot >= remoteItems.size())
224+
throw new IllegalArgumentException("Slot out of bounds: " + slot);
225+
226+
remoteItems.set(slot, item);
227+
dirtyItems.set(slot);
228+
}
229+
230+
protected void setRemoteCarried(HashedStack item) {
231+
remoteCarried = item;
232+
dirtyCarried = true;
233+
}
234+
235+
protected void setRemoteOffHand(HashedStack item) {
236+
remoteOffHand = item;
237+
dirtyOffHand = true;
238+
}
198239

199240
/**
200241
* Sends all changes to the remote client.
@@ -204,29 +245,38 @@ public org.bukkit.inventory.ItemStack getCursor() {
204245
public void sendChangesToRemote(int pingId) {
205246
var packets = new ArrayList<Packet<? super ClientGamePacketListener>>();
206247

207-
for (int i = 0; i < items.size(); i++) {
208-
var item = items.get(i);
209-
if (!remoteItems.get(i).matches(item, HASH_GENERATOR)) {
210-
packets.add(new ClientboundContainerSetSlotPacket(containerId, incrementStateId(), i, item.copy()));
211-
remoteItems.set(i, HashedStack.create(item, HASH_GENERATOR));
248+
int itemSlot = -1;
249+
while ((itemSlot = dirtyItems.nextSetBit(itemSlot + 1)) != -1) {
250+
var item = items.get(itemSlot);
251+
var remoteItem = remoteItems.get(itemSlot);
252+
if (remoteItem == DIRTY_MARKER || !remoteItem.matches(item, HASH_GENERATOR)) {
253+
packets.add(new ClientboundContainerSetSlotPacket(containerId, incrementStateId(), itemSlot, item.copy()));
254+
remoteItems.set(itemSlot, HashedStack.create(item, HASH_GENERATOR));
212255
}
213256
}
257+
dirtyItems.clear();
214258

215-
var offHand = serverPlayer.getOffhandItem();
216-
if (!remoteOffHand.matches(offHand, HASH_GENERATOR)) {
217-
packets.add(new ClientboundContainerSetSlotPacket(serverPlayer.inventoryMenu.containerId, incrementStateId(), OFF_HAND_SLOT, offHand.copy()));
218-
remoteOffHand = HashedStack.create(offHand, HASH_GENERATOR);
259+
if (dirtyOffHand) {
260+
var offHand = serverPlayer.getOffhandItem();
261+
if (remoteOffHand == DIRTY_MARKER || !remoteOffHand.matches(offHand, HASH_GENERATOR)) {
262+
packets.add(new ClientboundContainerSetSlotPacket(serverPlayer.inventoryMenu.containerId, incrementStateId(), OFF_HAND_SLOT, offHand.copy()));
263+
remoteOffHand = HashedStack.create(offHand, HASH_GENERATOR);
264+
}
265+
dirtyOffHand = false;
219266
}
220267

221-
if (!remoteCarried.matches(carried, HASH_GENERATOR)) {
222-
packets.add(new ClientboundSetCursorItemPacket(carried.copy()));
223-
remoteCarried = HashedStack.create(carried, HASH_GENERATOR);
268+
if (dirtyCarried) {
269+
if (remoteCarried == DIRTY_MARKER || !remoteCarried.matches(carried, HASH_GENERATOR)) {
270+
packets.add(new ClientboundSetCursorItemPacket(carried.copy()));
271+
remoteCarried = HashedStack.create(carried, HASH_GENERATOR);
272+
}
273+
dirtyCarried = false;
224274
}
225275

226-
for (int i = 0; i < dataSlots.length; i++) {
227-
if (dataSlots[i] != remoteDataSlots[i]) {
228-
packets.add(new ClientboundContainerSetDataPacket(containerId, i, dataSlots[i]));
229-
remoteDataSlots[i] = dataSlots[i];
276+
for (int dataSlot = 0; dataSlot < dataSlots.length; dataSlot++) {
277+
if (dataSlots[dataSlot] != remoteDataSlots[dataSlot]) {
278+
packets.add(new ClientboundContainerSetDataPacket(containerId, dataSlot, dataSlots[dataSlot]));
279+
remoteDataSlots[dataSlot] = dataSlots[dataSlot];
230280
}
231281
}
232282

@@ -244,6 +294,7 @@ public void sendCarriedToRemote() {
244294
var content = new ClientboundSetCursorItemPacket(carried.copy());
245295
PacketListener.getInstance().injectOutgoing(player, content);
246296
remoteCarried = HashedStack.create(carried, HASH_GENERATOR);
297+
dirtyCarried = false;
247298
}
248299

249300
/**
@@ -302,6 +353,10 @@ private void markRemoteSynced() {
302353
}
303354
remoteCarried = HashedStack.create(carried, HASH_GENERATOR);
304355
System.arraycopy(dataSlots, 0, remoteDataSlots, 0, dataSlots.length);
356+
357+
dirtyItems.clear();
358+
dirtyCarried = false;
359+
dirtyOffHand = false;
305360
}
306361
//</editor-fold>
307362

@@ -451,9 +506,9 @@ protected UpdateType handleClick(ServerboundContainerClickPacket packet) {
451506
HashedStack stack = entry.getValue();
452507
if (slot < 0 || slot > remoteItems.size())
453508
continue;
454-
remoteItems.set(slot, stack);
509+
setRemoteItem(slot, stack);
455510
}
456-
remoteCarried = DIRTY_MARKER;
511+
setRemoteCarried(DIRTY_MARKER);
457512

458513
if (packet.clickType() == net.minecraft.world.inventory.ClickType.QUICK_CRAFT) {
459514
if (!handleDragClick(packet))
@@ -491,7 +546,7 @@ private void handleNormalClick(ServerboundContainerClickPacket packet) {
491546
yield ClickType.NUMBER_KEY;
492547
}
493548
case 40 -> {
494-
remoteOffHand = DIRTY_MARKER;
549+
setRemoteOffHand(DIRTY_MARKER);
495550
yield ClickType.SWAP_OFFHAND;
496551
}
497552
default -> ClickType.UNKNOWN;

‎invui/src/main/java/xyz/xenondevs/invui/internal/menu/CustomCrafterMenu.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public boolean isSlotDisabled(int slot) {
7272
*/
7373
public void setSlotDisabled(int slot, boolean state) {
7474
dataSlots[slot] = state ? 1 : 0;
75-
remoteItems.set(slot, HashedStack.EMPTY);
75+
setRemoteItem(slot, HashedStack.EMPTY);
7676
}
7777

7878
/**

‎invui/src/main/java/xyz/xenondevs/invui/internal/menu/CustomGrindstoneMenu.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public void setItem(int slot, @Nullable ItemStack item) {
2626

2727
// client-side prediction clears output slot when input slots are modified
2828
if (slot == 0 || slot == 1) {
29-
remoteItems.set(2, HashedStack.EMPTY);
29+
setRemoteItem(2, HashedStack.EMPTY);
3030
}
3131
}
3232

‎invui/src/main/java/xyz/xenondevs/invui/internal/menu/CustomSmithingTableMenu.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public void setItem(int slot, @Nullable ItemStack item) {
2626

2727
// client-side prediction clears output slot when input slots are modified
2828
if (slot == 0 || slot == 1 || slot == 2) {
29-
remoteItems.set(3, HashedStack.EMPTY);
29+
setRemoteItem(3, HashedStack.EMPTY);
3030
}
3131
}
3232

‎invui/src/main/java/xyz/xenondevs/invui/internal/menu/CustomStonecutterMenu.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public void setItem(int slot, org.bukkit.inventory.@Nullable ItemStack item) {
8989
super.setItem(slot, item);
9090
if (slot == 0) {
9191
// client-side prediction may clear the output slot (e.g. when shift-clicking into input)
92-
remoteItems.set(1, DIRTY_MARKER);
92+
setRemoteItem(1, DIRTY_MARKER);
9393
}
9494
}
9595

@@ -121,8 +121,8 @@ private void setNmsButtons(List<? extends ItemStack> buttons) {
121121
// this also triggers result and selected slot to be reset, requiring them to be resent
122122
var setInputPacket = new ClientboundContainerSetSlotPacket(containerId, incrementStateId(), 0, ItemStack.EMPTY);
123123
PacketListener.getInstance().injectOutgoing(player, setInputPacket);
124-
remoteItems.set(0, HashedStack.EMPTY);
125-
remoteItems.set(1, HashedStack.EMPTY);
124+
setRemoteItem(0, HashedStack.EMPTY);
125+
setRemoteItem(1, HashedStack.EMPTY);
126126
remoteDataSlots[0] = -1;
127127
sendChangesToRemote(-1);
128128
}
@@ -155,7 +155,7 @@ public UpdateType handleButtonClick(int clicked) {
155155
int prev = dataSlots[0];
156156
dataSlots[0] = clicked;
157157
remoteDataSlots[0] = clicked;
158-
remoteItems.set(1, HashedStack.EMPTY);
158+
setRemoteItem(1, HashedStack.EMPTY);
159159

160160
if (clickHandler != null)
161161
clickHandler.accept(prev, clicked);

0 commit comments

Comments
 (0)