From ad1e12a3bbc793c1378c0f1de88b45379de93920 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Wed, 28 Feb 2024 20:07:56 -0500 Subject: [PATCH 01/10] Remove unfinished/obsolete registry & block optimizations These patches were unfinished, are known to be buggy, and won't make sense in modern versions of Minecraft, where many of the underlying issues have been addressed in other ways --- .../DirectObjectMixin.java | 24 - .../MappedRegistryMixin.java | 17 - .../StateDefinitionMixin.java | 26 - .../StateHolderMixin.java | 58 -- .../core/config/ModernFixEarlyConfig.java | 2 - .../registry/DirectStorageBiMap.java | 184 ------ .../registry/DirectStorageRegistryObject.java | 8 - .../modernfix/registry/RegistryStorage.java | 33 - .../modernfix/registry/TransformingBiMap.java | 224 ------- .../rewrite_registry/ForgeRegistryMixin.java | 65 -- .../ForgeRegistrySnapshotMixin.java | 35 - .../forge/registry/FastForgeRegistry.java | 602 ------------------ 12 files changed, 1278 deletions(-) delete mode 100644 common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/compact_mojang_registries/DirectObjectMixin.java delete mode 100644 common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_block_codecs/StateDefinitionMixin.java delete mode 100644 common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_block_codecs/StateHolderMixin.java delete mode 100644 common/src/main/java/org/embeddedt/modernfix/registry/DirectStorageBiMap.java delete mode 100644 common/src/main/java/org/embeddedt/modernfix/registry/DirectStorageRegistryObject.java delete mode 100644 common/src/main/java/org/embeddedt/modernfix/registry/RegistryStorage.java delete mode 100644 common/src/main/java/org/embeddedt/modernfix/registry/TransformingBiMap.java delete mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/rewrite_registry/ForgeRegistryMixin.java delete mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/rewrite_registry/ForgeRegistrySnapshotMixin.java delete mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/registry/FastForgeRegistry.java diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/compact_mojang_registries/DirectObjectMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/compact_mojang_registries/DirectObjectMixin.java deleted file mode 100644 index 02d346d2..00000000 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/compact_mojang_registries/DirectObjectMixin.java +++ /dev/null @@ -1,24 +0,0 @@ -package org.embeddedt.modernfix.common.mixin.perf.compact_mojang_registries; - -import net.minecraft.resources.ResourceLocation; -import net.minecraft.world.item.Item; -import net.minecraft.world.level.block.Block; -import org.embeddedt.modernfix.annotation.IgnoreOutsideDev; -import org.embeddedt.modernfix.registry.DirectStorageRegistryObject; -import org.spongepowered.asm.mixin.Mixin; - -@Mixin({ Block.class, Item.class }) -@IgnoreOutsideDev -public class DirectObjectMixin implements DirectStorageRegistryObject { - private ResourceLocation mfix$resourceKey; - - @Override - public ResourceLocation mfix$getResourceKey() { - return mfix$resourceKey; - } - - @Override - public void mfix$setResourceKey(ResourceLocation key) { - mfix$resourceKey = key; - } -} diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/compact_mojang_registries/MappedRegistryMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/compact_mojang_registries/MappedRegistryMixin.java index c5e9c813..492176cf 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/compact_mojang_registries/MappedRegistryMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/compact_mojang_registries/MappedRegistryMixin.java @@ -1,17 +1,11 @@ package org.embeddedt.modernfix.common.mixin.perf.compact_mojang_registries; -import com.google.common.collect.BiMap; -import com.google.common.collect.ImmutableSet; import com.mojang.serialization.Lifecycle; import net.minecraft.core.MappedRegistry; import net.minecraft.core.Registry; import net.minecraft.resources.ResourceKey; -import net.minecraft.resources.ResourceLocation; import org.embeddedt.modernfix.annotation.IgnoreOutsideDev; -import org.embeddedt.modernfix.core.ModernFixMixinPlugin; -import org.embeddedt.modernfix.registry.DirectStorageRegistryObject; import org.embeddedt.modernfix.registry.LifecycleMap; -import org.embeddedt.modernfix.registry.RegistryStorage; import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Mutable; @@ -29,12 +23,6 @@ public abstract class MappedRegistryMixin extends Registry { @Final @Mutable private Map lifecycles; - @Shadow @Final @Mutable - private BiMap storage; - @Shadow @Final @Mutable - private BiMap, T> keyStorage; - - private static final ImmutableSet MFIX$NEW_STORAGE_KEYS = ImmutableSet.of(new ResourceLocation("block"), new ResourceLocation("item")); protected MappedRegistryMixin(ResourceKey> resourceKey, Lifecycle lifecycle) { super(resourceKey, lifecycle); @@ -43,10 +31,5 @@ public abstract class MappedRegistryMixin extends Registry { @Inject(method = "", at = @At("RETURN")) private void replaceStorage(CallbackInfo ci) { this.lifecycles = new LifecycleMap<>(); - if(MFIX$NEW_STORAGE_KEYS.contains(this.key().location())) { - ModernFixMixinPlugin.instance.logger.info("Using experimental registry storage for {}", this.key()); - this.storage = (BiMap) RegistryStorage.createStorage(); - this.keyStorage = (BiMap, T>)RegistryStorage.createKeyStorage(this.key(), (BiMap)this.storage); - } } } diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_block_codecs/StateDefinitionMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_block_codecs/StateDefinitionMixin.java deleted file mode 100644 index 82ae09df..00000000 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_block_codecs/StateDefinitionMixin.java +++ /dev/null @@ -1,26 +0,0 @@ -package org.embeddedt.modernfix.common.mixin.perf.dynamic_block_codecs; - -import net.minecraft.world.level.block.Block; -import net.minecraft.world.level.block.state.StateDefinition; -import net.minecraft.world.level.block.state.StateHolder; -import net.minecraft.world.level.block.state.properties.Property; -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.ModifyVariable; - -import java.util.Map; -import java.util.function.Function; - -@Mixin(StateDefinition.class) -public class StateDefinitionMixin> { - @Shadow @Final private O owner; - - @ModifyVariable(method = "", at = @At("HEAD"), ordinal = 0, argsOnly = true) - private static > StateDefinition.Factory replaceMapCodec(StateDefinition.Factory factory, Function function, O object, StateDefinition.Factory factory2, Map> map) { - if(object instanceof Block) - return (o, m, c) -> factory.create(o, m, null); - return factory; - } -} diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_block_codecs/StateHolderMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_block_codecs/StateHolderMixin.java deleted file mode 100644 index 540c7495..00000000 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_block_codecs/StateHolderMixin.java +++ /dev/null @@ -1,58 +0,0 @@ -package org.embeddedt.modernfix.common.mixin.perf.dynamic_block_codecs; - -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; -import com.mojang.serialization.Codec; -import com.mojang.serialization.Decoder; -import com.mojang.serialization.Encoder; -import com.mojang.serialization.MapCodec; -import net.minecraft.world.level.block.Block; -import net.minecraft.world.level.block.state.BlockState; -import net.minecraft.world.level.block.state.StateDefinition; -import net.minecraft.world.level.block.state.StateHolder; -import net.minecraft.world.level.block.state.properties.Property; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.injection.At; -import org.spongepowered.asm.mixin.injection.Redirect; - -import java.util.concurrent.ExecutionException; -import java.util.function.Function; -import java.util.function.Supplier; - -@Mixin(StateHolder.class) -public class StateHolderMixin { - private static final LoadingCache> MODERNFIX_CODEC_CACHE = CacheBuilder.newBuilder() - .maximumSize(100000) - .build(new CacheLoader>() { - @Override - public MapCodec load(Block block) throws Exception { - Supplier stateSupplier = block::defaultBlockState; - MapCodec mapCodec = MapCodec.of(Encoder.empty(), Decoder.unit(stateSupplier)); - for(Property property : block.getStateDefinition().getProperties()) { - mapCodec = StateDefinition.appendPropertyCodec(mapCodec, stateSupplier, property.getName(), property); - } - return mapCodec; - } - }); - - @Redirect(method = "codec", at = @At(value = "INVOKE", target = "Lcom/mojang/serialization/Codec;dispatch(Ljava/lang/String;Ljava/util/function/Function;Ljava/util/function/Function;)Lcom/mojang/serialization/Codec;", remap = false)) - private static > Codec obtainCodec(Codec codec, String typeKey, Function type, Function> codecFn, Codec codecMethodArg, Function stateSupplier) { - return codec.dispatch(typeKey, type, block -> { - if(block instanceof Block) { - S state = stateSupplier.apply(block); - if(state.getValues().isEmpty()) - return Codec.unit(state); - MapCodec mapCodec; - try { - mapCodec = (MapCodec)MODERNFIX_CODEC_CACHE.get((Block)block); - } catch (ExecutionException e) { - throw new RuntimeException(e); - } - return mapCodec.fieldOf("Properties").codec(); - } else { - return codecFn.apply(block); - } - }); - } -} 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 6af610ef..65696139 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 @@ -168,9 +168,7 @@ public class ModernFixEarlyConfig { .put("mixin.perf.dynamic_resources", false) .putConditionally(() -> !isFabric, "mixin.perf.async_jei", false) .put("mixin.perf.reuse_datapacks", false) - .put("mixin.perf.dynamic_block_codecs", false) .put("mixin.feature.direct_stack_trace", false) - .putConditionally(ModernFixPlatformHooks.INSTANCE::isDevEnv, "mixin.perf.rewrite_registry", false) .put("mixin.perf.clear_mixin_classinfo", false) .put("mixin.bugfix.packet_leak", false) .put("mixin.perf.deduplicate_location", false) diff --git a/common/src/main/java/org/embeddedt/modernfix/registry/DirectStorageBiMap.java b/common/src/main/java/org/embeddedt/modernfix/registry/DirectStorageBiMap.java deleted file mode 100644 index 9aa1442a..00000000 --- a/common/src/main/java/org/embeddedt/modernfix/registry/DirectStorageBiMap.java +++ /dev/null @@ -1,184 +0,0 @@ -package org.embeddedt.modernfix.registry; - -import com.google.common.collect.BiMap; -import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; -import org.jetbrains.annotations.NotNull; - -import java.util.*; -import java.util.function.BiConsumer; -import java.util.function.Function; -import java.util.stream.Collectors; - -@SuppressWarnings("unchecked") -public class DirectStorageBiMap implements BiMap { - private final Function keyGetter; - private final BiConsumer keySetter; - private final Map forwardMap; - - public DirectStorageBiMap(Function keyGetter, BiConsumer keySetter) { - Objects.requireNonNull(keyGetter); - Objects.requireNonNull(keySetter); - this.keyGetter = keyGetter; - this.keySetter = keySetter; - this.forwardMap = new Object2ObjectOpenHashMap<>(); - } - - @Override - public int size() { - return this.forwardMap.size(); - } - - @Override - public boolean isEmpty() { - return this.forwardMap.isEmpty(); - } - - @Override - public boolean containsKey(Object o) { - return this.forwardMap.containsKey(o); - } - - @Override - public boolean containsValue(Object o) { - return o != null && keyGetter.apply((V)o) != null; - } - - @Override - public V get(Object o) { - return this.forwardMap.get(o); - } - - @Override - public V put(K key, V value) { - if(this.forwardMap.containsKey(key) || (value != null && keyGetter.apply(value) != null)) - throw new IllegalArgumentException("Already have mapping for " + key); - return forcePut(key, value); - } - - @Override - public V remove(Object o) { - return put((K)o, null); - } - - @Override - public V forcePut(K key, V value) { - V previousValue = this.forwardMap.put(key, value); - if(previousValue != null) - keySetter.accept(previousValue, null); - if(value != null) - keySetter.accept(value, key); - return previousValue; - } - - @Override - public void putAll(Map map) { - map.forEach(this::put); - } - - @Override - public void clear() { - for(V value : this.forwardMap.values()) { - if(value != null) - keySetter.accept(value, null); - } - this.forwardMap.clear(); - } - - @NotNull - @Override - public Set keySet() { - return this.forwardMap.keySet(); - } - - @Override - public Set values() { - return new HashSet<>(this.forwardMap.values()); - } - - @NotNull - @Override - public Set> entrySet() { - return this.forwardMap.entrySet(); - } - - @Override - public BiMap inverse() { - return new Reverse(); - } - - class Reverse implements BiMap { - @Override - public int size() { - return DirectStorageBiMap.this.size(); - } - - @Override - public boolean isEmpty() { - return DirectStorageBiMap.this.isEmpty(); - } - - @Override - public boolean containsKey(Object o) { - return DirectStorageBiMap.this.containsValue(o); - } - - @Override - public boolean containsValue(Object o) { - return DirectStorageBiMap.this.containsKey(o); - } - - @Override - public K get(Object o) { - return o == null ? null : keyGetter.apply((V)o); - } - - @Override - public K put(V key, K value) { - throw new UnsupportedOperationException(); - } - - @Override - public K remove(Object o) { - throw new UnsupportedOperationException(); - } - - @Override - public K forcePut(V key, K value) { - throw new UnsupportedOperationException(); - } - - @Override - public void putAll(Map map) { - throw new UnsupportedOperationException(); - } - - @Override - public void clear() { - throw new UnsupportedOperationException(); - } - - @NotNull - @Override - public Set keySet() { - return DirectStorageBiMap.this.values(); - } - - @Override - public Set values() { - return DirectStorageBiMap.this.keySet(); - } - - @NotNull - @Override - public Set> entrySet() { - return DirectStorageBiMap.this.entrySet().stream() - .map(entry -> new AbstractMap.SimpleImmutableEntry<>(entry.getValue(), entry.getKey())) - .collect(Collectors.toSet()); - } - - @Override - public BiMap inverse() { - return DirectStorageBiMap.this; - } - } -} diff --git a/common/src/main/java/org/embeddedt/modernfix/registry/DirectStorageRegistryObject.java b/common/src/main/java/org/embeddedt/modernfix/registry/DirectStorageRegistryObject.java deleted file mode 100644 index c18ac699..00000000 --- a/common/src/main/java/org/embeddedt/modernfix/registry/DirectStorageRegistryObject.java +++ /dev/null @@ -1,8 +0,0 @@ -package org.embeddedt.modernfix.registry; - -import net.minecraft.resources.ResourceLocation; - -public interface DirectStorageRegistryObject { - ResourceLocation mfix$getResourceKey(); - void mfix$setResourceKey(ResourceLocation key); -} diff --git a/common/src/main/java/org/embeddedt/modernfix/registry/RegistryStorage.java b/common/src/main/java/org/embeddedt/modernfix/registry/RegistryStorage.java deleted file mode 100644 index 5d2fb207..00000000 --- a/common/src/main/java/org/embeddedt/modernfix/registry/RegistryStorage.java +++ /dev/null @@ -1,33 +0,0 @@ -package org.embeddedt.modernfix.registry; - -import com.google.common.collect.BiMap; -import net.minecraft.core.Registry; -import net.minecraft.resources.ResourceKey; -import net.minecraft.resources.ResourceLocation; - -import java.util.Map; -import java.util.function.Function; - -public class RegistryStorage { - public static BiMap createStorage() { - return new DirectStorageBiMap<>(DirectStorageRegistryObject::mfix$getResourceKey, DirectStorageRegistryObject::mfix$setResourceKey); - } - - public static BiMap, DirectStorageRegistryObject> createKeyStorage(ResourceKey> registryKey, BiMap storage) { - if(storage instanceof DirectStorageBiMap) { - // silently ignore put/putAll calls on this map - return new TransformingBiMap, DirectStorageRegistryObject>(storage, loc -> ResourceKey.create(registryKey, loc), ResourceKey::location, Function.identity(), Function.identity()) { - @Override - public DirectStorageRegistryObject put(ResourceKey key, DirectStorageRegistryObject value) { - return null; - } - - @Override - public void putAll(Map, ? extends DirectStorageRegistryObject> map) { - - } - }; - } else - throw new UnsupportedOperationException(); - } -} diff --git a/common/src/main/java/org/embeddedt/modernfix/registry/TransformingBiMap.java b/common/src/main/java/org/embeddedt/modernfix/registry/TransformingBiMap.java deleted file mode 100644 index 2fb1beb1..00000000 --- a/common/src/main/java/org/embeddedt/modernfix/registry/TransformingBiMap.java +++ /dev/null @@ -1,224 +0,0 @@ -package org.embeddedt.modernfix.registry; - -import com.google.common.collect.BiMap; -import com.google.common.collect.Collections2; -import com.google.common.collect.Iterators; -import org.jetbrains.annotations.NotNull; - -import java.util.*; -import java.util.function.Function; - -public class TransformingBiMap implements BiMap { - private final BiMap delegate; - private final Function keyFwd; - private final Function keyBack; - private final Function valueFwd; - private final Function valueBack; - - public TransformingBiMap(BiMap map, Function keyFwd, Function keyBack, Function valueFwd, Function valueBack) { - this.delegate = map; - this.keyFwd = keyFwd; - this.keyBack = keyBack; - this.valueFwd = valueFwd; - this.valueBack = valueBack; - } - - private KFrom keyBack(KTo key) { - return key == null ? null : this.keyBack.apply(key); - } - - private KTo keyFwd(KFrom key) { - return key == null ? null : this.keyFwd.apply(key); - } - - private VFrom valueBack(VTo value) { - return value == null ? null : this.valueBack.apply(value); - } - - private VTo valueFwd(VFrom value) { - return value == null ? null : this.valueFwd.apply(value); - } - - @Override - public int size() { - return this.delegate.size(); - } - - @Override - public boolean isEmpty() { - return this.delegate.isEmpty(); - } - - @Override - public boolean containsKey(Object o) { - return this.delegate.containsKey(keyBack((KTo)o)); - } - - @Override - public boolean containsValue(Object o) { - return false; - } - - @Override - public VTo get(Object o) { - return valueFwd(this.delegate.get(keyBack((KTo)o))); - } - - @Override - public VTo put(KTo key, VTo value) { - return valueFwd(this.delegate.put(keyBack(key), valueBack(value))); - } - - @Override - public VTo remove(Object o) { - return valueFwd(this.delegate.remove(keyBack((KTo)o))); - } - - @Override - public VTo forcePut(KTo key, VTo value) { - return valueFwd(this.delegate.forcePut(keyBack(key), valueBack(value))); - } - - @Override - public void putAll(Map map) { - map.forEach((key, value) -> { - this.delegate.put(keyBack(key), valueBack(value)); - }); - } - - @Override - public void clear() { - this.delegate.clear(); - } - - @NotNull - @Override - public Set keySet() { - return new TransformingSet<>(this.delegate.keySet(), this.keyFwd, this.keyBack); - } - - @Override - public Set values() { - return new TransformingSet<>(this.delegate.values(), this.valueFwd, this.valueBack); - } - - @NotNull - @Override - public Set> entrySet() { - return new TransformingSet<>(this.delegate.entrySet(), entry -> { - return new AbstractMap.SimpleImmutableEntry<>(keyFwd(entry.getKey()), valueFwd(entry.getValue())); - }, entry -> { - return new AbstractMap.SimpleImmutableEntry<>(keyBack(entry.getKey()), valueBack(entry.getValue())); - }); - } - - @Override - public BiMap inverse() { - return new TransformingBiMap<>(this.delegate.inverse(), this.valueFwd, this.valueBack, this.keyFwd, this.keyBack); - } - - static class TransformingSet implements Set { - private final Set delegate; - private final Function forward; - private final Function reverse; - - public TransformingSet(Set set, Function forward, Function reverse) { - this.delegate = set; - this.forward = forward; - this.reverse = reverse; - } - - private TypeTo forward(TypeFrom t) { - return t == null ? null : this.forward.apply(t); - } - - private TypeFrom reverse(TypeTo t) { - return t == null ? null : this.reverse.apply(t); - } - - @Override - public int size() { - return this.delegate.size(); - } - - @Override - public boolean isEmpty() { - return this.delegate.isEmpty(); - } - - @Override - public boolean contains(Object o) { - return this.delegate.contains(reverse((TypeTo)o)); - } - - @NotNull - @Override - public Iterator iterator() { - return Iterators.transform(this.delegate.iterator(), this::forward); - } - - @NotNull - @Override - public Object[] toArray() { - Object[] array = this.delegate.toArray(); - for(int i = 0; i < array.length; i++) { - array[i] = this.forward((TypeFrom)array[i]); - } - return array; - } - - @NotNull - @Override - public T[] toArray(@NotNull T[] ts) { - if(ts.length >= this.delegate.size()) { - Object[] setContents = toArray(); - System.arraycopy(setContents, 0, ts, 0, Math.min(setContents.length, ts.length)); - if(ts.length > setContents.length) - ts[setContents.length] = null; - return ts; - } else { - T[] realArray = Arrays.copyOf(ts, this.delegate.size()); - Iterator iterator = this.iterator(); - int i = 0; - while(iterator.hasNext()) - realArray[i++] = (T)iterator.next(); - return realArray; - } - } - - @Override - public boolean add(TypeTo typeFrom) { - return this.delegate.add(reverse(typeFrom)); - } - - @Override - public boolean remove(Object o) { - return this.delegate.remove(reverse((TypeTo)o)); - } - - @Override - public boolean containsAll(@NotNull Collection collection) { - return this.delegate.containsAll(Collections2.transform(collection, obj -> reverse((TypeTo)obj))); - } - - @Override - public boolean addAll(@NotNull Collection collection) { - return this.delegate.addAll(Collections2.transform(collection, this::reverse)); - } - - @Override - public boolean retainAll(@NotNull Collection collection) { - return this.delegate.retainAll(Collections2.transform(collection, obj -> reverse((TypeTo)obj))); - } - - @Override - public boolean removeAll(@NotNull Collection collection) { - return this.delegate.removeAll(Collections2.transform(collection, obj -> reverse((TypeTo)obj))); - } - - @Override - public void clear() { - this.delegate.clear(); - } - } -} diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/rewrite_registry/ForgeRegistryMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/rewrite_registry/ForgeRegistryMixin.java deleted file mode 100644 index 1e955230..00000000 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/rewrite_registry/ForgeRegistryMixin.java +++ /dev/null @@ -1,65 +0,0 @@ -package org.embeddedt.modernfix.forge.mixin.perf.rewrite_registry; - -import com.google.common.collect.BiMap; -import net.minecraft.core.Registry; -import net.minecraft.resources.ResourceKey; -import net.minecraft.resources.ResourceLocation; -import net.minecraftforge.registries.ForgeRegistry; -import net.minecraftforge.registries.IForgeRegistryEntry; -import org.embeddedt.modernfix.annotation.IgnoreOutsideDev; -import org.embeddedt.modernfix.forge.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; -import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; - -@Mixin(value = ForgeRegistry.class, remap = false) -@IgnoreOutsideDev -public class ForgeRegistryMixin> { - @Shadow - @Final - @Mutable - private BiMap ids; - - @Shadow @Final @Mutable private BiMap, V> keys; - - @Shadow @Final private ResourceKey> key; - - @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) { - 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/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/rewrite_registry/ForgeRegistrySnapshotMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/rewrite_registry/ForgeRegistrySnapshotMixin.java deleted file mode 100644 index b8dbf343..00000000 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/rewrite_registry/ForgeRegistrySnapshotMixin.java +++ /dev/null @@ -1,35 +0,0 @@ -package org.embeddedt.modernfix.forge.mixin.perf.rewrite_registry; - -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.embeddedt.modernfix.annotation.IgnoreOutsideDev; -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) -@IgnoreOutsideDev -public class ForgeRegistrySnapshotMixin { - @Shadow(remap = false) @Final @Mutable public Map ids; - - @Shadow(remap = false) @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/forge/src/main/java/org/embeddedt/modernfix/forge/registry/FastForgeRegistry.java b/forge/src/main/java/org/embeddedt/modernfix/forge/registry/FastForgeRegistry.java deleted file mode 100644 index 9d4c2f5c..00000000 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/registry/FastForgeRegistry.java +++ /dev/null @@ -1,602 +0,0 @@ -package org.embeddedt.modernfix.forge.registry; - -import com.google.common.collect.BiMap; -import com.google.common.collect.Iterators; -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.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; - -import java.util.*; -import java.util.function.BiConsumer; -import java.util.function.Consumer; -import java.util.function.Function; - -public class FastForgeRegistry> { - private final BiMap ids; - private final DataFieldBiMap names; - private final DataFieldBiMap> keys; - private final DataFieldBiMap owners; - private final ResourceKey> registryKey; - - private final ObjectArrayList valuesById; - private final Object2ObjectOpenHashMap infoByValue; - - private void storeId(V value, int id) { - RegistryValueData pair = infoByValue.computeIfAbsent(value, k -> new RegistryValueData()); - pair.id = id; - } - - private void updateInfoPairAndClearIfNull(V v, Consumer consumer) { - infoByValue.compute(v, (oldValue, oldPair) -> { - if(oldPair == null) - oldPair = new RegistryValueData(); - consumer.accept(oldPair); - if(oldPair.isEmpty()) - return null; - else - return oldPair; - }); - } - - private void ensureArrayCanFitId(int id) { - int desiredSize = id + 1; - while(valuesById.size() < desiredSize) { - valuesById.add(null); - } - } - - 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(); - this.owners.clearUnsafe(); - } - - public FastForgeRegistry(ResourceKey> registryKey) { - this.registryKey = registryKey; - this.valuesById = new ObjectArrayList<>(); - 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 - public V put(@Nullable Integer key, @Nullable V value) { - 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 - @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.id = -1); - } - 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) { - RegistryValueData pair = infoByValue.get(key); - if(pair == null) - return null; - return pair.id == -1 ? null : pair.id; - } - - @Override - public Integer remove(Object key) { - RegistryValueData pair = infoByValue.get(key); - if(pair == null) - return null; - int id = pair.id; - if(id != -1) - valuesById.set(id, null); - updateInfoPairAndClearIfNull((V)key, p -> p.id = -1); - return id == -1 ? null : 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.id = -1; - return pair.isEmpty(); - }); - } - - @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); - } - } - }; - } - - public void optimize() { - this.keys.optimize(); - this.owners.optimize(); - this.names.optimize(); - this.infoByValue.trim(); - } - - public BiMap getIds() { - return ids; - } - - public BiMap, V> getKeys() { - return keys; - } - - public BiMap getNames() { - 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) { - 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) { - 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; - - 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(); - } - } - - static class RegistryValueData { - public ResourceKey key; - public ResourceLocation location; - public int id = -1; - public Object overrideOwner; - - boolean isEmpty() { - return key == null && location == null && id == -1 && overrideOwner == null; - } - } -} From 124112259eabdeb75c66f2c7bed73134c061d786 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 1 Mar 2024 11:32:24 -0500 Subject: [PATCH 02/10] Reduce load factor of ForgeRegistry delegate map --- .../mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java index bf045369..4488dc67 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java @@ -1,5 +1,6 @@ package org.embeddedt.modernfix.forge.mixin.perf.forge_registry_alloc; +import it.unimi.dsi.fastutil.Hash; import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; import net.minecraft.core.Holder; import net.minecraft.resources.ResourceKey; @@ -19,7 +20,7 @@ public abstract class ForgeRegistryMixin { // are a bottleneck in many areas (e.g. render type lookup) @Shadow @Final private Map> delegatesByName = new Object2ObjectOpenHashMap<>(); - @Shadow @Final private Map> delegatesByValue = new Object2ObjectOpenHashMap<>(); + @Shadow @Final private Map> delegatesByValue = new Object2ObjectOpenHashMap<>(Hash.DEFAULT_INITIAL_SIZE, 0.5F); /** * @author embeddedt From e7632b7f0b632fd310683a97c95a65f5db317ea4 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 2 Mar 2024 12:28:04 -0500 Subject: [PATCH 03/10] Remove Blueprint memory leak patch --- forge/build.gradle | 2 -- .../ObjectModificationManagerMixin.java | 27 ------------------- 2 files changed, 29 deletions(-) delete mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/blueprint_modif_memory_leak/ObjectModificationManagerMixin.java diff --git a/forge/build.gradle b/forge/build.gradle index 5403d9b6..1efefbe7 100644 --- a/forge/build.gradle +++ b/forge/build.gradle @@ -61,8 +61,6 @@ dependencies { modCompileOnly("curse.maven:supermartijncore-454372:4455391") modCompileOnly("vazkii.patchouli:Patchouli:1.19.2-77") - modCompileOnly("curse.maven:blueprint-382216:3991478") - // runtime remapping at home for (extraModJar in fileTree(dir: extraModsDir, include: '*.jar')) { def basename = extraModJar.name.substring(0, extraModJar.name.length() - ".jar".length()) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/blueprint_modif_memory_leak/ObjectModificationManagerMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/blueprint_modif_memory_leak/ObjectModificationManagerMixin.java deleted file mode 100644 index 2be15e95..00000000 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/blueprint_modif_memory_leak/ObjectModificationManagerMixin.java +++ /dev/null @@ -1,27 +0,0 @@ -package org.embeddedt.modernfix.forge.mixin.bugfix.blueprint_modif_memory_leak; - -import com.google.gson.Gson; -import com.teamabnormals.blueprint.core.util.modification.ObjectModificationManager; -import com.teamabnormals.blueprint.core.util.modification.selection.SelectionSpace; -import net.minecraft.server.packs.resources.SimpleJsonResourceReloadListener; -import org.embeddedt.modernfix.annotation.RequiresMod; -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.callback.CallbackInfo; - -@Mixin(ObjectModificationManager.class) -@RequiresMod("blueprint") -public abstract class ObjectModificationManagerMixin extends SimpleJsonResourceReloadListener { - @Shadow(remap = false) protected SelectionSpace selectionSpace; - - public ObjectModificationManagerMixin(Gson gson, String string) { - super(gson, string); - } - - @Inject(method = "apply(Ljava/util/Map;Lnet/minecraft/server/packs/resources/ResourceManager;Lnet/minecraft/util/profiling/ProfilerFiller;)V", at = @At("RETURN"), remap = false) - private void clearSelectionSpace(CallbackInfo ci) { - this.selectionSpace = consumer -> {}; - } -} From 96db279d58e663743f9081bae8e7c8f0736d7f84 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 2 Mar 2024 13:14:41 -0500 Subject: [PATCH 04/10] Merge Forge mixins that didn't automerge --- .../ModelDataManagerMixin.java | 56 --------------- .../LivingEntityMixin.java | 24 ------- .../ForgeRegistryMixin.java | 69 ------------------- .../LivingEntityRendererMixin.java | 19 +++-- .../PlayerRendererMixin.java | 21 +++--- .../entity_pose_stack/PoseStackAccessor.java | 2 +- 6 files changed, 20 insertions(+), 171 deletions(-) delete mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/model_data_manager_cme/ModelDataManagerMixin.java delete mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_cap_retrieval/LivingEntityMixin.java delete mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java rename {forge/src/main/java/org/embeddedt/modernfix/forge => neoforge/src/main/java/org/embeddedt/modernfix/neoforge}/mixin/bugfix/entity_pose_stack/LivingEntityRendererMixin.java (64%) rename {forge/src/main/java/org/embeddedt/modernfix/forge => neoforge/src/main/java/org/embeddedt/modernfix/neoforge}/mixin/bugfix/entity_pose_stack/PlayerRendererMixin.java (59%) rename {forge/src/main/java/org/embeddedt/modernfix/forge => neoforge/src/main/java/org/embeddedt/modernfix/neoforge}/mixin/bugfix/entity_pose_stack/PoseStackAccessor.java (83%) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/model_data_manager_cme/ModelDataManagerMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/model_data_manager_cme/ModelDataManagerMixin.java deleted file mode 100644 index 5fba1b2e..00000000 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/model_data_manager_cme/ModelDataManagerMixin.java +++ /dev/null @@ -1,56 +0,0 @@ -package org.embeddedt.modernfix.forge.mixin.bugfix.model_data_manager_cme; - -import net.minecraft.client.Minecraft; -import net.minecraft.core.BlockPos; -import net.minecraft.world.level.ChunkPos; -import net.minecraftforge.client.model.data.ModelDataManager; -import org.embeddedt.modernfix.annotation.ClientOnlyMixin; -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.ModifyArg; -import org.spongepowered.asm.mixin.injection.Redirect; - -import java.util.Collections; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Function; - -/** - * Fix several concurrency issues in the default ModelDataManager. - */ -@Mixin(ModelDataManager.class) -@ClientOnlyMixin -public abstract class ModelDataManagerMixin { - @Shadow protected abstract void refreshAt(ChunkPos chunk); - - @Shadow @Final private Map> needModelDataRefresh; - - /** - * Make the set of positions to refresh a real concurrent hash set rather than relying on synchronizedSet, - * because the returned iterator won't be thread-safe otherwise. See https://github.com/AppliedEnergistics/Applied-Energistics-2/issues/7511 - */ - @ModifyArg(method = "requestRefresh", at = @At(value = "INVOKE", target = "Ljava/util/Map;computeIfAbsent(Ljava/lang/Object;Ljava/util/function/Function;)Ljava/lang/Object;", ordinal = 0), index = 1, remap = false) - private Function> changeTypeOfSetUsed(Function> mappingFunction) { - return pos -> Collections.newSetFromMap(new ConcurrentHashMap<>()); - } - - @Redirect(method = "getAt(Lnet/minecraft/world/level/ChunkPos;)Ljava/util/Map;", at = @At(value = "INVOKE", target = "Lnet/minecraftforge/client/model/data/ModelDataManager;refreshAt(Lnet/minecraft/world/level/ChunkPos;)V"), remap = false) - private void onlyRefreshOnMainThread(ModelDataManager instance, ChunkPos pos) { - // Only refresh model data on the main thread. This prevents calling getBlockEntity from worker threads - // which could cause weird CMEs or other behavior. - // Avoid the loop if no model data needs to be refreshed, to prevent unnecessary allocation. - if(Minecraft.getInstance().isSameThread() && !needModelDataRefresh.isEmpty()) { - // Refresh the given chunk, and all its neighbors. This is less efficient than the default code - // but we have no choice since we need to not do refreshing on workers, and blocks might - // try to access model data in neighboring chunks. - for(int x = -1; x <= 1; x++) { - for(int z = -1; z <= 1; z++) { - refreshAt(new ChunkPos(pos.x + x, pos.z + z)); - } - } - } - } -} diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_cap_retrieval/LivingEntityMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_cap_retrieval/LivingEntityMixin.java deleted file mode 100644 index 68e86aab..00000000 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_cap_retrieval/LivingEntityMixin.java +++ /dev/null @@ -1,24 +0,0 @@ -package org.embeddedt.modernfix.forge.mixin.perf.forge_cap_retrieval; - -import net.minecraft.core.Direction; -import net.minecraft.world.entity.LivingEntity; -import net.minecraftforge.common.capabilities.Capability; -import net.minecraftforge.common.capabilities.ForgeCapabilities; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.injection.At; -import org.spongepowered.asm.mixin.injection.Redirect; - -import javax.annotation.Nullable; - -@Mixin(LivingEntity.class) -public class LivingEntityMixin { - /** - * @author embeddedt (issue noted by XFactHD) - * @reason check capability equality before checking that entity is alive, the latter requires a lot more - * indirection - */ - @Redirect(method = "getCapability", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/entity/LivingEntity;isAlive()Z")) - private boolean checkAliveAfterCap(LivingEntity entity, Capability capability, @Nullable Direction facing) { - return capability == ForgeCapabilities.ITEM_HANDLER && entity.isAlive(); - } -} diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java deleted file mode 100644 index 4488dc67..00000000 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java +++ /dev/null @@ -1,69 +0,0 @@ -package org.embeddedt.modernfix.forge.mixin.perf.forge_registry_alloc; - -import it.unimi.dsi.fastutil.Hash; -import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; -import net.minecraft.core.Holder; -import net.minecraft.resources.ResourceKey; -import net.minecraft.resources.ResourceLocation; -import net.minecraftforge.registries.ForgeRegistry; -import org.spongepowered.asm.mixin.Final; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.Overwrite; -import org.spongepowered.asm.mixin.Shadow; - -import java.util.Locale; -import java.util.Map; - -@Mixin(value = ForgeRegistry.class, remap = false) -public abstract class ForgeRegistryMixin { - // Replace the backing maps with fastutil maps for a bit more speed, since value->holder lookups in particular - // are a bottleneck in many areas (e.g. render type lookup) - @Shadow @Final private Map> delegatesByName = new Object2ObjectOpenHashMap<>(); - - @Shadow @Final private Map> delegatesByValue = new Object2ObjectOpenHashMap<>(Hash.DEFAULT_INITIAL_SIZE, 0.5F); - - /** - * @author embeddedt - * @reason stop allocating so many unneeded objects. stop. - */ - @Overwrite - public Holder.Reference getDelegateOrThrow(ResourceLocation location) { - Holder.Reference holder = delegatesByName.get(location); - - if (holder == null) { - throw new IllegalArgumentException(String.format(Locale.ENGLISH, "No delegate exists for location %s", location)); - } - - return holder; - } - - /** - * @author embeddedt - * @reason see above - */ - @Overwrite - public Holder.Reference getDelegateOrThrow(ResourceKey rkey) { - Holder.Reference holder = delegatesByName.get(rkey.location()); - - if (holder == null) { - throw new IllegalArgumentException(String.format(Locale.ENGLISH, "No delegate exists for key %s", rkey)); - } - - return holder; - } - - /** - * @author embeddedt - * @reason see above - */ - @Overwrite - public Holder.Reference getDelegateOrThrow(V value) { - Holder.Reference holder = delegatesByValue.get(value); - - if (holder == null) { - throw new IllegalArgumentException(String.format(Locale.ENGLISH, "No delegate exists for value %s", value)); - } - - return holder; - } -} diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/entity_pose_stack/LivingEntityRendererMixin.java b/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/bugfix/entity_pose_stack/LivingEntityRendererMixin.java similarity index 64% rename from forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/entity_pose_stack/LivingEntityRendererMixin.java rename to neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/bugfix/entity_pose_stack/LivingEntityRendererMixin.java index 10109809..186934c7 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/entity_pose_stack/LivingEntityRendererMixin.java +++ b/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/bugfix/entity_pose_stack/LivingEntityRendererMixin.java @@ -1,10 +1,10 @@ -package org.embeddedt.modernfix.forge.mixin.bugfix.entity_pose_stack; +package org.embeddedt.modernfix.neoforge.mixin.bugfix.entity_pose_stack; import com.mojang.blaze3d.vertex.PoseStack; import net.minecraft.client.renderer.entity.LivingEntityRenderer; -import net.minecraftforge.client.event.RenderLivingEvent; -import net.minecraftforge.eventbus.api.Event; -import net.minecraftforge.eventbus.api.IEventBus; +import net.neoforged.neoforge.client.event.RenderLivingEvent; +import net.neoforged.bus.api.Event; +import net.neoforged.bus.api.IEventBus; import org.embeddedt.modernfix.annotation.ClientOnlyMixin; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.injection.At; @@ -13,18 +13,17 @@ import org.spongepowered.asm.mixin.injection.Redirect; @Mixin(LivingEntityRenderer.class) @ClientOnlyMixin public class LivingEntityRendererMixin { - @Redirect(method = "render(Lnet/minecraft/world/entity/LivingEntity;FFLcom/mojang/blaze3d/vertex/PoseStack;Lnet/minecraft/client/renderer/MultiBufferSource;I)V", at = @At(value = "INVOKE", target = "Lnet/minecraftforge/eventbus/api/IEventBus;post(Lnet/minecraftforge/eventbus/api/Event;)Z", ordinal = 0)) - private boolean fireCheckingPoseStack(IEventBus instance, Event event) { + @Redirect(method = "render(Lnet/minecraft/world/entity/LivingEntity;FFLcom/mojang/blaze3d/vertex/PoseStack;Lnet/minecraft/client/renderer/MultiBufferSource;I)V", at = @At(value = "INVOKE", target = "Lnet/neoforged/bus/api/IEventBus;post(Lnet/neoforged/bus/api/Event;)Lnet/neoforged/bus/api/Event;", ordinal = 0)) + private Event fireCheckingPoseStack(IEventBus instance, Event event) { PoseStack stack = ((RenderLivingEvent)event).getPoseStack(); int size = ((PoseStackAccessor)stack).getPoseStack().size(); - if (instance.post(event)) { + instance.post(event); + if (((RenderLivingEvent.Pre)event).isCanceled()) { // Pop the stack if someone pushed it in the event while (((PoseStackAccessor)stack).getPoseStack().size() > size) { stack.popPose(); } - return true; - } else { - return false; } + return event; } } diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/entity_pose_stack/PlayerRendererMixin.java b/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/bugfix/entity_pose_stack/PlayerRendererMixin.java similarity index 59% rename from forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/entity_pose_stack/PlayerRendererMixin.java rename to neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/bugfix/entity_pose_stack/PlayerRendererMixin.java index 71180460..82ade461 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/entity_pose_stack/PlayerRendererMixin.java +++ b/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/bugfix/entity_pose_stack/PlayerRendererMixin.java @@ -1,10 +1,10 @@ -package org.embeddedt.modernfix.forge.mixin.bugfix.entity_pose_stack; +package org.embeddedt.modernfix.neoforge.mixin.bugfix.entity_pose_stack; import com.mojang.blaze3d.vertex.PoseStack; import net.minecraft.client.renderer.entity.player.PlayerRenderer; -import net.minecraftforge.client.event.RenderPlayerEvent; -import net.minecraftforge.eventbus.api.Event; -import net.minecraftforge.eventbus.api.IEventBus; +import net.neoforged.bus.api.Event; +import net.neoforged.bus.api.IEventBus; +import net.neoforged.neoforge.client.event.RenderLivingEvent; import org.embeddedt.modernfix.annotation.ClientOnlyMixin; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.injection.At; @@ -13,18 +13,17 @@ import org.spongepowered.asm.mixin.injection.Redirect; @Mixin(PlayerRenderer.class) @ClientOnlyMixin public class PlayerRendererMixin { - @Redirect(method = "render(Lnet/minecraft/client/player/AbstractClientPlayer;FFLcom/mojang/blaze3d/vertex/PoseStack;Lnet/minecraft/client/renderer/MultiBufferSource;I)V", at = @At(value = "INVOKE", target = "Lnet/minecraftforge/eventbus/api/IEventBus;post(Lnet/minecraftforge/eventbus/api/Event;)Z", ordinal = 0)) - private boolean fireCheckingPoseStack(IEventBus instance, Event event) { - PoseStack stack = ((RenderPlayerEvent)event).getPoseStack(); + @Redirect(method = "render(Lnet/minecraft/client/player/AbstractClientPlayer;FFLcom/mojang/blaze3d/vertex/PoseStack;Lnet/minecraft/client/renderer/MultiBufferSource;I)V", at = @At(value = "INVOKE", target = "Lnet/neoforged/bus/api/IEventBus;post(Lnet/neoforged/bus/api/Event;)Lnet/neoforged/bus/api/Event;", ordinal = 0)) + private Event fireCheckingPoseStack(IEventBus instance, Event event) { + PoseStack stack = ((RenderLivingEvent)event).getPoseStack(); int size = ((PoseStackAccessor)stack).getPoseStack().size(); - if (instance.post(event)) { + instance.post(event); + if (((RenderLivingEvent.Pre)event).isCanceled()) { // Pop the stack if someone pushed it in the event while (((PoseStackAccessor)stack).getPoseStack().size() > size) { stack.popPose(); } - return true; - } else { - return false; } + return event; } } diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/entity_pose_stack/PoseStackAccessor.java b/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/bugfix/entity_pose_stack/PoseStackAccessor.java similarity index 83% rename from forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/entity_pose_stack/PoseStackAccessor.java rename to neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/bugfix/entity_pose_stack/PoseStackAccessor.java index a0a8ec30..ee018ed3 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/entity_pose_stack/PoseStackAccessor.java +++ b/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/bugfix/entity_pose_stack/PoseStackAccessor.java @@ -1,4 +1,4 @@ -package org.embeddedt.modernfix.forge.mixin.bugfix.entity_pose_stack; +package org.embeddedt.modernfix.neoforge.mixin.bugfix.entity_pose_stack; import com.mojang.blaze3d.vertex.PoseStack; import org.embeddedt.modernfix.annotation.ClientOnlyMixin; From 48b492b9068ceb6b06ff70058e3dc658a88c0e1c Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 2 Mar 2024 13:17:19 -0500 Subject: [PATCH 05/10] Bump NeoForge --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 75eb5a18..7b587d2c 100644 --- a/gradle.properties +++ b/gradle.properties @@ -7,7 +7,7 @@ mixinextras_version=0.3.2 mod_id=modernfix minecraft_version=1.20.4 enabled_platforms=fabric,neoforge -forge_version=20.4.132-beta +forge_version=20.4.190 # parchment_version=2023.07.09 refined_storage_version=4392788 jei_version=16.0.0.28 From 967d39997f1fc1dfc751b95965da3d28475e522a Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 2 Mar 2024 13:50:35 -0500 Subject: [PATCH 06/10] Remove buffer builder leak fix since you can now close them properly --- .../BufferBuilderMixin.java | 57 ------------------- .../modernfix/render/UnsafeBufferHelper.java | 51 ----------------- 2 files changed, 108 deletions(-) delete mode 100644 common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/buffer_builder_leak/BufferBuilderMixin.java delete mode 100644 common/src/main/java/org/embeddedt/modernfix/render/UnsafeBufferHelper.java diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/buffer_builder_leak/BufferBuilderMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/buffer_builder_leak/BufferBuilderMixin.java deleted file mode 100644 index 432e9117..00000000 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/buffer_builder_leak/BufferBuilderMixin.java +++ /dev/null @@ -1,57 +0,0 @@ -package org.embeddedt.modernfix.common.mixin.bugfix.buffer_builder_leak; - -import com.mojang.blaze3d.vertex.BufferBuilder; -import org.embeddedt.modernfix.ModernFix; -import org.embeddedt.modernfix.annotation.ClientOnlyMixin; -import org.embeddedt.modernfix.render.UnsafeBufferHelper; -import org.spongepowered.asm.mixin.Dynamic; -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.callback.CallbackInfo; - -import java.nio.ByteBuffer; - -@Mixin(value = BufferBuilder.class, priority = 1500) -@ClientOnlyMixin -public class BufferBuilderMixin { - @Shadow private ByteBuffer buffer; - @Shadow private boolean closed; - - private static boolean leakReported = false; - - private boolean mfix$shouldFree = true; - - @Dynamic - @Inject(method = "flywheel$injectForRender", at = @At("RETURN"), remap = false, require = 0) - private void preventFree(CallbackInfo ci) { - mfix$shouldFree = false; - } - - /** - * Ensure UnsafeBufferHelper is classloaded early, to avoid Forge's event transformer showing an error in the log. - */ - @Inject(method = "", at = @At(value = "RETURN")) - private static void initUnsafeBufferHelper(CallbackInfo ci) { - UnsafeBufferHelper.init(); - } - - @Override - protected void finalize() throws Throwable { - try { - ByteBuffer buf = buffer; - // can be null if a mod already tried to free the buffer - if(!this.closed && buf != null && mfix$shouldFree) { - if(!leakReported) { - leakReported = true; - ModernFix.LOGGER.warn("One or more BufferBuilders have been leaked, ModernFix will attempt to correct this."); - } - UnsafeBufferHelper.free(buf); - buffer = null; - } - } finally { - super.finalize(); - } - } -} diff --git a/common/src/main/java/org/embeddedt/modernfix/render/UnsafeBufferHelper.java b/common/src/main/java/org/embeddedt/modernfix/render/UnsafeBufferHelper.java deleted file mode 100644 index 69ab0cfe..00000000 --- a/common/src/main/java/org/embeddedt/modernfix/render/UnsafeBufferHelper.java +++ /dev/null @@ -1,51 +0,0 @@ -package org.embeddedt.modernfix.render; - -import org.embeddedt.modernfix.ModernFix; -import org.lwjgl.system.MemoryUtil; -import sun.misc.Unsafe; - -import java.lang.reflect.Field; -import java.nio.ByteBuffer; - -/** - * Helper that frees ByteBuffers allocated by BufferBuilders, and nulls out the address pointer - * to prevent double frees. - * - * @author Moulberry - */ -public class UnsafeBufferHelper { - private static final MemoryUtil.MemoryAllocator ALLOCATOR = MemoryUtil.getAllocator(false); - - private static sun.misc.Unsafe UNSAFE = null; - private static long ADDRESS = -1; - - static { - try { - final Field theUnsafe = Unsafe.class.getDeclaredField("theUnsafe"); - theUnsafe.setAccessible(true); - UNSAFE = (Unsafe)theUnsafe.get(null); - - final Field addressField = MemoryUtil.class.getDeclaredField("ADDRESS"); - addressField.setAccessible(true); - ADDRESS = addressField.getLong(null); - } catch(Throwable t) { - ModernFix.LOGGER.error("Could load unsafe/buffer address", t); - } - } - - public static void init() { - - } - - public static void free(ByteBuffer buf) { - if(UNSAFE != null && ADDRESS >= 0) { - // set the address to 0 to prevent double free - long address = UNSAFE.getAndSetLong(buf, ADDRESS, 0); - if(address != 0) { - ALLOCATOR.free(address); - } - } else { - ALLOCATOR.free(MemoryUtil.memAddress0(buf)); - } - } -} From 761703b4ab6d28a6cba5752c5f11e5700882ba42 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 2 Mar 2024 21:35:50 -0500 Subject: [PATCH 07/10] Cast to correct event type --- .../mixin/bugfix/entity_pose_stack/PlayerRendererMixin.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/bugfix/entity_pose_stack/PlayerRendererMixin.java b/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/bugfix/entity_pose_stack/PlayerRendererMixin.java index 82ade461..f38abc42 100644 --- a/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/bugfix/entity_pose_stack/PlayerRendererMixin.java +++ b/neoforge/src/main/java/org/embeddedt/modernfix/neoforge/mixin/bugfix/entity_pose_stack/PlayerRendererMixin.java @@ -4,7 +4,7 @@ import com.mojang.blaze3d.vertex.PoseStack; import net.minecraft.client.renderer.entity.player.PlayerRenderer; import net.neoforged.bus.api.Event; import net.neoforged.bus.api.IEventBus; -import net.neoforged.neoforge.client.event.RenderLivingEvent; +import net.neoforged.neoforge.client.event.RenderPlayerEvent; import org.embeddedt.modernfix.annotation.ClientOnlyMixin; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.injection.At; @@ -15,10 +15,10 @@ import org.spongepowered.asm.mixin.injection.Redirect; public class PlayerRendererMixin { @Redirect(method = "render(Lnet/minecraft/client/player/AbstractClientPlayer;FFLcom/mojang/blaze3d/vertex/PoseStack;Lnet/minecraft/client/renderer/MultiBufferSource;I)V", at = @At(value = "INVOKE", target = "Lnet/neoforged/bus/api/IEventBus;post(Lnet/neoforged/bus/api/Event;)Lnet/neoforged/bus/api/Event;", ordinal = 0)) private Event fireCheckingPoseStack(IEventBus instance, Event event) { - PoseStack stack = ((RenderLivingEvent)event).getPoseStack(); + PoseStack stack = ((RenderPlayerEvent)event).getPoseStack(); int size = ((PoseStackAccessor)stack).getPoseStack().size(); instance.post(event); - if (((RenderLivingEvent.Pre)event).isCanceled()) { + if (((RenderPlayerEvent.Pre)event).isCanceled()) { // Pop the stack if someone pushed it in the event while (((PoseStackAccessor)stack).getPoseStack().size() > size) { stack.popPose(); From a45783647fc199d4b08c9cc6575cce314790ad6a Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 4 Mar 2024 16:37:50 -0500 Subject: [PATCH 08/10] Make the block model cache thread-local instead of using a lock --- .../BlockModelShaperMixin.java | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/BlockModelShaperMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/BlockModelShaperMixin.java index bed4cb16..11f99989 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/BlockModelShaperMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_resources/BlockModelShaperMixin.java @@ -1,12 +1,12 @@ package org.embeddedt.modernfix.common.mixin.perf.dynamic_resources; +import it.unimi.dsi.fastutil.objects.Reference2ReferenceLinkedOpenHashMap; import net.minecraft.client.renderer.block.BlockModelShaper; import net.minecraft.client.resources.model.BakedModel; import net.minecraft.client.resources.model.ModelManager; import net.minecraft.client.resources.model.ModelResourceLocation; import net.minecraft.world.level.block.state.BlockState; import org.embeddedt.modernfix.annotation.ClientOnlyMixin; -import org.embeddedt.modernfix.dynamicresources.DynamicModelCache; import org.embeddedt.modernfix.dynamicresources.ModelLocationCache; import org.embeddedt.modernfix.util.DynamicOverridableMap; import org.spongepowered.asm.mixin.*; @@ -25,7 +25,7 @@ public class BlockModelShaperMixin { @Shadow @Final @Mutable private Map modelByStateCache; - private final DynamicModelCache mfix$modelCache = new DynamicModelCache<>(k -> this.cacheBlockModel((BlockState)k), false); + private ThreadLocal> mfix$modelCache = ThreadLocal.withInitial(Reference2ReferenceLinkedOpenHashMap::new); @Inject(method = "", at = @At("RETURN")) private void replaceModelMap(CallbackInfo ci) { @@ -39,7 +39,7 @@ public class BlockModelShaperMixin { */ @Overwrite public void rebuildCache() { - this.mfix$modelCache.clear(); + this.mfix$modelCache = ThreadLocal.withInitial(Reference2ReferenceLinkedOpenHashMap::new); } private BakedModel cacheBlockModel(BlockState state) { @@ -59,6 +59,18 @@ public class BlockModelShaperMixin { */ @Overwrite public BakedModel getBlockModel(BlockState state) { - return this.mfix$modelCache.get(state); + Reference2ReferenceLinkedOpenHashMap map = this.mfix$modelCache.get(); + BakedModel model = map.get(state); + + if(model != null) { + return model; + } + + model = this.cacheBlockModel(state); + map.putAndMoveToFirst(state, model); + if(map.size() > 500) { + map.removeLast(); + } + return model; } } From 40b9ac6002467f737a87b201fcf662005525b45f Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 8 Mar 2024 17:49:46 -0500 Subject: [PATCH 09/10] Disable pose stack bugfix when OptiFine is installed Related: #376 --- .../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 65696139..b86c734c 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 @@ -228,6 +228,7 @@ public class ModernFixEarlyConfig { disableIfModPresent("mixin.perf.reuse_datapacks", "tac"); disableIfModPresent("mixin.launch.class_search_cache", "optifine"); disableIfModPresent("mixin.perf.faster_texture_stitching", "optifine"); + disableIfModPresent("mixin.bugfix.entity_pose_stack", "optifine"); disableIfModPresent("mixin.perf.datapack_reload_exceptions", "cyanide"); disableIfModPresent("mixin.perf.faster_texture_loading", "stitch", "optifine", "changed"); if(isFabric) { From 36f2564d6adda3c1cb1811d9075c5e2a475e8afd Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sun, 10 Mar 2024 21:53:18 -0400 Subject: [PATCH 10/10] Store delegates on registry objects to avoid map lookup --- .../DelegateHolderMixin.java | 30 ++++++++++++++++ .../ForgeRegistryMixin.java | 36 +++++++++++++++++-- .../forge/registry/DelegateHolder.java | 10 ++++++ 3 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/DelegateHolderMixin.java create mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/registry/DelegateHolder.java diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/DelegateHolderMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/DelegateHolderMixin.java new file mode 100644 index 00000000..4737ac1c --- /dev/null +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/DelegateHolderMixin.java @@ -0,0 +1,30 @@ +package org.embeddedt.modernfix.forge.mixin.perf.forge_registry_alloc; + +import net.minecraft.core.Holder; +import net.minecraft.core.Registry; +import net.minecraft.resources.ResourceKey; +import net.minecraft.world.item.Item; +import net.minecraft.world.level.block.Block; +import org.embeddedt.modernfix.forge.registry.DelegateHolder; +import org.spongepowered.asm.mixin.Mixin; + +@Mixin({Block.class, Item.class}) +public class DelegateHolderMixin implements DelegateHolder { + private Holder.Reference mfix$delegate; + private ResourceKey> mfix$key; + + @Override + public Holder.Reference mfix$getDelegate(ResourceKey> registryKey) { + if(mfix$key == registryKey) { + return mfix$delegate; + } else { + return null; + } + } + + @Override + public void mfix$setDelegate(ResourceKey> registryKey, Holder.Reference holder) { + this.mfix$delegate = holder; + this.mfix$key = registryKey; + } +} diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java index 4488dc67..5d19c0d1 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_registry_alloc/ForgeRegistryMixin.java @@ -3,13 +3,19 @@ package org.embeddedt.modernfix.forge.mixin.perf.forge_registry_alloc; import it.unimi.dsi.fastutil.Hash; import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; import net.minecraft.core.Holder; +import net.minecraft.core.Registry; import net.minecraft.resources.ResourceKey; import net.minecraft.resources.ResourceLocation; import net.minecraftforge.registries.ForgeRegistry; +import net.minecraftforge.registries.RegistryManager; +import org.embeddedt.modernfix.forge.registry.DelegateHolder; import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Overwrite; import org.spongepowered.asm.mixin.Shadow; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Inject; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; import java.util.Locale; import java.util.Map; @@ -22,6 +28,10 @@ public abstract class ForgeRegistryMixin { @Shadow @Final private Map> delegatesByValue = new Object2ObjectOpenHashMap<>(Hash.DEFAULT_INITIAL_SIZE, 0.5F); + @Shadow public abstract ResourceKey> getRegistryKey(); + + @Shadow @Final private RegistryManager stage; + /** * @author embeddedt * @reason stop allocating so many unneeded objects. stop. @@ -54,14 +64,34 @@ public abstract class ForgeRegistryMixin { /** * @author embeddedt - * @reason see above + * @reason store delegates that are accessed extremely regularly on the registry entry itself, rather than + * going through a map lookup + */ + @Inject(method = "bindDelegate", at = @At("RETURN")) + private void attachDelegate(ResourceKey rkey, V value, CallbackInfoReturnable> cir) { + // Only attach delegates for the ACTIVE registry. The Forge registry system is weird and seems to keep multiple + // copies of itself at once. + if(this.stage == RegistryManager.ACTIVE && value instanceof DelegateHolder) { + ((DelegateHolder)value).mfix$setDelegate(this.getRegistryKey(), cir.getReturnValue()); + } + } + + /** + * @author embeddedt + * @reason skip map lookup for hot delegates, avoid allocations otherwise */ @Overwrite public Holder.Reference getDelegateOrThrow(V value) { - Holder.Reference holder = delegatesByValue.get(value); + Holder.Reference holder = null; + if (this.stage == RegistryManager.ACTIVE && value instanceof DelegateHolder) { + holder = ((DelegateHolder)value).mfix$getDelegate(this.getRegistryKey()); + } if (holder == null) { - throw new IllegalArgumentException(String.format(Locale.ENGLISH, "No delegate exists for value %s", value)); + holder = delegatesByValue.get(value); + if (holder == null) { + throw new IllegalArgumentException(String.format(Locale.ENGLISH, "No delegate exists for value %s", value)); + } } return holder; diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/registry/DelegateHolder.java b/forge/src/main/java/org/embeddedt/modernfix/forge/registry/DelegateHolder.java new file mode 100644 index 00000000..acc06cab --- /dev/null +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/registry/DelegateHolder.java @@ -0,0 +1,10 @@ +package org.embeddedt.modernfix.forge.registry; + +import net.minecraft.core.Holder; +import net.minecraft.core.Registry; +import net.minecraft.resources.ResourceKey; + +public interface DelegateHolder { + Holder.Reference mfix$getDelegate(ResourceKey> registryKey); + void mfix$setDelegate(ResourceKey> registryKey, Holder.Reference holder); +}