From 9fbb97d0faec0784afaeb95684b16e97457f2fde Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 17 Jul 2023 13:20:46 -0400 Subject: [PATCH 1/3] Use Guava multimap for resource finder instead of custom "multi"map --- .../classloading/ModernFixResourceFinder.java | 41 ++++++------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ModernFixResourceFinder.java b/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ModernFixResourceFinder.java index 01b76015..211e0d52 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ModernFixResourceFinder.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ModernFixResourceFinder.java @@ -1,5 +1,6 @@ package org.embeddedt.modernfix.forge.classloading; +import com.google.common.base.Joiner; import com.google.common.collect.*; import net.minecraftforge.fml.loading.FMLLoader; import net.minecraftforge.fml.loading.LoadingModList; @@ -22,7 +23,7 @@ import java.util.regex.Pattern; import java.util.stream.Stream; public class ModernFixResourceFinder { - private static Map>> urlsForClass = null; + private static Multimap urlsForClass = null; private static final Class MINECRAFT_LOCATOR; private static Field explodedDirModsField = null; private static final Logger LOGGER = LogManager.getLogger("ModernFixResourceFinder"); @@ -35,8 +36,10 @@ public class ModernFixResourceFinder { } } + private static final Joiner SLASH_JOINER = Joiner.on('/'); + public static synchronized void init() throws ReflectiveOperationException { - urlsForClass = new HashMap<>(); + ImmutableMultimap.Builder urlBuilder = ImmutableMultimap.builder(); Interner pathInterner = Interners.newStrongInterner(); //LOGGER.info("Start building list of class locations..."); for(ModFileInfo fileInfo : LoadingModList.get().getModFiles()) { @@ -50,33 +53,15 @@ public class ModernFixResourceFinder { stream .map(root::relativize) .forEach(path -> { - String strPath = path.toString(); - Pair pathPair = Pair.of(fileInfo.getMods().get(0).getModId(), pathInterner.intern(strPath)); - List> urlList = urlsForClass.get(strPath); - if(urlList != null) { - if(urlList.size() > 1) - urlList.add(pathPair); - else { - /* Convert singleton to real list */ - ArrayList> newList = new ArrayList<>(urlList); - newList.add(pathPair); - urlsForClass.put(strPath, newList); - } - } else { - /* Use a singleton list initially to keep memory usage down */ - urlsForClass.put(strPath, Collections.singletonList(pathPair)); - } + String strPath = pathInterner.intern(SLASH_JOINER.join(path)); + urlBuilder.put(strPath, fileInfo.getMods().get(0).getModId()); }); } catch(IOException e) { throw new RuntimeException(e); } } } - for(List> list : urlsForClass.values()) { - if(list instanceof ArrayList) - ((ArrayList>)list).trimToSize(); - } - urlsForClass = ImmutableMap.copyOf(urlsForClass); + urlsForClass = urlBuilder.build(); //LOGGER.info("Finish building"); } @@ -105,12 +90,12 @@ public class ModernFixResourceFinder { private static final Pattern SLASH_REPLACER = Pattern.compile("/+"); public static Enumeration findAllURLsForResource(String input) { - input = SLASH_REPLACER.matcher(input).replaceAll("/"); - List> urlList = urlsForClass.get(input); - if(urlList != null) { - return Iterators.asEnumeration(urlList.stream().map(pair -> { + String pathInput = SLASH_REPLACER.matcher(input).replaceAll("/"); + Collection urlList = urlsForClass.get(pathInput); + if(!urlList.isEmpty()) { + return Iterators.asEnumeration(urlList.stream().map(modId -> { try { - return new URL("modjar://" + pair.getLeft() + "/" + pair.getRight()); + return new URL("modjar://" + modId + "/" + pathInput); } catch(MalformedURLException e) { throw new RuntimeException(e); } From 5ec070843d7eb3f293b7c72305600d1467d9b486 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 17 Jul 2023 13:42:50 -0400 Subject: [PATCH 2/3] Use CachedResourcePath in resource finder Reduces memory usage by ~50% --- .../resources/CachedResourcePath.java | 2 +- .../classloading/ModernFixResourceFinder.java | 33 ++++++++++--------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/resources/CachedResourcePath.java b/common/src/main/java/org/embeddedt/modernfix/resources/CachedResourcePath.java index abd260f7..79a04254 100644 --- a/common/src/main/java/org/embeddedt/modernfix/resources/CachedResourcePath.java +++ b/common/src/main/java/org/embeddedt/modernfix/resources/CachedResourcePath.java @@ -14,7 +14,7 @@ public class CachedResourcePath { public static final Interner PATH_COMPONENT_INTERNER = Interners.newStrongInterner(); private static final Splitter SLASH_SPLITTER = Splitter.on('/'); - private static final String[] NO_PREFIX = new String[0]; + public static final String[] NO_PREFIX = new String[0]; public CachedResourcePath(String[] prefix, Path path) { this(prefix, path, path.getNameCount(), true); diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ModernFixResourceFinder.java b/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ModernFixResourceFinder.java index 211e0d52..528cb4f7 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ModernFixResourceFinder.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ModernFixResourceFinder.java @@ -1,15 +1,21 @@ package org.embeddedt.modernfix.forge.classloading; -import com.google.common.base.Joiner; -import com.google.common.collect.*; +import com.google.common.collect.ImmutableMultimap; +import com.google.common.collect.Iterators; +import com.google.common.collect.Multimap; import net.minecraftforge.fml.loading.FMLLoader; import net.minecraftforge.fml.loading.LoadingModList; -import net.minecraftforge.fml.loading.moddiscovery.*; +import net.minecraftforge.fml.loading.moddiscovery.AbstractJarFileLocator; +import net.minecraftforge.fml.loading.moddiscovery.ExplodedDirectoryLocator; +import net.minecraftforge.fml.loading.moddiscovery.ModFile; +import net.minecraftforge.fml.loading.moddiscovery.ModFileInfo; import net.minecraftforge.forgespi.locating.IModFile; import net.minecraftforge.forgespi.locating.IModLocator; import org.apache.commons.lang3.tuple.Pair; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.embeddedt.modernfix.resources.CachedResourcePath; +import org.embeddedt.modernfix.util.FileUtil; import java.io.IOException; import java.lang.reflect.Field; @@ -19,11 +25,10 @@ import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; import java.util.*; -import java.util.regex.Pattern; import java.util.stream.Stream; public class ModernFixResourceFinder { - private static Multimap urlsForClass = null; + private static Multimap urlsForClass = null; private static final Class MINECRAFT_LOCATOR; private static Field explodedDirModsField = null; private static final Logger LOGGER = LogManager.getLogger("ModernFixResourceFinder"); @@ -36,11 +41,10 @@ public class ModernFixResourceFinder { } } - private static final Joiner SLASH_JOINER = Joiner.on('/'); - public static synchronized void init() throws ReflectiveOperationException { - ImmutableMultimap.Builder urlBuilder = ImmutableMultimap.builder(); - Interner pathInterner = Interners.newStrongInterner(); + // Make sure FileUtil is classloaded now to avoid issues + FileUtil.normalize(""); + ImmutableMultimap.Builder urlBuilder = ImmutableMultimap.builder(); //LOGGER.info("Start building list of class locations..."); for(ModFileInfo fileInfo : LoadingModList.get().getModFiles()) { ModFile file = fileInfo.getFile(); @@ -53,8 +57,8 @@ public class ModernFixResourceFinder { stream .map(root::relativize) .forEach(path -> { - String strPath = pathInterner.intern(SLASH_JOINER.join(path)); - urlBuilder.put(strPath, fileInfo.getMods().get(0).getModId()); + CachedResourcePath p = new CachedResourcePath(CachedResourcePath.NO_PREFIX, path); + urlBuilder.put(p, fileInfo.getMods().get(0).getModId()); }); } catch(IOException e) { throw new RuntimeException(e); @@ -87,12 +91,11 @@ public class ModernFixResourceFinder { throw new UnsupportedOperationException("Unknown ModLocator type: " + locator.getClass().getName()); } - private static final Pattern SLASH_REPLACER = Pattern.compile("/+"); - public static Enumeration findAllURLsForResource(String input) { - String pathInput = SLASH_REPLACER.matcher(input).replaceAll("/"); - Collection urlList = urlsForClass.get(pathInput); + // CachedResourcePath normalizes already + Collection urlList = urlsForClass.get(new CachedResourcePath(input)); if(!urlList.isEmpty()) { + String pathInput = FileUtil.normalize(input); return Iterators.asEnumeration(urlList.stream().map(modId -> { try { return new URL("modjar://" + modId + "/" + pathInput); From c3e3dff805a74735347335ed191fe004e29dcf6b Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 17 Jul 2023 16:07:51 -0400 Subject: [PATCH 3/3] Return all possible states for model if given location is not an MRL --- .../fabric/mixin/perf/dynamic_resources/ModelBakeryMixin.java | 2 +- .../forge/mixin/perf/dynamic_resources/ModelBakeryMixin.java | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/fabric/src/main/java/org/embeddedt/modernfix/fabric/mixin/perf/dynamic_resources/ModelBakeryMixin.java b/fabric/src/main/java/org/embeddedt/modernfix/fabric/mixin/perf/dynamic_resources/ModelBakeryMixin.java index 7e647e5f..12c30ce1 100644 --- a/fabric/src/main/java/org/embeddedt/modernfix/fabric/mixin/perf/dynamic_resources/ModelBakeryMixin.java +++ b/fabric/src/main/java/org/embeddedt/modernfix/fabric/mixin/perf/dynamic_resources/ModelBakeryMixin.java @@ -442,7 +442,7 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { @Redirect(method = "loadModel", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/block/state/StateDefinition;getPossibleStates()Lcom/google/common/collect/ImmutableList;")) private ImmutableList loadOnlyRelevantBlockState(StateDefinition stateDefinition, ResourceLocation location) { - if(this.inTextureGatheringPass) + if(this.inTextureGatheringPass || !(location instanceof ModelResourceLocation)) return stateDefinition.getPossibleStates(); else return ModelBakeryHelpers.getBlockStatesForMRL(stateDefinition, (ModelResourceLocation)location); diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakeryMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakeryMixin.java index cbf5eea1..9362b008 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakeryMixin.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakeryMixin.java @@ -304,7 +304,9 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { } @Redirect(method = "loadModel", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/block/state/StateDefinition;getPossibleStates()Lcom/google/common/collect/ImmutableList;")) private ImmutableList loadOnlyRelevantBlockState(StateDefinition stateDefinition, ResourceLocation location) { - return ModelBakeryHelpers.getBlockStatesForMRL(stateDefinition, (ModelResourceLocation)location); + if(!(location instanceof ModelResourceLocation)) + return stateDefinition.getPossibleStates(); + return ModelBakeryHelpers.getBlockStatesForMRL(stateDefinition, (ModelResourceLocation)location); } @Override