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 6f33dd5f..61f86bd4 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.*; @@ -15,7 +16,6 @@ import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; import java.util.Map; - @Mixin(BlockModelShaper.class) @ClientOnlyMixin public class BlockModelShaperMixin { @@ -24,10 +24,25 @@ public class BlockModelShaperMixin { @Shadow private Map modelByStateCache; + private final DynamicModelCache mfix$modelCache = new DynamicModelCache<>(k -> this.cacheBlockModel((BlockState)k), false); + @Inject(method = { "", "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(); + } + + 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; } /** @@ -36,11 +51,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/forge_registry_lambda/RegistryDelegateMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_lambda/RegistryDelegateMixin.java new file mode 100644 index 00000000..918b5a30 --- /dev/null +++ b/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; + } + } +} diff --git a/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/perf/dynamic_resources/ItemModelMesherForgeMixin.java b/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/perf/dynamic_resources/ItemModelMesherForgeMixin.java index 5697f927..bb06a10a 100644 --- a/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/perf/dynamic_resources/ItemModelMesherForgeMixin.java +++ b/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/perf/dynamic_resources/ItemModelMesherForgeMixin.java @@ -9,6 +9,7 @@ import net.minecraft.resources.ResourceLocation; import net.minecraft.world.item.Item; import net.neoforged.neoforge.client.model.RegistryAwareItemModelShaper; 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.*; @@ -26,6 +27,8 @@ public abstract class ItemModelMesherForgeMixin extends ItemModelShaper { private Map, ModelResourceLocation> overrideLocations; + private final DynamicModelCache> mfix$modelCache = new DynamicModelCache<>(k -> this.mfix$getModelSlow((Holder.Reference)k), true); + public ItemModelMesherForgeMixin(ModelManager arg) { super(arg); } @@ -49,6 +52,11 @@ public abstract class ItemModelMesherForgeMixin extends ItemModelShaper { return map; } + private BakedModel mfix$getModelSlow(Holder.Reference 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 @@ -57,8 +65,7 @@ public abstract class ItemModelMesherForgeMixin extends ItemModelShaper { @Overwrite @Override public BakedModel getItemModel(Item item) { - ModelResourceLocation map = mfix$getLocationForge(item.builtInRegistryHolder()); - return map == null ? null : getModelManager().getModel(map); + return this.mfix$modelCache.get(item.builtInRegistryHolder()); } /** @@ -79,5 +86,7 @@ public abstract class ItemModelMesherForgeMixin extends ItemModelShaper { **/ @Overwrite @Override - public void rebuildCache() {} + public void rebuildCache() { + this.mfix$modelCache.clear(); + } }