From 9992915f565578949cad8d9bb8ceb360e0db5c85 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 31 May 2022 18:32:23 -0400 Subject: replace MemoryCache with custom cache This was causing significant frame stutters for some players since the migration to .NET 5 in Stardew Valley 1.5.5. --- src/SMAPI/Framework/Reflection/CacheEntry.cs | 30 ------ src/SMAPI/Framework/Reflection/Reflector.cs | 110 +++++++++++---------- src/SMAPI/Framework/SCore.cs | 2 + .../Framework/Utilities/IntervalMemoryCache.cs | 57 +++++++++++ src/SMAPI/SMAPI.csproj | 1 - 5 files changed, 117 insertions(+), 83 deletions(-) delete mode 100644 src/SMAPI/Framework/Reflection/CacheEntry.cs create mode 100644 src/SMAPI/Framework/Utilities/IntervalMemoryCache.cs (limited to 'src') diff --git a/src/SMAPI/Framework/Reflection/CacheEntry.cs b/src/SMAPI/Framework/Reflection/CacheEntry.cs deleted file mode 100644 index 27f48a1f..00000000 --- a/src/SMAPI/Framework/Reflection/CacheEntry.cs +++ /dev/null @@ -1,30 +0,0 @@ -using System.Diagnostics.CodeAnalysis; -using System.Reflection; - -namespace StardewModdingAPI.Framework.Reflection -{ - /// A cached member reflection result. - internal readonly struct CacheEntry - { - /********* - ** Accessors - *********/ - /// Whether the lookup found a valid match. - [MemberNotNullWhen(true, nameof(CacheEntry.MemberInfo))] - public bool IsValid => this.MemberInfo != null; - - /// The reflection data for this member (or null if invalid). - public MemberInfo? MemberInfo { get; } - - - /********* - ** Public methods - *********/ - /// Construct an instance. - /// The reflection data for this member (or null if invalid). - public CacheEntry(MemberInfo? memberInfo) - { - this.MemberInfo = memberInfo; - } - } -} diff --git a/src/SMAPI/Framework/Reflection/Reflector.cs b/src/SMAPI/Framework/Reflection/Reflector.cs index 79575c26..502a8519 100644 --- a/src/SMAPI/Framework/Reflection/Reflector.cs +++ b/src/SMAPI/Framework/Reflection/Reflector.cs @@ -1,6 +1,6 @@ using System; using System.Reflection; -using System.Runtime.Caching; +using StardewModdingAPI.Framework.Utilities; namespace StardewModdingAPI.Framework.Reflection { @@ -12,10 +12,7 @@ namespace StardewModdingAPI.Framework.Reflection ** Fields *********/ /// The cached fields and methods found via reflection. - private readonly MemoryCache Cache = new(typeof(Reflector).FullName!); - - /// The sliding cache expiration time. - private readonly TimeSpan SlidingCacheExpiry = TimeSpan.FromMinutes(5); + private readonly IntervalMemoryCache Cache = new(); /********* @@ -136,6 +133,15 @@ namespace StardewModdingAPI.Framework.Reflection return method!; } + /**** + ** Management + ****/ + /// Start a new cache interval, clearing stale reflection lookups. + public void NewCacheInterval() + { + this.Cache.StartNewInterval(); + } + /********* ** Private methods @@ -149,20 +155,23 @@ namespace StardewModdingAPI.Framework.Reflection private IReflectedField? GetFieldFromHierarchy(Type type, object? obj, string name, BindingFlags bindingFlags) { bool isStatic = bindingFlags.HasFlag(BindingFlags.Static); - FieldInfo? field = this.GetCached($"field::{isStatic}::{type.FullName}::{name}", () => - { - for (Type? curType = type; curType != null; curType = curType.BaseType) + FieldInfo? field = this.GetCached( + 'f', type, name, isStatic, + fetch: () => { - FieldInfo? fieldInfo = curType.GetField(name, bindingFlags); - if (fieldInfo != null) + for (Type? curType = type; curType != null; curType = curType.BaseType) { - type = curType; - return fieldInfo; + FieldInfo? fieldInfo = curType.GetField(name, bindingFlags); + if (fieldInfo != null) + { + type = curType; + return fieldInfo; + } } - } - return null; - }); + return null; + } + ); return field != null ? new ReflectedField(type, obj, field, isStatic) @@ -178,20 +187,23 @@ namespace StardewModdingAPI.Framework.Reflection private IReflectedProperty? GetPropertyFromHierarchy(Type type, object? obj, string name, BindingFlags bindingFlags) { bool isStatic = bindingFlags.HasFlag(BindingFlags.Static); - PropertyInfo? property = this.GetCached($"property::{isStatic}::{type.FullName}::{name}", () => - { - for (Type? curType = type; curType != null; curType = curType.BaseType) + PropertyInfo? property = this.GetCached( + 'p', type, name, isStatic, + fetch: () => { - PropertyInfo? propertyInfo = curType.GetProperty(name, bindingFlags); - if (propertyInfo != null) + for (Type? curType = type; curType != null; curType = curType.BaseType) { - type = curType; - return propertyInfo; + PropertyInfo? propertyInfo = curType.GetProperty(name, bindingFlags); + if (propertyInfo != null) + { + type = curType; + return propertyInfo; + } } - } - return null; - }); + return null; + } + ); return property != null ? new ReflectedProperty(type, obj, property, isStatic) @@ -206,47 +218,41 @@ namespace StardewModdingAPI.Framework.Reflection private IReflectedMethod? GetMethodFromHierarchy(Type type, object? obj, string name, BindingFlags bindingFlags) { bool isStatic = bindingFlags.HasFlag(BindingFlags.Static); - MethodInfo? method = this.GetCached($"method::{isStatic}::{type.FullName}::{name}", () => - { - for (Type? curType = type; curType != null; curType = curType.BaseType) + MethodInfo? method = this.GetCached( + 'm', type, name, isStatic, + fetch: () => { - MethodInfo? methodInfo = curType.GetMethod(name, bindingFlags); - if (methodInfo != null) + for (Type? curType = type; curType != null; curType = curType.BaseType) { - type = curType; - return methodInfo; + MethodInfo? methodInfo = curType.GetMethod(name, bindingFlags); + if (methodInfo != null) + { + type = curType; + return methodInfo; + } } - } - return null; - }); + return null; + } + ); return method != null - ? new ReflectedMethod(type, obj, method, isStatic: bindingFlags.HasFlag(BindingFlags.Static)) + ? new ReflectedMethod(type, obj, method, isStatic: isStatic) : null; } /// Get a method or field through the cache. /// The expected type. - /// The cache key. + /// A letter representing the member type (like 'm' for method). + /// The type whose members are being reflected. + /// The member name. + /// Whether the member is static. /// Fetches a new value to cache. - private TMemberInfo? GetCached(string key, Func fetch) + private TMemberInfo? GetCached(char memberType, Type type, string memberName, bool isStatic, Func fetch) where TMemberInfo : MemberInfo { - // get from cache - if (this.Cache.Contains(key)) - { - CacheEntry entry = (CacheEntry)this.Cache[key]; - return entry.IsValid - ? (TMemberInfo)entry.MemberInfo - : default; - } - - // fetch & cache new value - TMemberInfo? result = fetch(); - CacheEntry cacheEntry = new(result); - this.Cache.Add(key, cacheEntry, new CacheItemPolicy { SlidingExpiration = this.SlidingCacheExpiry }); - return result; + string key = $"{memberType}{(isStatic ? 's' : 'i')}{type.FullName}:{memberName}"; + return (TMemberInfo?)this.Cache.GetOrSet(key, fetch); } } } diff --git a/src/SMAPI/Framework/SCore.cs b/src/SMAPI/Framework/SCore.cs index 67f78400..c453562f 100644 --- a/src/SMAPI/Framework/SCore.cs +++ b/src/SMAPI/Framework/SCore.cs @@ -1164,6 +1164,8 @@ namespace StardewModdingAPI.Framework protected void OnNewDayAfterFade() { this.EventManager.DayEnding.RaiseEmpty(); + + this.Reflection.NewCacheInterval(); } /// A callback invoked after an asset is fully loaded through a content manager. diff --git a/src/SMAPI/Framework/Utilities/IntervalMemoryCache.cs b/src/SMAPI/Framework/Utilities/IntervalMemoryCache.cs new file mode 100644 index 00000000..d2b69f51 --- /dev/null +++ b/src/SMAPI/Framework/Utilities/IntervalMemoryCache.cs @@ -0,0 +1,57 @@ +using System; +using System.Collections.Generic; + +namespace StardewModdingAPI.Framework.Utilities +{ + /// A memory cache with sliding expiry based on custom intervals, with no background processing. + /// The cache key type. + /// The cache value type. + /// This is optimized for small caches that are reset relatively rarely. Each cache entry is marked as hot (accessed since the interval started) or stale. + /// When a new interval is started, stale entries are cleared and hot entries become stale. + internal class IntervalMemoryCache + where TKey : notnull + { + /********* + ** Fields + *********/ + /// The cached values that were accessed during the current interval. + private Dictionary HotCache = new(); + + /// The cached values that will expire on the next interval. + private Dictionary StaleCache = new(); + + + /********* + ** Public methods + *********/ + /// Get a value from the cache, fetching it first if needed. + /// The unique key for the cached value. + /// Get the latest data if it's not in the cache yet. + public TValue GetOrSet(TKey cacheKey, Func get) + { + // from hot cache + if (this.HotCache.TryGetValue(cacheKey, out TValue? value)) + return value; + + // from stale cache + if (this.StaleCache.TryGetValue(cacheKey, out value)) + { + this.HotCache[cacheKey] = value; + return value; + } + + // new value + value = get(); + this.HotCache[cacheKey] = value; + return value; + } + + /// Start a new cache interval, removing any stale entries. + public void StartNewInterval() + { + this.StaleCache.Clear(); + if (this.HotCache.Count is not 0) + (this.StaleCache, this.HotCache) = (this.HotCache, this.StaleCache); // swap hot cache to stale + } + } +} diff --git a/src/SMAPI/SMAPI.csproj b/src/SMAPI/SMAPI.csproj index 27044679..95249bfd 100644 --- a/src/SMAPI/SMAPI.csproj +++ b/src/SMAPI/SMAPI.csproj @@ -28,7 +28,6 @@ - -- cgit