From 74a339bc2c8114e326998dd87d1b8fd0c864fa26 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 24 Jan 2025 09:32:06 -0500 Subject: [PATCH] Add more locking in various vanilla model loading paths --- .../ModelBakerImplMixin.java | 18 +++++++++ .../dynamic_resources/ModelBakeryMixin.java | 40 +++++++++++++++++-- .../modernfix/duck/IExtendedModelBakery.java | 3 ++ gradle.properties | 2 +- .../ModelBakerImplMixin.java | 22 +++++++++- 5 files changed, 79 insertions(+), 6 deletions(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/ModelBakerImplMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/ModelBakerImplMixin.java index b465efdb..e1b6c3e8 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/ModelBakerImplMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/ModelBakerImplMixin.java @@ -1,11 +1,17 @@ package org.embeddedt.modernfix.common.mixin.perf.dynamic_resources; import com.llamalad7.mixinextras.injector.ModifyReturnValue; +import com.llamalad7.mixinextras.injector.wrapmethod.WrapMethod; +import com.llamalad7.mixinextras.injector.wrapoperation.Operation; +import net.minecraft.client.resources.model.BakedModel; import net.minecraft.client.resources.model.ModelBakery; +import net.minecraft.client.resources.model.ModelState; import net.minecraft.client.resources.model.UnbakedModel; import net.minecraft.resources.ResourceLocation; import org.embeddedt.modernfix.ModernFix; import org.embeddedt.modernfix.annotation.ClientOnlyMixin; +import org.embeddedt.modernfix.duck.IExtendedModelBakery; +import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; import org.spongepowered.asm.mixin.Unique; @@ -16,6 +22,7 @@ import org.spongepowered.asm.mixin.injection.At; public abstract class ModelBakerImplMixin { @Shadow public abstract UnbakedModel getModel(ResourceLocation location); + @Shadow @Final private ModelBakery field_40571; @Unique private int mfix$getDepth = 0; @@ -36,4 +43,15 @@ public abstract class ModelBakerImplMixin { mfix$getDepth--; return model; } + + @WrapMethod(method = "bake(Lnet/minecraft/resources/ResourceLocation;Lnet/minecraft/client/resources/model/ModelState;)Lnet/minecraft/client/resources/model/BakedModel;") + private BakedModel mfix$lockWhenBaking(ResourceLocation location, ModelState transform, Operation original) { + var lock = ((IExtendedModelBakery)this.field_40571).mfix$getLock(); + lock.lock(); + try { + return original.call(location, transform); + } finally { + lock.unlock(); + } + } } diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/ModelBakeryMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/ModelBakeryMixin.java index 1f893d26..87bdc708 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/ModelBakeryMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/ModelBakeryMixin.java @@ -1,6 +1,7 @@ package org.embeddedt.modernfix.common.mixin.perf.dynamic_resources; import com.llamalad7.mixinextras.injector.ModifyExpressionValue; +import com.llamalad7.mixinextras.injector.wrapmethod.WrapMethod; import com.llamalad7.mixinextras.injector.wrapoperation.Operation; import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation; import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; @@ -108,6 +109,16 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { } } + @WrapMethod(method = "getModel") + private UnbakedModel mfix$lockWhenGettingModel(ResourceLocation modelLocation, Operation original) { + modelBakeryLock.lock(); + try { + return original.call(modelLocation); + } finally { + modelBakeryLock.unlock(); + } + } + @Override public UnbakedModel mfix$getMissingModel() { return missingModel; @@ -199,10 +210,26 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { private static final int MAXIMUM_CACHE_SIZE = 1000; private void runCleanup() { - ((LRUMap)this.unbakedCache).dropEntriesToMeetSize(MAXIMUM_CACHE_SIZE); - ((LRUMap)this.bakedCache).dropEntriesToMeetSize(MAXIMUM_CACHE_SIZE); - ((LRUMap)this.topLevelModels).dropEntriesToMeetSize(MAXIMUM_CACHE_SIZE); - ((LRUMap)this.bakedTopLevelModels).dropEntriesToMeetSize(MAXIMUM_CACHE_SIZE); + try { + ((LRUMap)this.unbakedCache).dropEntriesToMeetSize(MAXIMUM_CACHE_SIZE); + } catch(RuntimeException e) { + throw new IllegalStateException("Exception dropping entries in unbaked cache", e); + } + try { + ((LRUMap)this.bakedCache).dropEntriesToMeetSize(MAXIMUM_CACHE_SIZE); + } catch(RuntimeException e) { + throw new IllegalStateException("Exception dropping entries in baked cache", e); + } + try { + ((LRUMap)this.topLevelModels).dropEntriesToMeetSize(MAXIMUM_CACHE_SIZE); + } catch(RuntimeException e) { + throw new IllegalStateException("Exception dropping entries in top level models", e); + } + try { + ((LRUMap)this.bakedTopLevelModels).dropEntriesToMeetSize(MAXIMUM_CACHE_SIZE); + } catch(RuntimeException e) { + throw new IllegalStateException("Exception dropping entries in baked top level models", e); + } } @Override @@ -235,4 +262,9 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { public Map getBakedTopLevelModels() { return this.mfix$emulatedBakedRegistry; } + + @Override + public ReentrantLock mfix$getLock() { + return this.modelBakeryLock; + } } diff --git a/common/src/main/java/org/embeddedt/modernfix/duck/IExtendedModelBakery.java b/common/src/main/java/org/embeddedt/modernfix/duck/IExtendedModelBakery.java index cca44ea0..c37c495d 100644 --- a/common/src/main/java/org/embeddedt/modernfix/duck/IExtendedModelBakery.java +++ b/common/src/main/java/org/embeddedt/modernfix/duck/IExtendedModelBakery.java @@ -3,9 +3,12 @@ package org.embeddedt.modernfix.duck; import net.minecraft.client.resources.model.ModelResourceLocation; import net.minecraft.client.resources.model.UnbakedModel; +import java.util.concurrent.locks.ReentrantLock; + public interface IExtendedModelBakery { void mfix$tick(); void mfix$finishLoading(); UnbakedModel mfix$loadUnbakedModelDynamic(ModelResourceLocation location); UnbakedModel mfix$getMissingModel(); + ReentrantLock mfix$getLock(); } diff --git a/gradle.properties b/gradle.properties index 64b3bf63..dfb3bbd1 100644 --- a/gradle.properties +++ b/gradle.properties @@ -7,7 +7,7 @@ mixinextras_version=0.4.1 mod_id=modernfix minecraft_version=1.21.1 enabled_platforms=fabric,neoforge -forge_version=21.1.15 +forge_version=21.1.111 parchment_version=2024.07.07 parchment_mc_version=1.21 refined_storage_version=4392788 diff --git a/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/perf/dynamic_resources/ModelBakerImplMixin.java b/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/perf/dynamic_resources/ModelBakerImplMixin.java index 2d71dbed..3ce16067 100644 --- a/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/perf/dynamic_resources/ModelBakerImplMixin.java +++ b/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/perf/dynamic_resources/ModelBakerImplMixin.java @@ -1,8 +1,15 @@ package org.embeddedt.modernfix.neoforge.mixin.perf.dynamic_resources; +import com.llamalad7.mixinextras.injector.wrapmethod.WrapMethod; +import com.llamalad7.mixinextras.injector.wrapoperation.Operation; +import net.minecraft.client.renderer.texture.TextureAtlasSprite; +import net.minecraft.client.resources.model.BakedModel; +import net.minecraft.client.resources.model.Material; import net.minecraft.client.resources.model.ModelBakery; import net.minecraft.client.resources.model.ModelResourceLocation; +import net.minecraft.client.resources.model.ModelState; import net.minecraft.client.resources.model.UnbakedModel; +import net.minecraft.resources.ResourceLocation; import org.embeddedt.modernfix.annotation.ClientOnlyMixin; import org.embeddedt.modernfix.duck.IExtendedModelBakery; import org.spongepowered.asm.mixin.Final; @@ -10,7 +17,9 @@ import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Overwrite; import org.spongepowered.asm.mixin.Shadow; -@Mixin(targets = {"net/minecraft/client/resources/model/ModelBakery$ModelBakerImpl"}) +import java.util.function.Function; + +@Mixin(ModelBakery.ModelBakerImpl.class) @ClientOnlyMixin public class ModelBakerImplMixin { @Shadow @Final private ModelBakery field_40571; @@ -25,4 +34,15 @@ public class ModelBakerImplMixin { UnbakedModel model = bakery.mfix$loadUnbakedModelDynamic(location); return model == bakery.mfix$getMissingModel() ? null : model; } + + @WrapMethod(method = "bake(Lnet/minecraft/resources/ResourceLocation;Lnet/minecraft/client/resources/model/ModelState;Ljava/util/function/Function;)Lnet/minecraft/client/resources/model/BakedModel;", remap = false) + private BakedModel mfix$lockWhenBaking(ResourceLocation location, ModelState transform, Function textureGetter, Operation original) { + var lock = ((IExtendedModelBakery)this.field_40571).mfix$getLock(); + lock.lock(); + try { + return original.call(location, transform, textureGetter); + } finally { + lock.unlock(); + } + } }