From b5bcd8fd4a0b84c7a3d015638454ca793313ad6d Mon Sep 17 00:00:00 2001 From: TheMode Date: Tue, 25 Jan 2022 13:33:14 +0100 Subject: [PATCH] Avoid string internal during properties parsing Signed-off-by: TheMode --- gradle/libs.versions.toml | 2 +- .../server/instance/block/BlockImpl.java | 81 +++++++++++++------ .../server/utils/block/BlockUtils.java | 4 +- .../minestom/server/instance/BlockTest.java | 5 ++ 4 files changed, 66 insertions(+), 26 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 8e8c393cd..82ebbca38 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -7,7 +7,7 @@ adventure = "4.9.3" kotlin = "1.6.10" hydrazine = "1.7.2" dependencyGetter = "v1.0.1" -minestomData = "c7caadddbf" +minestomData = "eadcb99e14" hephaistos = "2.4.1" jetbrainsAnnotations = "23.0.0" diff --git a/src/main/java/net/minestom/server/instance/block/BlockImpl.java b/src/main/java/net/minestom/server/instance/block/BlockImpl.java index c69286466..82af30dc4 100644 --- a/src/main/java/net/minestom/server/instance/block/BlockImpl.java +++ b/src/main/java/net/minestom/server/instance/block/BlockImpl.java @@ -16,20 +16,19 @@ import org.jglrxavpok.hephaistos.nbt.NBTCompound; import org.jglrxavpok.hephaistos.nbt.mutable.MutableNBTCompound; import java.time.Duration; -import java.util.Arrays; -import java.util.Collection; -import java.util.Map; -import java.util.Objects; +import java.util.*; import java.util.function.Function; record BlockImpl(@NotNull Registry.BlockEntry registry, - @NotNull String[] propertiesArray, + @NotNull int[] propertiesArray, @Nullable NBTCompound nbt, @Nullable BlockHandler handler) implements Block { // Block state -> block object private static final ObjectArray BLOCK_STATE_MAP = new ObjectArray<>(); // Block id -> valid property keys (order is important for lookup) private static final ObjectArray PROPERTIES_KEYS = new ObjectArray<>(); + // Block id -> [property key -> values] + private static final ObjectArray PROPERTIES_VALUES = new ObjectArray<>(); // Block id -> Map private static final ObjectArray> POSSIBLE_STATES = new ObjectArray<>(); private static final Registry.Container CONTAINER = Registry.createContainer(Registry.Resource.BLOCKS, @@ -38,18 +37,25 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, final var stateObject = (Map) object.get("states"); // Retrieve properties - String[] possibleProperties = new String[0]; + String[] keys = new String[0]; + String[][] values = new String[0][]; { var properties = (Map) object.get("properties"); if (properties != null) { - possibleProperties = new String[properties.size()]; + keys = new String[properties.size()]; + values = new String[properties.size()][]; int i = 0; for (var entry : properties.entrySet()) { - possibleProperties[i++] = entry.getKey(); + final int entryIndex = i++; + keys[entryIndex] = entry.getKey(); + + final var v = (List) entry.getValue(); + values[entryIndex] = v.toArray(String[]::new); } } } - PROPERTIES_KEYS.set(blockId, possibleProperties); + PROPERTIES_KEYS.set(blockId, keys); + PROPERTIES_VALUES.set(blockId, values); // Retrieve block states { @@ -63,10 +69,19 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, final var stateOverride = (Map) stateEntry.getValue(); final var propertyMap = BlockUtils.parseProperties(query); - String[] propertiesArray = new String[possibleProperties.length]; + int[] propertiesArray = new int[keys.length]; int i = 0; for (var entry : propertyMap.entrySet()) { - propertiesArray[i++] = entry.getValue(); + final int entryIndex = i++; + final int keyIndex = ArrayUtils.indexOf(keys, entry.getKey()); + if (keyIndex == -1) { + throw new IllegalArgumentException("Unknown property key: " + entry.getKey()); + } + final int valueIndex = ArrayUtils.indexOf(values[keyIndex], entry.getValue()); + if (valueIndex == -1) { + throw new IllegalArgumentException("Unknown property value: " + entry.getValue()); + } + propertiesArray[entryIndex] = valueIndex; } final BlockImpl block = new BlockImpl(Registry.block(namespace, object, stateOverride), @@ -87,6 +102,8 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, .build(); static { + PROPERTIES_KEYS.trim(); + PROPERTIES_VALUES.trim(); BLOCK_STATE_MAP.trim(); POSSIBLE_STATES.trim(); } @@ -114,12 +131,19 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, @Override public @NotNull Block withProperty(@NotNull String property, @NotNull String value) { final String[] keys = PROPERTIES_KEYS.get(id()); - final int index = ArrayUtils.indexOf(keys, property); - if (index == -1) { + final String[][] values = PROPERTIES_VALUES.get(id()); + assert keys != null; + assert values != null; + final int keyIndex = ArrayUtils.indexOf(keys, property); + if (keyIndex == -1) { throw new IllegalArgumentException("Property " + property + " is not valid for block " + this); } + final int valueIndex = ArrayUtils.indexOf(values[keyIndex], value); + if (valueIndex == -1) { + throw new IllegalArgumentException("Value " + value + " is not valid for property " + property + " of block " + this); + } var properties = this.propertiesArray.clone(); - properties[index] = value; + properties[keyIndex] = valueIndex; return compute(properties); } @@ -127,14 +151,20 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, public @NotNull Block withProperties(@NotNull Map<@NotNull String, @NotNull String> properties) { if (properties.isEmpty()) return this; final String[] keys = PROPERTIES_KEYS.get(id()); + final String[][] values = PROPERTIES_VALUES.get(id()); assert keys != null; - String[] result = this.propertiesArray.clone(); + assert values != null; + int[] result = this.propertiesArray.clone(); for (var entry : properties.entrySet()) { - final int index = ArrayUtils.indexOf(keys, entry.getKey()); - if (index == -1) { + final int keyIndex = ArrayUtils.indexOf(keys, entry.getKey()); + if (keyIndex == -1) { throw new IllegalArgumentException("Property " + entry.getKey() + " is not valid for block " + this); } - result[index] = entry.getValue(); + final int valueIndex = ArrayUtils.indexOf(values[keyIndex], entry.getValue()); + if (valueIndex == -1) { + throw new IllegalArgumentException("Value " + entry.getValue() + " is not valid for property " + entry.getKey() + " of block " + this); + } + result[keyIndex] = valueIndex; } return compute(result); } @@ -155,9 +185,14 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, @Override public @Unmodifiable @NotNull Map properties() { final String[] keys = PROPERTIES_KEYS.get(id()); + final String[][] values = PROPERTIES_VALUES.get(id()); assert keys != null; - var map = new Object2ObjectArrayMap<>(keys, propertiesArray, keys.length); - return Map.class.cast(Object2ObjectMaps.unmodifiable(map)); + assert values != null; + String[] finalValues = new String[keys.length]; + for (int i = 0; i < keys.length; i++) { + finalValues[i] = values[i][propertiesArray[i]]; + } + return Map.class.cast(Object2ObjectMaps.unmodifiable(new Object2ObjectArrayMap<>(keys, finalValues, keys.length))); } @Override @@ -191,7 +226,7 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, return Objects.hash(stateId(), nbt, handler); } - private Block compute(String[] properties) { + private Block compute(int[] properties) { if (Arrays.equals(propertiesArray, properties)) return this; BlockImpl block = possibleProperties().get(new PropertiesHolder(properties)); if (block == null) @@ -200,10 +235,10 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, } private static final class PropertiesHolder { - private final String[] properties; + private final int[] properties; private final int hashCode; - public PropertiesHolder(String[] properties) { + public PropertiesHolder(int[] properties) { this.properties = properties; this.hashCode = Arrays.hashCode(properties); } diff --git a/src/main/java/net/minestom/server/utils/block/BlockUtils.java b/src/main/java/net/minestom/server/utils/block/BlockUtils.java index e3640f8fd..eebd62035 100644 --- a/src/main/java/net/minestom/server/utils/block/BlockUtils.java +++ b/src/main/java/net/minestom/server/utils/block/BlockUtils.java @@ -81,8 +81,8 @@ public class BlockUtils { if (equalIndex != -1 && equalIndex < index) { final String key = query.substring(start, equalIndex).trim(); final String value = query.substring(equalIndex + 1, index).trim(); - keys[entryCount] = key.intern(); - values[entryCount++] = value.intern(); + keys[entryCount] = key; + values[entryCount++] = value; } start = index + 1; } diff --git a/src/test/java/net/minestom/server/instance/BlockTest.java b/src/test/java/net/minestom/server/instance/BlockTest.java index 107d77f80..bc74cfd6f 100644 --- a/src/test/java/net/minestom/server/instance/BlockTest.java +++ b/src/test/java/net/minestom/server/instance/BlockTest.java @@ -38,6 +38,11 @@ public class BlockTest { Block block = Block.CHEST; assertEquals(block.properties(), Objects.requireNonNull(Block.fromBlockId(block.id())).properties()); + // Default state may change, but the test is required to ensure the `properties` method is working + assertEquals(Map.of("facing", "north", + "type", "single", + "waterlogged", "false"), block.properties()); + for (var possible : block.possibleStates()) { assertEquals(possible, block.withProperties(possible.properties())); }