From e7277b89d5cfc6dac9dfb61c17af66a87c1c9960 Mon Sep 17 00:00:00 2001 From: Phoenix-Starlight <64714532+Phoenix-Starlight@users.noreply.github.com> Date: Thu, 12 Oct 2023 19:19:46 -0700 Subject: [PATCH 1/3] Fix dynamic_sounds breaking on 1.16 (#259) An issue in Guava (https://github.com/google/guava/issues/3081) causes the removal listener to fire even when entries haven't actually been removed. We filter them to get around this. --- .../mixin/perf/dynamic_sounds/SoundBufferLibraryMixin.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_sounds/SoundBufferLibraryMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_sounds/SoundBufferLibraryMixin.java index 607657d5..921da3ef 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_sounds/SoundBufferLibraryMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_sounds/SoundBufferLibraryMixin.java @@ -1,6 +1,7 @@ package org.embeddedt.modernfix.common.mixin.perf.dynamic_sounds; import com.google.common.cache.CacheBuilder; +import com.google.common.cache.RemovalCause; import com.google.common.cache.RemovalNotification; import com.mojang.blaze3d.audio.SoundBuffer; import net.minecraft.client.sounds.SoundBufferLibrary; @@ -26,12 +27,15 @@ public abstract class SoundBufferLibraryMixin { @Shadow @Final @Mutable private Map> cache = CacheBuilder.newBuilder() .expireAfterAccess(DynamicSoundHelpers.MAX_SOUND_LIFETIME_SECS, TimeUnit.SECONDS) + .concurrencyLevel(1) // Excessive use of type hinting due to it assuming Object as the broadest correct type .>removalListener(this::onSoundRemoval) .build() .asMap(); private > void onSoundRemoval(RemovalNotification notification) { + if(notification.getCause() == RemovalCause.REPLACED && notification.getValue() == cache.get(notification.getKey())) + return; notification.getValue().thenAccept(SoundBuffer::discardAlBuffer); if(debugDynamicSoundLoading) { K k = notification.getKey(); From 49c1bc71bad8d1ffde886f6234997d8e900572db Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 13 Oct 2023 10:24:37 -0400 Subject: [PATCH 2/3] Patch modded shape caches to be thread-safe Related: #260 --- .../ShapeCacheCyclicMixin.java | 27 +++++++++++++++++ .../ShapeCacheRSMixin.java | 29 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/unsafe_modded_shape_caches/ShapeCacheCyclicMixin.java create mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/unsafe_modded_shape_caches/ShapeCacheRSMixin.java diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/unsafe_modded_shape_caches/ShapeCacheCyclicMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/unsafe_modded_shape_caches/ShapeCacheCyclicMixin.java new file mode 100644 index 00000000..3f1eb8e2 --- /dev/null +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/unsafe_modded_shape_caches/ShapeCacheCyclicMixin.java @@ -0,0 +1,27 @@ +package org.embeddedt.modernfix.forge.mixin.bugfix.unsafe_modded_shape_caches; + +import org.embeddedt.modernfix.ModernFix; +import org.embeddedt.modernfix.annotation.RequiresMod; +import org.spongepowered.asm.mixin.*; +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.concurrent.ConcurrentHashMap; + +/** + * see {@link ShapeCacheRSMixin} + */ +@Pseudo +@Mixin(targets = { "com/lothrazar/cyclic/block/cable/ShapeCache" }, remap = false) +@RequiresMod("cyclic") +@SuppressWarnings("rawtypes") +public class ShapeCacheCyclicMixin { + @Shadow @Final @Mutable private static final Map CACHE = new ConcurrentHashMap(); + + @Inject(method = "", at = @At("RETURN")) + private static void mfix$notify(CallbackInfo ci) { + ModernFix.LOGGER.info("Made Cyclic shape cache map thread-safe"); + } +} diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/unsafe_modded_shape_caches/ShapeCacheRSMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/unsafe_modded_shape_caches/ShapeCacheRSMixin.java new file mode 100644 index 00000000..c2a43839 --- /dev/null +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/unsafe_modded_shape_caches/ShapeCacheRSMixin.java @@ -0,0 +1,29 @@ +package org.embeddedt.modernfix.forge.mixin.bugfix.unsafe_modded_shape_caches; + +import org.embeddedt.modernfix.ModernFix; +import org.embeddedt.modernfix.annotation.RequiresMod; +import org.spongepowered.asm.mixin.*; +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.concurrent.ConcurrentHashMap; + +/** + * Some mods use a custom shape cache with a non-thread-safe map. There is no reason why this wouldn't cause crashes + * in vanilla as well if getShape was called on two threads at once. It seems more likely to happen with ModernFix + * installed due to the dynamic blockstate cache generation, so we solve it by making the maps thread-safe. + */ +@Pseudo +@Mixin(targets = { "com/refinedmods/refinedstorage/block/shape/ShapeCache" }, remap = false) +@RequiresMod("refinedstorage") +@SuppressWarnings("rawtypes") +public class ShapeCacheRSMixin { + @Shadow @Final @Mutable private static final Map CACHE = new ConcurrentHashMap(); + + @Inject(method = "", at = @At("RETURN")) + private static void mfix$notify(CallbackInfo ci) { + ModernFix.LOGGER.info("Made Refined Storage shape cache map thread-safe"); + } +} From 8059fc46727b6b33c244ec71c52881387d5e0de0 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 13 Oct 2023 10:33:08 -0400 Subject: [PATCH 3/3] Make clear_fabric_mapping_tables work on Fabric Loader 0.14.23 --- .../modernfix/fabric/mappings/MappingsClearer.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/fabric/src/main/java/org/embeddedt/modernfix/fabric/mappings/MappingsClearer.java b/fabric/src/main/java/org/embeddedt/modernfix/fabric/mappings/MappingsClearer.java index 1e392e57..75341b3f 100644 --- a/fabric/src/main/java/org/embeddedt/modernfix/fabric/mappings/MappingsClearer.java +++ b/fabric/src/main/java/org/embeddedt/modernfix/fabric/mappings/MappingsClearer.java @@ -21,6 +21,14 @@ public class MappingsClearer { if(FabricLoader.getInstance().isDevelopmentEnvironment()) return; // never do this in dev CommonModUtil.runWithoutCrash(() -> { + // For now, force the mapping resolver to be initialized. Fabric Loader 0.14.23 stops initializing it early, + // which means that we actually need to keep the TinyMappingFactory tree around for initialization to work + // later. We force init it now because then we can store less in memory. + // Comparing heap dumps on 0.14.23 suggests a savings of 20MB by doing it our way, since many mods will + // end up needing the mapping resolver. + // This will need to be revisited when https://github.com/FabricMC/fabric-loader/pull/812 is merged. + FabricLoader.getInstance().getMappingResolver(); + // clear notch->intermediary mappings MappingConfiguration config = FabricLauncherBase.getLauncher().getMappingConfiguration(); Field mappingsField = MappingConfiguration.class.getDeclaredField("mappings");