From 18151e271fbe05977bfce6b2fe68f9ea4fa7969b Mon Sep 17 00:00:00 2001 From: Madeline Miller Date: Sat, 17 Feb 2024 16:00:47 +1000 Subject: [PATCH 1/3] Slightly optimise and improve debugability of state creation code --- .../worldedit/world/block/BlockState.java | 50 +++++++++++++------ 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/world/block/BlockState.java b/worldedit-core/src/main/java/com/sk89q/worldedit/world/block/BlockState.java index ec1d36c4d1..dfd72d76ac 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/world/block/BlockState.java +++ b/worldedit-core/src/main/java/com/sk89q/worldedit/world/block/BlockState.java @@ -79,22 +79,30 @@ public void setInternalId(BlockState blockState, int internalId) { this.emptyBaseBlock = new BaseBlock(this); } + /** + * Generates a map of all possible states for a block type. + * + * @param blockType The block type + * @return The map of states + */ static Map, Object>, BlockState> generateStateMap(BlockType blockType) { - ImmutableMap.Builder, Object>, BlockState> stateMapBuilder = ImmutableMap.builder(); List> properties = blockType.getProperties(); + ImmutableMap.Builder, Object>, BlockState> stateMapBuilder = null; if (!properties.isEmpty()) { - List> separatedValues = Lists.newArrayList(); + // Create a list of lists of values, with a copy of the underlying lists + List> separatedValues = Lists.newArrayListWithCapacity(properties.size()); for (Property prop : properties) { - List vals = Lists.newArrayList(); - vals.addAll(prop.getValues()); - separatedValues.add(vals); + separatedValues.add(Lists.newArrayList(prop.getValues())); } + List> valueLists = Lists.cartesianProduct(separatedValues); + stateMapBuilder = ImmutableMap.builderWithExpectedSize(valueLists.size()); for (List valueList : valueLists) { Map, Object> valueMap = Maps.newTreeMap(Comparator.comparing(Property::getName)); BlockState stateMaker = new BlockState(blockType); - for (int i = 0; i < valueList.size(); i++) { + int valueCount = valueList.size(); + for (int i = 0; i < valueCount; i++) { Property property = properties.get(i); Object value = valueList.get(i); valueMap.put(property, value); @@ -104,27 +112,39 @@ static Map, Object>, BlockState> generateStateMap(BlockType bloc } } - ImmutableMap, Object>, BlockState> stateMap = stateMapBuilder.build(); + ImmutableMap, Object>, BlockState> stateMap; - if (stateMap.isEmpty()) { + if (stateMapBuilder == null) { // No properties. stateMap = ImmutableMap.of(ImmutableMap.of(), new BlockState(blockType)); + } else { + stateMap = stateMapBuilder.build(); } + Watchdog watchdog = WorldEdit.getInstance().getPlatformManager().queryCapability(Capability.GAME_HOOKS) + .getWatchdog(); + long startTime = System.currentTimeMillis(); + for (BlockState state : stateMap.values()) { state.populate(stateMap); - } - // Sometimes loading can take a while. This is the perfect spot to let MC know we're working. - Watchdog watchdog = WorldEdit.getInstance().getPlatformManager().queryCapability(Capability.GAME_HOOKS) - .getWatchdog(); - if (watchdog != null) { - watchdog.tick(); + // Sometimes loading can take a while. This is the perfect spot to let MC know we're working. + if (watchdog != null) { + watchdog.tick(); + } + } + if (System.currentTimeMillis() - startTime > 60000) { + WorldEdit.logger.warn("Took more than 60 seconds to generate complete state map for " + blockType.getId() + ". This block is likely improperly using properties. State count: " + stateMap.size()); } return stateMap; } + /** + * Creates the underlying state table for object lookups. + * + * @param stateMap The state map to generate the table from + */ private void populate(Map, Object>, BlockState> stateMap) { final ImmutableTable.Builder, Object, BlockState> states = ImmutableTable.builder(); @@ -148,7 +168,7 @@ private void populate(Map, Object>, BlockState> stateMap) { } private Map, Object> withValue(final Property property, final V value) { - final ImmutableMap.Builder, Object> values = ImmutableMap.builder(); + final ImmutableMap.Builder, Object> values = ImmutableMap.builderWithExpectedSize(this.values.size()); for (Map.Entry, Object> entry : this.values.entrySet()) { if (entry.getKey().equals(property)) { values.put(entry.getKey(), value); From 0bb906752e87556a4c25eb968aee27669e439afb Mon Sep 17 00:00:00 2001 From: Madeline Miller Date: Sat, 17 Feb 2024 16:05:39 +1000 Subject: [PATCH 2/3] Make 5s rather than 60, and add time taken to the output --- .../java/com/sk89q/worldedit/world/block/BlockState.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/world/block/BlockState.java b/worldedit-core/src/main/java/com/sk89q/worldedit/world/block/BlockState.java index dfd72d76ac..646e423500 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/world/block/BlockState.java +++ b/worldedit-core/src/main/java/com/sk89q/worldedit/world/block/BlockState.java @@ -133,8 +133,9 @@ static Map, Object>, BlockState> generateStateMap(BlockType bloc watchdog.tick(); } } - if (System.currentTimeMillis() - startTime > 60000) { - WorldEdit.logger.warn("Took more than 60 seconds to generate complete state map for " + blockType.getId() + ". This block is likely improperly using properties. State count: " + stateMap.size()); + long timeTaken = System.currentTimeMillis() - startTime; + if (timeTaken > 5000) { + WorldEdit.logger.warn("Took more than 5 seconds to generate complete state map for " + blockType.getId() + ". This block is likely improperly using properties. State count: " + stateMap.size() + ". " + timeTaken + "ms elapsed."); } return stateMap; From a41f56082d8ead3b8d6581f37243d371e8dd68d0 Mon Sep 17 00:00:00 2001 From: Madeline Miller Date: Sat, 17 Feb 2024 16:17:04 +1000 Subject: [PATCH 3/3] Use an ImmutableList here --- .../main/java/com/sk89q/worldedit/world/block/BlockState.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/world/block/BlockState.java b/worldedit-core/src/main/java/com/sk89q/worldedit/world/block/BlockState.java index 646e423500..97e1876a44 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/world/block/BlockState.java +++ b/worldedit-core/src/main/java/com/sk89q/worldedit/world/block/BlockState.java @@ -19,6 +19,7 @@ package com.sk89q.worldedit.world.block; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableTable; import com.google.common.collect.Lists; @@ -93,7 +94,7 @@ static Map, Object>, BlockState> generateStateMap(BlockType bloc // Create a list of lists of values, with a copy of the underlying lists List> separatedValues = Lists.newArrayListWithCapacity(properties.size()); for (Property prop : properties) { - separatedValues.add(Lists.newArrayList(prop.getValues())); + separatedValues.add(ImmutableList.copyOf(prop.getValues())); } List> valueLists = Lists.cartesianProduct(separatedValues);