From 467d4818d835ba9b8b6c3b0bc0fd515d6182f17a Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sun, 30 Jul 2023 20:19:59 -0400 Subject: [PATCH 1/6] Default reuse_datapacks to false This optimization is only meaningful when swapping worlds (probably uncommon among players) and it's hard to predict what mods will have issues. Can be enabled by modpack devs for their own packs/testing if they wish --- .../embeddedt/modernfix/core/config/ModernFixEarlyConfig.java | 1 + 1 file changed, 1 insertion(+) diff --git a/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java b/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java index da24f0ac..5c589b05 100644 --- a/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java +++ b/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java @@ -160,6 +160,7 @@ public class ModernFixEarlyConfig { private static final ImmutableMap DEFAULT_SETTING_OVERRIDES = new DefaultSettingMapBuilder() .put("mixin.perf.dynamic_resources", false) + .put("mixin.perf.reuse_datapacks", false) .put("mixin.feature.direct_stack_trace", false) .putConditionally(ModernFixPlatformHooks.INSTANCE::isDevEnv, "mixin.perf.rewrite_registry", false) .put("mixin.perf.clear_mixin_classinfo", false) From 4fe235cdbd9ef58870ab260489c6bc45e2fec510 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 31 Jul 2023 11:14:38 -0400 Subject: [PATCH 2/6] Workaround for mods instantiating PathResourcePack incorrectly --- .../forge/load/ModResourcePackPathFixer.java | 21 +++++++++++++++++++ .../ModFileResourcePackMixin.java | 17 +++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/load/ModResourcePackPathFixer.java diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/load/ModResourcePackPathFixer.java b/forge/src/main/java/org/embeddedt/modernfix/forge/load/ModResourcePackPathFixer.java new file mode 100644 index 00000000..d9e9f7c7 --- /dev/null +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/load/ModResourcePackPathFixer.java @@ -0,0 +1,21 @@ +package org.embeddedt.modernfix.forge.load; + +import net.minecraftforge.fml.ModList; +import net.minecraftforge.forgespi.language.IModFileInfo; +import net.minecraftforge.forgespi.locating.IModFile; + +import java.nio.file.Path; +import java.util.IdentityHashMap; + +public class ModResourcePackPathFixer { + private static final IdentityHashMap modFileByPath = new IdentityHashMap<>(); + + public static synchronized IModFile getModFileByRootPath(Path path) { + if(modFileByPath.size() == 0) { + for(IModFileInfo info : ModList.get().getModFiles()) { + modFileByPath.put(info.getFile().getFilePath(), info.getFile()); + } + } + return modFileByPath.get(path); + } +} diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/resourcepacks/ModFileResourcePackMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/resourcepacks/ModFileResourcePackMixin.java index ff0046d2..a095389c 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/resourcepacks/ModFileResourcePackMixin.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/resourcepacks/ModFileResourcePackMixin.java @@ -2,7 +2,10 @@ package org.embeddedt.modernfix.forge.mixin.perf.resourcepacks; import net.minecraft.server.packs.PackType; import net.minecraft.resources.ResourceLocation; +import net.minecraftforge.forgespi.locating.IModFile; import net.minecraftforge.resource.PathResourcePack; +import org.embeddedt.modernfix.ModernFix; +import org.embeddedt.modernfix.forge.load.ModResourcePackPathFixer; import org.embeddedt.modernfix.resources.ICachingResourcePack; import org.embeddedt.modernfix.resources.PackResourcesCacheEngine; import org.embeddedt.modernfix.util.PackTypeHelper; @@ -25,12 +28,26 @@ public abstract class ModFileResourcePackMixin implements ICachingResourcePack { private PackResourcesCacheEngine cacheEngine; + private IModFile mfix$resolveFileOverride; + @Inject(method = "", at = @At("TAIL")) private void cacheResources(String packName, Path source, CallbackInfo ci) { + // handle buggy mods instantiating at the root path, but only if they didn't override at all + // (otherwise they may have handled resolve() already) + if(((Object)this).getClass() == PathResourcePack.class) + this.mfix$resolveFileOverride = ModResourcePackPathFixer.getModFileByRootPath(source); + if(this.mfix$resolveFileOverride != null) + ModernFix.LOGGER.warn("PathResourcePack base class instantiated with root path of mod file {}. This probably means a mod should be calling ResourcePackLoader.createPackForMod instead. Applying workaround.", mfix$resolveFileOverride.getFileName()); invalidateCache(); PackResourcesCacheEngine.track(this); } + @Inject(method = "resolve", at = @At("HEAD"), cancellable = true, remap = false) + private void resolveViaModFile(String[] paths, CallbackInfoReturnable cir) { + if(this.mfix$resolveFileOverride != null) + cir.setReturnValue(this.mfix$resolveFileOverride.findResource(paths)); + } + private PackResourcesCacheEngine generateResourceCache() { synchronized (this) { PackResourcesCacheEngine engine = this.cacheEngine; From 3e4f1ab23aebb0436261e8c7bcf2f91a67f9878f Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 31 Jul 2023 13:13:39 -0400 Subject: [PATCH 3/6] Improve registry performance with large entry counts --- .../MappedRegistryMixin.java | 36 +++++++++++++++++++ .../ResourceKeyMixin.java | 4 +-- 2 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/mojang_registry_size/MappedRegistryMixin.java rename {forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/fast_registry_validation => common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/mojang_registry_size}/ResourceKeyMixin.java (85%) diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/mojang_registry_size/MappedRegistryMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/mojang_registry_size/MappedRegistryMixin.java new file mode 100644 index 00000000..ad9adc3c --- /dev/null +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/mojang_registry_size/MappedRegistryMixin.java @@ -0,0 +1,36 @@ +package org.embeddedt.modernfix.common.mixin.perf.mojang_registry_size; + +import it.unimi.dsi.fastutil.objects.ObjectArrayList; +import it.unimi.dsi.fastutil.objects.ObjectList; +import net.minecraft.core.MappedRegistry; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Redirect; + +@Mixin(MappedRegistry.class) +public class MappedRegistryMixin { + /** + * Avoid copying the ID list to a slightly larger one every time an entry is added to the registry. + * The original behavior causes O(n) time complexity for registration. + */ + @Redirect( + method = "registerMapping(ILnet/minecraft/resources/ResourceKey;Ljava/lang/Object;Lcom/mojang/serialization/Lifecycle;Z)Ljava/lang/Object;", + at = @At(value = "INVOKE", target = "Lit/unimi/dsi/fastutil/objects/ObjectList;size(I)V") + ) + private void setSizeSmart(ObjectList list, int size) { + if(list instanceof ObjectArrayList && size > list.size()) { + int requestedSize = size; + /* choose next power of two, or this value if it is a power of two */ + int p2Size = Integer.highestOneBit(size); + if(p2Size != size) + size = p2Size << 1; + // grow backing array to power-of-two size, this will return instantly in most cases + ((ObjectArrayList)list).ensureCapacity(size); + // write null entries to fill size, to match the behavior of list.size(int) + while(list.size() < requestedSize) + list.add(null); + } else { + list.size(size); + } + } +} diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/fast_registry_validation/ResourceKeyMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/mojang_registry_size/ResourceKeyMixin.java similarity index 85% rename from forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/fast_registry_validation/ResourceKeyMixin.java rename to common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/mojang_registry_size/ResourceKeyMixin.java index 0c62d3fe..7d6701db 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/fast_registry_validation/ResourceKeyMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/mojang_registry_size/ResourceKeyMixin.java @@ -1,4 +1,4 @@ -package org.embeddedt.modernfix.forge.mixin.perf.fast_registry_validation; +package org.embeddedt.modernfix.common.mixin.perf.mojang_registry_size; import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; import net.minecraft.resources.ResourceKey; @@ -12,7 +12,7 @@ import java.util.Map; @Mixin(ResourceKey.class) public class ResourceKeyMixin { - private static Map>> INTERNING_MAP = new Object2ObjectOpenHashMap<>(); + private static final 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) { From 9d6f51695a0ad6c46f911c546472cdb29ca38fea Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 31 Jul 2023 13:14:48 -0400 Subject: [PATCH 4/6] Update MappedRegistry mixin for 1.18 --- .../mixin/perf/mojang_registry_size/MappedRegistryMixin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/mojang_registry_size/MappedRegistryMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/mojang_registry_size/MappedRegistryMixin.java index ad9adc3c..d3574148 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/mojang_registry_size/MappedRegistryMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/mojang_registry_size/MappedRegistryMixin.java @@ -14,7 +14,7 @@ public class MappedRegistryMixin { * The original behavior causes O(n) time complexity for registration. */ @Redirect( - method = "registerMapping(ILnet/minecraft/resources/ResourceKey;Ljava/lang/Object;Lcom/mojang/serialization/Lifecycle;Z)Ljava/lang/Object;", + method = "registerMapping(ILnet/minecraft/resources/ResourceKey;Ljava/lang/Object;Lcom/mojang/serialization/Lifecycle;Z)Lnet/minecraft/core/Holder;", at = @At(value = "INVOKE", target = "Lit/unimi/dsi/fastutil/objects/ObjectList;size(I)V") ) private void setSizeSmart(ObjectList list, int size) { From e540c9d58dffe8c1b28df82a4e8fbf6fc130d691 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 31 Jul 2023 14:22:30 -0400 Subject: [PATCH 5/6] Improve FerriteCore memory usage for blocks with one state --- .../blockstate/FerriteCorePostProcess.java | 56 +++++++++++++++++++ .../StateDefinitionMixin.java | 8 +++ 2 files changed, 64 insertions(+) create mode 100644 common/src/main/java/org/embeddedt/modernfix/blockstate/FerriteCorePostProcess.java diff --git a/common/src/main/java/org/embeddedt/modernfix/blockstate/FerriteCorePostProcess.java b/common/src/main/java/org/embeddedt/modernfix/blockstate/FerriteCorePostProcess.java new file mode 100644 index 00000000..372fb27d --- /dev/null +++ b/common/src/main/java/org/embeddedt/modernfix/blockstate/FerriteCorePostProcess.java @@ -0,0 +1,56 @@ +package org.embeddedt.modernfix.blockstate; + +import it.unimi.dsi.fastutil.objects.Object2IntArrayMap; +import it.unimi.dsi.fastutil.objects.Object2IntMap; +import it.unimi.dsi.fastutil.objects.Object2IntMaps; +import net.minecraft.world.level.block.state.StateDefinition; +import net.minecraft.world.level.block.state.StateHolder; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.reflect.Field; + +public class FerriteCorePostProcess { + private static final boolean willPostProcess; + + private static final MethodHandle theTable, toKeyIndex; + + static { + boolean success = true; + MethodHandle table = null, keyIndex = null; + try { + Class fastMap = Class.forName("malte0811.ferritecore.fastmap.FastMap"); + Field field = fastMap.getDeclaredField("toKeyIndex"); + field.setAccessible(true); + keyIndex = MethodHandles.publicLookup().unreflectSetter(field); + field = StateHolder.class.getDeclaredField("ferritecore_globalTable"); + field.setAccessible(true); + table = MethodHandles.publicLookup().unreflectGetter(field); + } catch(ReflectiveOperationException | RuntimeException e) { + e.printStackTrace(); + success = false; + } + willPostProcess = success; + theTable = table; + toKeyIndex = keyIndex; + } + + private static final Object2IntMap EMPTY_MAP = Object2IntMaps.unmodifiable(new Object2IntArrayMap<>()); + + public static > void postProcess(StateDefinition state) { + if(!willPostProcess) + return; + try { + if(state.getProperties().size() == 0) { + for(S holder : state.getPossibleStates()) { + // deduplicate Object2IntMap objects from FerriteCore + // will probably be fixed upstream at some point, but likely not for older versions + Object table = theTable.invoke(holder); + toKeyIndex.invoke(table, EMPTY_MAP); + } + } + } catch(Throwable e) { + e.printStackTrace(); + } + } +} diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/state_definition_construct/StateDefinitionMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/state_definition_construct/StateDefinitionMixin.java index 1d32168d..cf7f3dfb 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/state_definition_construct/StateDefinitionMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/state_definition_construct/StateDefinitionMixin.java @@ -6,11 +6,14 @@ import net.minecraft.world.level.block.state.StateHolder; import net.minecraft.world.level.block.state.properties.Property; import org.embeddedt.modernfix.annotation.RequiresMod; import org.embeddedt.modernfix.blockstate.FakeStateMap; +import org.embeddedt.modernfix.blockstate.FerriteCorePostProcess; import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Inject; import org.spongepowered.asm.mixin.injection.ModifyVariable; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; import java.util.Map; @@ -27,4 +30,9 @@ public class StateDefinitionMixin> { } return new FakeStateMap<>(numStates); } + + @Inject(method = "", at = @At("TAIL")) + private void postProcess(CallbackInfo ci) { + FerriteCorePostProcess.postProcess((StateDefinition)(Object)this); + } } From 49fa5bf14ac24c8583fa424db82409ad422dbc98 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 31 Jul 2023 15:23:37 -0400 Subject: [PATCH 6/6] Lock FerriteCore deduplication fix to dev only --- .../state_definition_construct/StateDefinitionMixin.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/state_definition_construct/StateDefinitionMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/state_definition_construct/StateDefinitionMixin.java index cf7f3dfb..70210f7b 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/state_definition_construct/StateDefinitionMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/state_definition_construct/StateDefinitionMixin.java @@ -7,6 +7,7 @@ import net.minecraft.world.level.block.state.properties.Property; import org.embeddedt.modernfix.annotation.RequiresMod; import org.embeddedt.modernfix.blockstate.FakeStateMap; import org.embeddedt.modernfix.blockstate.FerriteCorePostProcess; +import org.embeddedt.modernfix.platform.ModernFixPlatformHooks; import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; @@ -33,6 +34,8 @@ public class StateDefinitionMixin> { @Inject(method = "", at = @At("TAIL")) private void postProcess(CallbackInfo ci) { - FerriteCorePostProcess.postProcess((StateDefinition)(Object)this); + // keep in dev only until upstream FC releases + if(ModernFixPlatformHooks.INSTANCE.isDevEnv()) + FerriteCorePostProcess.postProcess((StateDefinition)(Object)this); } }