From ccfc282cfcccaa08ef40f29ddeea8e582e25e626 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Thu, 7 Dec 2023 22:13:49 -0500 Subject: [PATCH 1/2] Remove hot allocations in ForgeRegistry#getDelegateOrThrow --- .../ForgeRegistryMixin.java | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java new file mode 100644 index 00000000..920136ae --- /dev/null +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java @@ -0,0 +1,65 @@ +package org.embeddedt.modernfix.forge.mixin.perf.forge_registry_alloc; + +import net.minecraft.core.Holder; +import net.minecraft.resources.ResourceKey; +import net.minecraft.resources.ResourceLocation; +import net.minecraftforge.registries.ForgeRegistry; +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.Locale; +import java.util.Map; + +@Mixin(value = ForgeRegistry.class, remap = false) +public abstract class ForgeRegistryMixin { + @Shadow @Final private Map> delegatesByName; + + @Shadow @Final private Map> delegatesByValue; + + /** + * @author embeddedt + * @reason stop allocating so many unneeded objects. stop. + */ + @Overwrite + public Holder.Reference getDelegateOrThrow(ResourceLocation location) { + Holder.Reference holder = delegatesByName.get(location); + + if (holder == null) { + throw new IllegalArgumentException(String.format(Locale.ENGLISH, "No delegate exists for location %s", location)); + } + + return holder; + } + + /** + * @author embeddedt + * @reason see above + */ + @Overwrite + public Holder.Reference getDelegateOrThrow(ResourceKey rkey) { + Holder.Reference holder = delegatesByName.get(rkey.location()); + + if (holder == null) { + throw new IllegalArgumentException(String.format(Locale.ENGLISH, "No delegate exists for key %s", rkey)); + } + + return holder; + } + + /** + * @author embeddedt + * @reason see above + */ + @Overwrite + public Holder.Reference getDelegateOrThrow(V value) { + Holder.Reference holder = delegatesByValue.get(value); + + if (holder == null) { + throw new IllegalArgumentException(String.format(Locale.ENGLISH, "No delegate exists for value %s", value)); + } + + return holder; + } +} From 9464f07a5a3394c1c389e694352e0f71c68e84c7 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 9 Dec 2023 14:32:52 -0500 Subject: [PATCH 2/2] Replace hacky capturing with MixinExtras --- .../ModelBakerImplMixin.java | 36 +++---------------- .../ModelBakerImplMixin.java | 33 +++-------------- 2 files changed, 10 insertions(+), 59 deletions(-) diff --git a/fabric/src/main/java/org/embeddedt/modernfix/fabric/mixin/perf/dynamic_resources/ModelBakerImplMixin.java b/fabric/src/main/java/org/embeddedt/modernfix/fabric/mixin/perf/dynamic_resources/ModelBakerImplMixin.java index 03995c6f..4db81275 100644 --- a/fabric/src/main/java/org/embeddedt/modernfix/fabric/mixin/perf/dynamic_resources/ModelBakerImplMixin.java +++ b/fabric/src/main/java/org/embeddedt/modernfix/fabric/mixin/perf/dynamic_resources/ModelBakerImplMixin.java @@ -1,5 +1,7 @@ package org.embeddedt.modernfix.fabric.mixin.perf.dynamic_resources; +import com.llamalad7.mixinextras.injector.wrapoperation.Operation; +import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation; import net.fabricmc.loader.api.FabricLoader; import net.minecraft.client.renderer.texture.TextureAtlasSprite; import net.minecraft.client.resources.model.*; @@ -18,7 +20,6 @@ import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject; -import org.spongepowered.asm.mixin.injection.ModifyVariable; import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; import java.lang.invoke.MethodHandle; @@ -53,10 +54,6 @@ public abstract class ModelBakerImplMixin implements IExtendedModelBaker { } } - private ResourceLocation capturedLocation; - private UnbakedModel capturedModel; - private ModelState capturedState; - private boolean throwIfMissing; @Override @@ -64,11 +61,6 @@ public abstract class ModelBakerImplMixin implements IExtendedModelBaker { throwIfMissing = true; } - @Inject(method = "bake", at = @At("HEAD")) - private void captureState(ResourceLocation rl, ModelState state, CallbackInfoReturnable cir) { - capturedState = state; - } - @Inject(method = "getModel", at = @At("HEAD"), cancellable = true) private void obtainModel(ResourceLocation arg, CallbackInfoReturnable cir) { if(debugDynamicModelLoading) @@ -109,10 +101,6 @@ public abstract class ModelBakerImplMixin implements IExtendedModelBaker { } cir.setReturnValue(toReplace); cir.getReturnValue().resolveParents(this.field_40571::getModel); - if(capturedLocation == null) { - capturedLocation = arg; - capturedModel = cir.getReturnValue(); - } if(cir.getReturnValue() == extendedBakery.mfix$getUnbakedMissingModel()) { if(arg != ModelBakery.MISSING_MODEL_LOCATION) { if(debugDynamicModelLoading) @@ -123,31 +111,17 @@ public abstract class ModelBakerImplMixin implements IExtendedModelBaker { } } - @ModifyVariable(method = "bake", at = @At(value = "INVOKE_ASSIGN", target = "Lnet/minecraft/client/resources/model/UnbakedModel;bake(Lnet/minecraft/client/resources/model/ModelBaker;Ljava/util/function/Function;Lnet/minecraft/client/resources/model/ModelState;Lnet/minecraft/resources/ResourceLocation;)Lnet/minecraft/client/resources/model/BakedModel;")) - private BakedModel unifyMissingBakedModel(BakedModel model) { - // Save these variables in case the nested model calls getModel somehow - ResourceLocation location = this.capturedLocation; - UnbakedModel unbakedModel = this.capturedModel; - ModelState state = this.capturedState; - - // Safety check - if(location == null) { - return model; - } + @WrapOperation(method = "bake", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/resources/model/UnbakedModel;bake(Lnet/minecraft/client/resources/model/ModelBaker;Ljava/util/function/Function;Lnet/minecraft/client/resources/model/ModelState;Lnet/minecraft/resources/ResourceLocation;)Lnet/minecraft/client/resources/model/BakedModel;")) + private BakedModel callBakedModelIntegration(UnbakedModel unbakedModel, ModelBaker baker, Function spriteGetter, ModelState state, ResourceLocation location, Operation operation) { + BakedModel model = operation.call(unbakedModel, baker, spriteGetter, state, location); for(ModernFixClientIntegration integration : ModernFixClient.CLIENT_INTEGRATIONS) { model = integration.onBakedModelLoad(location, unbakedModel, model, state, this.field_40571); } - // Allow more capturing - this.capturedLocation = null; - this.capturedModel = null; - return model; } - - /** * @author embeddedt * @reason emulate old function, to allow injectors to work diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakerImplMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakerImplMixin.java index c3527eb5..7cd6d6d9 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakerImplMixin.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakerImplMixin.java @@ -1,6 +1,8 @@ package org.embeddedt.modernfix.forge.mixin.perf.dynamic_resources; import com.llamalad7.mixinextras.injector.ModifyExpressionValue; +import com.llamalad7.mixinextras.injector.wrapoperation.Operation; +import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation; import net.minecraft.client.renderer.texture.TextureAtlasSprite; import net.minecraft.client.resources.model.*; import net.minecraft.resources.ResourceLocation; @@ -34,10 +36,6 @@ public abstract class ModelBakerImplMixin implements IModelBakerImpl, IExtendedM mfix$ignoreCache = true; } - private ResourceLocation capturedLocation; - private UnbakedModel capturedModel; - private ModelState capturedState; - private boolean throwIfMissing; @Override @@ -45,11 +43,6 @@ public abstract class ModelBakerImplMixin implements IModelBakerImpl, IExtendedM throwIfMissing = true; } - @Inject(method = "bake(Lnet/minecraft/resources/ResourceLocation;Lnet/minecraft/client/resources/model/ModelState;Ljava/util/function/Function;)Lnet/minecraft/client/resources/model/BakedModel;", at = @At("HEAD"), remap = false) - private void captureState(ResourceLocation arg, ModelState state, Function sprites, CallbackInfoReturnable cir) { - capturedState = state; - } - @Inject(method = "getModel", at = @At("HEAD"), cancellable = true) private void obtainModel(ResourceLocation arg, CallbackInfoReturnable cir) { if(debugDynamicModelLoading) @@ -77,10 +70,6 @@ public abstract class ModelBakerImplMixin implements IModelBakerImpl, IExtendedM } cir.setReturnValue(toReplace); cir.getReturnValue().resolveParents(this.field_40571::getModel); - if(capturedLocation == null) { - capturedLocation = arg; - capturedModel = cir.getReturnValue(); - } if(cir.getReturnValue() == extendedBakery.mfix$getUnbakedMissingModel()) { if(arg != ModelBakery.MISSING_MODEL_LOCATION) { if(debugDynamicModelLoading) @@ -96,26 +85,14 @@ public abstract class ModelBakerImplMixin implements IModelBakerImpl, IExtendedM return mfix$ignoreCache ? null : o; } - @ModifyExpressionValue(method = "bake(Lnet/minecraft/resources/ResourceLocation;Lnet/minecraft/client/resources/model/ModelState;Ljava/util/function/Function;)Lnet/minecraft/client/resources/model/BakedModel;", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/resources/model/UnbakedModel;bake(Lnet/minecraft/client/resources/model/ModelBaker;Ljava/util/function/Function;Lnet/minecraft/client/resources/model/ModelState;Lnet/minecraft/resources/ResourceLocation;)Lnet/minecraft/client/resources/model/BakedModel;")) - private BakedModel unifyMissingBakedModel(BakedModel model) { - // Save these variables in case the nested model calls getModel somehow - ResourceLocation location = this.capturedLocation; - UnbakedModel unbakedModel = this.capturedModel; - ModelState state = this.capturedState; - - // Safety check - if(location == null) { - return model; - } + @WrapOperation(method = "bake(Lnet/minecraft/resources/ResourceLocation;Lnet/minecraft/client/resources/model/ModelState;Ljava/util/function/Function;)Lnet/minecraft/client/resources/model/BakedModel;", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/resources/model/UnbakedModel;bake(Lnet/minecraft/client/resources/model/ModelBaker;Ljava/util/function/Function;Lnet/minecraft/client/resources/model/ModelState;Lnet/minecraft/resources/ResourceLocation;)Lnet/minecraft/client/resources/model/BakedModel;")) + private BakedModel callBakedModelIntegration(UnbakedModel unbakedModel, ModelBaker baker, Function spriteGetter, ModelState state, ResourceLocation location, Operation operation) { + BakedModel model = operation.call(unbakedModel, baker, spriteGetter, state, location); for(ModernFixClientIntegration integration : ModernFixClient.CLIENT_INTEGRATIONS) { model = integration.onBakedModelLoad(location, unbakedModel, model, state, this.field_40571); } - // Allow more capturing - this.capturedLocation = null; - this.capturedModel = null; - return model; } }