From ae1051e81397a875bc4b34aa01d8148dc9344fda Mon Sep 17 00:00:00 2001 From: Pieter-Jan Briers Date: Wed, 19 Jun 2024 22:49:22 +0200 Subject: [PATCH] Cache non-existence of ResourceCache TryGetResource. Many patterns (both in engine and content) make use of regular TryGetResource returning null. The problem is that if the resource doesn't exist, it won't be cached and the code attempts to load it from disk *every single time*. For example, opening an inventory in SS14 would hang the client for ages on some UI themes due to the UITheme texture fallback system constantly trying to load a texture that doesn't exist. --- RELEASE-NOTES.md | 2 +- .../ResourceCache.Preload.cs | 4 +- .../ResourceManagement/ResourceCache.cs | 52 +++++++++++-------- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 4cabeaaf1..6b3425ba2 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -47,7 +47,7 @@ END TEMPLATE--> ### Other -*None yet* +* Non-existent resources are cached by `IResourceCache.TryGetResource`. This avoids the game constantly trying to re-load non-existent resources in common patterns such as UI theme texture fallbacks. ### Internal diff --git a/Robust.Client/ResourceManagement/ResourceCache.Preload.cs b/Robust.Client/ResourceManagement/ResourceCache.Preload.cs index cc567565e..da768589f 100644 --- a/Robust.Client/ResourceManagement/ResourceCache.Preload.cs +++ b/Robust.Client/ResourceManagement/ResourceCache.Preload.cs @@ -47,7 +47,7 @@ namespace Robust.Client.ResourceManagement { sawmill.Debug("Preloading textures..."); var sw = Stopwatch.StartNew(); - var resList = GetTypeDict(); + var resList = GetTypeData().Resources; var texList = _manager.ContentFindFiles("/Textures/") // Skip PNG files inside RSIs. @@ -119,7 +119,7 @@ namespace Robust.Client.ResourceManagement private void PreloadRsis(ISawmill sawmill) { var sw = Stopwatch.StartNew(); - var resList = GetTypeDict(); + var resList = GetTypeData().Resources; var rsiList = _manager.ContentFindFiles("/Textures/") .Where(p => p.ToString().EndsWith(".rsi/meta.json")) diff --git a/Robust.Client/ResourceManagement/ResourceCache.cs b/Robust.Client/ResourceManagement/ResourceCache.cs index 9d48643cd..cdcea4fcc 100644 --- a/Robust.Client/ResourceManagement/ResourceCache.cs +++ b/Robust.Client/ResourceManagement/ResourceCache.cs @@ -17,9 +17,7 @@ namespace Robust.Client.ResourceManagement; /// internal sealed partial class ResourceCache : ResourceManager, IResourceCacheInternal, IDisposable { - private readonly Dictionary> _cachedResources = - new(); - + private readonly Dictionary _cachedResources = new(); private readonly Dictionary _fallbacks = new(); public T GetResource(string path, bool useFallback = true) where T : BaseResource, new() @@ -29,8 +27,8 @@ internal sealed partial class ResourceCache : ResourceManager, IResourceCacheInt public T GetResource(ResPath path, bool useFallback = true) where T : BaseResource, new() { - var cache = GetTypeDict(); - if (cache.TryGetValue(path, out var cached)) + var cache = GetTypeData(); + if (cache.Resources.TryGetValue(path, out var cached)) { return (T) cached; } @@ -40,7 +38,7 @@ internal sealed partial class ResourceCache : ResourceManager, IResourceCacheInt { var dependencies = IoCManager.Instance!; resource.Load(dependencies, path); - cache[path] = resource; + cache.Resources[path] = resource; return resource; } catch (Exception e) @@ -67,24 +65,31 @@ internal sealed partial class ResourceCache : ResourceManager, IResourceCacheInt public bool TryGetResource(ResPath path, [NotNullWhen(true)] out T? resource) where T : BaseResource, new() { - var cache = GetTypeDict(); - if (cache.TryGetValue(path, out var cached)) + var cache = GetTypeData(); + if (cache.Resources.TryGetValue(path, out var cached)) { resource = (T) cached; return true; } + if (cache.NonExistent.Contains(path)) + { + resource = null; + return false; + } + var _resource = new T(); try { var dependencies = IoCManager.Instance!; _resource.Load(dependencies, path); resource = _resource; - cache[path] = resource; + cache.Resources[path] = resource; return true; } catch (FileNotFoundException) { + cache.NonExistent.Add(path); resource = null; return false; } @@ -109,9 +114,9 @@ internal sealed partial class ResourceCache : ResourceManager, IResourceCacheInt public void ReloadResource(ResPath path) where T : BaseResource, new() { - var cache = GetTypeDict(); + var cache = GetTypeData(); - if (!cache.TryGetValue(path, out var res)) + if (!cache.Resources.TryGetValue(path, out var res)) { return; } @@ -145,7 +150,7 @@ internal sealed partial class ResourceCache : ResourceManager, IResourceCacheInt public void CacheResource(ResPath path, T resource) where T : BaseResource, new() { - GetTypeDict()[path] = resource; + GetTypeData().Resources[path] = resource; } public T GetFallback() where T : BaseResource, new() @@ -168,7 +173,7 @@ internal sealed partial class ResourceCache : ResourceManager, IResourceCacheInt public IEnumerable> GetAllResources() where T : BaseResource, new() { - return GetTypeDict().Select(p => new KeyValuePair(p.Key, (T) p.Value)); + return GetTypeData().Resources.Select(p => new KeyValuePair(p.Key, (T) p.Value)); } public event Action? OnRawTextureLoaded; @@ -193,7 +198,7 @@ internal sealed partial class ResourceCache : ResourceManager, IResourceCacheInt if (disposing) { - foreach (var res in _cachedResources.Values.SelectMany(dict => dict.Values)) + foreach (var res in _cachedResources.Values.SelectMany(dict => dict.Resources.Values)) { res.Dispose(); } @@ -210,15 +215,9 @@ internal sealed partial class ResourceCache : ResourceManager, IResourceCacheInt #endregion IDisposable Members [MethodImpl(MethodImplOptions.AggressiveInlining)] - protected Dictionary GetTypeDict() + private TypeData GetTypeData() { - if (!_cachedResources.TryGetValue(typeof(T), out var ret)) - { - ret = new Dictionary(); - _cachedResources.Add(typeof(T), ret); - } - - return ret; + return _cachedResources.GetOrNew(typeof(T)); } public void TextureLoaded(TextureLoadedEventArgs eventArgs) @@ -230,4 +229,13 @@ internal sealed partial class ResourceCache : ResourceManager, IResourceCacheInt { OnRsiLoaded?.Invoke(eventArgs); } + + private sealed class TypeData + { + public readonly Dictionary Resources = new(); + + // List of resources which DON'T exist. + // Needed to avoid innocuous TryGet calls repeatedly trying to re-load non-existent resources from disk. + public readonly HashSet NonExistent = new(); + } }