From d3563a385476285d82ca3ef69a46c008f3386e0a Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Wed, 24 Nov 2021 19:07:12 +0000 Subject: [PATCH] Cleanup resource mount reloading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Subscribe to the "on add reload listener" event, otherwise we don't get reloads beyond the first one! This means we no longer need to cast the resource manager to a reloadable one. - Change the mount cache so it's keyed on path, rather than "path ✕ manager". - Update the reload listener just to use the mount cache, rather than having its own separate list. I really don't understand what I was thinking before. --- .../computercraft/ComputerCraftAPIImpl.java | 3 +- .../core/filesystem/ResourceMount.java | 82 ++++++++----------- .../computercraft/shared/CommonHooks.java | 8 ++ 3 files changed, 42 insertions(+), 51 deletions(-) diff --git a/src/main/java/dan200/computercraft/ComputerCraftAPIImpl.java b/src/main/java/dan200/computercraft/ComputerCraftAPIImpl.java index 502460c45..5f775437d 100644 --- a/src/main/java/dan200/computercraft/ComputerCraftAPIImpl.java +++ b/src/main/java/dan200/computercraft/ComputerCraftAPIImpl.java @@ -28,6 +28,7 @@ import dan200.computercraft.shared.peripheral.modem.wireless.WirelessNetwork; import dan200.computercraft.shared.util.IDAssigner; import dan200.computercraft.shared.wired.WiredNode; import net.minecraft.resources.IReloadableResourceManager; +import net.minecraft.resources.IResourceManager; import net.minecraft.tileentity.TileEntity; import net.minecraft.util.Direction; import net.minecraft.util.ResourceLocation; @@ -101,7 +102,7 @@ public final class ComputerCraftAPIImpl implements IComputerCraftAPI @Override public IMount createResourceMount( @Nonnull String domain, @Nonnull String subPath ) { - IReloadableResourceManager manager = (IReloadableResourceManager) ServerLifecycleHooks.getCurrentServer().getDataPackRegistries().getResourceManager(); + IResourceManager manager = ServerLifecycleHooks.getCurrentServer().getDataPackRegistries().getResourceManager(); ResourceMount mount = ResourceMount.get( domain, subPath, manager ); return mount.exists( "" ) ? mount : null; } diff --git a/src/main/java/dan200/computercraft/core/filesystem/ResourceMount.java b/src/main/java/dan200/computercraft/core/filesystem/ResourceMount.java index e82db2d94..6a30c1ecf 100644 --- a/src/main/java/dan200/computercraft/core/filesystem/ResourceMount.java +++ b/src/main/java/dan200/computercraft/core/filesystem/ResourceMount.java @@ -7,19 +7,17 @@ package dan200.computercraft.core.filesystem; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; -import com.google.common.collect.MapMaker; import com.google.common.io.ByteStreams; import dan200.computercraft.ComputerCraft; import dan200.computercraft.api.filesystem.IMount; import dan200.computercraft.core.apis.handles.ArrayByteChannel; import dan200.computercraft.shared.util.IoUtil; -import net.minecraft.resources.IReloadableResourceManager; +import net.minecraft.client.resources.ReloadListener; +import net.minecraft.profiler.IProfiler; import net.minecraft.resources.IResource; import net.minecraft.resources.IResourceManager; import net.minecraft.util.ResourceLocation; import net.minecraft.util.ResourceLocationException; -import net.minecraftforge.resource.IResourceType; -import net.minecraftforge.resource.ISelectiveResourceReloadListener; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -28,9 +26,10 @@ import java.io.IOException; import java.io.InputStream; import java.nio.channels.Channels; import java.nio.channels.ReadableByteChannel; -import java.util.*; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.concurrent.TimeUnit; -import java.util.function.Predicate; public final class ResourceMount implements IMount { @@ -58,50 +57,37 @@ public final class ResourceMount implements IMount .weigher( ( k, v ) -> v.length ) .build(); - private static final MapMaker CACHE_TEMPLATE = new MapMaker().weakValues().concurrencyLevel( 1 ); - /** * Maintain a cache of currently loaded resource mounts. This cache is invalidated when currentManager changes. */ - private static final Map> MOUNT_CACHE = new WeakHashMap<>( 2 ); + private static final Map MOUNT_CACHE = new HashMap<>( 2 ); private final String namespace; private final String subPath; - private final IReloadableResourceManager manager; + private IResourceManager manager; @Nullable private FileEntry root; - public static ResourceMount get( String namespace, String subPath, IReloadableResourceManager manager ) + public static ResourceMount get( String namespace, String subPath, IResourceManager manager ) { - Map cache; - + ResourceLocation path = new ResourceLocation( namespace, subPath ); synchronized( MOUNT_CACHE ) { - cache = MOUNT_CACHE.get( manager ); - if( cache == null ) MOUNT_CACHE.put( manager, cache = CACHE_TEMPLATE.makeMap() ); - } - - ResourceLocation path = new ResourceLocation( namespace, subPath ); - synchronized( cache ) - { - ResourceMount mount = cache.get( path ); - if( mount == null ) cache.put( path, mount = new ResourceMount( namespace, subPath, manager ) ); + ResourceMount mount = MOUNT_CACHE.get( path ); + if( mount == null ) MOUNT_CACHE.put( path, mount = new ResourceMount( namespace, subPath, manager ) ); return mount; } } - private ResourceMount( String namespace, String subPath, IReloadableResourceManager manager ) + private ResourceMount( String namespace, String subPath, IResourceManager manager ) { this.namespace = namespace; this.subPath = subPath; - this.manager = manager; - - Listener.INSTANCE.add( manager, this ); - if( root == null ) load(); + load( manager ); } - private void load() + private void load( IResourceManager manager ) { boolean hasAny = false; String existingNamespace = null; @@ -118,6 +104,7 @@ public final class ResourceMount implements IMount hasAny = true; } + this.manager = manager; root = hasAny ? newRoot : null; if( !hasAny ) @@ -293,35 +280,30 @@ public final class ResourceMount implements IMount } /** - * A {@link ISelectiveResourceReloadListener} which reloads any associated mounts. - * - * While people should really be keeping a permanent reference to this, some people construct it every - * method call, so let's make this as small as possible. + * A {@link ReloadListener} which reloads any associated mounts and correctly updates the resource manager they + * point to. */ - static class Listener implements ISelectiveResourceReloadListener + public static final ReloadListener RELOAD_LISTENER = new ReloadListener() { - private static final Listener INSTANCE = new Listener(); - - private final Set mounts = Collections.newSetFromMap( new WeakHashMap<>() ); - private final Set managers = Collections.newSetFromMap( new WeakHashMap<>() ); - + @Nonnull @Override - public void onResourceManagerReload( @Nonnull IResourceManager manager ) + protected Void prepare( @Nonnull IResourceManager manager, @Nonnull IProfiler profiler ) { - // FIXME: Remove this. We need this patch in order to prevent trying to load ReloadRequirements. - onResourceManagerReload( manager, x -> true ); + profiler.push( "Reloading ComputerCraft mounts" ); + try + { + for( ResourceMount mount : MOUNT_CACHE.values() ) mount.load( manager ); + } + finally + { + profiler.pop(); + } + return null; } @Override - public synchronized void onResourceManagerReload( @Nonnull IResourceManager manager, @Nonnull Predicate predicate ) + protected void apply( @Nonnull Void result, @Nonnull IResourceManager manager, @Nonnull IProfiler profiler ) { - for( ResourceMount mount : mounts ) mount.load(); } - - synchronized void add( IReloadableResourceManager manager, ResourceMount mount ) - { - if( managers.add( manager ) ) manager.registerReloadListener( this ); - mounts.add( mount ); - } - } + }; } diff --git a/src/main/java/dan200/computercraft/shared/CommonHooks.java b/src/main/java/dan200/computercraft/shared/CommonHooks.java index 6e4b5f75c..60e9da7c0 100644 --- a/src/main/java/dan200/computercraft/shared/CommonHooks.java +++ b/src/main/java/dan200/computercraft/shared/CommonHooks.java @@ -8,6 +8,7 @@ package dan200.computercraft.shared; import dan200.computercraft.ComputerCraft; import dan200.computercraft.core.apis.http.NetworkUtils; import dan200.computercraft.core.computer.MainThread; +import dan200.computercraft.core.filesystem.ResourceMount; import dan200.computercraft.core.tracking.ComputerMBean; import dan200.computercraft.core.tracking.Tracking; import dan200.computercraft.shared.command.CommandComputerCraft; @@ -23,6 +24,7 @@ import net.minecraft.loot.TableLootEntry; import net.minecraft.server.MinecraftServer; import net.minecraft.server.dedicated.DedicatedServer; import net.minecraft.util.ResourceLocation; +import net.minecraftforge.event.AddReloadListenerEvent; import net.minecraftforge.event.LootTableLoadEvent; import net.minecraftforge.event.RegisterCommandsEvent; import net.minecraftforge.event.TickEvent; @@ -138,4 +140,10 @@ public final class CommonHooks .name( "computercraft_treasure" ) .build() ); } + + @SubscribeEvent + public static void onAddReloadListeners( AddReloadListenerEvent event ) + { + event.addListener( ResourceMount.RELOAD_LISTENER ); + } }