From 469c564c1beaebb4bfd4633c1959db8754c8205f Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 28 Apr 2023 15:53:39 -0400 Subject: [PATCH 01/16] Suppress model bakery errors if there are too many --- .../dynamic_resources/ModelBakeryMixin.java | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java index ea1928e3..49010ac4 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java @@ -11,6 +11,7 @@ import com.google.common.collect.Sets; import com.google.gson.*; import com.mojang.datafixers.util.Pair; import com.mojang.math.Transformation; +import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; import net.minecraft.Util; import net.minecraft.client.color.block.BlockColors; import net.minecraft.client.renderer.block.model.BlockModel; @@ -56,6 +57,7 @@ import java.util.*; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.function.BiConsumer; +import java.util.function.BiFunction; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -194,12 +196,24 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { return ImmutableList.of(); } + private static final int ERROR_THRESHOLD = 200; + + private void logOrSuppressError(Object2IntOpenHashMap suppressionMap, String type, ResourceLocation location, Throwable e) { + int numErrors; + synchronized (suppressionMap) { + numErrors = suppressionMap.computeInt(location.getNamespace(), (k, oldVal) -> (oldVal == null ? 1 : oldVal + 1)); + } + if(numErrors <= ERROR_THRESHOLD) + ModernFix.LOGGER.error("Error reading {} {}: {}", type, location, e); + } + /** * Load all blockstate JSONs and model files, collect textures. */ private void gatherModelMaterials(Set materialSet) { Stopwatch stopwatch = Stopwatch.createStarted(); List>> blockStateData = new ArrayList<>(); + final Object2IntOpenHashMap blockstateErrors = new Object2IntOpenHashMap<>(); for(ResourceLocation blockstate : blockStateFiles) { blockStateData.add(CompletableFuture.supplyAsync(() -> { ResourceLocation fileLocation = new ResourceLocation(blockstate.getNamespace(), "blockstates/" + blockstate.getPath() + ".json"); @@ -207,7 +221,7 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { JsonParser parser = new JsonParser(); return Pair.of(blockstate, parser.parse(new InputStreamReader(resource.getInputStream(), StandardCharsets.UTF_8))); } catch(IOException | JsonParseException e) { - ModernFix.LOGGER.error("Error reading blockstate {}: {}", blockstate, e); + logOrSuppressError(blockstateErrors, "blockstate", blockstate, e); } return Pair.of(blockstate, null); }, ModernFix.resourceReloadExecutor())); @@ -256,11 +270,17 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { } } catch(RuntimeException e) { - ModernFix.LOGGER.error("Error with blockstate {}: {}", pair.getFirst(), e); + logOrSuppressError(blockstateErrors, "blockstate", pair.getFirst(), e); } } } + blockstateErrors.object2IntEntrySet().forEach(entry -> { + if(entry.getIntValue() > ERROR_THRESHOLD) { + ModernFix.LOGGER.error("Suppressed additional {} blockstate errors for domain {}", entry.getIntValue(), entry.getKey()); + } + }); + blockstateErrors.clear(); blockStateData = null; Map basicModels = new HashMap<>(); basicModels.put(MISSING_MODEL_LOCATION, (BlockModel)missingModel); @@ -278,7 +298,7 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { JsonParser parser = new JsonParser(); return Pair.of(model, parser.parse(new InputStreamReader(resource.getInputStream(), StandardCharsets.UTF_8))); } catch(IOException | JsonParseException e) { - ModernFix.LOGGER.error("Error reading model {}: {}", fileLocation, e); + logOrSuppressError(blockstateErrors, "model", fileLocation, e); return Pair.of(fileLocation, null); } }, ModernFix.resourceReloadExecutor())); @@ -298,12 +318,17 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { continue; } } catch(Throwable e) { - ModernFix.LOGGER.warn("Unable to parse {}: {}", pair.getFirst(), e); + logOrSuppressError(blockstateErrors, "model", pair.getFirst(), e); } basicModels.put(pair.getFirst(), (BlockModel)missingModel); } UVController.useDummyUv.set(Boolean.FALSE); } + blockstateErrors.object2IntEntrySet().forEach(entry -> { + if(entry.getIntValue() > ERROR_THRESHOLD) { + ModernFix.LOGGER.error("Suppressed additional {} model errors for domain {}", entry.getIntValue(), entry.getKey()); + } + }); modelFiles = null; Function modelGetter = loc -> { UnbakedModel m = basicModels.get(loc); From 4195b15946ad9eeed9fa088f0895635bf1a4a67e Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 28 Apr 2023 15:53:51 -0400 Subject: [PATCH 02/16] Don't waste time interning paths we are checking exist --- .../modernfix/util/CachedResourcePath.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/embeddedt/modernfix/util/CachedResourcePath.java b/src/main/java/org/embeddedt/modernfix/util/CachedResourcePath.java index 5b32b41f..889311ae 100644 --- a/src/main/java/org/embeddedt/modernfix/util/CachedResourcePath.java +++ b/src/main/java/org/embeddedt/modernfix/util/CachedResourcePath.java @@ -25,30 +25,30 @@ public class CachedResourcePath { private static final String[] NO_PREFIX = new String[0]; public CachedResourcePath(Path path) { - this(NO_PREFIX, path, path.getNameCount()); + this(NO_PREFIX, path, path.getNameCount(), true); } public CachedResourcePath(String s) { // normalize so we can guarantee there are no empty sections - this(NO_PREFIX, SLASH_SPLITTER.splitToList(FileUtil.normalize(s))); + this(NO_PREFIX, SLASH_SPLITTER.splitToList(FileUtil.normalize(s)), false); } - public CachedResourcePath(String[] prefixElements, Collection collection) { - this(prefixElements, collection, collection.size()); + public CachedResourcePath(String[] prefixElements, Collection collection, boolean intern) { + this(prefixElements, collection, collection.size(), intern); } - public CachedResourcePath(String[] prefixElements, Iterable path, int count) { + public CachedResourcePath(String[] prefixElements, Iterable path, int count, boolean intern) { String[] components = new String[prefixElements.length + count]; int i = 0; while(i < prefixElements.length) { - components[i] = PATH_COMPONENT_INTERNER.intern(prefixElements[i]); + components[i] = intern ? PATH_COMPONENT_INTERNER.intern(prefixElements[i]) : prefixElements[i]; i++; } for(Object component : path) { String s = component.toString(); if(s.length() == 0) continue; - components[i] = PATH_COMPONENT_INTERNER.intern(s); + components[i] = intern ? PATH_COMPONENT_INTERNER.intern(s) : s; i++; } pathComponents = components; From 77e9309d2b3d0a55a229c00b85171f6a4c15e1fc Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 28 Apr 2023 17:14:09 -0400 Subject: [PATCH 03/16] Registry optimizations --- .../modernfix/mixin/devenv/GameDataMixin.java | 16 +++++++++ .../ForgeRegistryMixin.java | 33 +++++++++++++++++++ .../ForgeRegistrySnapshotMixin.java | 33 +++++++++++++++++++ .../ResourceKeyMixin.java | 28 ++++++++++++++++ .../resources/META-INF/accesstransformer.cfg | 3 +- src/main/resources/modernfix.mixins.json | 5 ++- 6 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/embeddedt/modernfix/mixin/devenv/GameDataMixin.java create mode 100644 src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistrySnapshotMixin.java create mode 100644 src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ResourceKeyMixin.java diff --git a/src/main/java/org/embeddedt/modernfix/mixin/devenv/GameDataMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/devenv/GameDataMixin.java new file mode 100644 index 00000000..34f531a3 --- /dev/null +++ b/src/main/java/org/embeddedt/modernfix/mixin/devenv/GameDataMixin.java @@ -0,0 +1,16 @@ +package org.embeddedt.modernfix.mixin.devenv; + +import net.minecraft.resources.ResourceLocation; +import net.minecraftforge.registries.ForgeRegistry; +import net.minecraftforge.registries.GameData; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Redirect; + +@Mixin(GameData.class) +public class GameDataMixin { + + @Redirect(method = "*", at = @At(value = "INVOKE", target = "Lnet/minecraftforge/registries/ForgeRegistry;dump(Lnet/minecraft/resources/ResourceLocation;)V")) + private static void noDump(ForgeRegistry reg, ResourceLocation id) { + } +} diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java index 6f27929f..8300cf9f 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java @@ -2,11 +2,17 @@ package org.embeddedt.modernfix.mixin.perf.fast_registry_validation; import net.minecraftforge.fml.common.ObfuscationReflectionHelper; import net.minecraftforge.registries.ForgeRegistry; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.Marker; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Inject; import org.spongepowered.asm.mixin.injection.Redirect; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; import java.lang.reflect.Method; +import java.util.BitSet; @Mixin(value = ForgeRegistry.class, remap = false) public class ForgeRegistryMixin { @@ -25,4 +31,31 @@ public class ForgeRegistryMixin { } return bitSetTrimMethod; } + + private int expectedNextBit = -1; + + /** + * Avoid calling nextClearBit and scanning the whole registry for every block registration. + */ + @Redirect(method = "add(ILnet/minecraftforge/registries/IForgeRegistryEntry;Ljava/lang/String;)I", at = @At(value = "INVOKE", target = "Ljava/util/BitSet;nextClearBit(I)I")) + private int useCachedBit(BitSet availabilityMap, int minimum) { + int bit = availabilityMap.nextClearBit(expectedNextBit != -1 ? expectedNextBit : minimum); + expectedNextBit = bit + 1; + return bit; + } + + @Inject(method = { "sync", "clear", "block" }, at = @At("HEAD")) + private void clearBitCache(CallbackInfo ci) { + expectedNextBit = -1; + } + + @Inject(method = "markDummy", at = @At(value = "INVOKE", target = "Ljava/util/BitSet;clear(I)V")) + private void clearBitCache2(CallbackInfoReturnable cir) { + expectedNextBit = -1; + } + + @Redirect(method = "add(ILnet/minecraftforge/registries/IForgeRegistryEntry;Ljava/lang/String;)I", at = @At(value = "INVOKE", target = "Lorg/apache/logging/log4j/Logger;trace(Lorg/apache/logging/log4j/Marker;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)V")) + private void skipTrace(Logger logger, Marker marker, String s, Object o, Object o1, Object o2, Object o3, Object o4) { + + } } diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistrySnapshotMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistrySnapshotMixin.java new file mode 100644 index 00000000..ad941ab0 --- /dev/null +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistrySnapshotMixin.java @@ -0,0 +1,33 @@ +package org.embeddedt.modernfix.mixin.perf.fast_registry_validation; + +import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; +import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; +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.Mutable; +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.callback.CallbackInfo; + +import java.util.Map; +import java.util.Set; + +@Mixin(ForgeRegistry.Snapshot.class) +public class ForgeRegistrySnapshotMixin { + @Shadow @Final @Mutable public Map ids; + + @Shadow @Final @Mutable public Set dummied; + + /** + * The only good reason to use tree maps here is to keep the order the same. But we are tracking IDs + * anyway so order shouldn't matter. We replace the maps that will be most used. + */ + @Inject(method = "", at = @At("RETURN")) + private void replaceSnapshotMaps(CallbackInfo ci) { + this.ids = new Object2ObjectOpenHashMap<>(); + this.dummied = new ObjectOpenHashSet<>(); + } +} diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ResourceKeyMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ResourceKeyMixin.java new file mode 100644 index 00000000..804dce98 --- /dev/null +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ResourceKeyMixin.java @@ -0,0 +1,28 @@ +package org.embeddedt.modernfix.mixin.perf.fast_registry_validation; + +import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; +import net.minecraft.resources.ResourceKey; +import net.minecraft.resources.ResourceLocation; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Inject; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; + +import java.util.Map; + +@Mixin(ResourceKey.class) +public class ResourceKeyMixin { + private static Map>> INTERNING_MAP = new Object2ObjectOpenHashMap<>(); + @Inject(method = "create(Lnet/minecraft/resources/ResourceLocation;Lnet/minecraft/resources/ResourceLocation;)Lnet/minecraft/resources/ResourceKey;", at = @At("HEAD"), cancellable = true) + private static void createEfficient(ResourceLocation parent, ResourceLocation location, CallbackInfoReturnable> cir) { + synchronized (ResourceKey.class) { + Map> keys = INTERNING_MAP.computeIfAbsent(parent, k -> new Object2ObjectOpenHashMap<>()); + ResourceKey key = keys.get(location); + if(key == null) { + key = new ResourceKey<>(parent, location); + keys.put(location, key); + } + cir.setReturnValue((ResourceKey)key); + } + } +} diff --git a/src/main/resources/META-INF/accesstransformer.cfg b/src/main/resources/META-INF/accesstransformer.cfg index 22dba02c..30e53ddd 100644 --- a/src/main/resources/META-INF/accesstransformer.cfg +++ b/src/main/resources/META-INF/accesstransformer.cfg @@ -25,4 +25,5 @@ public net.minecraft.block.AbstractBlock$Properties field_235806_h_ # requiresCo public net.minecraft.block.AbstractBlock$Properties field_200959_g # destroyTime public net.minecraft.world.server.ServerChunkProvider$ChunkExecutor public net.minecraft.nbt.CompoundNBT (Ljava/util/Map;)V # -public net.minecraft.client.renderer.texture.TextureAtlasSprite (Lnet/minecraft/client/renderer/texture/TextureAtlas;Lnet/minecraft/client/renderer/texture/TextureAtlasSprite$Info;IIIIILcom/mojang/blaze3d/platform/NativeImage;)V # \ No newline at end of file +public net.minecraft.client.renderer.texture.TextureAtlasSprite (Lnet/minecraft/client/renderer/texture/TextureAtlas;Lnet/minecraft/client/renderer/texture/TextureAtlasSprite$Info;IIIIILcom/mojang/blaze3d/platform/NativeImage;)V # +public net.minecraft.resources.ResourceKey (Lnet/minecraft/resources/ResourceLocation;Lnet/minecraft/resources/ResourceLocation;)V # \ No newline at end of file diff --git a/src/main/resources/modernfix.mixins.json b/src/main/resources/modernfix.mixins.json index a9af9c66..f8320632 100644 --- a/src/main/resources/modernfix.mixins.json +++ b/src/main/resources/modernfix.mixins.json @@ -64,6 +64,8 @@ "perf.kubejs.CustomIngredientMixin", "perf.nbt_memory_usage.CompoundTagMixin", "perf.fast_registry_validation.ForgeRegistryMixin", + "perf.fast_registry_validation.ForgeRegistrySnapshotMixin", + "perf.fast_registry_validation.ResourceKeyMixin", "perf.cache_strongholds.ChunkGeneratorMixin", "perf.cache_upgraded_structures.StructureManagerMixin", "perf.cache_strongholds.ServerLevelMixin", @@ -73,7 +75,8 @@ "perf.compress_blockstate.BlockBehaviourMixin", "perf.dedup_blockstate_flattening_map.BlockStateDataMixin", "perf.dedup_blockstate_flattening_map.ChunkPalettedStorageFixMixin", - "devenv.MinecraftServerMixin" + "devenv.MinecraftServerMixin", + "devenv.GameDataMixin" ], "client": [ "core.MinecraftMixin", From b9cb33b1ef95e01ab1320236e045be513d0a0408 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 28 Apr 2023 17:27:33 -0400 Subject: [PATCH 04/16] Dynamically generate item model location cache --- .../dynamicresources/ModelLocationCache.java | 23 +++++++++++++++++-- .../ItemModelShaperMixin.java | 9 +++++++- .../dynamic_resources/ItemRendererMixin.java | 20 ++++++++++++++++ src/main/resources/modernfix.mixins.json | 1 + 4 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ItemRendererMixin.java diff --git a/src/main/java/org/embeddedt/modernfix/dynamicresources/ModelLocationCache.java b/src/main/java/org/embeddedt/modernfix/dynamicresources/ModelLocationCache.java index 25886e20..11938b3b 100644 --- a/src/main/java/org/embeddedt/modernfix/dynamicresources/ModelLocationCache.java +++ b/src/main/java/org/embeddedt/modernfix/dynamicresources/ModelLocationCache.java @@ -10,8 +10,10 @@ import net.minecraft.Util; import net.minecraft.client.renderer.block.BlockModelShaper; import net.minecraft.client.resources.model.ModelResourceLocation; import net.minecraft.core.Registry; +import net.minecraft.world.item.Item; import net.minecraft.world.level.block.Block; import net.minecraft.world.level.block.state.BlockState; +import net.minecraftforge.registries.IRegistryDelegate; import java.util.ArrayList; import java.util.Collections; @@ -20,7 +22,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; public class ModelLocationCache { - private static final LoadingCache locationCache = CacheBuilder.newBuilder() + private static final LoadingCache blockLocationCache = CacheBuilder.newBuilder() .maximumSize(10000) .build(new CacheLoader() { @Override @@ -29,9 +31,26 @@ public class ModelLocationCache { } }); + private static final LoadingCache itemLocationCache = CacheBuilder.newBuilder() + .maximumSize(10000) + .build(new CacheLoader() { + @Override + public ModelResourceLocation load(Item key) throws Exception { + return new ModelResourceLocation(Registry.ITEM.getKey(key), "inventory"); + } + }); + public static ModelResourceLocation get(BlockState state) { try { - return locationCache.get(state); + return blockLocationCache.get(state); + } catch(ExecutionException e) { + throw new RuntimeException(e.getCause()); + } + } + + public static ModelResourceLocation get(Item item) { + try { + return itemLocationCache.get(item); } catch(ExecutionException e) { throw new RuntimeException(e.getCause()); } diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ItemModelShaperMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ItemModelShaperMixin.java index 0ca70841..86b708bd 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ItemModelShaperMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ItemModelShaperMixin.java @@ -7,6 +7,7 @@ import net.minecraft.client.resources.model.ModelResourceLocation; import net.minecraft.world.item.Item; import net.minecraftforge.client.ItemModelMesherForge; import net.minecraftforge.registries.IRegistryDelegate; +import org.embeddedt.modernfix.dynamicresources.ModelLocationCache; import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Overwrite; @@ -22,6 +23,8 @@ public abstract class ItemModelShaperMixin extends ItemModelShaper { super(arg); } + private static final ModelResourceLocation SENTINEL = new ModelResourceLocation("modernfix", "sentinel"); + /** * @reason Get the stored location for that item and meta, and get the model * from that location from the model manager. @@ -29,7 +32,11 @@ public abstract class ItemModelShaperMixin extends ItemModelShaper { @Overwrite @Override public BakedModel getItemModel(Item item) { - ModelResourceLocation map = locations.get(item.delegate); + ModelResourceLocation map = locations.getOrDefault(item.delegate, SENTINEL); + if(map == SENTINEL) { + /* generate the appropriate location from our cache */ + map = ModelLocationCache.get(item); + } return map == null ? null : getModelManager().getModel(map); } diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ItemRendererMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ItemRendererMixin.java new file mode 100644 index 00000000..3e47df2d --- /dev/null +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ItemRendererMixin.java @@ -0,0 +1,20 @@ +package org.embeddedt.modernfix.mixin.perf.dynamic_resources; + +import net.minecraft.client.renderer.ItemModelShaper; +import net.minecraft.client.renderer.entity.ItemRenderer; +import net.minecraft.client.resources.model.ModelResourceLocation; +import net.minecraft.world.item.Item; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Redirect; + +@Mixin(ItemRenderer.class) +public class ItemRendererMixin { + /** + * Don't waste space putting all these locations into the cache, compute them on demand later. + */ + @Redirect(method = "", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/renderer/ItemModelShaper;register(Lnet/minecraft/world/item/Item;Lnet/minecraft/client/resources/model/ModelResourceLocation;)V")) + private void skipDefaultRegistration(ItemModelShaper shaper, Item item, ModelResourceLocation mrl) { + + } +} diff --git a/src/main/resources/modernfix.mixins.json b/src/main/resources/modernfix.mixins.json index f8320632..71141447 100644 --- a/src/main/resources/modernfix.mixins.json +++ b/src/main/resources/modernfix.mixins.json @@ -92,6 +92,7 @@ "perf.dynamic_resources.BlockElementFaceDeserializerMixin", "perf.dynamic_resources.BlockModelShaperMixin", "perf.dynamic_resources.ItemModelShaperMixin", + "perf.dynamic_resources.ItemRendererMixin", "perf.dynamic_resources.ModelBakeryMixin", "perf.dynamic_resources.ae2.RegistrationMixin", "perf.dynamic_resources.ctm.TextureMetadataHandlerMixin", From 3f24f50744fdffee6ee0d41e3fa5fc688db853ec Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 28 Apr 2023 19:14:31 -0400 Subject: [PATCH 05/16] Initial version of fast registry --- .../ForgeRegistryMixin.java | 36 +- .../modernfix/registry/FastForgeRegistry.java | 674 ++++++++++++++++++ 2 files changed, 708 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java index 8300cf9f..2745e3f1 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java @@ -1,10 +1,19 @@ package org.embeddedt.modernfix.mixin.perf.fast_registry_validation; +import com.google.common.collect.BiMap; +import net.minecraft.core.Registry; +import net.minecraft.resources.ResourceKey; +import net.minecraft.resources.ResourceLocation; import net.minecraftforge.fml.common.ObfuscationReflectionHelper; import net.minecraftforge.registries.ForgeRegistry; +import net.minecraftforge.registries.IForgeRegistryEntry; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.Marker; +import org.embeddedt.modernfix.registry.FastForgeRegistry; +import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.Mutable; +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.Redirect; @@ -12,10 +21,10 @@ import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; import java.lang.reflect.Method; -import java.util.BitSet; +import java.util.*; @Mixin(value = ForgeRegistry.class, remap = false) -public class ForgeRegistryMixin { +public class ForgeRegistryMixin> { private static Method bitSetTrimMethod = null; private static boolean bitSetTrimMethodRetrieved = false; @@ -54,8 +63,31 @@ public class ForgeRegistryMixin { expectedNextBit = -1; } + /* @Redirect(method = "add(ILnet/minecraftforge/registries/IForgeRegistryEntry;Ljava/lang/String;)I", at = @At(value = "INVOKE", target = "Lorg/apache/logging/log4j/Logger;trace(Lorg/apache/logging/log4j/Marker;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)V")) private void skipTrace(Logger logger, Marker marker, String s, Object o, Object o1, Object o2, Object o3, Object o4) { } + + */ + + @Shadow @Final @Mutable private BiMap ids; + + @Shadow @Final @Mutable private BiMap, V> keys; + + @Shadow @Final private ResourceKey> key; + + @Shadow @Final @Mutable private BiMap names; + + /** + * The following code replaces the Forge HashBiMaps with a more efficient data structure based around + * an array list for IDs and one HashMap going from value -> information. + */ + @Inject(method = "", at = @At("RETURN")) + private void replaceBackingMaps(CallbackInfo ci) { + FastForgeRegistry fastReg = new FastForgeRegistry<>(this.key); + this.ids = fastReg.getIds(); + this.keys = fastReg.getKeys(); + this.names = fastReg.getNames(); + } } diff --git a/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java b/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java new file mode 100644 index 00000000..f01208c7 --- /dev/null +++ b/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java @@ -0,0 +1,674 @@ +package org.embeddedt.modernfix.registry; + +import com.google.common.collect.BiMap; +import com.google.common.collect.Iterators; +import com.google.common.collect.Maps; +import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; +import it.unimi.dsi.fastutil.objects.ObjectArrayList; +import net.minecraft.core.Registry; +import net.minecraft.resources.ResourceKey; +import net.minecraft.resources.ResourceLocation; +import net.minecraftforge.registries.IForgeRegistryEntry; +import org.apache.commons.lang3.tuple.MutablePair; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.*; +import java.util.function.BiConsumer; +import java.util.function.Consumer; +import java.util.stream.Collectors; + +public class FastForgeRegistry> { + private BiMap ids; + private BiMap names; + private BiMap, V> keys; + private ResourceKey> registryKey; + + private ObjectArrayList valuesById; + private Map, Integer>> infoByValue; + private Map, V> valuesByKey = new Object2ObjectOpenHashMap<>(); + + private void storeId(V value, int id) { + MutablePair, Integer> pair = infoByValue.computeIfAbsent(value, k -> new MutablePair<>(null, null)); + pair.setRight(id); + } + + private void updateInfoPairAndClearIfNull(V v, Consumer, Integer>> consumer) { + infoByValue.compute(v, (oldValue, oldPair) -> { + if(oldPair == null) + oldPair = new MutablePair<>(null, null); + consumer.accept(oldPair); + if(oldPair.getLeft() == null && oldPair.getRight() == null) + return null; + else + return oldPair; + }); + } + + private void ensureArrayCanFitId(int id) { + int desiredSize = id + 1; + while(valuesById.size() < desiredSize) { + valuesById.add(null); + } + } + + public FastForgeRegistry(ResourceKey> registryKey) { + this.registryKey = registryKey; + this.valuesById = new ObjectArrayList<>(); + this.infoByValue = new HashMap<>(); + this.ids = new BiMap() { + @Nullable + @Override + public V put(@Nullable Integer key, @Nullable V value) { + ensureArrayCanFitId(key); + V oldValue = valuesById.get(key); + if(oldValue != null) + throw new IllegalArgumentException("Existing mapping"); + valuesById.set(key, value); + storeId(value, key); + return null; + } + + @Nullable + @Override + public V forcePut(@Nullable Integer key, @Nullable V value) { + ensureArrayCanFitId(key); + V oldValue = valuesById.set(key, value); + if(oldValue != null) { + updateInfoPairAndClearIfNull(oldValue, pair -> pair.setRight(null)); + } + storeId(value, key); + return oldValue; + } + + @Override + public void putAll(Map map) { + map.forEach(this::put); + } + + @Override + public Set values() { + return Collections.unmodifiableSet(infoByValue.keySet()); + } + + @Override + public BiMap inverse() { + return new BiMap() { + @Nullable + @Override + public Integer put(@Nullable V key, @Nullable Integer value) { + throw new UnsupportedOperationException(); + } + + @Nullable + @Override + public Integer forcePut(@Nullable V key, @Nullable Integer value) { + throw new UnsupportedOperationException(); + } + + @Override + public void putAll(Map map) { + throw new UnsupportedOperationException(); + } + + @Override + public Set values() { + throw new UnsupportedOperationException(); + } + + @Override + public BiMap inverse() { + throw new UnsupportedOperationException(); + } + + @Override + public int size() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isEmpty() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsKey(Object key) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsValue(Object value) { + throw new UnsupportedOperationException(); + } + + @Override + public Integer get(Object key) { + MutablePair, Integer> pair = infoByValue.get(key); + if(pair == null) + return null; + return pair.getRight(); + } + + @Override + public Integer remove(Object key) { + MutablePair, Integer> pair = infoByValue.get(key); + if(pair == null) + return null; + int id = pair.getRight(); + valuesById.set(id, null); + updateInfoPairAndClearIfNull((V)key, p -> p.setRight(null)); + return id; + } + + @Override + public void clear() { + throw new UnsupportedOperationException(); + } + + @NotNull + @Override + public Set keySet() { + throw new UnsupportedOperationException(); + } + + @NotNull + @Override + public Set> entrySet() { + throw new UnsupportedOperationException(); + } + }; + } + + @Override + public int size() { + return infoByValue.size(); + } + + @Override + public boolean isEmpty() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsKey(Object key) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsValue(Object value) { + throw new UnsupportedOperationException(); + } + + @Override + public V get(Object key) { + int id = (Integer)key; + if(id < 0 || id >= valuesById.size()) + return null; + else + return valuesById.get(id); + } + + @Override + public V remove(Object key) { + throw new UnsupportedOperationException(); + } + + @Override + public void clear() { + valuesById.clear(); + infoByValue.values().removeIf(pair -> { + pair.setRight(null); + return pair.getLeft() == null && pair.getRight() == null; + }); + } + + @NotNull + @Override + public Set keySet() { + throw new UnsupportedOperationException(); + } + + @NotNull + @Override + public Set> entrySet() { + throw new UnsupportedOperationException(); + } + + @Override + public void forEach(BiConsumer action) { + for(int i = 0 ; i < valuesById.size(); i++) { + V val = valuesById.get(i); + if(val != null) + action.accept(i, val); + } + } + }; + this.keys = new BiMap, V>() { + @Nullable + @Override + public V put(@Nullable ResourceKey key, @Nullable V value) { + if(valuesByKey.get(key) != null) + throw new IllegalArgumentException("Existing mapping"); + return forcePut(key, value); + } + + @Nullable + @Override + public V forcePut(@Nullable ResourceKey key, @Nullable V value) { + V oldValue = valuesByKey.put(key, value); + if(oldValue != null) { + updateInfoPairAndClearIfNull(oldValue, p -> p.setLeft(null)); + } + updateInfoPairAndClearIfNull(value, p -> p.setLeft(key)); + return oldValue; + } + + @Override + public void putAll(Map, ? extends V> map) { + map.forEach(this::put); + } + + @Override + public Set values() { + throw new UnsupportedOperationException(); + } + + @Override + public BiMap> inverse() { + return new BiMap>() { + @Nullable + @Override + public ResourceKey put(@Nullable V key, @Nullable ResourceKey value) { + throw new UnsupportedOperationException(); + } + + @Nullable + @Override + public ResourceKey forcePut(@Nullable V key, @Nullable ResourceKey value) { + throw new UnsupportedOperationException(); + } + + @Override + public void putAll(Map> map) { + throw new UnsupportedOperationException(); + } + + @Override + public Set> values() { + throw new UnsupportedOperationException(); + } + + @Override + public BiMap, V> inverse() { + throw new UnsupportedOperationException(); + } + + @Override + public int size() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isEmpty() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsKey(Object key) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsValue(Object value) { + throw new UnsupportedOperationException(); + } + + @Override + public ResourceKey get(Object key) { + MutablePair, Integer> pair = infoByValue.get(key); + if(pair == null) + return null; + else + return pair.getLeft(); + } + + @Override + public ResourceKey remove(Object key) { + MutablePair, Integer> pair = infoByValue.get(key); + if(pair == null) + return null; + else { + ResourceKey rk = pair.getLeft(); + valuesByKey.remove(rk); + updateInfoPairAndClearIfNull((V)key, p -> p.setLeft(null)); + return rk; + } + } + + @Override + public void clear() { + throw new UnsupportedOperationException(); + } + + @NotNull + @Override + public Set keySet() { + throw new UnsupportedOperationException(); + } + + @NotNull + @Override + public Set>> entrySet() { + throw new UnsupportedOperationException(); + } + }; + } + + @Override + public int size() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isEmpty() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsKey(Object key) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsValue(Object value) { + throw new UnsupportedOperationException(); + } + + @Override + public V get(Object key) { + return null; + } + + @Override + public V remove(Object key) { + throw new UnsupportedOperationException(); + } + + @Override + public void clear() { + valuesByKey.values().forEach(v -> updateInfoPairAndClearIfNull(v, p -> p.setLeft(null))); + valuesByKey.clear(); + } + + @NotNull + @Override + public Set> keySet() { + throw new UnsupportedOperationException(); + } + + @NotNull + @Override + public Set, V>> entrySet() { + return valuesByKey.entrySet(); + } + }; + this.names = new BiMap() { + @Nullable + @Override + public V put(@Nullable ResourceLocation key, @Nullable V value) { + // not needed + return null; + } + + @Nullable + @Override + public V forcePut(@Nullable ResourceLocation key, @Nullable V value) { + return null; + } + + @Override + public void putAll(Map map) { + map.forEach(this::put); + } + + @Override + public Set values() { + return infoByValue.keySet(); + } + + @Override + public BiMap inverse() { + return new BiMap() { + @Nullable + @Override + public ResourceLocation put(@Nullable V key, @Nullable ResourceLocation value) { + throw new UnsupportedOperationException(); + } + + @Nullable + @Override + public ResourceLocation forcePut(@Nullable V key, @Nullable ResourceLocation value) { + throw new UnsupportedOperationException(); + } + + @Override + public void putAll(Map map) { + throw new UnsupportedOperationException(); + } + + @Override + public Set values() { + throw new UnsupportedOperationException(); + } + + @Override + public BiMap inverse() { + throw new UnsupportedOperationException(); + } + + @Override + public int size() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isEmpty() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsKey(Object key) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsValue(Object value) { + throw new UnsupportedOperationException(); + } + + @Override + public ResourceLocation get(Object key) { + MutablePair, Integer> pair = infoByValue.get(key); + if(pair == null || pair.getLeft() == null) + return null; + else + return pair.getLeft().location(); + } + + @Override + public ResourceLocation remove(Object key) { + throw new UnsupportedOperationException(); + } + + @Override + public void clear() { + throw new UnsupportedOperationException(); + } + + @NotNull + @Override + public Set keySet() { + throw new UnsupportedOperationException(); + } + + @NotNull + @Override + public Set> entrySet() { + throw new UnsupportedOperationException(); + } + }; + } + + @Override + public int size() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isEmpty() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsKey(Object key) { + ResourceKey rk = ResourceKey.create(FastForgeRegistry.this.registryKey, (ResourceLocation)key); + return valuesByKey.containsKey(rk); + } + + @Override + public boolean containsValue(Object value) { + return infoByValue.containsValue(value); + } + + @Override + public V get(Object key) { + ResourceKey rk = ResourceKey.create(FastForgeRegistry.this.registryKey, (ResourceLocation)key); + return valuesByKey.get(rk); + } + + @Override + public V remove(Object key) { + // we need to return a non-null value to prevent Forge throwing, but the actual removal is done by this.keys + return get(key); + } + + @Override + public void clear() { + // ditto + } + + @NotNull + @Override + public Set keySet() { + return new FastForgeRegistryLocationSet(valuesByKey.keySet()); + } + + @NotNull + @Override + public Set> entrySet() { + return valuesByKey.entrySet().stream().map(entry -> new AbstractMap.SimpleImmutableEntry<>(entry.getKey().location(), entry.getValue())).collect(Collectors.toSet()); + } + }; + } + + public BiMap getIds() { + return ids; + } + + public BiMap, V> getKeys() { + return keys; + } + + public BiMap getNames() { + return names; + } + + class FastForgeRegistryLocationSet implements Set { + private final Set> backingSet; + + public FastForgeRegistryLocationSet(Set> backingSet) { + this.backingSet = backingSet; + } + + @Override + public int size() { + return backingSet.size(); + } + + @Override + public boolean isEmpty() { + return backingSet.isEmpty(); + } + + @Override + public boolean contains(Object o) { + return backingSet.contains(ResourceKey.create(FastForgeRegistry.this.registryKey, (ResourceLocation)o)); + } + + @NotNull + @Override + public Iterator iterator() { + return Iterators.transform(backingSet.iterator(), ResourceKey::location); + } + + @NotNull + @Override + public Object[] toArray() { + Object[] keyArray = backingSet.toArray(); + for(int i = 0; i < keyArray.length; i++) { + keyArray[i] = ((ResourceKey)keyArray[i]).location(); + } + return keyArray; + } + + @NotNull + @Override + public T[] toArray(@NotNull T[] a) { + Object[] keyArray = backingSet.toArray(); + T[] finalArray = Arrays.copyOf(a, keyArray.length); + for(int i = 0; i < keyArray.length; i++) { + finalArray[i] = (T)((ResourceKey)keyArray[i]).location(); + } + return finalArray; + } + + @Override + public boolean add(ResourceLocation resourceLocation) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean remove(Object o) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsAll(@NotNull Collection c) { + for(Object o : c) { + if(!contains(o)) + return false; + } + return true; + } + + @Override + public boolean addAll(@NotNull Collection c) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean retainAll(@NotNull Collection c) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean removeAll(@NotNull Collection c) { + throw new UnsupportedOperationException(); + } + + @Override + public void clear() { + throw new UnsupportedOperationException(); + } + } +} From 599bdee17357c86b3aed61a92a6bbe3d217c17e7 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 28 Apr 2023 19:22:38 -0400 Subject: [PATCH 06/16] Refactor registry data into a custom object --- .../modernfix/registry/FastForgeRegistry.java | 61 +++++++++++-------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java b/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java index f01208c7..c03e9940 100644 --- a/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java +++ b/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java @@ -2,14 +2,12 @@ package org.embeddedt.modernfix.registry; import com.google.common.collect.BiMap; import com.google.common.collect.Iterators; -import com.google.common.collect.Maps; import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; import it.unimi.dsi.fastutil.objects.ObjectArrayList; import net.minecraft.core.Registry; import net.minecraft.resources.ResourceKey; import net.minecraft.resources.ResourceLocation; import net.minecraftforge.registries.IForgeRegistryEntry; -import org.apache.commons.lang3.tuple.MutablePair; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -25,20 +23,20 @@ public class FastForgeRegistry> { private ResourceKey> registryKey; private ObjectArrayList valuesById; - private Map, Integer>> infoByValue; + private Map infoByValue; private Map, V> valuesByKey = new Object2ObjectOpenHashMap<>(); private void storeId(V value, int id) { - MutablePair, Integer> pair = infoByValue.computeIfAbsent(value, k -> new MutablePair<>(null, null)); - pair.setRight(id); + RegistryValueData pair = infoByValue.computeIfAbsent(value, k -> new RegistryValueData()); + pair.id = id; } - private void updateInfoPairAndClearIfNull(V v, Consumer, Integer>> consumer) { + private void updateInfoPairAndClearIfNull(V v, Consumer consumer) { infoByValue.compute(v, (oldValue, oldPair) -> { if(oldPair == null) - oldPair = new MutablePair<>(null, null); + oldPair = new RegistryValueData(); consumer.accept(oldPair); - if(oldPair.getLeft() == null && oldPair.getRight() == null) + if(oldPair.isEmpty()) return null; else return oldPair; @@ -75,7 +73,7 @@ public class FastForgeRegistry> { ensureArrayCanFitId(key); V oldValue = valuesById.set(key, value); if(oldValue != null) { - updateInfoPairAndClearIfNull(oldValue, pair -> pair.setRight(null)); + updateInfoPairAndClearIfNull(oldValue, pair -> pair.id = null); } storeId(value, key); return oldValue; @@ -143,20 +141,20 @@ public class FastForgeRegistry> { @Override public Integer get(Object key) { - MutablePair, Integer> pair = infoByValue.get(key); + RegistryValueData pair = infoByValue.get(key); if(pair == null) return null; - return pair.getRight(); + return pair.id; } @Override public Integer remove(Object key) { - MutablePair, Integer> pair = infoByValue.get(key); + RegistryValueData pair = infoByValue.get(key); if(pair == null) return null; - int id = pair.getRight(); + int id = pair.id; valuesById.set(id, null); - updateInfoPairAndClearIfNull((V)key, p -> p.setRight(null)); + updateInfoPairAndClearIfNull((V)key, p -> p.id = null); return id; } @@ -217,8 +215,8 @@ public class FastForgeRegistry> { public void clear() { valuesById.clear(); infoByValue.values().removeIf(pair -> { - pair.setRight(null); - return pair.getLeft() == null && pair.getRight() == null; + pair.id = null; + return pair.isEmpty(); }); } @@ -257,9 +255,9 @@ public class FastForgeRegistry> { public V forcePut(@Nullable ResourceKey key, @Nullable V value) { V oldValue = valuesByKey.put(key, value); if(oldValue != null) { - updateInfoPairAndClearIfNull(oldValue, p -> p.setLeft(null)); + updateInfoPairAndClearIfNull(oldValue, p -> p.key = null); } - updateInfoPairAndClearIfNull(value, p -> p.setLeft(key)); + updateInfoPairAndClearIfNull(value, p -> p.key = key); return oldValue; } @@ -325,22 +323,22 @@ public class FastForgeRegistry> { @Override public ResourceKey get(Object key) { - MutablePair, Integer> pair = infoByValue.get(key); + RegistryValueData pair = infoByValue.get(key); if(pair == null) return null; else - return pair.getLeft(); + return (ResourceKey)pair.key; } @Override public ResourceKey remove(Object key) { - MutablePair, Integer> pair = infoByValue.get(key); + RegistryValueData pair = infoByValue.get(key); if(pair == null) return null; else { - ResourceKey rk = pair.getLeft(); + ResourceKey rk = (ResourceKey)pair.key; valuesByKey.remove(rk); - updateInfoPairAndClearIfNull((V)key, p -> p.setLeft(null)); + updateInfoPairAndClearIfNull((V)key, p -> p.key = null); return rk; } } @@ -396,7 +394,7 @@ public class FastForgeRegistry> { @Override public void clear() { - valuesByKey.values().forEach(v -> updateInfoPairAndClearIfNull(v, p -> p.setLeft(null))); + valuesByKey.values().forEach(v -> updateInfoPairAndClearIfNull(v, p -> p.key = null)); valuesByKey.clear(); } @@ -488,11 +486,11 @@ public class FastForgeRegistry> { @Override public ResourceLocation get(Object key) { - MutablePair, Integer> pair = infoByValue.get(key); - if(pair == null || pair.getLeft() == null) + RegistryValueData pair = infoByValue.get(key); + if(pair == null || pair.key == null) return null; else - return pair.getLeft().location(); + return pair.key.location(); } @Override @@ -671,4 +669,13 @@ public class FastForgeRegistry> { throw new UnsupportedOperationException(); } } + + static class RegistryValueData { + public ResourceKey key; + public Integer id; + + boolean isEmpty() { + return key == null && id == null; + } + } } From 433db3409a0934c67fbb3c603f719366866dcbbe Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 28 Apr 2023 21:30:51 -0400 Subject: [PATCH 07/16] Continue optimizing --- .../ForgeRegistryMixin.java | 34 +- .../modernfix/registry/FastForgeRegistry.java | 579 ++++++++---------- 2 files changed, 275 insertions(+), 338 deletions(-) diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java index 2745e3f1..c00a2c85 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java @@ -63,14 +63,11 @@ public class ForgeRegistryMixin> { expectedNextBit = -1; } - /* @Redirect(method = "add(ILnet/minecraftforge/registries/IForgeRegistryEntry;Ljava/lang/String;)I", at = @At(value = "INVOKE", target = "Lorg/apache/logging/log4j/Logger;trace(Lorg/apache/logging/log4j/Marker;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)V")) private void skipTrace(Logger logger, Marker marker, String s, Object o, Object o1, Object o2, Object o3, Object o4) { } - */ - @Shadow @Final @Mutable private BiMap ids; @Shadow @Final @Mutable private BiMap, V> keys; @@ -79,15 +76,38 @@ public class ForgeRegistryMixin> { @Shadow @Final @Mutable private BiMap names; + @Shadow @Final @Mutable private BiMap owners; + + private FastForgeRegistry fastRegistry; + /** * The following code replaces the Forge HashBiMaps with a more efficient data structure based around * an array list for IDs and one HashMap going from value -> information. */ @Inject(method = "", at = @At("RETURN")) private void replaceBackingMaps(CallbackInfo ci) { - FastForgeRegistry fastReg = new FastForgeRegistry<>(this.key); - this.ids = fastReg.getIds(); - this.keys = fastReg.getKeys(); - this.names = fastReg.getNames(); + this.fastRegistry = new FastForgeRegistry<>(this.key); + this.ids = fastRegistry.getIds(); + this.keys = fastRegistry.getKeys(); + this.names = fastRegistry.getNames(); + this.owners = fastRegistry.getOwners(); } + + @Inject(method = "freeze", at = @At("RETURN")) + private void optimizeRegistry(CallbackInfo ci) { + this.fastRegistry.optimize(); + } + + /* + @Redirect(method = "sync", at = @At(value = "INVOKE", target = "Lcom/google/common/collect/BiMap;clear()V")) + private void clearBiMap(BiMap map) { + if(map == this.owners) { + this.fastRegistry.clear(); + } else if(map == this.keys || map == this.names || map == this.ids) { + // do nothing, the registry is faster at clearing everything at once + } else + map.clear(); + } + + */ } diff --git a/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java b/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java index c03e9940..ecee2a18 100644 --- a/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java +++ b/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java @@ -14,17 +14,18 @@ import org.jetbrains.annotations.Nullable; import java.util.*; import java.util.function.BiConsumer; import java.util.function.Consumer; +import java.util.function.Function; import java.util.stream.Collectors; public class FastForgeRegistry> { private BiMap ids; - private BiMap names; - private BiMap, V> keys; + private DataFieldBiMap names; + private DataFieldBiMap> keys; + private DataFieldBiMap owners; private ResourceKey> registryKey; private ObjectArrayList valuesById; - private Map infoByValue; - private Map, V> valuesByKey = new Object2ObjectOpenHashMap<>(); + private Object2ObjectOpenHashMap infoByValue; private void storeId(V value, int id) { RegistryValueData pair = infoByValue.computeIfAbsent(value, k -> new RegistryValueData()); @@ -50,10 +51,23 @@ public class FastForgeRegistry> { } } + public void clear() { + this.infoByValue.clear(); + for(int i = 0; i < this.valuesById.size(); i++) { + this.valuesById.set(i, null); + } + this.names.clearUnsafe(); + this.keys.clearUnsafe(); + } + public FastForgeRegistry(ResourceKey> registryKey) { this.registryKey = registryKey; this.valuesById = new ObjectArrayList<>(); - this.infoByValue = new HashMap<>(); + this.infoByValue = new Object2ObjectOpenHashMap<>(); + this.keys = new DataFieldBiMap<>(p -> (ResourceKey) p.key, (p, k) -> p.key = k); + this.owners = new DataFieldBiMap<>(p -> p.overrideOwner, (p, k) -> p.overrideOwner = k); + this.names = new DataFieldBiMap<>(p -> p.location, (p, l) -> p.location = l); + // IDs require a specialized implementation, as we back the K->V direction with an array this.ids = new BiMap() { @Nullable @Override @@ -241,332 +255,13 @@ public class FastForgeRegistry> { } } }; - this.keys = new BiMap, V>() { - @Nullable - @Override - public V put(@Nullable ResourceKey key, @Nullable V value) { - if(valuesByKey.get(key) != null) - throw new IllegalArgumentException("Existing mapping"); - return forcePut(key, value); - } + } - @Nullable - @Override - public V forcePut(@Nullable ResourceKey key, @Nullable V value) { - V oldValue = valuesByKey.put(key, value); - if(oldValue != null) { - updateInfoPairAndClearIfNull(oldValue, p -> p.key = null); - } - updateInfoPairAndClearIfNull(value, p -> p.key = key); - return oldValue; - } - - @Override - public void putAll(Map, ? extends V> map) { - map.forEach(this::put); - } - - @Override - public Set values() { - throw new UnsupportedOperationException(); - } - - @Override - public BiMap> inverse() { - return new BiMap>() { - @Nullable - @Override - public ResourceKey put(@Nullable V key, @Nullable ResourceKey value) { - throw new UnsupportedOperationException(); - } - - @Nullable - @Override - public ResourceKey forcePut(@Nullable V key, @Nullable ResourceKey value) { - throw new UnsupportedOperationException(); - } - - @Override - public void putAll(Map> map) { - throw new UnsupportedOperationException(); - } - - @Override - public Set> values() { - throw new UnsupportedOperationException(); - } - - @Override - public BiMap, V> inverse() { - throw new UnsupportedOperationException(); - } - - @Override - public int size() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean isEmpty() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean containsKey(Object key) { - throw new UnsupportedOperationException(); - } - - @Override - public boolean containsValue(Object value) { - throw new UnsupportedOperationException(); - } - - @Override - public ResourceKey get(Object key) { - RegistryValueData pair = infoByValue.get(key); - if(pair == null) - return null; - else - return (ResourceKey)pair.key; - } - - @Override - public ResourceKey remove(Object key) { - RegistryValueData pair = infoByValue.get(key); - if(pair == null) - return null; - else { - ResourceKey rk = (ResourceKey)pair.key; - valuesByKey.remove(rk); - updateInfoPairAndClearIfNull((V)key, p -> p.key = null); - return rk; - } - } - - @Override - public void clear() { - throw new UnsupportedOperationException(); - } - - @NotNull - @Override - public Set keySet() { - throw new UnsupportedOperationException(); - } - - @NotNull - @Override - public Set>> entrySet() { - throw new UnsupportedOperationException(); - } - }; - } - - @Override - public int size() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean isEmpty() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean containsKey(Object key) { - throw new UnsupportedOperationException(); - } - - @Override - public boolean containsValue(Object value) { - throw new UnsupportedOperationException(); - } - - @Override - public V get(Object key) { - return null; - } - - @Override - public V remove(Object key) { - throw new UnsupportedOperationException(); - } - - @Override - public void clear() { - valuesByKey.values().forEach(v -> updateInfoPairAndClearIfNull(v, p -> p.key = null)); - valuesByKey.clear(); - } - - @NotNull - @Override - public Set> keySet() { - throw new UnsupportedOperationException(); - } - - @NotNull - @Override - public Set, V>> entrySet() { - return valuesByKey.entrySet(); - } - }; - this.names = new BiMap() { - @Nullable - @Override - public V put(@Nullable ResourceLocation key, @Nullable V value) { - // not needed - return null; - } - - @Nullable - @Override - public V forcePut(@Nullable ResourceLocation key, @Nullable V value) { - return null; - } - - @Override - public void putAll(Map map) { - map.forEach(this::put); - } - - @Override - public Set values() { - return infoByValue.keySet(); - } - - @Override - public BiMap inverse() { - return new BiMap() { - @Nullable - @Override - public ResourceLocation put(@Nullable V key, @Nullable ResourceLocation value) { - throw new UnsupportedOperationException(); - } - - @Nullable - @Override - public ResourceLocation forcePut(@Nullable V key, @Nullable ResourceLocation value) { - throw new UnsupportedOperationException(); - } - - @Override - public void putAll(Map map) { - throw new UnsupportedOperationException(); - } - - @Override - public Set values() { - throw new UnsupportedOperationException(); - } - - @Override - public BiMap inverse() { - throw new UnsupportedOperationException(); - } - - @Override - public int size() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean isEmpty() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean containsKey(Object key) { - throw new UnsupportedOperationException(); - } - - @Override - public boolean containsValue(Object value) { - throw new UnsupportedOperationException(); - } - - @Override - public ResourceLocation get(Object key) { - RegistryValueData pair = infoByValue.get(key); - if(pair == null || pair.key == null) - return null; - else - return pair.key.location(); - } - - @Override - public ResourceLocation remove(Object key) { - throw new UnsupportedOperationException(); - } - - @Override - public void clear() { - throw new UnsupportedOperationException(); - } - - @NotNull - @Override - public Set keySet() { - throw new UnsupportedOperationException(); - } - - @NotNull - @Override - public Set> entrySet() { - throw new UnsupportedOperationException(); - } - }; - } - - @Override - public int size() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean isEmpty() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean containsKey(Object key) { - ResourceKey rk = ResourceKey.create(FastForgeRegistry.this.registryKey, (ResourceLocation)key); - return valuesByKey.containsKey(rk); - } - - @Override - public boolean containsValue(Object value) { - return infoByValue.containsValue(value); - } - - @Override - public V get(Object key) { - ResourceKey rk = ResourceKey.create(FastForgeRegistry.this.registryKey, (ResourceLocation)key); - return valuesByKey.get(rk); - } - - @Override - public V remove(Object key) { - // we need to return a non-null value to prevent Forge throwing, but the actual removal is done by this.keys - return get(key); - } - - @Override - public void clear() { - // ditto - } - - @NotNull - @Override - public Set keySet() { - return new FastForgeRegistryLocationSet(valuesByKey.keySet()); - } - - @NotNull - @Override - public Set> entrySet() { - return valuesByKey.entrySet().stream().map(entry -> new AbstractMap.SimpleImmutableEntry<>(entry.getKey().location(), entry.getValue())).collect(Collectors.toSet()); - } - }; + public void optimize() { + this.keys.optimize(); + this.owners.optimize(); + this.names.optimize(); + this.infoByValue.trim(); } public BiMap getIds() { @@ -581,6 +276,226 @@ public class FastForgeRegistry> { return names; } + public DataFieldBiMap getOwners() { + return owners; + } + + /** + * Custom BiMap implementation that uses one internal hash map for the K->V direction, and the shared global + * information hash map for the V->K direction. + */ + class DataFieldBiMap implements BiMap { + public final Object2ObjectOpenHashMap valuesByKey = new Object2ObjectOpenHashMap<>(); + private final Function getter; + private final BiConsumer setter; + + public DataFieldBiMap(Function getter, BiConsumer setter) { + this.getter = getter; + this.setter = setter; + } + + public void optimize() { + this.valuesByKey.trim(); + } + + public void clearUnsafe() { + this.valuesByKey.clear(); + } + + private V forcePut(@Nullable K key, @Nullable V value, boolean throwOnExisting) { + V oldValue = valuesByKey.put(key, value); + if(oldValue != null) { + if(throwOnExisting) { + valuesByKey.put(key, oldValue); + throw new IllegalArgumentException("Existing mapping"); + } else { + updateInfoPairAndClearIfNull(oldValue, p -> setter.accept(p, null)); + } + } + updateInfoPairAndClearIfNull(value, p -> setter.accept(p, key)); + return oldValue; + } + + @Nullable + @Override + public V put(@Nullable K key, @Nullable V value) { + return forcePut(key, value, true); + } + + @Nullable + @Override + public V forcePut(@Nullable K key, @Nullable V value) { + return forcePut(key, value, false); + } + + @Override + public void putAll(Map map) { + map.forEach(this::put); + } + + @Override + public Set values() { + return Collections.unmodifiableSet(infoByValue.keySet()); + } + + private DataFieldBiMapInverse inverse = null; + + @Override + public BiMap inverse() { + if(inverse == null) + inverse = new DataFieldBiMapInverse<>(this); + return inverse; + } + + @Override + public int size() { + return valuesByKey.size(); + } + + @Override + public boolean isEmpty() { + return valuesByKey.isEmpty(); + } + + @Override + public boolean containsKey(Object key) { + return valuesByKey.containsKey(key); + } + + @Override + public boolean containsValue(Object value) { + return infoByValue.containsKey(value); + } + + @Override + public V get(Object key) { + return valuesByKey.get(key); + } + + @Override + public V remove(Object key) { + V value = get(key); + if(value == null) + return null; + inverse().remove(value); + return value; + } + + @Override + public void clear() { + valuesByKey.values().forEach(v -> updateInfoPairAndClearIfNull(v, p -> p.key = null)); + valuesByKey.clear(); + } + + @NotNull + @Override + public Set keySet() { + return Collections.unmodifiableSet(valuesByKey.keySet()); + } + + @NotNull + @Override + public Set> entrySet() { + return Collections.unmodifiableSet(valuesByKey.entrySet()); + } + + + } + + class DataFieldBiMapInverse implements BiMap { + private final DataFieldBiMap forward; + + public DataFieldBiMapInverse(DataFieldBiMap forward) { + this.forward = forward; + } + + @Nullable + @Override + public K put(@Nullable V key, @Nullable K value) { + throw new UnsupportedOperationException(); + } + + @Nullable + @Override + public K forcePut(@Nullable V key, @Nullable K value) { + throw new UnsupportedOperationException(); + } + + @Override + public void putAll(Map map) { + throw new UnsupportedOperationException(); + } + + @Override + public Set values() { + throw new UnsupportedOperationException(); + } + + @Override + public BiMap inverse() { + return forward; + } + + @Override + public int size() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isEmpty() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsKey(Object key) { + return infoByValue.containsKey(key); + } + + @Override + public boolean containsValue(Object value) { + return forward.valuesByKey.containsKey(value); + } + + @Override + public K get(Object key) { + RegistryValueData pair = infoByValue.get(key); + if(pair == null) + return null; + else + return forward.getter.apply(pair); + } + + @Override + public K remove(Object key) { + RegistryValueData pair = infoByValue.get(key); + if(pair == null) + return null; + else { + K rk = forward.getter.apply(pair); + forward.valuesByKey.remove(rk); + updateInfoPairAndClearIfNull((V)key, p -> forward.setter.accept(p, null)); + return rk; + } + } + + @Override + public void clear() { + throw new UnsupportedOperationException(); + } + + @NotNull + @Override + public Set keySet() { + throw new UnsupportedOperationException(); + } + + @NotNull + @Override + public Set> entrySet() { + throw new UnsupportedOperationException(); + } + } + class FastForgeRegistryLocationSet implements Set { private final Set> backingSet; @@ -672,10 +587,12 @@ public class FastForgeRegistry> { static class RegistryValueData { public ResourceKey key; + public ResourceLocation location; public Integer id; + public Object overrideOwner; boolean isEmpty() { - return key == null && id == null; + return key == null && location == null && id == null && overrideOwner == null; } } } From c09c4ccf68f3a2f5ba746990fbed9aaf37e6fda2 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 28 Apr 2023 21:33:57 -0400 Subject: [PATCH 08/16] Optimize clear() --- .../ForgeRegistryMixin.java | 3 --- .../modernfix/registry/FastForgeRegistry.java | 15 ++++++++------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java index c00a2c85..09b7daf7 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/fast_registry_validation/ForgeRegistryMixin.java @@ -98,7 +98,6 @@ public class ForgeRegistryMixin> { this.fastRegistry.optimize(); } - /* @Redirect(method = "sync", at = @At(value = "INVOKE", target = "Lcom/google/common/collect/BiMap;clear()V")) private void clearBiMap(BiMap map) { if(map == this.owners) { @@ -108,6 +107,4 @@ public class ForgeRegistryMixin> { } else map.clear(); } - - */ } diff --git a/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java b/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java index ecee2a18..673cec5e 100644 --- a/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java +++ b/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java @@ -18,14 +18,14 @@ import java.util.function.Function; import java.util.stream.Collectors; public class FastForgeRegistry> { - private BiMap ids; - private DataFieldBiMap names; - private DataFieldBiMap> keys; - private DataFieldBiMap owners; - private ResourceKey> registryKey; + private final BiMap ids; + private final DataFieldBiMap names; + private final DataFieldBiMap> keys; + private final DataFieldBiMap owners; + private final ResourceKey> registryKey; - private ObjectArrayList valuesById; - private Object2ObjectOpenHashMap infoByValue; + private final ObjectArrayList valuesById; + private final Object2ObjectOpenHashMap infoByValue; private void storeId(V value, int id) { RegistryValueData pair = infoByValue.computeIfAbsent(value, k -> new RegistryValueData()); @@ -58,6 +58,7 @@ public class FastForgeRegistry> { } this.names.clearUnsafe(); this.keys.clearUnsafe(); + this.owners.clearUnsafe(); } public FastForgeRegistry(ResourceKey> registryKey) { From 8b71c823c4397a182c24f88ca9f76c6a71a19613 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 28 Apr 2023 21:40:22 -0400 Subject: [PATCH 09/16] Unbox IDs --- .../modernfix/registry/FastForgeRegistry.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java b/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java index 673cec5e..d56e6136 100644 --- a/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java +++ b/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java @@ -88,7 +88,7 @@ public class FastForgeRegistry> { ensureArrayCanFitId(key); V oldValue = valuesById.set(key, value); if(oldValue != null) { - updateInfoPairAndClearIfNull(oldValue, pair -> pair.id = null); + updateInfoPairAndClearIfNull(oldValue, pair -> pair.id = -1); } storeId(value, key); return oldValue; @@ -159,7 +159,7 @@ public class FastForgeRegistry> { RegistryValueData pair = infoByValue.get(key); if(pair == null) return null; - return pair.id; + return pair.id == -1 ? null : pair.id; } @Override @@ -168,9 +168,10 @@ public class FastForgeRegistry> { if(pair == null) return null; int id = pair.id; - valuesById.set(id, null); - updateInfoPairAndClearIfNull((V)key, p -> p.id = null); - return id; + if(id != -1) + valuesById.set(id, null); + updateInfoPairAndClearIfNull((V)key, p -> p.id = -1); + return id == -1 ? null : id; } @Override @@ -230,7 +231,7 @@ public class FastForgeRegistry> { public void clear() { valuesById.clear(); infoByValue.values().removeIf(pair -> { - pair.id = null; + pair.id = -1; return pair.isEmpty(); }); } @@ -589,11 +590,11 @@ public class FastForgeRegistry> { static class RegistryValueData { public ResourceKey key; public ResourceLocation location; - public Integer id; + public int id; public Object overrideOwner; boolean isEmpty() { - return key == null && location == null && id == null && overrideOwner == null; + return key == null && location == null && id == -1 && overrideOwner == null; } } } From 39e9dfab99b762b241f276d7b3075ff4d98e9dad Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 28 Apr 2023 22:06:21 -0400 Subject: [PATCH 10/16] Use getResources on resource packs we can trust --- .../dynamic_resources/ModelBakeryMixin.java | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java index 49010ac4..51efcde1 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java @@ -12,6 +12,7 @@ import com.google.gson.*; import com.mojang.datafixers.util.Pair; import com.mojang.math.Transformation; import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; +import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; import net.minecraft.Util; import net.minecraft.client.color.block.BlockColors; import net.minecraft.client.renderer.block.model.BlockModel; @@ -20,8 +21,11 @@ import net.minecraft.client.renderer.texture.AtlasSet; import net.minecraft.client.renderer.texture.TextureAtlas; import net.minecraft.client.renderer.texture.TextureAtlasSprite; import net.minecraft.client.renderer.texture.TextureManager; +import net.minecraft.client.resources.ClientPackSource; import net.minecraft.client.resources.model.*; import net.minecraft.resources.ResourceLocation; +import net.minecraft.server.packs.*; +import net.minecraft.server.packs.resources.FallbackResourceManager; import net.minecraft.server.packs.resources.Resource; import net.minecraft.server.packs.resources.ResourceManager; import net.minecraft.util.profiling.ProfilerFiller; @@ -35,6 +39,8 @@ import net.minecraftforge.client.model.ModelLoader; import net.minecraftforge.client.model.ModelLoaderRegistry; import net.minecraftforge.common.MinecraftForge; import net.minecraftforge.fml.ModLoader; +import net.minecraftforge.fml.packs.DelegatingResourcePack; +import net.minecraftforge.fml.packs.ModFileResourcePack; import net.minecraftforge.registries.ForgeRegistries; import net.minecraftforge.registries.ForgeRegistryEntry; import org.apache.commons.lang3.tuple.Triple; @@ -207,6 +213,15 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { ModernFix.LOGGER.error("Error reading {} {}: {}", type, location, e); } + private boolean trustedResourcePack(PackResources pack) { + return pack instanceof VanillaPackResources || + pack instanceof ModFileResourcePack || + pack instanceof ClientPackSource || + pack instanceof DelegatingResourcePack || + pack instanceof FolderPackResources || + pack instanceof FilePackResources; + } + /** * Load all blockstate JSONs and model files, collect textures. */ @@ -214,6 +229,58 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { Stopwatch stopwatch = Stopwatch.createStarted(); List>> blockStateData = new ArrayList<>(); final Object2IntOpenHashMap blockstateErrors = new Object2IntOpenHashMap<>(); + /* + * First, gather all vanilla packs, and use listResources on them. This will allow us to (hopefully) avoid + * scanning most packs a lot. + */ + List allPackResources = new ArrayList<>(this.resourceManager.listPacks().collect(Collectors.toList())); + Collections.reverse(allPackResources); + ObjectOpenHashSet allAvailableModels = new ObjectOpenHashSet<>(), allAvailableStates = new ObjectOpenHashSet<>(); + allPackResources.removeIf(pack -> { + if(trustedResourcePack(pack)) { + for(String namespace : pack.getNamespaces(PackType.CLIENT_RESOURCES)) { + Collection allBlockstates = pack.getResources(PackType.CLIENT_RESOURCES, namespace, "blockstates", Integer.MAX_VALUE, p -> p.endsWith(".json")); + for(ResourceLocation blockstate : allBlockstates) { + allAvailableStates.add(new ResourceLocation(blockstate.getNamespace(), blockstate.getPath().replace("blockstates/", "").replace(".json", ""))); + } + Collection allModels = pack.getResources(PackType.CLIENT_RESOURCES, namespace, "models", Integer.MAX_VALUE, p -> p.endsWith(".json")); + for(ResourceLocation blockstate : allModels) { + allAvailableModels.add(new ResourceLocation(blockstate.getNamespace(), blockstate.getPath().replace("models/", "").replace(".json", ""))); + } + } + return true; + } + ModernFix.LOGGER.debug("Pack with class {} needs manual scan", pack.getClass().getName()); + return false; + }); + if(allPackResources.size() > 0) { + /* Now make a fallback resource manager and use it on the remaining packs to see if they actually contain these files */ + FallbackResourceManager frm = new FallbackResourceManager(PackType.CLIENT_RESOURCES, "dummy"); + for(int i = allPackResources.size() - 1; i >= 0; i--) { + frm.add(allPackResources.get(i)); + } + for(ResourceLocation blockstate : blockStateFiles) { + ResourceLocation fileLocation = new ResourceLocation(blockstate.getNamespace(), "blockstates/" + blockstate.getPath() + ".json"); + try(Resource resource = frm.getResource(fileLocation)) { + allAvailableStates.add(blockstate); + } catch(IOException ignored) { + } + } + for(ResourceLocation model : modelFiles) { + ResourceLocation fileLocation = new ResourceLocation(model.getNamespace(), "models/" + model.getPath() + ".json"); + try(Resource resource = frm.getResource(fileLocation)) { + allAvailableModels.add(model); + } catch(IOException ignored) { + } + } + } + // We now have a list of all vanilla resources known to exist. Delete anything that we don't have + blockStateFiles.retainAll(allAvailableStates); + modelFiles.retainAll(allAvailableModels); + allAvailableModels.clear(); + allAvailableModels.trim(); + allAvailableStates.clear(); + allAvailableStates.trim(); for(ResourceLocation blockstate : blockStateFiles) { blockStateData.add(CompletableFuture.supplyAsync(() -> { ResourceLocation fileLocation = new ResourceLocation(blockstate.getNamespace(), "blockstates/" + blockstate.getPath() + ".json"); From 12a7483d4dbcbb0b35d55eebc9378d9aa4e85024 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 28 Apr 2023 22:06:35 -0400 Subject: [PATCH 11/16] Add large registry test option --- .../java/org/embeddedt/modernfix/ModernFix.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/main/java/org/embeddedt/modernfix/ModernFix.java b/src/main/java/org/embeddedt/modernfix/ModernFix.java index 5a761ba3..5ee1161f 100644 --- a/src/main/java/org/embeddedt/modernfix/ModernFix.java +++ b/src/main/java/org/embeddedt/modernfix/ModernFix.java @@ -5,8 +5,10 @@ import net.minecraft.Util; import net.minecraft.server.level.ChunkHolder; import net.minecraft.server.level.ChunkMap; import net.minecraft.server.level.ServerLevel; +import net.minecraft.world.item.Item; import net.minecraftforge.api.distmarker.Dist; import net.minecraftforge.common.MinecraftForge; +import net.minecraftforge.event.RegistryEvent; import net.minecraftforge.eventbus.api.EventPriority; import net.minecraftforge.eventbus.api.SubscribeEvent; import net.minecraftforge.fml.*; @@ -21,6 +23,7 @@ import net.minecraftforge.fml.event.server.FMLServerStoppedEvent; import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext; import net.minecraftforge.fml.loading.FMLLoader; import net.minecraftforge.fml.network.FMLNetworkConstants; +import net.minecraftforge.registries.ForgeRegistries; import org.apache.commons.lang3.tuple.Pair; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -101,6 +104,7 @@ public class ModernFix { MinecraftForge.EVENT_BUS.register(this); FMLJavaModLoadingContext.get().getModEventBus().addListener(this::commonSetup); FMLJavaModLoadingContext.get().getModEventBus().addListener(this::onLoadComplete); + FMLJavaModLoadingContext.get().getModEventBus().addGenericListener(Item.class, this::registerItems); DistExecutor.unsafeRunWhenOn(Dist.CLIENT, () -> () -> MinecraftForge.EVENT_BUS.register(new ModernFixClient())); ModLoadingContext.get().registerExtensionPoint(ExtensionPoint.DISPLAYTEST, () -> Pair.of(() -> FMLNetworkConstants.IGNORESERVERONLY, (a, b) -> true)); ModLoadingContext.get().registerConfig(ModConfig.Type.COMMON, ModernFixConfig.COMMON_CONFIG); @@ -111,6 +115,15 @@ public class ModernFix { ModFileScanDataDeduplicator.deduplicate(); } + private void registerItems(RegistryEvent event) { + if(Boolean.getBoolean("modernfix.largeRegistryTest")) { + Item.Properties props = new Item.Properties(); + for(int i = 0; i < 1000000; i++) { + ForgeRegistries.ITEMS.register(new Item(props).setRegistryName("modernfix", "item_" + i)); + } + } + } + private static boolean dfuModPresent() { for(String modId : new String[] { "lazydfu", "datafixerslayer" }) { if(ModList.get().isLoaded(modId)) From 6665db3a69032e4c07465fc3052d9ce0c07d81ed Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 28 Apr 2023 22:34:44 -0400 Subject: [PATCH 12/16] Only use the fallback path for models/blockstates not discovered yet --- .../mixin/perf/dynamic_resources/ModelBakeryMixin.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java index 51efcde1..a68bd3d5 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java @@ -260,6 +260,8 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { frm.add(allPackResources.get(i)); } for(ResourceLocation blockstate : blockStateFiles) { + if(allAvailableStates.contains(blockstate)) + continue; // don't check ones we know exist ResourceLocation fileLocation = new ResourceLocation(blockstate.getNamespace(), "blockstates/" + blockstate.getPath() + ".json"); try(Resource resource = frm.getResource(fileLocation)) { allAvailableStates.add(blockstate); @@ -267,6 +269,8 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { } } for(ResourceLocation model : modelFiles) { + if(allAvailableModels.contains(model)) + continue; // don't check ones we know exist ResourceLocation fileLocation = new ResourceLocation(model.getNamespace(), "models/" + model.getPath() + ".json"); try(Resource resource = frm.getResource(fileLocation)) { allAvailableModels.add(model); From c0c789f29c7719aff2acac3a82cd3d4c53807779 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 28 Apr 2023 22:57:33 -0400 Subject: [PATCH 13/16] Fix registry replacement --- .../modernfix/registry/FastForgeRegistry.java | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java b/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java index d56e6136..88a65de0 100644 --- a/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java +++ b/src/main/java/org/embeddedt/modernfix/registry/FastForgeRegistry.java @@ -73,13 +73,14 @@ public class FastForgeRegistry> { @Nullable @Override public V put(@Nullable Integer key, @Nullable V value) { - ensureArrayCanFitId(key); - V oldValue = valuesById.get(key); - if(oldValue != null) - throw new IllegalArgumentException("Existing mapping"); - valuesById.set(key, value); - storeId(value, key); - return null; + RegistryValueData data = infoByValue.get(value); + int unboxedKey = key; + if(data != null && data.id != -1 && data.id != unboxedKey) + throw new IllegalArgumentException("Existing mapping for ID " + data.id + " value " + value + " when new ID " + unboxedKey + " was requested"); + ensureArrayCanFitId(unboxedKey); + V oldValue = valuesById.set(unboxedKey, value); + storeId(value, unboxedKey); + return oldValue; } @Nullable @@ -305,14 +306,16 @@ public class FastForgeRegistry> { } private V forcePut(@Nullable K key, @Nullable V value, boolean throwOnExisting) { + if(throwOnExisting) { + RegistryValueData dataForValue = infoByValue.get(value); + // null check could be wrong if null is a valid value but doesn't matter in Forge's use + if(dataForValue != null && getter.apply(dataForValue) != null && !Objects.equals(getter.apply(dataForValue), key)) { + throw new IllegalArgumentException("Existing mapping for key " + key + " value " + value); + } + } V oldValue = valuesByKey.put(key, value); if(oldValue != null) { - if(throwOnExisting) { - valuesByKey.put(key, oldValue); - throw new IllegalArgumentException("Existing mapping"); - } else { - updateInfoPairAndClearIfNull(oldValue, p -> setter.accept(p, null)); - } + updateInfoPairAndClearIfNull(oldValue, p -> setter.accept(p, null)); } updateInfoPairAndClearIfNull(value, p -> setter.accept(p, key)); return oldValue; @@ -590,7 +593,7 @@ public class FastForgeRegistry> { static class RegistryValueData { public ResourceKey key; public ResourceLocation location; - public int id; + public int id = -1; public Object overrideOwner; boolean isEmpty() { From e4ef3103cc62a2e96fb9d8480940c42b4a31c740 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 29 Apr 2023 09:17:41 -0400 Subject: [PATCH 14/16] Fix CTM crash --- .../dynamic_resources/ctm/TextureMetadataHandlerMixin.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ctm/TextureMetadataHandlerMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ctm/TextureMetadataHandlerMixin.java index dd15d1bb..67ad067d 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ctm/TextureMetadataHandlerMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ctm/TextureMetadataHandlerMixin.java @@ -15,6 +15,7 @@ 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.Redirect; import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; import team.chisel.ctm.CTM; import team.chisel.ctm.client.model.AbstractCTMBakedModel; @@ -36,6 +37,11 @@ public abstract class TextureMetadataHandlerMixin { MinecraftForge.EVENT_BUS.addListener(this::onDynamicModelBake); } + @Redirect(method = "onModelBake", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/resources/model/BakedModel;isCustomRenderer()Z")) + private boolean checkModelValid(BakedModel model) { + return model == null || model.isCustomRenderer(); + } + public void onDynamicModelBake(DynamicModelBakeEvent event) { UnbakedModel rootModel = event.getUnbakedModel(); BakedModel baked = event.getModel(); From 9b50a50baddc4f39d544cd779f7db6af176926a0 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 29 Apr 2023 09:22:55 -0400 Subject: [PATCH 15/16] Allow mutating CanonizingStringMap.keySet() --- .../java/org/embeddedt/modernfix/util/CanonizingStringMap.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java b/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java index 737e9bca..e5bb2f2d 100644 --- a/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java +++ b/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java @@ -98,7 +98,8 @@ public class CanonizingStringMap implements Map { @NotNull @Override public Set keySet() { - return Collections.unmodifiableSet(this.backingMap.keySet()); + // has to be modifiable because mods (cough, Tinkers) use it to clear the tag + return this.backingMap.keySet(); } @NotNull From 960dd4074ec68523e807af90138f2b91cc0a8c9a Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 29 Apr 2023 09:38:13 -0400 Subject: [PATCH 16/16] Clean up scanner + abstract pack scanning to separate function --- .../dynamic_resources/ModelBakeryMixin.java | 61 ++++++++++--------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java index a68bd3d5..4e973f57 100644 --- a/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java +++ b/src/main/java/org/embeddedt/modernfix/mixin/perf/dynamic_resources/ModelBakeryMixin.java @@ -222,6 +222,26 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { pack instanceof FilePackResources; } + private void gatherAdditionalViaManualScan(List untrustedPacks, Set knownLocations, + Collection uncertainLocations, String filePrefix) { + if(untrustedPacks.size() > 0) { + /* Now make a fallback resource manager and use it on the remaining packs to see if they actually contain these files */ + FallbackResourceManager frm = new FallbackResourceManager(PackType.CLIENT_RESOURCES, "dummy"); + for (int i = untrustedPacks.size() - 1; i >= 0; i--) { + frm.add(untrustedPacks.get(i)); + } + for (ResourceLocation blockstate : uncertainLocations) { + if (knownLocations.contains(blockstate)) + continue; // don't check ones we know exist + ResourceLocation fileLocation = new ResourceLocation(blockstate.getNamespace(), filePrefix + blockstate.getPath() + ".json"); + try (Resource resource = frm.getResource(fileLocation)) { + knownLocations.add(blockstate); + } catch (IOException ignored) { + } + } + } + } + /** * Load all blockstate JSONs and model files, collect textures. */ @@ -253,38 +273,14 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { ModernFix.LOGGER.debug("Pack with class {} needs manual scan", pack.getClass().getName()); return false; }); - if(allPackResources.size() > 0) { - /* Now make a fallback resource manager and use it on the remaining packs to see if they actually contain these files */ - FallbackResourceManager frm = new FallbackResourceManager(PackType.CLIENT_RESOURCES, "dummy"); - for(int i = allPackResources.size() - 1; i >= 0; i--) { - frm.add(allPackResources.get(i)); - } - for(ResourceLocation blockstate : blockStateFiles) { - if(allAvailableStates.contains(blockstate)) - continue; // don't check ones we know exist - ResourceLocation fileLocation = new ResourceLocation(blockstate.getNamespace(), "blockstates/" + blockstate.getPath() + ".json"); - try(Resource resource = frm.getResource(fileLocation)) { - allAvailableStates.add(blockstate); - } catch(IOException ignored) { - } - } - for(ResourceLocation model : modelFiles) { - if(allAvailableModels.contains(model)) - continue; // don't check ones we know exist - ResourceLocation fileLocation = new ResourceLocation(model.getNamespace(), "models/" + model.getPath() + ".json"); - try(Resource resource = frm.getResource(fileLocation)) { - allAvailableModels.add(model); - } catch(IOException ignored) { - } - } - } - // We now have a list of all vanilla resources known to exist. Delete anything that we don't have + + gatherAdditionalViaManualScan(allPackResources, allAvailableStates, blockStateFiles, "blockstates/"); + // We now have a list of all blockstates known to exist. Delete anything that we don't have blockStateFiles.retainAll(allAvailableStates); - modelFiles.retainAll(allAvailableModels); - allAvailableModels.clear(); - allAvailableModels.trim(); allAvailableStates.clear(); allAvailableStates.trim(); + + for(ResourceLocation blockstate : blockStateFiles) { blockStateData.add(CompletableFuture.supplyAsync(() -> { ResourceLocation fileLocation = new ResourceLocation(blockstate.getNamespace(), "blockstates/" + blockstate.getPath() + ".json"); @@ -353,6 +349,13 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { }); blockstateErrors.clear(); blockStateData = null; + + /* figure out which models we should actually load */ + gatherAdditionalViaManualScan(allPackResources, allAvailableModels, modelFiles, "models/"); + modelFiles.retainAll(allAvailableModels); + allAvailableModels.clear(); + allAvailableModels.trim(); + Map basicModels = new HashMap<>(); basicModels.put(MISSING_MODEL_LOCATION, (BlockModel)missingModel); basicModels.put(new ResourceLocation("builtin/generated"), GENERATION_MARKER);