From 6ecede145ec4de17ddfbddd4ae5f94afb7bc448b Mon Sep 17 00:00:00 2001 From: TheMode Date: Sat, 25 Jun 2022 10:36:50 +0200 Subject: [PATCH] Improve tag performance + concurrency tests (#1165) --- .../server/tag/TagPathRehashTest.java | 50 ++ .../minestom/server/tag/TagRehashTest.java | 50 ++ .../server/tag/TagUpdatePathRehashTest.java | 57 ++ .../minestom/server/tag/TagUpdateTest.java | 23 +- .../net/minestom/server/item/ItemMeta.java | 4 +- .../net/minestom/server/tag/Serializers.java | 8 +- .../net/minestom/server/tag/StaticIntMap.java | 87 +++ .../java/net/minestom/server/tag/Tag.java | 5 + .../minestom/server/tag/TagHandlerImpl.java | 498 +++++++++--------- .../net/minestom/server/tag/TagRecord.java | 6 +- .../minestom/server/item/ItemBlockTest.java | 62 +++ .../minestom/server/item/ItemDisplayTest.java | 55 ++ .../minestom/server/item/ItemEnchantTest.java | 42 ++ .../net/minestom/server/item/ItemTest.java | 78 --- .../minestom/server/tag/TagAtomicTest.java | 43 ++ .../tag/TagHandlerReadableCopyTest.java | 63 +++ .../java/net/minestom/server/tag/TagTest.java | 8 + 17 files changed, 787 insertions(+), 352 deletions(-) create mode 100644 jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagPathRehashTest.java create mode 100644 jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagRehashTest.java create mode 100644 jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagUpdatePathRehashTest.java create mode 100644 src/main/java/net/minestom/server/tag/StaticIntMap.java create mode 100644 src/test/java/net/minestom/server/item/ItemBlockTest.java create mode 100644 src/test/java/net/minestom/server/item/ItemDisplayTest.java create mode 100644 src/test/java/net/minestom/server/item/ItemEnchantTest.java create mode 100644 src/test/java/net/minestom/server/tag/TagHandlerReadableCopyTest.java diff --git a/jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagPathRehashTest.java b/jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagPathRehashTest.java new file mode 100644 index 000000000..33ed9379a --- /dev/null +++ b/jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagPathRehashTest.java @@ -0,0 +1,50 @@ +package net.minestom.server.tag; + +import org.openjdk.jcstress.annotations.*; +import org.openjdk.jcstress.infra.results.LL_Result; + +import java.util.ArrayList; +import java.util.List; + +import static org.openjdk.jcstress.annotations.Expect.ACCEPTABLE; + +@JCStressTest +@Outcome(id = "1, 198", expect = ACCEPTABLE) +@Outcome(id = "1, 99", expect = ACCEPTABLE) +@Outcome(id = "2, 198", expect = ACCEPTABLE) +@Outcome(id = "2, 99", expect = ACCEPTABLE) +@State +public class TagPathRehashTest { + private static final int MAX_SIZE = 500; + private static final List> TAGS; + + static { + List> tags = new ArrayList<>(); + for (int i = 0; i < MAX_SIZE; i++) { + tags.add(Tag.Integer("key" + i).path("path")); + } + TAGS = List.copyOf(tags); + } + + private final TagHandler handler = TagHandler.newHandler(); + + @Actor + public void actor1() { + for (int i = 0; i < MAX_SIZE; i++) { + handler.setTag(TAGS.get(i), i); + } + } + + @Actor + public void actor2() { + for (int i = 0; i < MAX_SIZE; i++) { + handler.setTag(TAGS.get(i), i * 2); + } + } + + @Arbiter + public void arbiter(LL_Result r) { + r.r1 = handler.getTag(TAGS.get(1)); + r.r2 = handler.getTag(TAGS.get(99)); + } +} diff --git a/jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagRehashTest.java b/jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagRehashTest.java new file mode 100644 index 000000000..4c5cc85f4 --- /dev/null +++ b/jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagRehashTest.java @@ -0,0 +1,50 @@ +package net.minestom.server.tag; + +import org.openjdk.jcstress.annotations.*; +import org.openjdk.jcstress.infra.results.LL_Result; + +import java.util.ArrayList; +import java.util.List; + +import static org.openjdk.jcstress.annotations.Expect.ACCEPTABLE; + +@JCStressTest +@Outcome(id = "1, 198", expect = ACCEPTABLE) +@Outcome(id = "1, 99", expect = ACCEPTABLE) +@Outcome(id = "2, 198", expect = ACCEPTABLE) +@Outcome(id = "2, 99", expect = ACCEPTABLE) +@State +public class TagRehashTest { + private static final int MAX_SIZE = 500; + private static final List> TAGS; + + static { + List> tags = new ArrayList<>(); + for (int i = 0; i < MAX_SIZE; i++) { + tags.add(Tag.Integer("key" + i)); + } + TAGS = List.copyOf(tags); + } + + private final TagHandler handler = TagHandler.newHandler(); + + @Actor + public void actor1() { + for (int i = 0; i < MAX_SIZE; i++) { + handler.setTag(TAGS.get(i), i); + } + } + + @Actor + public void actor2() { + for (int i = 0; i < MAX_SIZE; i++) { + handler.setTag(TAGS.get(i), i * 2); + } + } + + @Arbiter + public void arbiter(LL_Result r) { + r.r1 = handler.getTag(TAGS.get(1)); + r.r2 = handler.getTag(TAGS.get(99)); + } +} diff --git a/jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagUpdatePathRehashTest.java b/jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagUpdatePathRehashTest.java new file mode 100644 index 000000000..b0b91ef53 --- /dev/null +++ b/jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagUpdatePathRehashTest.java @@ -0,0 +1,57 @@ +package net.minestom.server.tag; + +import org.openjdk.jcstress.annotations.*; +import org.openjdk.jcstress.infra.results.L_Result; + +import java.util.ArrayList; +import java.util.List; + +import static org.openjdk.jcstress.annotations.Expect.ACCEPTABLE; + +@JCStressTest +@Outcome(id = "2000", expect = ACCEPTABLE) +@State +public class TagUpdatePathRehashTest { + private static final Tag TAG = Tag.Integer("key").path("path").defaultValue(0); + + private static final int MAX_SIZE = 500; + private static final List> TAGS; + + static { + List> tags = new ArrayList<>(); + for (int i = 0; i < MAX_SIZE; i++) { + tags.add(Tag.Integer("key" + i).path("path")); + } + TAGS = List.copyOf(tags); + } + + private final TagHandler handler = TagHandler.newHandler(); + + @Actor + public void actor1() { + for (int i = 0; i < 1000; i++) { + handler.updateTag(TAG, integer -> integer + 1); + } + } + + @Actor + public void actor2() { + for (int i = 0; i < 1000; i++) { + handler.updateTag(TAG, integer -> integer + 1); + } + } + + @Actor + public void actor3() { + // May be able to disturb actor1/2 + for (int i = 0; i < MAX_SIZE; i++) { + handler.setTag(TAGS.get(i), i); + } + } + + @Arbiter + public void arbiter(L_Result r) { + r.r1 = handler.getTag(TAG); + } +} + diff --git a/jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagUpdateTest.java b/jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagUpdateTest.java index d2b77c56e..61c7ff633 100644 --- a/jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagUpdateTest.java +++ b/jcstress-tests/src/jcstress/java/net/minestom/server/tag/TagUpdateTest.java @@ -1,36 +1,33 @@ package net.minestom.server.tag; -import org.openjdk.jcstress.annotations.Actor; -import org.openjdk.jcstress.annotations.JCStressTest; -import org.openjdk.jcstress.annotations.Outcome; -import org.openjdk.jcstress.annotations.State; +import org.openjdk.jcstress.annotations.*; import org.openjdk.jcstress.infra.results.L_Result; import static org.openjdk.jcstress.annotations.Expect.ACCEPTABLE; @JCStressTest -@Outcome(id = "null", expect = ACCEPTABLE) -@Outcome(id = "3", expect = ACCEPTABLE) -@Outcome(id = "4", expect = ACCEPTABLE) -@Outcome(id = "10", expect = ACCEPTABLE) -@Outcome(id = "11", expect = ACCEPTABLE) +@Outcome(id = "2000", expect = ACCEPTABLE) @State public class TagUpdateTest { - private static final Tag TAG = Tag.Integer("key"); + private static final Tag TAG = Tag.Integer("key").defaultValue(0); private final TagHandler handler = TagHandler.newHandler(); @Actor public void actor1() { - handler.updateAndGetTag(TAG, integer -> integer == null ? 3 : integer + 1); + for (int i = 0; i < 1000; i++) { + handler.updateAndGetTag(TAG, integer -> integer + 1); + } } @Actor public void actor2() { - handler.updateAndGetTag(TAG, integer -> integer == null ? 10 : integer + 1); + for (int i = 0; i < 1000; i++) { + handler.updateAndGetTag(TAG, integer -> integer + 1); + } } - @Actor + @Arbiter public void arbiter(L_Result r) { r.r1 = handler.getTag(TAG); } diff --git a/src/main/java/net/minestom/server/item/ItemMeta.java b/src/main/java/net/minestom/server/item/ItemMeta.java index 2cdfb9859..0a60dc3f5 100644 --- a/src/main/java/net/minestom/server/item/ItemMeta.java +++ b/src/main/java/net/minestom/server/item/ItemMeta.java @@ -153,7 +153,7 @@ public sealed interface ItemMeta extends TagReadable, Writeable @Contract("_ -> this") default @NotNull Builder canPlaceOn(@NotNull Set<@NotNull Block> blocks) { - return set(ItemTags.CAN_PLACE_ON, List.copyOf(blocks)); + return set(ItemTags.CAN_PLACE_ON, blocks.stream().map(block -> Block.fromNamespaceId(block.name())).toList()); } @Contract("_ -> this") @@ -163,7 +163,7 @@ public sealed interface ItemMeta extends TagReadable, Writeable @Contract("_ -> this") default @NotNull Builder canDestroy(@NotNull Set<@NotNull Block> blocks) { - return set(ItemTags.CAN_DESTROY, List.copyOf(blocks)); + return set(ItemTags.CAN_DESTROY, blocks.stream().map(block -> Block.fromNamespaceId(block.name())).toList()); } @Contract("_ -> this") diff --git a/src/main/java/net/minestom/server/tag/Serializers.java b/src/main/java/net/minestom/server/tag/Serializers.java index 662bd4e06..952a23d4c 100644 --- a/src/main/java/net/minestom/server/tag/Serializers.java +++ b/src/main/java/net/minestom/server/tag/Serializers.java @@ -12,8 +12,6 @@ import java.util.function.Function; * Basic serializers for {@link Tag tags}. */ final class Serializers { - static final Entry PATH = new Entry<>(NBTType.TAG_Compound, TagHandlerImpl::fromCompound, TagHandlerImpl::asCompound); - static final Entry BYTE = new Entry<>(NBTType.TAG_Byte, NBTByte::getValue, NBT::Byte); static final Entry BOOLEAN = new Entry<>(NBTType.TAG_Byte, NBTByte::asBoolean, NBT::Boolean); static final Entry SHORT = new Entry<>(NBTType.TAG_Short, NBTShort::getValue, NBT::Short); @@ -44,7 +42,11 @@ final class Serializers { }); } - record Entry(NBTType nbtType, Function reader, Function writer) { + record Entry(NBTType nbtType, Function reader, Function writer, boolean isPath) { + Entry(NBTType nbtType, Function reader, Function writer) { + this(nbtType, reader, writer, false); + } + T read(N nbt) { return reader.apply(nbt); } diff --git a/src/main/java/net/minestom/server/tag/StaticIntMap.java b/src/main/java/net/minestom/server/tag/StaticIntMap.java new file mode 100644 index 000000000..e2648205f --- /dev/null +++ b/src/main/java/net/minestom/server/tag/StaticIntMap.java @@ -0,0 +1,87 @@ +package net.minestom.server.tag; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Range; + +import java.util.Arrays; +import java.util.function.Consumer; + +sealed interface StaticIntMap permits StaticIntMap.Array { + + T get(@Range(from = 0, to = Integer.MAX_VALUE) int key); + + void forValues(@NotNull Consumer consumer); + + @NotNull StaticIntMap copy(); + + // Methods potentially causing re-hashing + + void put(@Range(from = 0, to = Integer.MAX_VALUE) int key, T value); + + void remove(@Range(from = 0, to = Integer.MAX_VALUE) int key); + + void updateContent(@NotNull StaticIntMap content); + + final class Array implements StaticIntMap { + private static final Object[] EMPTY_ARRAY = new Object[0]; + + private T[] array; + + public Array(T[] array) { + this.array = array; + } + + public Array() { + //noinspection unchecked + this.array = (T[]) EMPTY_ARRAY; + } + + @Override + public T get(int key) { + final T[] array = this.array; + return key < array.length ? array[key] : null; + } + + @Override + public void forValues(@NotNull Consumer consumer) { + final T[] array = this.array; + for (T value : array) { + if (value != null) consumer.accept(value); + } + } + + @Override + public @NotNull StaticIntMap copy() { + return new Array<>(array.clone()); + } + + @Override + public void put(int key, T value) { + T[] array = this.array; + if (key >= array.length) { + array = updateArray(Arrays.copyOf(array, key * 2 + 1)); + } + array[key] = value; + } + + @Override + public void updateContent(@NotNull StaticIntMap content) { + if (content instanceof StaticIntMap.Array arrayMap) { + updateArray(arrayMap.array.clone()); + } else { + throw new IllegalArgumentException("Invalid content type: " + content.getClass()); + } + } + + @Override + public void remove(int key) { + T[] array = this.array; + if (key < array.length) array[key] = null; + } + + T[] updateArray(T[] result) { + this.array = result; + return result; + } + } +} diff --git a/src/main/java/net/minestom/server/tag/Tag.java b/src/main/java/net/minestom/server/tag/Tag.java index 40da7db4e..dee669f0f 100644 --- a/src/main/java/net/minestom/server/tag/Tag.java +++ b/src/main/java/net/minestom/server/tag/Tag.java @@ -210,6 +210,11 @@ public class Tag { return supplier != null ? supplier.get() : null; } + final T copyValue(@NotNull T value) { + final UnaryOperator copier = copy; + return copier != null ? copier.apply(value) : value; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/src/main/java/net/minestom/server/tag/TagHandlerImpl.java b/src/main/java/net/minestom/server/tag/TagHandlerImpl.java index 3044e5450..1f8efc4f1 100644 --- a/src/main/java/net/minestom/server/tag/TagHandlerImpl.java +++ b/src/main/java/net/minestom/server/tag/TagHandlerImpl.java @@ -1,8 +1,7 @@ package net.minestom.server.tag; -import it.unimi.dsi.fastutil.ints.Int2ObjectMap; -import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap; import net.minestom.server.utils.PropertyUtils; +import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.UnknownNullability; @@ -13,355 +12,348 @@ import org.jglrxavpok.hephaistos.nbt.NBTType; import org.jglrxavpok.hephaistos.nbt.mutable.MutableNBTCompound; import java.lang.invoke.VarHandle; -import java.util.function.Supplier; import java.util.function.UnaryOperator; final class TagHandlerImpl implements TagHandler { private static final boolean CACHE_ENABLE = PropertyUtils.getBoolean("minestom.tag-handler-cache", true); + static final Serializers.Entry NODE_SERIALIZER = new Serializers.Entry<>(NBTType.TAG_Compound, entries -> fromCompound(entries).root, Node::compound, true); - private final TagHandlerImpl parent; - private volatile SPMCMap entries; - private Cache cache; + private final Node root; + private volatile Node copy; - TagHandlerImpl(TagHandlerImpl parent) { - this.parent = parent; - this.entries = new SPMCMap(this); - this.cache = null; + TagHandlerImpl(Node root) { + this.root = root; } TagHandlerImpl() { - this(null); + this.root = new Node(); } static TagHandlerImpl fromCompound(NBTCompoundLike compoundLike) { final NBTCompound compound = compoundLike.toCompound(); - TagHandlerImpl handler = new TagHandlerImpl(null); + TagHandlerImpl handler = new TagHandlerImpl(); TagNbtSeparator.separate(compound, entry -> handler.setTag(entry.tag(), entry.value())); + handler.root.compound = compound; return handler; } @Override public @UnknownNullability T getTag(@NotNull Tag tag) { - return read(entries, tag, this::asCompound); + VarHandle.fullFence(); + return root.getTag(tag); } @Override public void setTag(@NotNull Tag tag, @Nullable T value) { - TagHandlerImpl local = this; - final Tag.PathEntry[] paths = tag.path; - final boolean present = value != null; - final int tagIndex = tag.index; - final boolean isView = tag.isView(); - synchronized (this) { - if (paths != null) { - if ((local = traversePathWrite(this, paths, present)) == null) - return; // Tried to remove an absent tag. Do nothing - } - SPMCMap entries = local.entries; - if (present) { - if (!isView) { - Entry previous = entries.get(tagIndex); - if (previous != null && previous.tag().shareValue(tag)) { - previous.updateValue(value); - } else { - entries.put(tagIndex, valueToEntry(local, tag, value)); - } - } else { - local.updateContent((NBTCompound) tag.entry.write(value)); - return; - } - } else { - // Remove recursively - if (!isView) { - if (entries.remove(tagIndex) == null) return; - } else { - entries.clear(); - } - if (paths != null) { - TagHandlerImpl tmp = local; - int i = paths.length; - do { - if (!tmp.entries.isEmpty()) break; - tmp = tmp.parent; - tmp.entries.remove(paths[--i].index()); - } while (i > 0); + // Handle view tags + if (tag.isView()) { + synchronized (this) { + Node syncNode = traversePathWrite(root, tag, value != null); + if (syncNode != null) { + syncNode.updateContent(value != null ? (NBTCompound) tag.entry.write(value) : NBTCompound.EMPTY); + syncNode.invalidate(); } } - entries.invalidate(); - assert !local.entries.rehashed; + return; } - } - - private Entry valueToEntry(TagHandlerImpl parent, Tag tag, @NotNull T value) { - if (value instanceof NBT nbt) { - if (nbt instanceof NBTCompound compound) { - var handler = new TagHandlerImpl(parent); - handler.updateContent(compound); - return new PathEntry(tag.getKey(), handler); + // Normal tag + final int tagIndex = tag.index; + VarHandle.fullFence(); + Node node = traversePathWrite(root, tag, value != null); + if (node == null) + return; // Tried to remove an absent tag. Do nothing + StaticIntMap> entries = node.entries; + if (value != null) { + Entry previous = entries.get(tagIndex); + if (previous != null && previous.tag().shareValue(tag)) { + previous.updateValue(tag.copyValue(value)); } else { - final var nbtEntry = TagNbtSeparator.separateSingle(tag.getKey(), nbt); - return new TagEntry<>(nbtEntry.tag(), nbtEntry.value()); + synchronized (this) { + node = traversePathWrite(root, tag, true); + node.entries.put(tagIndex, valueToEntry(node, tag, value)); + } } } else { - final UnaryOperator copy = tag.copy; - if (copy != null) value = copy.apply(value); - return new TagEntry<>(tag, value); + synchronized (this) { + node = traversePathWrite(root, tag, false); + if (node != null) node.entries.remove(tagIndex); + } } + node.invalidate(); } @Override - public synchronized void updateTag(@NotNull Tag tag, @NotNull UnaryOperator<@UnknownNullability T> value) { - setTag(tag, value.apply(getTag(tag))); + public void updateTag(@NotNull Tag tag, @NotNull UnaryOperator<@UnknownNullability T> value) { + updateTag0(tag, value, false); } @Override - public synchronized @UnknownNullability T updateAndGetTag(@NotNull Tag tag, @NotNull UnaryOperator<@UnknownNullability T> value) { - final T next = value.apply(getTag(tag)); - setTag(tag, next); - return next; + public @UnknownNullability T updateAndGetTag(@NotNull Tag tag, @NotNull UnaryOperator<@UnknownNullability T> value) { + return updateTag0(tag, value, false); } @Override - public synchronized @UnknownNullability T getAndUpdateTag(@NotNull Tag tag, @NotNull UnaryOperator<@UnknownNullability T> value) { - final T prev = getTag(tag); - setTag(tag, value.apply(prev)); - return prev; + public @UnknownNullability T getAndUpdateTag(@NotNull Tag tag, @NotNull UnaryOperator<@UnknownNullability T> value) { + return updateTag0(tag, value, true); + } + + private synchronized T updateTag0(@NotNull Tag tag, @NotNull UnaryOperator value, boolean returnPrevious) { + final int tagIndex = tag.index; + final Node node = traversePathWrite(root, tag, true); + StaticIntMap> entries = node.entries; + + final Entry previousEntry = entries.get(tagIndex); + final T previousValue = previousEntry != null ? (T) previousEntry.value() : tag.createDefault(); + final T newValue = value.apply(previousValue); + if (newValue != null) entries.put(tagIndex, valueToEntry(node, tag, newValue)); + else entries.remove(tagIndex); + + node.invalidate(); + return returnPrevious ? previousValue : newValue; } @Override public @NotNull TagReadable readableCopy() { - return updatedCache(); - } - - @Override - public @NotNull TagHandler copy() { - return fromCompound(asCompound()); - } - - @Override - public void updateContent(@NotNull NBTCompoundLike compound) { - final TagHandlerImpl converted = fromCompound(compound); - synchronized (this) { - this.cache = converted.cache; - this.entries = new SPMCMap(this, converted.entries); + Node copy = this.copy; + if (copy == null) { + synchronized (this) { + this.copy = copy = root.copy(null); + } } + return copy; + } + + @Override + public synchronized @NotNull TagHandler copy() { + return new TagHandlerImpl(root.copy(null)); + } + + @Override + public synchronized void updateContent(@NotNull NBTCompoundLike compound) { + this.root.updateContent(compound); } @Override public @NotNull NBTCompound asCompound() { - return updatedCache().compound; + VarHandle.fullFence(); + return root.compound(); } - private static TagHandlerImpl traversePathWrite(TagHandlerImpl root, Tag.PathEntry[] paths, - boolean present) { - TagHandlerImpl local = root; + private static Node traversePathRead(Node node, Tag tag) { + final Tag.PathEntry[] paths = tag.path; + if (paths == null) return node; + for (var path : paths) { + final Entry entry = node.entries.get(path.index()); + if (entry == null || (node = entry.toNode()) == null) + return null; + } + return node; + } + + @Contract("_, _, true -> !null") + private Node traversePathWrite(Node root, Tag tag, + boolean present) { + final Tag.PathEntry[] paths = tag.path; + if (paths == null) return root; + Node local = root; for (Tag.PathEntry path : paths) { final int pathIndex = path.index(); final Entry entry = local.entries.get(pathIndex); - if (entry instanceof PathEntry pathEntry) { + if (entry != null && entry.tag.entry.isPath()) { // Existing path, continue navigating - assert pathEntry.value.parent == local : "Path parent is invalid: " + pathEntry.value.parent + " != " + local; - local = pathEntry.value; + final Node tmp = (Node) entry.value; + assert tmp.parent == local : "Path parent is invalid: " + tmp.parent + " != " + local; + local = tmp; } else { if (!present) return null; - // Empty path, create a new handler. - // Slow path is taken if the entry comes from a Structure tag, requiring conversion from NBT - TagHandlerImpl tmp = local; - local = new TagHandlerImpl(tmp); - if (entry != null && entry.nbt() instanceof NBTCompound compound) { - local.updateContent(compound); + synchronized (this) { + var synEntry = local.entries.get(pathIndex); + if (synEntry != null && synEntry.tag.entry.isPath()) { + // Existing path, continue navigating + final Node tmp = (Node) synEntry.value; + assert tmp.parent == local : "Path parent is invalid: " + tmp.parent + " != " + local; + local = tmp; + continue; + } + + // Empty path, create a new handler. + // Slow path is taken if the entry comes from a Structure tag, requiring conversion from NBT + Node tmp = local; + local = new Node(tmp); + if (synEntry != null && synEntry.nbt() instanceof NBTCompound compound) { + local.updateContent(compound); + } + tmp.entries.put(pathIndex, Entry.makePathEntry(path.name(), local)); } - tmp.entries.put(pathIndex, new PathEntry(path.name(), local)); } } return local; } - private synchronized Cache updatedCache() { - Cache cache; - if (!CACHE_ENABLE || (cache = this.cache) == null) { - final SPMCMap entries = this.entries; - if (!entries.isEmpty()) { - MutableNBTCompound tmp = new MutableNBTCompound(); - for (Entry entry : entries.values()) { - if (entry != null) tmp.put(entry.tag().getKey(), entry.nbt()); - } - cache = new Cache(entries.clone(), tmp.toCompound()); - } else cache = Cache.EMPTY; - this.cache = cache; - } - return cache; - } - - private static T read(Int2ObjectOpenHashMap> entries, Tag tag, - Supplier rootCompoundSupplier) { - final Tag.PathEntry[] paths = tag.path; - TagHandlerImpl pathHandler = null; - if (paths != null) { - if ((pathHandler = traversePathRead(paths, entries)) == null) - return tag.createDefault(); // Must be a path-able entry, but not present - entries = pathHandler.entries; - } - - if (tag.isView()) { - return tag.read(pathHandler != null ? - pathHandler.asCompound() : rootCompoundSupplier.get()); - } - - final Entry entry; - if ((entry = entries.get(tag.index)) == null) { - return tag.createDefault(); - } - if (entry.tag().shareValue(tag)) { - // The tag used to write the entry is compatible with the one used to get - // return the value directly - //noinspection unchecked - return (T) entry.value(); - } - // Value must be parsed from nbt if the tag is different - final NBT nbt = entry.nbt(); - final Serializers.Entry serializerEntry = tag.entry; - final NBTType type = serializerEntry.nbtType(); - return type == null || type == nbt.getID() ? serializerEntry.read(nbt) : tag.createDefault(); - } - - private static TagHandlerImpl traversePathRead(Tag.PathEntry[] paths, - Int2ObjectOpenHashMap> entries) { - assert paths != null && paths.length > 0; - TagHandlerImpl result = null; - for (var path : paths) { - final Entry entry; - if ((entry = entries.get(path.index())) == null) - return null; - if (entry instanceof PathEntry pathEntry) { - result = pathEntry.value; - } else if (entry.nbt() instanceof NBTCompound compound) { - // Slow path forcing a conversion of the structure to NBTCompound - // TODO should the handler be cached inside the entry? - result = fromCompound(compound); + private Entry valueToEntry(Node parent, Tag tag, @NotNull T value) { + if (value instanceof NBT nbt) { + if (nbt instanceof NBTCompound compound) { + final TagHandlerImpl handler = fromCompound(compound); + return Entry.makePathEntry(tag, new Node(parent, handler.root.entries)); } else { - // Entry is not path-able - return null; + final var nbtEntry = TagNbtSeparator.separateSingle(tag.getKey(), nbt); + return new Entry<>(nbtEntry.tag(), nbtEntry.value()); } - assert result != null; - entries = result.entries; + } else { + return new Entry<>(tag, tag.copyValue(value)); } - assert result != null; - return result; } - private record Cache(Int2ObjectOpenHashMap> entries, NBTCompound compound) implements TagReadable { - static final Cache EMPTY = new Cache(new Int2ObjectOpenHashMap<>(), NBTCompound.EMPTY); + final class Node implements TagReadable { + final Node parent; + final StaticIntMap> entries; + NBTCompound compound; + + public Node(Node parent, StaticIntMap> entries) { + this.parent = parent; + this.entries = entries; + } + + Node(Node parent) { + this(parent, new StaticIntMap.Array<>()); + } + + Node() { + this(null); + } @Override public @UnknownNullability T getTag(@NotNull Tag tag) { - return read(entries, tag, () -> compound); + final Node node = traversePathRead(this, tag); + if (node == null) + return tag.createDefault(); // Must be a path-able entry, but not present + if (tag.isView()) return tag.read(node.compound()); + + final StaticIntMap> entries = node.entries; + final Entry entry = entries.get(tag.index); + if (entry == null) + return tag.createDefault(); // Not present + if (entry.tag().shareValue(tag)) { + // The tag used to write the entry is compatible with the one used to get + // return the value directly + //noinspection unchecked + return (T) entry.value(); + } + // Value must be parsed from nbt if the tag is different + final NBT nbt = entry.nbt(); + final Serializers.Entry serializerEntry = tag.entry; + final NBTType type = serializerEntry.nbtType(); + return type == null || type == nbt.getID() ? serializerEntry.read(nbt) : tag.createDefault(); + } + + void updateContent(@NotNull NBTCompoundLike compoundLike) { + final NBTCompound compound = compoundLike.toCompound(); + final TagHandlerImpl converted = fromCompound(compound); + this.entries.updateContent(converted.root.entries); + this.compound = compound; + } + + NBTCompound compound() { + NBTCompound compound; + if (!CACHE_ENABLE || (compound = this.compound) == null) { + MutableNBTCompound tmp = new MutableNBTCompound(); + this.entries.forValues(entry -> { + final Tag tag = entry.tag(); + final NBT nbt = entry.nbt(); + if (!tag.entry.isPath() || !((NBTCompound) nbt).isEmpty()) { + tmp.put(tag.getKey(), nbt); + } + }); + this.compound = compound = tmp.toCompound(); + } + return compound; + } + + @Contract("null -> !null") + Node copy(Node parent) { + MutableNBTCompound tmp = new MutableNBTCompound(); + Node result = new Node(parent, new StaticIntMap.Array<>()); + StaticIntMap> entries = result.entries; + this.entries.forValues(entry -> { + Tag tag = entry.tag; + Object value = entry.value; + NBT nbt; + if (value instanceof Node node) { + Node copy = node.copy(result); + if (copy == null) + return; // Empty node + value = copy; + nbt = copy.compound; + assert nbt != null : "Node copy should also compute the compound"; + } else { + nbt = entry.nbt(); + } + + tmp.put(tag.getKey(), nbt); + entries.put(tag.index, valueToEntry(result, tag, value)); + }); + if (tmp.isEmpty() && parent != null) + return null; // Empty child node + result.compound = tmp.toCompound(); + return result; + } + + void invalidate() { + Node tmp = this; + do tmp.compound = null; + while ((tmp = tmp.parent) != null); + TagHandlerImpl.this.copy = null; } } - private sealed interface Entry - permits TagEntry, PathEntry { - Tag tag(); - - T value(); - - NBT nbt(); - - void updateValue(T value); - } - - private static final class TagEntry implements Entry { + private static final class Entry { private final Tag tag; - volatile T value; - volatile NBT nbt; + T value; + NBT nbt; - TagEntry(Tag tag, T value) { + Entry(Tag tag, T value) { this.tag = tag; this.value = value; } - @Override + static Entry makePathEntry(String path, Node node) { + return new Entry<>(Tag.tag(path, NODE_SERIALIZER), node); + } + + static Entry makePathEntry(Tag tag, Node node) { + return makePathEntry(tag.getKey(), node); + } + public Tag tag() { return tag; } - @Override public T value() { return value; } - @Override public NBT nbt() { + if (tag.entry.isPath()) return ((Node) value).compound(); NBT nbt = this.nbt; if (nbt == null) this.nbt = nbt = tag.entry.write(value); return nbt; } - @Override public void updateValue(T value) { + assert !tag.entry.isPath(); this.value = value; this.nbt = null; } - } - private record PathEntry(Tag tag, - TagHandlerImpl value) implements Entry { - PathEntry(String key, TagHandlerImpl value) { - this(Tag.tag(key, Serializers.PATH), value); - } - - @Override - public NBTCompound nbt() { - return value.asCompound(); - } - - @Override - public void updateValue(TagHandlerImpl value) { - throw new UnsupportedOperationException("Cannot update a path entry"); - } - } - - static final class SPMCMap extends Int2ObjectOpenHashMap> { - final TagHandlerImpl handler; - volatile boolean rehashed; - - SPMCMap(TagHandlerImpl handler) { - super(); - this.handler = handler; - assertState(); - } - - SPMCMap(TagHandlerImpl handler, Int2ObjectMap> m) { - super(m.size(), DEFAULT_LOAD_FACTOR); - this.handler = handler; - assertState(); - putAll(m); - } - - @Override - protected void rehash(int newSize) { - assertState(); - this.handler.entries = new SPMCMap(handler, this); - this.rehashed = true; - } - - @Override - public SPMCMap clone() { - return (SPMCMap) super.clone(); - } - - void invalidate() { - if (!CACHE_ENABLE) return; - TagHandlerImpl tmp = handler; - do { - tmp.cache = null; - } while ((tmp = tmp.parent) != null); - VarHandle.fullFence(); - } - - private void assertState() { - assert !rehashed; - assert handler != null; + Node toNode() { + if (tag.entry.isPath()) return (Node) value; + if (nbt() instanceof NBTCompound compound) { + // Slow path forcing a conversion of the structure to NBTCompound + // TODO should the handler be cached inside the entry? + return fromCompound(compound).root; + } + // Entry is not path-able + return null; } } } diff --git a/src/main/java/net/minestom/server/tag/TagRecord.java b/src/main/java/net/minestom/server/tag/TagRecord.java index a186d46c5..3c15d198c 100644 --- a/src/main/java/net/minestom/server/tag/TagRecord.java +++ b/src/main/java/net/minestom/server/tag/TagRecord.java @@ -71,9 +71,9 @@ final class TagRecord { } static final class Serializer implements TagSerializer { - Constructor constructor; - Entry[] entries; - Serializers.Entry serializerEntry; + final Constructor constructor; + final Entry[] entries; + final Serializers.Entry serializerEntry; Serializer(Constructor constructor, Entry[] entries) { this.constructor = constructor; diff --git a/src/test/java/net/minestom/server/item/ItemBlockTest.java b/src/test/java/net/minestom/server/item/ItemBlockTest.java new file mode 100644 index 000000000..733f5c476 --- /dev/null +++ b/src/test/java/net/minestom/server/item/ItemBlockTest.java @@ -0,0 +1,62 @@ +package net.minestom.server.item; + +import net.minestom.server.instance.block.Block; +import org.junit.jupiter.api.Test; + +import static net.minestom.server.api.TestUtils.assertEqualsSNBT; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class ItemBlockTest { + + @Test + public void canPlace() { + var item = ItemStack.builder(Material.STONE) + .meta(builder -> builder.canPlaceOn(Block.STONE)) + .build(); + assertTrue(item.meta().getCanPlaceOn().contains(Block.STONE)); + } + + @Test + public void canPlaceNbt() { + var item = ItemStack.builder(Material.STONE) + .meta(builder -> builder.canPlaceOn(Block.STONE)) + .build(); + assertEqualsSNBT(""" + {"CanPlaceOn":["minecraft:stone"]} + """, item.meta().toNBT()); + } + + @Test + public void canPlaceMismatchProperties() { + var item = ItemStack.builder(Material.STONE) + .meta(builder -> builder.canPlaceOn(Block.SANDSTONE_STAIRS.withProperty("facing", "south"))) + .build(); + assertTrue(item.meta().getCanPlaceOn().contains(Block.SANDSTONE_STAIRS)); + } + + @Test + public void canDestroy() { + var item = ItemStack.builder(Material.STONE) + .meta(builder -> builder.canDestroy(Block.STONE)) + .build(); + assertTrue(item.meta().getCanDestroy().contains(Block.STONE)); + } + + @Test + public void canDestroyNbt() { + var item = ItemStack.builder(Material.STONE) + .meta(builder -> builder.canDestroy(Block.STONE)) + .build(); + assertEqualsSNBT(""" + {"CanDestroy":["minecraft:stone"]} + """, item.meta().toNBT()); + } + + @Test + public void canDestroyMismatchProperties() { + var item = ItemStack.builder(Material.STONE) + .meta(builder -> builder.canDestroy(Block.SANDSTONE_STAIRS.withProperty("facing", "south"))) + .build(); + assertTrue(item.meta().getCanDestroy().contains(Block.SANDSTONE_STAIRS)); + } +} diff --git a/src/test/java/net/minestom/server/item/ItemDisplayTest.java b/src/test/java/net/minestom/server/item/ItemDisplayTest.java new file mode 100644 index 000000000..87700dc15 --- /dev/null +++ b/src/test/java/net/minestom/server/item/ItemDisplayTest.java @@ -0,0 +1,55 @@ +package net.minestom.server.item; + +import net.kyori.adventure.text.Component; +import net.kyori.adventure.text.serializer.gson.GsonComponentSerializer; +import org.jglrxavpok.hephaistos.nbt.NBTString; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.*; + +public class ItemDisplayTest { + + @Test + public void lore() { + var item = ItemStack.of(Material.DIAMOND_SWORD); + assertEquals(List.of(), item.getLore()); + assertNull(item.meta().toNBT().get("display")); + + { + var lore = List.of(Component.text("Hello")); + item = item.withLore(lore); + assertEquals(lore, item.getLore()); + var loreNbt = item.meta().toNBT().getCompound("display").getList("Lore"); + assertNotNull(loreNbt); + assertEquals(1, loreNbt.getSize()); + assertEquals(lore, loreNbt.asListView().stream().map(line -> GsonComponentSerializer.gson().deserialize(line.getValue())).toList()); + } + + { + var lore = List.of(Component.text("Hello"), Component.text("World")); + item = item.withLore(lore); + assertEquals(lore, item.getLore()); + var loreNbt = item.meta().toNBT().getCompound("display").getList("Lore"); + assertNotNull(loreNbt); + assertEquals(2, loreNbt.getSize()); + assertEquals(lore, loreNbt.asListView().stream().map(line -> GsonComponentSerializer.gson().deserialize(line.getValue())).toList()); + } + + { + var lore = Stream.of("string test").map(Component::text).toList(); + item = item.withLore(lore); + assertEquals(lore, item.getLore()); + var loreNbt = item.meta().toNBT().getCompound("display").getList("Lore"); + assertNotNull(loreNbt); + assertEquals(1, loreNbt.getSize()); + assertEquals(lore, loreNbt.asListView().stream().map(line -> GsonComponentSerializer.gson().deserialize(line.getValue())).toList()); + } + + // Ensure that lore can be properly removed without residual (display compound) + item = item.withLore(List.of()); + assertNull(item.meta().toNBT().get("display")); + } +} diff --git a/src/test/java/net/minestom/server/item/ItemEnchantTest.java b/src/test/java/net/minestom/server/item/ItemEnchantTest.java new file mode 100644 index 000000000..46f818bcb --- /dev/null +++ b/src/test/java/net/minestom/server/item/ItemEnchantTest.java @@ -0,0 +1,42 @@ +package net.minestom.server.item; + +import org.junit.jupiter.api.Test; + +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class ItemEnchantTest { + + @Test + public void enchant() { + var item = ItemStack.of(Material.DIAMOND_SWORD); + var enchantments = item.meta().getEnchantmentMap(); + assertTrue(enchantments.isEmpty(), "items do not have enchantments by default"); + + item = item.withMeta(meta -> meta.enchantment(Enchantment.EFFICIENCY, (short) 10)); + enchantments = item.meta().getEnchantmentMap(); + assertEquals(enchantments.size(), 1); + assertEquals(enchantments.get(Enchantment.EFFICIENCY), (short) 10); + + item = item.withMeta(meta -> meta.enchantment(Enchantment.INFINITY, (short) 5)); + enchantments = item.meta().getEnchantmentMap(); + assertEquals(enchantments.size(), 2); + assertEquals(enchantments.get(Enchantment.EFFICIENCY), (short) 10); + assertEquals(enchantments.get(Enchantment.INFINITY), (short) 5); + + item = item.withMeta(meta -> meta.enchantments(Map.of())); + enchantments = item.meta().getEnchantmentMap(); + assertTrue(enchantments.isEmpty()); + + // Ensure that enchantments can still be modified after being emptied + item = item.withMeta(meta -> meta.enchantment(Enchantment.EFFICIENCY, (short) 10)); + enchantments = item.meta().getEnchantmentMap(); + assertEquals(enchantments.get(Enchantment.EFFICIENCY), (short) 10); + + item = item.withMeta(ItemMeta.Builder::clearEnchantment); + enchantments = item.meta().getEnchantmentMap(); + assertTrue(enchantments.isEmpty()); + } +} diff --git a/src/test/java/net/minestom/server/item/ItemTest.java b/src/test/java/net/minestom/server/item/ItemTest.java index 76d027c95..02aedebd5 100644 --- a/src/test/java/net/minestom/server/item/ItemTest.java +++ b/src/test/java/net/minestom/server/item/ItemTest.java @@ -2,14 +2,8 @@ package net.minestom.server.item; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.format.NamedTextColor; -import net.kyori.adventure.text.serializer.gson.GsonComponentSerializer; -import org.jglrxavpok.hephaistos.nbt.NBTString; import org.junit.jupiter.api.Test; -import java.util.List; -import java.util.Map; -import java.util.stream.Stream; - import static org.junit.jupiter.api.Assertions.*; public class ItemTest { @@ -78,78 +72,6 @@ public class ItemTest { assertEquals(createItem(), item, "Items must be equal if created from the same meta nbt"); } - @Test - public void testEnchant() { - var item = ItemStack.of(Material.DIAMOND_SWORD); - var enchantments = item.meta().getEnchantmentMap(); - assertTrue(enchantments.isEmpty(), "items do not have enchantments by default"); - - item = item.withMeta(meta -> meta.enchantment(Enchantment.EFFICIENCY, (short) 10)); - enchantments = item.meta().getEnchantmentMap(); - assertEquals(enchantments.size(), 1); - assertEquals(enchantments.get(Enchantment.EFFICIENCY), (short) 10); - - item = item.withMeta(meta -> meta.enchantment(Enchantment.INFINITY, (short) 5)); - enchantments = item.meta().getEnchantmentMap(); - assertEquals(enchantments.size(), 2); - assertEquals(enchantments.get(Enchantment.EFFICIENCY), (short) 10); - assertEquals(enchantments.get(Enchantment.INFINITY), (short) 5); - - item = item.withMeta(meta -> meta.enchantments(Map.of())); - enchantments = item.meta().getEnchantmentMap(); - assertTrue(enchantments.isEmpty()); - - // Ensure that enchantments can still be modified after being emptied - item = item.withMeta(meta -> meta.enchantment(Enchantment.EFFICIENCY, (short) 10)); - enchantments = item.meta().getEnchantmentMap(); - assertEquals(enchantments.get(Enchantment.EFFICIENCY), (short) 10); - - item = item.withMeta(ItemMeta.Builder::clearEnchantment); - enchantments = item.meta().getEnchantmentMap(); - assertTrue(enchantments.isEmpty()); - } - - @Test - public void testLore() { - var item = ItemStack.of(Material.DIAMOND_SWORD); - assertEquals(List.of(), item.getLore()); - assertNull(item.meta().toNBT().get("display")); - - { - var lore = List.of(Component.text("Hello")); - item = item.withLore(lore); - assertEquals(lore, item.getLore()); - var loreNbt = item.meta().toNBT().getCompound("display").getList("Lore"); - assertNotNull(loreNbt); - assertEquals(1, loreNbt.getSize()); - assertEquals(lore, loreNbt.asListView().stream().map(line -> GsonComponentSerializer.gson().deserialize(line.getValue())).toList()); - } - - { - var lore = List.of(Component.text("Hello"), Component.text("World")); - item = item.withLore(lore); - assertEquals(lore, item.getLore()); - var loreNbt = item.meta().toNBT().getCompound("display").getList("Lore"); - assertNotNull(loreNbt); - assertEquals(2, loreNbt.getSize()); - assertEquals(lore, loreNbt.asListView().stream().map(line -> GsonComponentSerializer.gson().deserialize(line.getValue())).toList()); - } - - { - var lore = Stream.of("string test").map(Component::text).toList(); - item = item.withLore(lore); - assertEquals(lore, item.getLore()); - var loreNbt = item.meta().toNBT().getCompound("display").getList("Lore"); - assertNotNull(loreNbt); - assertEquals(1, loreNbt.getSize()); - assertEquals(lore, loreNbt.asListView().stream().map(line -> GsonComponentSerializer.gson().deserialize(line.getValue())).toList()); - } - - // Ensure that lore can be properly removed without residual (display compound) - item = item.withLore(List.of()); - assertNull(item.meta().toNBT().get("display")); - } - @Test public void testBuilderReuse() { var builder = ItemStack.builder(Material.DIAMOND); diff --git a/src/test/java/net/minestom/server/tag/TagAtomicTest.java b/src/test/java/net/minestom/server/tag/TagAtomicTest.java index dea05b232..272d42ebc 100644 --- a/src/test/java/net/minestom/server/tag/TagAtomicTest.java +++ b/src/test/java/net/minestom/server/tag/TagAtomicTest.java @@ -2,6 +2,7 @@ package net.minestom.server.tag; import org.junit.jupiter.api.Test; +import static net.minestom.server.api.TestUtils.assertEqualsSNBT; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; @@ -23,6 +24,48 @@ public class TagAtomicTest { assertEquals(10, handler.getTag(tag)); } + @Test + public void updateDefault() { + var tag = Tag.Integer("coin").defaultValue(25); + var handler = TagHandler.newHandler(); + handler.updateTag(tag, integer -> { + assertEquals(25, integer); + return 5; + }); + assertEquals(5, handler.getTag(tag)); + handler.updateTag(tag, integer -> { + assertEquals(5, integer); + return 10; + }); + assertEquals(10, handler.getTag(tag)); + } + + @Test + public void updateRemoval() { + var tag = Tag.Integer("coin"); + var handler = TagHandler.newHandler(); + handler.setTag(tag, 5); + handler.updateTag(tag, integer -> { + assertEquals(5, integer); + return null; + }); + assertNull(handler.getTag(tag)); + assertEqualsSNBT("{}", handler.asCompound()); + } + + @Test + public void updateRemovalPath() { + var tag = Tag.Integer("coin").path("path"); + var handler = TagHandler.newHandler(); + handler.setTag(tag, 5); + handler.updateTag(tag, integer -> { + assertEquals(5, integer); + return null; + }); + assertNull(handler.getTag(tag)); + assertEqualsSNBT("{}", handler.asCompound()); + } + @Test public void updateAndGet() { var tag = Tag.Integer("coin"); diff --git a/src/test/java/net/minestom/server/tag/TagHandlerReadableCopyTest.java b/src/test/java/net/minestom/server/tag/TagHandlerReadableCopyTest.java new file mode 100644 index 000000000..68354b49b --- /dev/null +++ b/src/test/java/net/minestom/server/tag/TagHandlerReadableCopyTest.java @@ -0,0 +1,63 @@ +package net.minestom.server.tag; + +import org.junit.jupiter.api.Test; + +import static net.minestom.server.api.TestUtils.assertEqualsSNBT; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; + +public class TagHandlerReadableCopyTest { + + @Test + public void copyCache() { + var tag = Tag.String("key"); + var handler = TagHandler.newHandler(); + handler.setTag(tag, "test"); + + var copy = handler.readableCopy(); + assertEquals(handler.getTag(tag), copy.getTag(tag)); + + handler.setTag(tag, "test2"); + assertEquals("test2", handler.getTag(tag)); + assertEquals("test", copy.getTag(tag)); + } + + @Test + public void copyCachePath() { + var tag = Tag.String("key").path("path"); + var handler = TagHandler.newHandler(); + handler.setTag(tag, "test"); + assertEqualsSNBT(""" + {"path":{"key":"test"}} + """, handler.asCompound()); + + var copy = handler.readableCopy(); + handler.setTag(tag, "test2"); + assertEquals("test2", handler.getTag(tag)); + assertEquals("test", copy.getTag(tag)); + } + + @Test + public void copyCacheReuse() { + var handler = TagHandler.newHandler(); + handler.setTag(Tag.String("key"), "test"); + assertSame(handler.readableCopy(), handler.readableCopy()); + } + + @Test + public void copyRehashing() { + var tag = Tag.String("key"); + var handler = TagHandler.newHandler(); + handler.setTag(tag, "test"); + var copy = handler.readableCopy(); + for (int i = 0; i < 1000; i++) { + handler.setTag(Tag.Integer("copyRehashing" + i), i); + } + assertEquals("test", handler.getTag(tag)); + assertEquals("test", copy.getTag(tag)); + + handler.setTag(tag, "test2"); + assertEquals("test2", handler.getTag(tag)); + assertEquals("test", copy.getTag(tag)); + } +} diff --git a/src/test/java/net/minestom/server/tag/TagTest.java b/src/test/java/net/minestom/server/tag/TagTest.java index c9a9abc75..d1376f66c 100644 --- a/src/test/java/net/minestom/server/tag/TagTest.java +++ b/src/test/java/net/minestom/server/tag/TagTest.java @@ -62,6 +62,14 @@ public class TagTest { assertEquals(mutable.toCompound(), handler.asCompound(), "NBT is not the same"); } + @Test + public void fromNbtCache() { + // Ensure that TagHandler#asCompound reuse the same compound used for construction + var compound = NBT.Compound(Map.of("key", NBT.Int(5))); + var handler = TagHandler.fromCompound(compound); + assertSame(compound, handler.asCompound(), "NBT is not the same"); + } + @Test public void defaultValue() { var nullable = Tag.String("key");