From ad6425f7e9fbdc1b94d5072e038ddff3da0388ef Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 25 Jan 2025 15:47:00 -0500 Subject: [PATCH] Improve bulk dynamic model loading performance Filtering by blockstate has been removed as it seems to be slower now than just loading all the models. This will need to be revisited if we end up with issues from Pedestals again. --- .../BlockStateModelLoaderMixin.java | 45 +++++++++++++++++-- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/BlockStateModelLoaderMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/BlockStateModelLoaderMixin.java index a2247c04..8fc41b55 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/BlockStateModelLoaderMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/BlockStateModelLoaderMixin.java @@ -1,10 +1,17 @@ package org.embeddedt.modernfix.common.mixin.perf.dynamic_resources; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import com.google.common.collect.ImmutableList; +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.Pair; import it.unimi.dsi.fastutil.objects.Object2IntMap; import it.unimi.dsi.fastutil.objects.Object2IntMaps; -import net.minecraft.client.Minecraft; +import it.unimi.dsi.fastutil.objects.ReferenceObjectImmutablePair; import net.minecraft.client.color.block.BlockColors; +import net.minecraft.client.renderer.block.model.BlockModelDefinition; import net.minecraft.client.resources.model.BlockStateModelLoader; import net.minecraft.client.resources.model.ModelResourceLocation; import net.minecraft.client.resources.model.UnbakedModel; @@ -15,10 +22,8 @@ import net.minecraft.util.profiling.ProfilerFiller; import net.minecraft.world.level.block.Block; import net.minecraft.world.level.block.state.BlockState; import net.minecraft.world.level.block.state.StateDefinition; -import org.embeddedt.modernfix.ModernFix; import org.embeddedt.modernfix.annotation.ClientOnlyMixin; import org.embeddedt.modernfix.duck.IBlockStateModelLoader; -import org.embeddedt.modernfix.dynamicresources.ModelBakeryHelpers; import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Mutable; @@ -31,7 +36,9 @@ import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; import java.util.Collections; import java.util.Iterator; import java.util.Map; +import java.util.concurrent.ExecutionException; import java.util.function.BiConsumer; +import java.util.function.Predicate; @Mixin(BlockStateModelLoader.class) @ClientOnlyMixin @@ -51,6 +58,9 @@ public abstract class BlockStateModelLoaderMixin implements IBlockStateModelLoad public void loadSpecificBlock(ModelResourceLocation location) { var optionalBlock = BuiltInRegistries.BLOCK.getOptional(location.id()); if(optionalBlock.isPresent()) { + // embeddedt note - filtering is currently disabled as it's quite inefficient to do vs. just loading + // the extra models and letting LRU deal with it + /* try { // Only filter states if we are in a world and not in the loading overlay filteredStates = (Minecraft.getInstance().getOverlay() == null && Minecraft.getInstance().level != null) ? ModelBakeryHelpers.getBlockStatesForMRL(optionalBlock.get().getStateDefinition(), location) : null; @@ -58,6 +68,7 @@ public abstract class BlockStateModelLoaderMixin implements IBlockStateModelLoad ModernFix.LOGGER.error("Exception filtering states on {}", location, e); filteredStates = null; } + */ try { this.loadBlockStateDefinitions(location.id(), optionalBlock.get().getStateDefinition()); } finally { @@ -75,4 +86,32 @@ public abstract class BlockStateModelLoaderMixin implements IBlockStateModelLoad private ImmutableList getFilteredStates(StateDefinition instance) { return this.filteredStates != null ? this.filteredStates : instance.getPossibleStates(); } + + // Add some caching around key hot paths + + private final Cache, BlockModelDefinition> cachedBlockModelDefs = CacheBuilder.newBuilder() + .maximumSize(100) + .build(); + + private static final Cache, String>, Predicate> cachedBlockStatePredicates = CacheBuilder.newBuilder() + .maximumSize(100) + .build(); + + @WrapOperation(method = "loadBlockStateDefinitions", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/resources/model/BlockStateModelLoader$LoadedJson;parse(Lnet/minecraft/resources/ResourceLocation;Lnet/minecraft/client/renderer/block/model/BlockModelDefinition$Context;)Lnet/minecraft/client/renderer/block/model/BlockModelDefinition;")) + private BlockModelDefinition avoidMultipleParses(BlockStateModelLoader.LoadedJson instance, ResourceLocation blockStateId, BlockModelDefinition.Context context, Operation original) { + try { + return cachedBlockModelDefs.get(ReferenceObjectImmutablePair.of(instance, blockStateId), () -> original.call(instance, blockStateId, context)); + } catch (ExecutionException e) { + throw new RuntimeException(e); + } + } + + @WrapMethod(method = "predicate") + private static Predicate memoizePredicate(StateDefinition stateDefentition, String properties, Operation> original) { + try { + return cachedBlockStatePredicates.get(Pair.of(stateDefentition, properties), () -> original.call(stateDefentition, properties)); + } catch (ExecutionException e) { + throw new RuntimeException(e); + } + } }