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 +++++++++++ 4 files changed, 117 insertions(+), 82 deletions(-) delete mode 100644 src/SMAPI/Framework/Reflection/CacheEntry.cs create mode 100644 src/SMAPI/Framework/Utilities/IntervalMemoryCache.cs (limited to 'src/SMAPI/Framework') 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 + } + } +} -- cgit From bf960ce283d794a11885a5fde6f123a4e6827853 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 31 May 2022 21:23:44 -0400 Subject: add backwards compatibility for mods using now-unused dependencies --- src/SMAPI/Framework/IModMetadata.cs | 4 ++ src/SMAPI/Framework/ModLoading/AssemblyLoader.cs | 38 +++++++++++++++++ .../ModLoading/Finders/LegacyAssemblyFinder.cs | 49 ++++++++++++++++++++++ .../ModLoading/InstructionHandleResult.cs | 11 ++++- src/SMAPI/Framework/ModLoading/ModMetadata.cs | 7 ++++ src/SMAPI/Framework/SCore.cs | 25 +++++++++++ 6 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 src/SMAPI/Framework/ModLoading/Finders/LegacyAssemblyFinder.cs (limited to 'src/SMAPI/Framework') diff --git a/src/SMAPI/Framework/IModMetadata.cs b/src/SMAPI/Framework/IModMetadata.cs index 7cee20b9..be25c070 100644 --- a/src/SMAPI/Framework/IModMetadata.cs +++ b/src/SMAPI/Framework/IModMetadata.cs @@ -88,6 +88,10 @@ namespace StardewModdingAPI.Framework /// The warning to set. IModMetadata SetWarning(ModWarning warning); + /// Remove a warning flag for the mod. + /// The warning to remove. + IModMetadata RemoveWarning(ModWarning warning); + /// Set the mod instance. /// The mod instance to set. /// The translations for this mod (if loaded). diff --git a/src/SMAPI/Framework/ModLoading/AssemblyLoader.cs b/src/SMAPI/Framework/ModLoading/AssemblyLoader.cs index fb5ebc01..e5aaa8ee 100644 --- a/src/SMAPI/Framework/ModLoading/AssemblyLoader.cs +++ b/src/SMAPI/Framework/ModLoading/AssemblyLoader.cs @@ -163,6 +163,29 @@ namespace StardewModdingAPI.Framework.ModLoading this.AssemblyDefinitionResolver.Add(assembly.Definition); } + // special case: clear legacy-DLL warnings if the mod bundles a copy + if (mod.Warnings.HasFlag(ModWarning.DetectedLegacyCachingDll)) + { + if (File.Exists(Path.Combine(mod.DirectoryPath, "System.Runtime.Caching.dll"))) + mod.RemoveWarning(ModWarning.DetectedLegacyCachingDll); + else + { + // remove duplicate warnings (System.Runtime.Caching.dll references these) + mod.RemoveWarning(ModWarning.DetectedLegacyConfigurationDll); + mod.RemoveWarning(ModWarning.DetectedLegacyPermissionsDll); + } + } + if (mod.Warnings.HasFlag(ModWarning.DetectedLegacyConfigurationDll)) + { + if (File.Exists(Path.Combine(mod.DirectoryPath, "System.Configuration.ConfigurationManager.dll"))) + mod.RemoveWarning(ModWarning.DetectedLegacyConfigurationDll); + } + if (mod.Warnings.HasFlag(ModWarning.DetectedLegacyPermissionsDll)) + { + if (File.Exists(Path.Combine(mod.DirectoryPath, "System.Security.Permissions.dll"))) + mod.RemoveWarning(ModWarning.DetectedLegacyPermissionsDll); + } + // throw if incompatibilities detected if (!assumeCompatible && mod.Warnings.HasFlag(ModWarning.BrokenCodeLoaded)) throw new IncompatibleInstructionException(); @@ -429,6 +452,21 @@ namespace StardewModdingAPI.Framework.ModLoading mod.SetWarning(ModWarning.AccessesShell); break; + case InstructionHandleResult.DetectedLegacyCachingDll: + template = $"{logPrefix}Detected reference to System.Runtime.Caching.dll, which will be removed in SMAPI 4.0.0."; + mod.SetWarning(ModWarning.DetectedLegacyCachingDll); + break; + + case InstructionHandleResult.DetectedLegacyConfigurationDll: + template = $"{logPrefix}Detected reference to System.Configuration.ConfigurationManager.dll, which will be removed in SMAPI 4.0.0."; + mod.SetWarning(ModWarning.DetectedLegacyConfigurationDll); + break; + + case InstructionHandleResult.DetectedLegacyPermissionsDll: + template = $"{logPrefix}Detected reference to System.Security.Permissions.dll, which will be removed in SMAPI 4.0.0."; + mod.SetWarning(ModWarning.DetectedLegacyPermissionsDll); + break; + case InstructionHandleResult.None: break; diff --git a/src/SMAPI/Framework/ModLoading/Finders/LegacyAssemblyFinder.cs b/src/SMAPI/Framework/ModLoading/Finders/LegacyAssemblyFinder.cs new file mode 100644 index 00000000..d3437b05 --- /dev/null +++ b/src/SMAPI/Framework/ModLoading/Finders/LegacyAssemblyFinder.cs @@ -0,0 +1,49 @@ +using Mono.Cecil; +using StardewModdingAPI.Framework.ModLoading.Framework; + +namespace StardewModdingAPI.Framework.ModLoading.Finders +{ + /// Detects assembly references which will break in SMAPI 4.0.0. + internal class LegacyAssemblyFinder : BaseInstructionHandler + { + /********* + ** Public methods + *********/ + /// Construct an instance. + public LegacyAssemblyFinder() + : base(defaultPhrase: "legacy assembly references") { } + + + /// + public override bool Handle(ModuleDefinition module) + { + foreach (AssemblyNameReference assembly in module.AssemblyReferences) + { + InstructionHandleResult flag = this.GetFlag(assembly); + if (flag is InstructionHandleResult.None) + continue; + + this.MarkFlag(flag); + } + + return false; + } + + + /********* + ** Private methods + *********/ + /// Get the instruction handle flag for the given assembly reference, if any. + /// The assembly reference. + private InstructionHandleResult GetFlag(AssemblyNameReference assemblyRef) + { + return assemblyRef.Name switch + { + "System.Configuration.ConfigurationManager" => InstructionHandleResult.DetectedLegacyConfigurationDll, + "System.Runtime.Caching" => InstructionHandleResult.DetectedLegacyCachingDll, + "System.Security.Permission" => InstructionHandleResult.DetectedLegacyPermissionsDll, + _ => InstructionHandleResult.None + }; + } + } +} diff --git a/src/SMAPI/Framework/ModLoading/InstructionHandleResult.cs b/src/SMAPI/Framework/ModLoading/InstructionHandleResult.cs index e3f108cb..476c30d0 100644 --- a/src/SMAPI/Framework/ModLoading/InstructionHandleResult.cs +++ b/src/SMAPI/Framework/ModLoading/InstructionHandleResult.cs @@ -30,6 +30,15 @@ namespace StardewModdingAPI.Framework.ModLoading DetectedFilesystemAccess, /// The instruction accesses the OS shell or processes directly. - DetectedShellAccess + DetectedShellAccess, + + /// The module references the legacy System.Configuration.ConfigurationManager assembly and doesn't include a copy in the mod folder, so it'll break in SMAPI 4.0.0. + DetectedLegacyConfigurationDll, + + /// The module references the legacy System.Runtime.Caching assembly and doesn't include a copy in the mod folder, so it'll break in SMAPI 4.0.0. + DetectedLegacyCachingDll, + + /// The module references the legacy System.Security.Permissions assembly and doesn't include a copy in the mod folder, so it'll break in SMAPI 4.0.0. + DetectedLegacyPermissionsDll } } diff --git a/src/SMAPI/Framework/ModLoading/ModMetadata.cs b/src/SMAPI/Framework/ModLoading/ModMetadata.cs index fe54634b..aa4d2d8c 100644 --- a/src/SMAPI/Framework/ModLoading/ModMetadata.cs +++ b/src/SMAPI/Framework/ModLoading/ModMetadata.cs @@ -138,6 +138,13 @@ namespace StardewModdingAPI.Framework.ModLoading return this; } + /// + public IModMetadata RemoveWarning(ModWarning warning) + { + this.ActualWarnings &= ~warning; + return this; + } + /// public IModMetadata SetMod(IMod mod, TranslationHelper translations) { diff --git a/src/SMAPI/Framework/SCore.cs b/src/SMAPI/Framework/SCore.cs index c453562f..731731d4 100644 --- a/src/SMAPI/Framework/SCore.cs +++ b/src/SMAPI/Framework/SCore.cs @@ -1679,6 +1679,31 @@ namespace StardewModdingAPI.Framework } #pragma warning restore CS0612, CS0618 + // log deprecation warnings + if (metadata.HasWarnings(ModWarning.DetectedLegacyCachingDll, ModWarning.DetectedLegacyConfigurationDll, ModWarning.DetectedLegacyPermissionsDll)) + { + string?[] referenced = + new[] + { + metadata.Warnings.HasFlag(ModWarning.DetectedLegacyConfigurationDll) ? "System.Configuration.ConfigurationManager" : null, + metadata.Warnings.HasFlag(ModWarning.DetectedLegacyCachingDll) ? "System.Runtime.Caching" : null, + metadata.Warnings.HasFlag(ModWarning.DetectedLegacyPermissionsDll) ? "System.Security.Permissions" : null + } + .Where(p => p is not null) + .ToArray(); + + foreach (string? name in referenced) + { + DeprecationManager.Warn( + metadata, + $"using {name} without bundling it", + "3.14.7", + DeprecationLevel.Notice, + logStackTrace: false + ); + } + } + // call entry method Context.HeuristicModsRunningCode.Push(metadata); try -- cgit