From 2e52db6e932abc310a3bbaa391ab492a5486847e Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 27 Jul 2024 12:43:48 -0400 Subject: [PATCH 1/6] Apply some simple optimizations for vanilla section meshing --- .../perf/chunk_meshing/RebuildTaskMixin.java | 31 +++++++++++++++ .../blockpos/SectionBlockPosIterator.java | 39 +++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/chunk_meshing/RebuildTaskMixin.java create mode 100644 common/src/main/java/org/embeddedt/modernfix/util/blockpos/SectionBlockPosIterator.java diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/chunk_meshing/RebuildTaskMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/chunk_meshing/RebuildTaskMixin.java new file mode 100644 index 00000000..b10b557b --- /dev/null +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/chunk_meshing/RebuildTaskMixin.java @@ -0,0 +1,31 @@ +package org.embeddedt.modernfix.common.mixin.perf.chunk_meshing; + +import com.llamalad7.mixinextras.sugar.Local; +import net.minecraft.client.renderer.chunk.RenderChunkRegion; +import net.minecraft.core.BlockPos; +import net.minecraft.world.level.block.state.BlockState; +import org.embeddedt.modernfix.util.blockpos.SectionBlockPosIterator; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Redirect; + +@Mixin(targets = { "net/minecraft/client/renderer/chunk/ChunkRenderDispatcher$RenderChunk$RebuildTask"}, priority = 2000) +public class RebuildTaskMixin { + /** + * @author embeddedt + * @reason Use a much faster iterator implementation than vanilla's Guava-based one. + */ + @Redirect(method = "compile", at = @At(value = "INVOKE", target = "Lnet/minecraft/core/BlockPos;betweenClosed(Lnet/minecraft/core/BlockPos;Lnet/minecraft/core/BlockPos;)Ljava/lang/Iterable;")) + private Iterable fastBetweenClosed(BlockPos firstPos, BlockPos secondPos) { + return () -> new SectionBlockPosIterator(firstPos); + } + + /** + * @author embeddedt + * @reason RenderChunkRegion.getBlockState is expensive, avoid calling it multiple times for the same position + */ + @Redirect(method = "compile", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/renderer/chunk/RenderChunkRegion;getBlockState(Lnet/minecraft/core/BlockPos;)Lnet/minecraft/world/level/block/state/BlockState;", ordinal = 1)) + private BlockState useExistingBlockState(RenderChunkRegion instance, BlockPos pos, @Local(ordinal = 0) BlockState state) { + return state; + } +} diff --git a/common/src/main/java/org/embeddedt/modernfix/util/blockpos/SectionBlockPosIterator.java b/common/src/main/java/org/embeddedt/modernfix/util/blockpos/SectionBlockPosIterator.java new file mode 100644 index 00000000..3521680d --- /dev/null +++ b/common/src/main/java/org/embeddedt/modernfix/util/blockpos/SectionBlockPosIterator.java @@ -0,0 +1,39 @@ +package org.embeddedt.modernfix.util.blockpos; + +import net.minecraft.core.BlockPos; + +import java.util.Iterator; +import java.util.NoSuchElementException; + +public class SectionBlockPosIterator implements Iterator { + private final BlockPos.MutableBlockPos pos = new BlockPos.MutableBlockPos(); + private int index = 0; + private final int baseX, baseY, baseZ; + + public SectionBlockPosIterator(int baseX, int baseY, int baseZ) { + this.baseX = baseX; + this.baseY = baseY; + this.baseZ = baseZ; + } + + public SectionBlockPosIterator(BlockPos pos) { + this(pos.getX(), pos.getY(), pos.getZ()); + } + + @Override + public boolean hasNext() { + return index < 4096; + } + + @Override + public BlockPos next() { + int i = index; + if (i >= 4096) { + throw new NoSuchElementException(); + } + index = i + 1; + var pos = this.pos; + pos.set(this.baseX + (i & 15), this.baseY + ((i >> 8) & 15), this.baseZ + ((i >> 4) & 15)); + return pos; + } +} From 6a365be734f0f794853bfa6a34be6564c51b941b Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 27 Jul 2024 12:44:02 -0400 Subject: [PATCH 2/6] Disable FAPI in dev --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index eb860025..140abcfd 100644 --- a/gradle.properties +++ b/gradle.properties @@ -27,7 +27,7 @@ diagonal_fences_version=4558828 spark_version=4587310 -use_fabric_api_at_runtime=true +use_fabric_api_at_runtime=false # Look up maven coordinates when changing shadow_version shadow_version=7.1.2 From 6f4212ebc8f8245b4f0c4cfbcb525966487e0fa4 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 27 Jul 2024 12:51:24 -0400 Subject: [PATCH 3/6] Unfreeze block registry ourselves in tests --- .../testing/util/BootstrapMinecraftExtension.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/fabric/src/test/java/org/embeddedt/modernfix/testing/util/BootstrapMinecraftExtension.java b/fabric/src/test/java/org/embeddedt/modernfix/testing/util/BootstrapMinecraftExtension.java index a2417e3c..5adb9bc6 100644 --- a/fabric/src/test/java/org/embeddedt/modernfix/testing/util/BootstrapMinecraftExtension.java +++ b/fabric/src/test/java/org/embeddedt/modernfix/testing/util/BootstrapMinecraftExtension.java @@ -1,12 +1,17 @@ package org.embeddedt.modernfix.testing.util; import net.minecraft.DetectedVersion; +import net.minecraft.core.MappedRegistry; +import net.minecraft.core.registries.BuiltInRegistries; import net.minecraft.server.Bootstrap; import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.Extension; import org.junit.jupiter.api.extension.ExtensionContext; +import java.lang.reflect.Field; +import java.util.IdentityHashMap; + /** * Simple extension to run vanilla bootstrap, inspired by AE2. */ @@ -15,6 +20,15 @@ public class BootstrapMinecraftExtension implements Extension, BeforeAllCallback public void beforeAll(ExtensionContext context) throws Exception { DetectedVersion.tryDetectVersion(); Bootstrap.bootStrap(); + // Allow blocks to be created in tests + Field field = MappedRegistry.class.getDeclaredField("unregisteredIntrusiveHolders"); + field.setAccessible(true); + if(field.get(BuiltInRegistries.BLOCK) == null) { + field.set(BuiltInRegistries.BLOCK, new IdentityHashMap<>()); + field = MappedRegistry.class.getDeclaredField("frozen"); + field.setAccessible(true); + field.setBoolean(BuiltInRegistries.BLOCK, false); + } } @Override From 87d1aad3d16e8029115fc75d9dc0bca93702389a Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 5 Aug 2024 09:48:10 -0400 Subject: [PATCH 4/6] Backport unbaked model caching from 1.21 --- .../dynamic_resources/ModelManagerMixin.java | 7 +++++-- .../embeddedt/modernfix/util/CacheUtil.java | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 common/src/main/java/org/embeddedt/modernfix/util/CacheUtil.java diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/ModelManagerMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/ModelManagerMixin.java index 392ba1c7..2a1ce44b 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/ModelManagerMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/ModelManagerMixin.java @@ -13,6 +13,7 @@ import net.minecraft.world.level.block.state.BlockState; import net.minecraft.world.level.block.state.StateDefinition; import org.embeddedt.modernfix.ModernFix; import org.embeddedt.modernfix.annotation.ClientOnlyMixin; +import org.embeddedt.modernfix.util.CacheUtil; import org.embeddedt.modernfix.util.LambdaMap; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; @@ -45,12 +46,14 @@ public class ModelManagerMixin { @Redirect(method = "reload", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/resources/model/ModelManager;loadBlockModels(Lnet/minecraft/server/packs/resources/ResourceManager;Ljava/util/concurrent/Executor;)Ljava/util/concurrent/CompletableFuture;")) private CompletableFuture> deferBlockModelLoad(ResourceManager manager, Executor executor) { - return CompletableFuture.completedFuture(new LambdaMap<>(location -> loadSingleBlockModel(manager, location))); + var cache = CacheUtil.simpleCacheForLambda(location -> loadSingleBlockModel(manager, location), 100L); + return CompletableFuture.completedFuture(new LambdaMap<>(location -> cache.getUnchecked(location))); } @Redirect(method = "reload", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/resources/model/ModelManager;loadBlockStates(Lnet/minecraft/server/packs/resources/ResourceManager;Ljava/util/concurrent/Executor;)Ljava/util/concurrent/CompletableFuture;")) private CompletableFuture>> deferBlockStateLoad(ResourceManager manager, Executor executor) { - return CompletableFuture.completedFuture(new LambdaMap<>(location -> loadSingleBlockState(manager, location))); + var cache = CacheUtil.>simpleCacheForLambda(location -> loadSingleBlockState(manager, location), 100L); + return CompletableFuture.completedFuture(new LambdaMap<>(location -> cache.getUnchecked(location))); } @Redirect(method = "loadModels", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/block/state/StateDefinition;getPossibleStates()Lcom/google/common/collect/ImmutableList;")) diff --git a/common/src/main/java/org/embeddedt/modernfix/util/CacheUtil.java b/common/src/main/java/org/embeddedt/modernfix/util/CacheUtil.java new file mode 100644 index 00000000..bdeebbe4 --- /dev/null +++ b/common/src/main/java/org/embeddedt/modernfix/util/CacheUtil.java @@ -0,0 +1,18 @@ +package org.embeddedt.modernfix.util; + +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; + +import java.util.function.Function; + +public class CacheUtil { + public static LoadingCache simpleCacheForLambda(Function function, long maxSize) { + return CacheBuilder.newBuilder().maximumSize(maxSize).build(new CacheLoader() { + @Override + public V load(K key) throws Exception { + return function.apply(key); + } + }); + } +} \ No newline at end of file From c0eaf29cb58cc4a2b5525bf67e842b39ca27f2ac Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Tue, 6 Aug 2024 20:27:25 -0400 Subject: [PATCH 5/6] Remove mod file scan data deduplicator This has no effect on newer FML versions since fields are now records --- .../ModFileScanDataDeduplicator.java | 89 ------------------- .../modernfix/forge/init/ModernFixForge.java | 2 - 2 files changed, 91 deletions(-) delete mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ModFileScanDataDeduplicator.java diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ModFileScanDataDeduplicator.java b/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ModFileScanDataDeduplicator.java deleted file mode 100644 index 61f0ad81..00000000 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ModFileScanDataDeduplicator.java +++ /dev/null @@ -1,89 +0,0 @@ -package org.embeddedt.modernfix.forge.classloading; - -import com.google.common.collect.Interner; -import com.google.common.collect.Interners; -import net.minecraftforge.fml.ModList; -import net.minecraftforge.forgespi.language.ModFileScanData; -import net.minecraftforge.forgespi.locating.IModFile; -import org.objectweb.asm.Type; - -import java.lang.reflect.Field; -import java.util.Set; -import java.util.function.Function; -import java.util.stream.Collectors; - -public class ModFileScanDataDeduplicator { - private final Interner typeInterner = Interners.newStrongInterner(); - - private final Function internerFn = type -> type != null ? typeInterner.intern(type) : null; - - private static Field classClazzField, parentField, interfacesField, annotationClazzField, annotationTypeField; - private static final boolean reflectionSuccessful; - - static { - boolean success = false; - try { - classClazzField = ModFileScanData.ClassData.class.getDeclaredField("clazz"); - classClazzField.setAccessible(true); - parentField = ModFileScanData.ClassData.class.getDeclaredField("parent"); - parentField.setAccessible(true); - interfacesField = ModFileScanData.ClassData.class.getDeclaredField("interfaces"); - interfacesField.setAccessible(true); - annotationClazzField = ModFileScanData.AnnotationData.class.getDeclaredField("clazz"); - annotationClazzField.setAccessible(true); - annotationTypeField = ModFileScanData.AnnotationData.class.getDeclaredField("annotationType"); - annotationTypeField.setAccessible(true); - success = true; - } catch(ReflectiveOperationException | RuntimeException e) { - } - reflectionSuccessful = success; - } - - ModFileScanDataDeduplicator() { - } - - private void runDeduplication() { - ModList.get().forEachModFile(this::deduplicateFile); - } - - private void deduplicateFile(IModFile file) { - ModFileScanData data = file.getScanResult(); - if(data != null) { - data.getClasses().forEach(this::deduplicateClass); - data.getAnnotations().forEach(this::deduplicateAnnotation); - } - } - - private void deduplicateClass(ModFileScanData.ClassData data) { - try { - Type type = (Type)classClazzField.get(data); - type = internerFn.apply(type); - classClazzField.set(data, type); - type = (Type)parentField.get(data); - type = internerFn.apply(type); - parentField.set(data, type); - Set types = (Set)interfacesField.get(data); - types = types.stream().map(internerFn).collect(Collectors.toSet()); - interfacesField.set(data, types); - } catch(ReflectiveOperationException e) { - } - } - - private void deduplicateAnnotation(ModFileScanData.AnnotationData data) { - try { - Type type = (Type)annotationClazzField.get(data); - type = internerFn.apply(type); - annotationClazzField.set(data, type); - type = (Type)annotationTypeField.get(data); - type = internerFn.apply(type); - annotationTypeField.set(data, type); - } catch(ReflectiveOperationException e) { - } - } - - public static void deduplicate() { - if(!reflectionSuccessful) - return; - new ModFileScanDataDeduplicator().runDeduplication(); - } -} diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java index d03b8e7d..347619d0 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java @@ -28,7 +28,6 @@ import org.embeddedt.modernfix.ModernFix; import org.embeddedt.modernfix.core.ModernFixMixinPlugin; import org.embeddedt.modernfix.entity.EntityDataIDSyncHandler; import org.embeddedt.modernfix.forge.ModernFixConfig; -import org.embeddedt.modernfix.forge.classloading.ModFileScanDataDeduplicator; import org.embeddedt.modernfix.forge.config.ConfigFixer; import org.embeddedt.modernfix.forge.config.NightConfigFixer; import org.embeddedt.modernfix.forge.packet.PacketHandler; @@ -51,7 +50,6 @@ public class ModernFixForge { ModLoadingContext.get().registerExtensionPoint(IExtensionPoint.DisplayTest.class, () -> new IExtensionPoint.DisplayTest(() -> NetworkConstants.IGNORESERVERONLY, (a, b) -> true)); ModLoadingContext.get().registerConfig(ModConfig.Type.COMMON, ModernFixConfig.COMMON_CONFIG); PacketHandler.register(); - ModFileScanDataDeduplicator.deduplicate(); ConfigFixer.replaceConfigHandlers(); } From 49464451ddc6b174740b2fd14057611441d26f40 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 9 Aug 2024 18:02:39 -0400 Subject: [PATCH 6/6] Make state_definition_construct degrade gracefully if map is used like a hashmap Related: #452 --- .../modernfix/blockstate/FakeStateMap.java | 27 ++++++++++++++++--- .../StateDefinitionMixin.java | 17 +++++------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/blockstate/FakeStateMap.java b/common/src/main/java/org/embeddedt/modernfix/blockstate/FakeStateMap.java index ff7cc745..7cc8f22c 100644 --- a/common/src/main/java/org/embeddedt/modernfix/blockstate/FakeStateMap.java +++ b/common/src/main/java/org/embeddedt/modernfix/blockstate/FakeStateMap.java @@ -1,5 +1,6 @@ package org.embeddedt.modernfix.blockstate; +import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; import net.minecraft.world.level.block.state.properties.Property; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -14,6 +15,7 @@ import java.util.*; */ public class FakeStateMap implements Map, Comparable>, S> { private final Map, Comparable>[] keys; + private Map, Comparable>, S> fastLookup; private final Object[] values; private int usedSlots; public FakeStateMap(int numStates) { @@ -34,22 +36,39 @@ public class FakeStateMap implements Map, Comparable>, S> @Override public boolean containsKey(Object o) { - throw new UnsupportedOperationException(); + return getFastLookup().containsKey(o); } @Override public boolean containsValue(Object o) { - throw new UnsupportedOperationException(); + return getFastLookup().containsValue(o); + } + + @SuppressWarnings("unchecked") + private Map, Comparable>, S> getFastLookup() { + if(fastLookup == null) { + var map = new Object2ObjectOpenHashMap, Comparable>, S>(usedSlots); + Map, Comparable>[] keys = this.keys; + Object[] values = this.values; + for(int i = 0; i < usedSlots; i++) { + map.put(keys[i], (S)values[i]); + } + fastLookup = map; + } + return fastLookup; } @Override public S get(Object o) { - throw new UnsupportedOperationException(); + return getFastLookup().get(o); } @Nullable @Override public S put(Map, Comparable> propertyComparableMap, S s) { + if(fastLookup != null) { + throw new IllegalStateException("Cannot populate map after fast lookup is built"); + } keys[usedSlots] = propertyComparableMap; values[usedSlots] = s; usedSlots++; @@ -70,7 +89,7 @@ public class FakeStateMap implements Map, Comparable>, S> @Override public void clear() { - for(int i = 0; i < this.keys.length; i++) { + for(int i = 0; i < usedSlots; i++) { this.keys[i] = null; this.values[i] = null; } 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 70210f7b..b24ed70d 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,23 +6,25 @@ 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.embeddedt.modernfix.platform.ModernFixPlatformHooks; 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; +// This optimization requires FerriteCore to be worthwhile, otherwise the FakeStateMap degrades to hash internally @Mixin(StateDefinition.class) @RequiresMod("ferritecore") public class StateDefinitionMixin> { @Shadow @Final private ImmutableSortedMap> propertiesByName; + /** + * @author embeddedt + * @reason write states into a custom array map for fast iteration by FerriteCore, no need to waste time hashing + * and growing + */ @ModifyVariable(method = "", at = @At(value = "STORE", ordinal = 0), ordinal = 1, index = 8) private Map, Comparable>, S> useArrayMap(Map, Comparable>, S> in) { int numStates = 1; @@ -31,11 +33,4 @@ public class StateDefinitionMixin> { } return new FakeStateMap<>(numStates); } - - @Inject(method = "", at = @At("TAIL")) - private void postProcess(CallbackInfo ci) { - // keep in dev only until upstream FC releases - if(ModernFixPlatformHooks.INSTANCE.isDevEnv()) - FerriteCorePostProcess.postProcess((StateDefinition)(Object)this); - } }