Merge remote-tracking branch 'origin/1.19.2' into 1.20

Author: embeddedt

common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/BlockModelShaperMixin.java

@@ -6,6 +6,7 @@
 import net.minecraft.client.resources.model.ModelResourceLocation;
 import net.minecraft.world.level.block.state.BlockState;
 import org.embeddedt.modernfix.annotation.ClientOnlyMixin;
+import org.embeddedt.modernfix.dynamicresources.DynamicModelCache;
 import org.embeddedt.modernfix.dynamicresources.ModelLocationCache;
 import org.embeddedt.modernfix.util.DynamicOverridableMap;
 import org.spongepowered.asm.mixin.*;
@@ -15,7 +16,6 @@
 
 import java.util.Map;
 
-
 @Mixin(BlockModelShaper.class)
 @ClientOnlyMixin
 public class BlockModelShaperMixin {
@@ -24,23 +24,33 @@ public class BlockModelShaperMixin {
     @Shadow
     private Map<BlockState, BakedModel> modelByStateCache;
 
+    private final DynamicModelCache<BlockState> mfix$modelCache = new DynamicModelCache<>(k -> this.cacheBlockModel((BlockState)k), false);
+
     @Inject(method = { "<init>", "replaceCache" }, at = @At("RETURN"))
     private void replaceModelMap(CallbackInfo ci) {
         // replace the backing map for mods which will access it
         this.modelByStateCache = new DynamicOverridableMap<>(state -> modelManager.getModel(ModelLocationCache.get(state)));
+        if(this.mfix$modelCache != null)
+            this.mfix$modelCache.clear();
     }
 
-    /**
-     * @author embeddedt
-     * @reason get the model from the dynamic model provider
-     */
-    @Overwrite
-    public BakedModel getBlockModel(BlockState state) {
+    private BakedModel cacheBlockModel(BlockState state) {
+        // Do all model system accesses in the unlocked path
         ModelResourceLocation mrl = ModelLocationCache.get(state);
         BakedModel model = mrl == null ? null : modelManager.getModel(mrl);
         if (model == null) {
             model = modelManager.getMissingModel();
         }
+
         return model;
     }
+
+    /**
+     * @author embeddedt
+     * @reason get the model from the dynamic model provider
+     */
+    @Overwrite
+    public BakedModel getBlockModel(BlockState state) {
+        return this.mfix$modelCache.get(state);
+    }
 }

common/src/main/java/org/embeddedt/modernfix/dynamicresources/DynamicModelCache.java

@@ -0,0 +1,79 @@
+package org.embeddedt.modernfix.dynamicresources;
+
+import it.unimi.dsi.fastutil.Function;
+import it.unimi.dsi.fastutil.objects.Reference2ReferenceLinkedOpenHashMap;
+import net.minecraft.client.resources.model.BakedModel;
+
+import java.util.concurrent.locks.StampedLock;
+
+/**
+ * The Mojang Triple-based baked cache system is too slow to be hitting on every model retrieval, so
+ * we need a fast, concurrency-safe wrapper on top.
+ */
+public class DynamicModelCache<K> {
+    private final Reference2ReferenceLinkedOpenHashMap<K, BakedModel> cache = new Reference2ReferenceLinkedOpenHashMap<>();
+    private final StampedLock lock = new StampedLock();
+    private final Function<K, BakedModel> modelRetriever;
+    private final boolean allowNulls;
+
+    public DynamicModelCache(Function<K, BakedModel> modelRetriever, boolean allowNulls) {
+        this.modelRetriever = modelRetriever;
+        this.allowNulls = allowNulls;
+    }
+
+    public void clear() {
+        long stamp = lock.writeLock();
+        try {
+            cache.clear();
+        } finally {
+            lock.unlock(stamp);
+        }
+    }
+
+    private boolean needToPopulate(K state) {
+        long stamp = lock.readLock();
+        try {
+            return !cache.containsKey(state);
+        } finally {
+            lock.unlock(stamp);
+        }
+    }
+
+    private BakedModel getModelFromCache(K state) {
+        long stamp = lock.readLock();
+        try {
+            return cache.get(state);
+        } finally {
+            lock.unlock(stamp);
+        }
+    }
+
+    private BakedModel cacheModel(K state) {
+        BakedModel model = modelRetriever.apply(state);
+
+        // Lock and modify our local, faster cache
+        long stamp = lock.writeLock();
+
+        try {
+            cache.putAndMoveToFirst(state, model);
+            // TODO: choose less arbitrary number
+            if(cache.size() >= 1000) {
+                cache.removeLast();
+            }
+        } finally {
+            lock.unlock(stamp);
+        }
+
+        return model;
+    }
+
+    public BakedModel get(K key) {
+        BakedModel model = getModelFromCache(key);
+
+        if(model == null && (!allowNulls || needToPopulate(key))) {
+            model = cacheModel(key);
+        }
+
+        return model;
+    }
+}

fabric/src/test/java/org/embeddedt/modernfix/dynamicresources/DynamicModelCacheTest.java

@@ -0,0 +1,26 @@
+package org.embeddedt.modernfix.dynamicresources;
+
+import net.minecraft.client.resources.model.BakedModel;
+import net.minecraft.client.resources.model.BuiltInModel;
+import net.minecraft.world.item.Item;
+import net.minecraft.world.item.Items;
+import org.embeddedt.modernfix.testing.util.BootstrapMinecraft;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+@BootstrapMinecraft
+public class DynamicModelCacheTest {
+    @Test
+    public void testCacheReturnsNullForNullGetter() {
+        DynamicModelCache<Item> cache = new DynamicModelCache(k -> null, true);
+        assertNull(cache.get(Items.STONE));
+    }
+
+    @Test
+    public void testCacheFunctions() {
+        BakedModel model = new BuiltInModel(null, null, null, false);
+        DynamicModelCache<Item> cache = new DynamicModelCache(k -> model, true);
+        assertEquals(model, cache.get(Items.STONE));
+    }
+}

forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ItemModelMesherForgeMixin.java

@@ -10,6 +10,7 @@
 import net.minecraftforge.client.model.ForgeItemModelShaper;
 import net.minecraftforge.registries.ForgeRegistries;
 import org.embeddedt.modernfix.annotation.ClientOnlyMixin;
+import org.embeddedt.modernfix.dynamicresources.DynamicModelCache;
 import org.embeddedt.modernfix.dynamicresources.ModelLocationCache;
 import org.embeddedt.modernfix.util.ItemMesherMap;
 import org.spongepowered.asm.mixin.*;
@@ -27,6 +28,8 @@ public abstract class ItemModelMesherForgeMixin extends ItemModelShaper {
 
     private Map<Holder.Reference<Item>, ModelResourceLocation> overrideLocations;
 
+    private final DynamicModelCache<Holder.Reference<Item>> mfix$modelCache = new DynamicModelCache<>(k -> this.mfix$getModelSlow((Holder.Reference<Item>)k), true);
+
     public ItemModelMesherForgeMixin(ModelManager arg) {
         super(arg);
     }
@@ -50,6 +53,11 @@ private void replaceLocationMap(CallbackInfo ci) {
         return map;
     }
 
+    private BakedModel mfix$getModelSlow(Holder.Reference<Item> key) {
+        ModelResourceLocation map = mfix$getLocationForge(key);
+        return map == null ? null : getModelManager().getModel(map);
+    }
+
     /**
      * @author embeddedt
      * @reason Get the stored location for that item and meta, and get the model
@@ -58,8 +66,7 @@ private void replaceLocationMap(CallbackInfo ci) {
     @Overwrite
     @Override
     public BakedModel getItemModel(Item item) {
-        ModelResourceLocation map = mfix$getLocationForge(ForgeRegistries.ITEMS.getDelegateOrThrow(item));
-        return map == null ? null : getModelManager().getModel(map);
+        return this.mfix$modelCache.get(ForgeRegistries.ITEMS.getDelegateOrThrow(item));
     }
 
     /**
@@ -80,5 +87,7 @@ public void register(Item item, ModelResourceLocation location) {
      **/
     @Overwrite
     @Override
-    public void rebuildCache() {}
+    public void rebuildCache() {
+        this.mfix$modelCache.clear();
+    }
 }

forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_lambda/RegistryDelegateMixin.java

@@ -0,0 +1,25 @@
+package org.embeddedt.modernfix.forge.mixin.perf.forge_registry_lambda;
+
+import net.minecraft.resources.ResourceLocation;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Overwrite;
+import org.spongepowered.asm.mixin.Shadow;
+
+@Mixin(targets = {"net/minecraftforge/registries/RegistryDelegate"})
+public class RegistryDelegateMixin {
+    @Shadow private ResourceLocation name;
+
+    /**
+     * @author embeddedt
+     * @reason avoid allocation in hashCode()
+     */
+    @Overwrite(remap = false)
+    public int hashCode() {
+        ResourceLocation name = this.name;
+        if(name != null) {
+            return name.hashCode();
+        } else {
+            return 0;
+        }
+    }
+}