New optimization: Deduplicate predicates used for multipart models

Saves a few 100 MB on FC

build.gradle

@@ -39,6 +39,7 @@ minecraft {
         client {
             workingDirectory project.file('run')
             arg "-mixin.config="+archivesBaseName+".mixins.json"
+            property 'mixin.env.disableRefMap', 'true'
 
 
             // Recommended logging level for the console
@@ -55,6 +56,7 @@ minecraft {
             workingDirectory project.file('run')
             arg "-mixin.config="+archivesBaseName+".mixins.json"
             arg "-nogui"
+            property 'mixin.env.disableRefMap', 'true'
 
 
             // Recommended logging level for the console

src/main/java/malte0811/ferritecore/CachedOrPredicates.java

@@ -0,0 +1,20 @@
+package malte0811.ferritecore;
+
+import net.minecraft.block.BlockState;
+
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Predicate;
+
+public class CachedOrPredicates {
+
+    public static final Map<List<Predicate<BlockState>>, Predicate<BlockState>> OR_PREDICATE_CACHE = new ConcurrentHashMap<>();
+
+    public static Predicate<BlockState> or(List<Predicate<BlockState>> list) {
+        return OR_PREDICATE_CACHE.computeIfAbsent(
+                list,
+                listInt -> state -> listInt.stream().anyMatch((predicate) -> predicate.test(state))
+        );
+    }
+}

src/main/java/malte0811/ferritecore/FastMap.java

@@ -52,7 +52,7 @@ public class FastMap<Value> {
         // It might be possible to speed this up by sorting the keys by their hash code and using a binary search,
         // however I do not think that it would actually be faster in practice.
         for (Key<?> key : keys) {
-            if (key.getProperty() == prop) {
+            if (key.getProperty().equals(prop)) {
                 return (Key<T>) key;
             }
         }

src/main/java/malte0811/ferritecore/mixin/AndConditionMixin.java

@@ -0,0 +1,36 @@
+package malte0811.ferritecore.mixin;
+
+import malte0811.ferritecore.util.PredicateHelper;
+import net.minecraft.block.Block;
+import net.minecraft.block.BlockState;
+import net.minecraft.client.renderer.model.multipart.AndCondition;
+import net.minecraft.client.renderer.model.multipart.ICondition;
+import net.minecraft.state.StateContainer;
+import org.spongepowered.asm.mixin.Final;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Overwrite;
+import org.spongepowered.asm.mixin.Shadow;
+
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Predicate;
+
+@Mixin(AndCondition.class)
+public class AndConditionMixin {
+    private static final Map<List<Predicate<BlockState>>, Predicate<BlockState>> COMBINED_PREDICATE_CACHE = new ConcurrentHashMap<>();
+
+    @Shadow @Final private Iterable<? extends ICondition> conditions;
+
+    /**
+     * @reason Use cached result predicates
+     * @author malte0811
+     */
+    @Overwrite
+    public Predicate<BlockState> getPredicate(StateContainer<Block, BlockState> stateContainer) {
+        return COMBINED_PREDICATE_CACHE.computeIfAbsent(
+                PredicateHelper.toCanonicalList(conditions, stateContainer),
+                listInt -> state -> listInt.stream().allMatch((predicate) -> predicate.test(state))
+        );
+    }
+}

src/main/java/malte0811/ferritecore/mixin/MixinPropertyValueCondition.java

@@ -1,45 +0,0 @@
-package malte0811.ferritecore.mixin;
-
-import net.minecraft.block.Block;
-import net.minecraft.block.BlockState;
-import net.minecraft.client.renderer.model.multipart.PropertyValueCondition;
-import net.minecraft.state.Property;
-import net.minecraft.state.StateContainer;
-import org.spongepowered.asm.mixin.Final;
-import org.spongepowered.asm.mixin.Mixin;
-import org.spongepowered.asm.mixin.Overwrite;
-import org.spongepowered.asm.mixin.Shadow;
-
-import java.util.Optional;
-import java.util.function.Predicate;
-
-@Mixin(PropertyValueCondition.class)
-public class MixinPropertyValueCondition {
-    @Shadow
-    @Final
-    private String key;
-    @Shadow
-    @Final
-    private String value;
-
-    /**
-     * @reason The vanilla implementation captures an Optional in the resulting lambda, which eats a lot of memory. A
-     * less aggressive Mixin would require fiddling with local variables, which is more effort than it would be worth
-     * for such an obscure method.
-     * @author malte0811
-     */
-    @Overwrite
-    private <T extends Comparable<T>>
-    Predicate<BlockState> makePropertyPredicate(
-            StateContainer<Block, BlockState> container, Property<T> property, String value
-    ) {
-        Optional<T> optional = property.parseValue(value);
-        if (!optional.isPresent()) {
-            throw new RuntimeException(String.format("Unknown value '%s' for property '%s' on '%s' in '%s'", value, this.key, container.getOwner().toString(), this.value));
-        } else {
-            // These are the only relevant lines. In vanilla optional.get is called inside the lambda
-            T unwrapped = optional.get();
-            return (state) -> state.get(property).equals(unwrapped);
-        }
-    }
-}

src/main/java/malte0811/ferritecore/mixin/OrConditionMixin.java

@@ -0,0 +1,37 @@
+package malte0811.ferritecore.mixin;
+
+import malte0811.ferritecore.CachedOrPredicates;
+import malte0811.ferritecore.util.PredicateHelper;
+import net.minecraft.block.Block;
+import net.minecraft.block.BlockState;
+import net.minecraft.client.renderer.model.multipart.AndCondition;
+import net.minecraft.client.renderer.model.multipart.ICondition;
+import net.minecraft.client.renderer.model.multipart.OrCondition;
+import net.minecraft.state.StateContainer;
+import org.spongepowered.asm.mixin.Final;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Overwrite;
+import org.spongepowered.asm.mixin.Shadow;
+
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Predicate;
+
+import malte0811.ferritecore.HackyGlobalState;
+
+@Mixin(OrCondition.class)
+public class OrConditionMixin {
+    @Shadow @Final private Iterable<? extends ICondition> conditions;
+
+    /**
+     * @reason Use cached result predicates
+     * @author malte0811
+     */
+    @Overwrite
+    public Predicate<BlockState> getPredicate(StateContainer<Block, BlockState> stateContainer) {
+        return CachedOrPredicates.or(PredicateHelper.toCanonicalList(conditions, stateContainer));
+    }
+}

src/main/java/malte0811/ferritecore/mixin/PropertyValueConditionMixin.java

@@ -0,0 +1,103 @@
+package malte0811.ferritecore.mixin;
+
+import malte0811.ferritecore.CachedOrPredicates;
+import net.minecraft.block.Block;
+import net.minecraft.block.BlockState;
+import net.minecraft.client.renderer.model.multipart.PropertyValueCondition;
+import net.minecraft.state.Property;
+import net.minecraft.state.StateContainer;
+import org.apache.commons.lang3.tuple.Pair;
+import org.spongepowered.asm.mixin.Final;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Overwrite;
+import org.spongepowered.asm.mixin.Shadow;
+ 
+import com.google.common.base.Splitter;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Predicate;
+
+import malte0811.ferritecore.HackyGlobalState;
+
+@Mixin(PropertyValueCondition.class)
+public class PropertyValueConditionMixin {
+    private static final Map<Pair<Property<?>, Comparable<?>>, Predicate<BlockState>> STATE_HAS_PROPERTY_CACHE = new ConcurrentHashMap<>();
+
+    @Shadow
+    @Final
+    private String key;
+    @Shadow
+    @Final
+    private String value;
+    @Shadow
+    @Final
+    private static Splitter SPLITTER;
+
+   /**
+    * @reason Use cached predicates in the case of multiple specified values
+    * TODO this should be possible with a less invasive mixin?
+    * @author malte0811
+    */
+   @Overwrite
+   public Predicate<BlockState> getPredicate(StateContainer<Block, BlockState> stateContainer) {
+      Property<?> property = stateContainer.getProperty(this.key);
+      if (property == null) {
+         throw new RuntimeException(String.format("Unknown property '%s' on '%s'", this.key, stateContainer.getOwner().toString()));
+      } else {
+         String valueNoInvert = this.value;
+         boolean invert = !valueNoInvert.isEmpty() && valueNoInvert.charAt(0) == '!';
+         if (invert) {
+            valueNoInvert = valueNoInvert.substring(1);
+         }
+
+         List<String> matchedStates = SPLITTER.splitToList(valueNoInvert);
+         if (matchedStates.isEmpty()) {
+            throw new RuntimeException(String.format("Empty value '%s' for property '%s' on '%s'", this.value, this.key, stateContainer.getOwner().toString()));
+         } else {
+            Predicate<BlockState> isMatchedState;
+            if (matchedStates.size() == 1) {
+               isMatchedState = this.makePropertyPredicate(stateContainer, property, valueNoInvert);
+            } else {
+               List<Predicate<BlockState>> subPredicates = matchedStates.stream()
+                       .map(subValue -> this.makePropertyPredicate(stateContainer, property, subValue))
+                       .collect(Collectors.toList());
+               isMatchedState = CachedOrPredicates.or(subPredicates);
+            }
+
+            return invert ? isMatchedState.negate() : isMatchedState;
+         }
+      }
+   }
+
+    /**
+     * @reason The vanilla implementation captures an Optional in the resulting lambda, which eats a lot of memory. A
+     * less aggressive Mixin would require fiddling with local variables, which is more effort than it would be worth
+     * for such an obscure method.
+     * Also adds a cache to avoid creating two Predicate instances for the same lookup
+     * @author malte0811
+     */
+    @Overwrite
+    private <T extends Comparable<T>>
+    Predicate<BlockState> makePropertyPredicate(
+            StateContainer<Block, BlockState> container, Property<T> property, String value
+    ) {
+        Optional<T> optional = property.parseValue(value);
+        if (!optional.isPresent()) {
+            throw new RuntimeException(String.format("Unknown value '%s' for property '%s' on '%s' in '%s'", value, this.key, container.getOwner().toString(), this.value));
+        } else {
+            T unwrapped = optional.get();
+            Pair<Property<?>, Comparable<?>> key = Pair.of(property, unwrapped);
+            return STATE_HAS_PROPERTY_CACHE.computeIfAbsent(
+                    key,
+                    pair -> {
+                        Comparable<?> valueInt = pair.getRight();
+                        Property<?> propInt = pair.getLeft();
+                        return state -> state.get(propInt).equals(valueInt);
+                    }
+            );
+        }
+    }
+}

src/main/java/malte0811/ferritecore/util/PredicateHelper.java

@@ -0,0 +1,25 @@
+package malte0811.ferritecore.util;
+
+import net.minecraft.block.Block;
+import net.minecraft.block.BlockState;
+import net.minecraft.client.renderer.model.multipart.ICondition;
+import net.minecraft.state.StateContainer;
+
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.function.Predicate;
+
+public class PredicateHelper {
+    public static List<Predicate<BlockState>> toCanonicalList(
+            Iterable<? extends ICondition> conditions, StateContainer<Block, BlockState> stateContainer
+    ) {
+        ArrayList<Predicate<BlockState>> list = new ArrayList<>();
+        for (ICondition cond : conditions) {
+            list.add(cond.getPredicate(stateContainer));
+        }
+        list.sort(Comparator.comparingInt(Predicate::hashCode));
+        list.trimToSize();
+        return list;
+    }
+}

src/main/resources/ferritecore.mixins.json

@@ -7,7 +7,9 @@
     "StateholderMixin"
   ],
   "client": [
-    "MixinPropertyValueCondition"
+    "OrConditionMixin",
+    "AndConditionMixin",
+    "PropertyValueConditionMixin"
   ],
   "injectors": {
     "defaultRequire": 1