Skip to content

Commit 9059a51

Browse files
Switch CustomContainerMenu identity cache to custom WeakIdentityMap
This avoids the overhead from locking
1 parent 7791a3e commit 9059a51

3 files changed

Lines changed: 136 additions & 27 deletions

File tree

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

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
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;
76
import com.google.common.hash.HashCode;
87
import com.mojang.serialization.DynamicOps;
98
import io.papermc.paper.adventure.PaperAdventure;
@@ -45,17 +44,13 @@
4544
import xyz.xenondevs.invui.Click;
4645
import xyz.xenondevs.invui.InvUI;
4746
import xyz.xenondevs.invui.internal.network.PacketListener;
48-
import xyz.xenondevs.invui.internal.util.FakeInventoryView;
49-
import xyz.xenondevs.invui.internal.util.InventoryUtils;
50-
import xyz.xenondevs.invui.internal.util.MathUtils;
51-
import xyz.xenondevs.invui.internal.util.PingData;
47+
import xyz.xenondevs.invui.internal.util.*;
5248
import xyz.xenondevs.invui.window.Window;
5349

5450
import java.time.Duration;
5551
import java.util.*;
5652
import java.util.concurrent.ConcurrentHashMap;
5753
import java.util.concurrent.ConcurrentLinkedQueue;
58-
import java.util.concurrent.ConcurrentMap;
5954

