diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/BlockModelShaperMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/BlockModelShaperMixin.java index 3f737e53..bed4cb16 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/BlockModelShaperMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/BlockModelShaperMixin.java @@ -6,6 +6,7 @@ import net.minecraft.client.resources.model.ModelManager; 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.*; @@ -24,6 +25,8 @@ public class BlockModelShaperMixin { @Shadow @Final @Mutable private Map modelByStateCache; + private final DynamicModelCache mfix$modelCache = new DynamicModelCache<>(k -> this.cacheBlockModel((BlockState)k), false); + @Inject(method = "", at = @At("RETURN")) private void replaceModelMap(CallbackInfo ci) { // replace the backing map for mods which will access it @@ -32,10 +35,22 @@ public class BlockModelShaperMixin { /** * @author embeddedt - * @reason no need to rebuild model cache, and location cache is done elsewhere + * @reason no need to rebuild vanilla model cache */ @Overwrite public void rebuildCache() { + this.mfix$modelCache.clear(); + } + + 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; } /** @@ -44,11 +59,6 @@ public class BlockModelShaperMixin { */ @Overwrite public BakedModel getBlockModel(BlockState state) { - ModelResourceLocation mrl = ModelLocationCache.get(state); - BakedModel model = mrl == null ? null : modelManager.getModel(mrl); - if (model == null) { - model = modelManager.getMissingModel(); - } - return model; + return this.mfix$modelCache.get(state); } } diff --git a/common/src/main/java/org/embeddedt/modernfix/dynamicresources/DynamicModelCache.java b/common/src/main/java/org/embeddedt/modernfix/dynamicresources/DynamicModelCache.java new file mode 100644 index 00000000..a2c90a9f --- /dev/null +++ b/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 { + private final Reference2ReferenceLinkedOpenHashMap cache = new Reference2ReferenceLinkedOpenHashMap<>(); + private final StampedLock lock = new StampedLock(); + private final Function modelRetriever; + private final boolean allowNulls; + + public DynamicModelCache(Function 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; + } +} diff --git a/fabric/src/test/java/org/embeddedt/modernfix/dynamicresources/DynamicModelCacheTest.java b/fabric/src/test/java/org/embeddedt/modernfix/dynamicresources/DynamicModelCacheTest.java new file mode 100644 index 00000000..0ed2b26c --- /dev/null +++ b/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 cache = new DynamicModelCache(k -> null, true); + assertNull(cache.get(Items.STONE)); + } + + @Test + public void testCacheFunctions() { + BakedModel model = new BuiltInModel(null, null, null, false); + DynamicModelCache cache = new DynamicModelCache(k -> model, true); + assertEquals(model, cache.get(Items.STONE)); + } +} diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ItemModelMesherForgeMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ItemModelMesherForgeMixin.java index d5792df3..b2d5a2ca 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ItemModelMesherForgeMixin.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ItemModelMesherForgeMixin.java @@ -7,6 +7,7 @@ import net.minecraft.client.resources.model.ModelResourceLocation; import net.minecraft.world.item.Item; import net.minecraftforge.registries.IRegistryDelegate; 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.*; @@ -24,6 +25,8 @@ public abstract class ItemModelMesherForgeMixin extends ItemModelShaper { private Map, ModelResourceLocation> overrideLocations; + private final DynamicModelCache> mfix$modelCache = new DynamicModelCache<>(k -> this.mfix$getModelSlow((IRegistryDelegate)k), true); + public ItemModelMesherForgeMixin(ModelManager arg) { super(arg); } @@ -47,6 +50,11 @@ public abstract class ItemModelMesherForgeMixin extends ItemModelShaper { return map; } + private BakedModel mfix$getModelSlow(IRegistryDelegate 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 @@ -55,8 +63,7 @@ public abstract class ItemModelMesherForgeMixin extends ItemModelShaper { @Overwrite @Override public BakedModel getItemModel(Item item) { - ModelResourceLocation map = mfix$getLocationForge(item.delegate); - return map == null ? null : getModelManager().getModel(map); + return this.mfix$modelCache.get(item.delegate); } /** @@ -77,5 +84,7 @@ public abstract class ItemModelMesherForgeMixin extends ItemModelShaper { **/ @Overwrite @Override - public void rebuildCache() {} + public void rebuildCache() { + this.mfix$modelCache.clear(); + } }