6055
/**
6156
* A packet-based container menu.
@@ -82,12 +77,7 @@ public abstract class CustomContainerMenu {
8277
*/
8378
private static final HashedPatchMap.HashGenerator HASH_GENERATOR = new HashedPatchMap.HashGenerator() {
8479

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()
80+
private final LoadingCache<TypedDataComponent<?>, Integer> cache = CacheBuilder.newBuilder()
9181
.expireAfterAccess(Duration.ofMinutes(1))
9282
.build(new CacheLoader<>() {
9383

@@ -104,11 +94,8 @@ public Integer load(TypedDataComponent<?> key) {
10494
});
10595

10696
@Override
107-
public Integer apply(TypedDataComponent<?> typedDataComponent) {
108-
return layer1.computeIfAbsent(
109-
typedDataComponent.value(),
110-
_ -> layer2.getUnchecked(typedDataComponent)
111-
);
97+
public Integer apply(TypedDataComponent<?> tdc) {
98+
return cache.getUnchecked(tdc);
11299
}
113100

114101
};
@@ -153,6 +140,23 @@ public Integer apply(TypedDataComponent<?> typedDataComponent) {
153140
private boolean dirtyCarried;
154141
private boolean dirtyOffHand;
155142

143+
private final HashedPatchMap.HashGenerator hashGenerator = new HashedPatchMap.HashGenerator() {
144+
145+
private final WeakIdentityToIntMap<Object> cache = new WeakIdentityToIntMap<>();
146+
147+
@Override
148+
public Integer apply(TypedDataComponent<?> tdc) {
149+
var c = tdc.value();
150+
var cachedValue = cache.get(c);
151+
if (cachedValue.isPresent())
152+
return cachedValue.getAsInt();
153+
var value = HASH_GENERATOR.apply(tdc);
154+
cache.putAssertAbsent(c, value);
155+
return value;
156+
}
157+
158+
};
159+
156160
/**
157161
* Creates a new {@link CustomContainerMenu} for the specified player.
158162
*
@@ -168,7 +172,7 @@ protected CustomContainerMenu(MenuType<?> menuType, org.bukkit.entity.Player pla
168172
int size = InventoryUtils.getSizeOf(menuType) + LOWER_INVENTORY_SIZE;
169173
this.items = NonNullList.withSize(size, ItemStack.EMPTY);
170174
this.remoteItems = NonNullList.withSize(size, HashedStack.EMPTY);
171-
this.remoteOffHand = HashedStack.create(serverPlayer.getOffhandItem(), HASH_GENERATOR);
175+
this.remoteOffHand = HashedStack.create(serverPlayer.getOffhandItem(), hashGenerator);
172176
this.dirtyItems = new BitSet(size);
173177

174178
int dataSize = InventoryUtils.getDataSlotCountOf(menuType);
@@ -249,26 +253,26 @@ public void sendChangesToRemote(int pingId) {
249253
while ((itemSlot = dirtyItems.nextSetBit(itemSlot + 1)) != -1) {
250254
var item = items.get(itemSlot);
251255
var remoteItem = remoteItems.get(itemSlot);
252-
if (remoteItem == DIRTY_MARKER || !remoteItem.matches(item, HASH_GENERATOR)) {
256+
if (remoteItem == DIRTY_MARKER || !remoteItem.matches(item, hashGenerator)) {
253257
packets.add(new ClientboundContainerSetSlotPacket(containerId, incrementStateId(), itemSlot, item.copy()));
254-
remoteItems.set(itemSlot, HashedStack.create(item, HASH_GENERATOR));
258+
remoteItems.set(itemSlot, HashedStack.create(item, hashGenerator));
255259
}
256260
}
257261
dirtyItems.clear();
258262

259263
if (dirtyOffHand) {
260264
var offHand = serverPlayer.getOffhandItem();
261-
if (remoteOffHand == DIRTY_MARKER || !remoteOffHand.matches(offHand, HASH_GENERATOR)) {
265+
if (remoteOffHand == DIRTY_MARKER || !remoteOffHand.matches(offHand, hashGenerator)) {
262266
packets.add(new ClientboundContainerSetSlotPacket(serverPlayer.inventoryMenu.containerId, incrementStateId(), OFF_HAND_SLOT, offHand.copy()));
263-
remoteOffHand = HashedStack.create(offHand, HASH_GENERATOR);
267+
remoteOffHand = HashedStack.create(offHand, hashGenerator);
264268
}
265269
dirtyOffHand = false;
266270
}
267271

268272
if (dirtyCarried) {
269-
if (remoteCarried == DIRTY_MARKER || !remoteCarried.matches(carried, HASH_GENERATOR)) {
273+
if (remoteCarried == DIRTY_MARKER || !remoteCarried.matches(carried, hashGenerator)) {
270274
packets.add(new ClientboundSetCursorItemPacket(carried.copy()));
271-
remoteCarried = HashedStack.create(carried, HASH_GENERATOR);
275+
remoteCarried = HashedStack.create(carried, hashGenerator);
272276
}
273277
dirtyCarried = false;
274278
}
@@ -293,7 +297,7 @@ public void sendChangesToRemote(int pingId) {
293297
public void sendCarriedToRemote() {
294298
var content = new ClientboundSetCursorItemPacket(carried.copy());
295299
PacketListener.getInstance().injectOutgoing(player, content);
296-
remoteCarried = HashedStack.create(carried, HASH_GENERATOR);
300+
remoteCarried = HashedStack.create(carried, hashGenerator);
297301
dirtyCarried = false;
298302
}
299303

@@ -349,9 +353,9 @@ private List<Packet<? super ClientGamePacketListener>> createContainerInitPacket
349353
*/
350354
private void markRemoteSynced() {
351355
for (int i = 0; i < items.size(); i++) {
352-
remoteItems.set(i, HashedStack.create(items.get(i), HASH_GENERATOR));
356+
remoteItems.set(i, HashedStack.create(items.get(i), hashGenerator));
353357
}
354-
remoteCarried = HashedStack.create(carried, HASH_GENERATOR);
358+
remoteCarried = HashedStack.create(carried, hashGenerator);
355359
System.arraycopy(dataSlots, 0, remoteDataSlots, 0, dataSlots.length);
356360

357361
dirtyItems.clear();
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package xyz.xenondevs.invui.internal.util;
2+
3+
import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
4+
import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
5+
import org.jspecify.annotations.Nullable;
6+
7+
import java.lang.ref.ReferenceQueue;
8+
import java.lang.ref.WeakReference;
9+
import java.util.ArrayList;
10+
import java.util.List;
11+
import java.util.OptionalInt;
12+
13+
public final class WeakIdentityToIntMap<K> {
14+
15+
private final Int2ObjectMap<@Nullable List<Entry<K>>> buckets = new Int2ObjectOpenHashMap<>();
16+
private final ReferenceQueue<K> queue = new ReferenceQueue<>();
17+
18+
public OptionalInt get(K key) {
19+
expunge();
20+
int hash = System.identityHashCode(key);
21+
List<Entry<K>> bucket = buckets.get(hash);
22+
if (bucket == null)
23+
return OptionalInt.empty();
24+
25+
for (Entry<K> e : bucket) {
26+
if (e.get() == key)
27+
return OptionalInt.of(e.value);
28+
}
29+
30+
return OptionalInt.empty();
31+
}
32+
33+
public void putAssertAbsent(K key, int value) {
34+
expunge();
35+
int hash = System.identityHashCode(key);
36+
List<Entry<K>> bucket = buckets.computeIfAbsent(hash, _ -> new ArrayList<>(2));
37+
assert bucket.stream().noneMatch(e -> e.get() == key) : "Key already present in map";
38+
bucket.add(new Entry<>(key, value, hash, queue));
39+
}
40+
41+
private void expunge() {
42+
Entry<?> e;
43+
while ((e = (Entry<?>) queue.poll()) != null) {
44+
List<Entry<K>> bucket = buckets.get(e.keyIdentityHash);
45+
if (bucket != null) {
46+
bucket.remove(e);
47+
if (bucket.isEmpty())
48+
buckets.remove(e.keyIdentityHash);
49+
}
50+
}
51+
}
52+
53+
private static class Entry<K> extends WeakReference<K> {
54+
55+
final int value;
56+
final int keyIdentityHash;
57+
58+
Entry(K key, int value, int keyIdentityHash, ReferenceQueue<K> queue) {
59+
super(key, queue);
60+
this.value = value;
61+
this.keyIdentityHash = keyIdentityHash;
62+
}
63+
64+
}
65+
66+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package xyz.xenondevs.invui.internal.util;
2+
3+
import org.junit.jupiter.api.Test;
4+
5+
import java.util.OptionalInt;
6+
7+
import static org.junit.jupiter.api.Assertions.assertEquals;
8+
9+
public class WeakIdentityToIntMapTest {
10+
11+
@Test
12+
public void testPutAndGet() {
13+
var map = new WeakIdentityToIntMap<>();
14+
var key = new Object();
15+
map.putAssertAbsent(key, 42);
16+
assertEquals(OptionalInt.of(42), map.get(key));
17+
}
18+
19+
@Test
20+
public void testGetMissing() {
21+
var map = new WeakIdentityToIntMap<>();
22+
assertEquals(OptionalInt.empty(), map.get(new Object()));
23+
}
24+
25+
@Test
26+
public void testIdentitySemantics() {
27+
var map = new WeakIdentityToIntMap<Key>();
28+
var key1 = new Key(1);
29+
var key2 = new Key(1);
30+
map.putAssertAbsent(key1, 1);
31+
map.putAssertAbsent(key2, 2);
32+
assertEquals(OptionalInt.of(1), map.get(key1));
33+
assertEquals(OptionalInt.of(2), map.get(key2));
34+
assertEquals(OptionalInt.empty(), map.get(new Key(1)));
35+
}
36+
37+
private record Key(int value) {}
38+
39+
}

0 commit comments

Comments
 (0)