fix(site-runtime): compile cache eviction counter+warning, configurable bound, never cache faulted compiles (review 2026-10-21 01/SR-M2)

This commit is contained in:
Joseph Doherty
2026-10-22 08:28:22 -04:00
parent b0a08bb63b
commit 96cab3f3d7
5 changed files with 144 additions and 8 deletions
@@ -38,8 +38,12 @@ public class ScriptCompilationService
/// <summary>Initializes a new instance of the ScriptCompilationService class.</summary>
/// <param name="logger">Logger instance.</param>
public ScriptCompilationService(ILogger<ScriptCompilationService> logger)
: this(logger, new SiteScriptCompileCache())
/// <param name="options">Site runtime options; supplies the compile-cache bound. Optional (default bound when null).</param>
public ScriptCompilationService(
ILogger<ScriptCompilationService> logger,
Microsoft.Extensions.Options.IOptions<SiteRuntimeOptions>? options = null)
: this(logger, new SiteScriptCompileCache(
options?.Value.ScriptCompileCacheMaxEntries ?? SiteScriptCompileCache.MaxEntries, logger))
{
}
@@ -228,7 +232,9 @@ public class ScriptCompilationService
catch (Exception ex)
{
_logger.LogError(ex, "Unexpected error compiling script {Script}", name);
return ScriptCompilationResult.Failed([$"Compilation exception: {ex.Message}"]);
// Faulted, not Failed: an unexpected exception says nothing about the code, so the
// cache must not memoise it (a transient fault would otherwise stick until eviction/restart).
return ScriptCompilationResult.Faulted([$"Compilation exception: {ex.Message}"]);
}
}
}
@@ -245,8 +251,15 @@ public class ScriptCompilationResult
/// <summary>List of error messages, empty if compilation succeeded.</summary>
public IReadOnlyList<string> Errors { get; }
private ScriptCompilationResult(bool success, Script<object?>? script, IReadOnlyList<string> errors)
/// <summary>
/// False for a result produced by an unexpected exception during compilation; such a result
/// is returned to the caller but never memoised by <see cref="SiteScriptCompileCache"/>.
/// </summary>
internal bool IsCacheable { get; }
private ScriptCompilationResult(bool success, Script<object?>? script, IReadOnlyList<string> errors, bool cacheable = true)
{
IsCacheable = cacheable;
IsSuccess = success;
CompiledScript = script;
Errors = errors;
@@ -263,6 +276,12 @@ public class ScriptCompilationResult
/// <returns>A <see cref="ScriptCompilationResult"/> with <see cref="IsSuccess"/> set to <c>false</c> and the provided error messages.</returns>
public static ScriptCompilationResult Failed(IReadOnlyList<string> errors) =>
new(false, null, errors);
/// <summary>Creates a failure caused by an unexpected compiler fault; not cacheable.</summary>
/// <param name="errors">List of error messages.</param>
/// <returns>A non-cacheable failed result.</returns>
internal static ScriptCompilationResult Faulted(IReadOnlyList<string> errors) =>
new(false, null, errors, cacheable: false);
}
/// <summary>
@@ -1,6 +1,7 @@
using System.Collections.Concurrent;
using System.Security.Cryptography;
using System.Text;
using Microsoft.Extensions.Logging;
namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
@@ -56,9 +57,15 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
/// </summary>
internal sealed class SiteScriptCompileCache
{
/// <summary>Upper bound on cached entries; crossing it evicts the oldest batch.</summary>
/// <summary>
/// Default upper bound on cached entries; crossing it evicts the oldest batch. The live bound
/// is <see cref="Capacity"/>, configurable via <c>SiteRuntimeOptions.ScriptCompileCacheMaxEntries</c>.
/// </summary>
internal const int MaxEntries = 1024;
/// <summary>Minimum interval between eviction Warning logs (a churning cache evicts constantly).</summary>
private static readonly TimeSpan EvictionWarnInterval = TimeSpan.FromMinutes(1);
/// <summary>
/// Fraction of the cache evicted in one sweep (⅛ = 128 entries at
/// <see cref="MaxEntries"/>). Batching amortises the O(n) scan across many inserts, so
@@ -69,10 +76,38 @@ internal sealed class SiteScriptCompileCache
private readonly ConcurrentDictionary<string, CacheEntry> _cache = new();
private readonly object _evictionLock = new();
private long _hits;
private long _evictions;
private long _evictionsAtLastWarn;
private long _lastWarnTicks;
private readonly int _capacity;
private readonly ILogger? _logger;
/// <summary>Monotonic access sequence; the recency stamp written onto entries.</summary>
private long _accessSequence;
/// <summary>Creates a cache bounded at <see cref="MaxEntries"/>.</summary>
public SiteScriptCompileCache() : this(MaxEntries, null) { }
/// <summary>Creates a cache with an explicit bound and an optional logger for eviction warnings.</summary>
/// <param name="capacity">Maximum cached entries (clamped to at least 8).</param>
/// <param name="logger">Receives the rate-limited eviction Warning; may be null.</param>
public SiteScriptCompileCache(int capacity, ILogger? logger)
{
_capacity = Math.Max(8, capacity);
_logger = logger;
}
/// <summary>The live entry bound for this cache instance.</summary>
public int Capacity => _capacity;
/// <summary>
/// Total entries evicted since construction. Eviction discards only the managed
/// <c>Script&lt;object?&gt;</c>; the assembly Roslyn loaded for it stays in the default load context
/// for the process lifetime, and a later miss for the same body loads a second copy. A rising value
/// therefore means monotonic working-set growth — raise <c>ScriptCompileCacheMaxEntries</c>.
/// </summary>
public long Evictions => Interlocked.Read(ref _evictions);
/// <summary>Number of cache hits observed by this cache instance.</summary>
public long Hits => Interlocked.Read(ref _hits);
@@ -102,7 +137,12 @@ internal sealed class SiteScriptCompileCache
var result = factory();
if (_cache.Count >= MaxEntries)
// A faulted compile (unexpected exception, e.g. a transient OOM) is NOT a verdict on the
// code, so it must not become sticky: only success and diagnostic failures are cached.
if (!result.IsCacheable)
return result;
if (_cache.Count >= _capacity)
EvictOldestBatch();
_cache[key] = new CacheEntry(result) { LastAccess = NextStamp() };
@@ -123,9 +163,9 @@ internal sealed class SiteScriptCompileCache
{
lock (_evictionLock)
{
if (_cache.Count < MaxEntries) return; // another thread already swept
if (_cache.Count < _capacity) return; // another thread already swept
var batch = Math.Max(1, MaxEntries / EvictionBatchDivisor);
var batch = Math.Max(1, _capacity / EvictionBatchDivisor);
// ConcurrentDictionary.ToArray() takes an ATOMIC snapshot. Enumerating (or
// LINQ-ing) the dictionary directly does not: LINQ's ToArray picks the
@@ -141,9 +181,30 @@ internal sealed class SiteScriptCompileCache
foreach (var victim in victims)
_cache.TryRemove(victim, out _);
Interlocked.Add(ref _evictions, victims.Length);
WarnEvicted();
}
}
/// <summary>Rate-limited eviction Warning (first sweep, then at most one summary per interval). Caller holds the eviction lock.</summary>
private void WarnEvicted()
{
if (_logger is null) return;
var now = Environment.TickCount64;
if (_lastWarnTicks != 0 && now - _lastWarnTicks < (long)EvictionWarnInterval.TotalMilliseconds) return;
var total = Interlocked.Read(ref _evictions);
var sinceLast = total - _evictionsAtLastWarn;
_evictionsAtLastWarn = total;
_lastWarnTicks = now == 0 ? 1 : now;
_logger.LogWarning(
"Script compile cache is at its {Capacity}-entry bound and evicted {Evicted} entries ({Total} total). " +
"Eviction does not unload the Roslyn assembly: each recompile of an evicted body loads another permanent " +
"copy. Raise ScadaBridge:SiteRuntime:ScriptCompileCacheMaxEntries if this repeats.",
_capacity, sinceLast, total);
}
/// <summary>
/// A cached compile result plus its recency stamp. A class (not a struct) so
/// <see cref="Touch"/> can update recency in place without replacing the dictionary
@@ -62,6 +62,14 @@ public class SiteRuntimeOptions
/// </summary>
public int ScriptExecutionMaxThreadCount { get; set; } = 32;
/// <summary>
/// Maximum entries in the site script compile cache (scripts + trigger expressions, keyed on
/// body hash). Crossing it evicts the oldest eighth. Size it above the site's count of
/// DISTINCT script/trigger bodies: eviction never unloads the Roslyn assembly, so a body that
/// is evicted and later recompiled loads a second permanent copy. Default: 1024.
/// </summary>
public int ScriptCompileCacheMaxEntries { get; set; } = 1024;
/// <summary>
/// Maximum number of trigger-expression evaluations allowed to run
/// concurrently across the whole process. Trigger expressions are non-blocking by
@@ -47,6 +47,10 @@ public sealed class SiteRuntimeOptionsValidator : OptionsValidatorBase<SiteRunti
"CEILING for the instance-scaled script-execution pool and a ceiling below the floor would " +
"silently shrink the configured pool.");
builder.RequireThat(options.ScriptCompileCacheMaxEntries >= 8,
$"ScadaBridge:SiteRuntime:ScriptCompileCacheMaxEntries must be at least 8 " +
$"(was {options.ScriptCompileCacheMaxEntries}); it bounds the compiled-script cache.");
builder.RequireThat(options.TriggerEvalMaxConcurrency > 0,
$"ScadaBridge:SiteRuntime:TriggerEvalMaxConcurrency must be greater than 0 " +
$"(was {options.TriggerEvalMaxConcurrency}); it gates concurrent trigger-expression evaluations " +
@@ -209,4 +209,48 @@ public class SiteScriptCompileCacheTests
$"cache grew past its bound: {_cache.Count} vs max {SiteScriptCompileCache.MaxEntries} " +
$"(+{tasks} racing-insert slack) after {tasks * perTask} distinct inserts");
}
/// <summary>A faulted compile (unexpected exception) is returned but never cached: the next call recompiles (review 2026-10-21 01/SR-M2 follow-up).</summary>
[Fact]
public void GetOrAdd_FaultedResult_IsNotCached_NextCallRecompiles()
{
var calls = 0;
ScriptCompilationResult Factory() =>
++calls == 1 ? ScriptCompilationResult.Faulted(["Compilation exception: boom"]) : Ok();
var first = _cache.GetOrAdd("return 7;", typeof(ScriptGlobals), Factory);
Assert.False(first.IsSuccess);
Assert.Equal(0, _cache.Count);
var second = _cache.GetOrAdd("return 7;", typeof(ScriptGlobals), Factory);
Assert.True(second.IsSuccess);
Assert.Equal(2, calls);
Assert.Equal(1, _cache.Count);
}
/// <summary>A diagnostic failure IS a verdict on the code and stays cached.</summary>
[Fact]
public void GetOrAdd_DiagnosticFailure_IsStillCached()
{
var calls = 0;
ScriptCompilationResult Factory() { calls++; return ScriptCompilationResult.Failed(["CS1002"]); }
_cache.GetOrAdd("bad", typeof(ScriptGlobals), Factory);
_cache.GetOrAdd("bad", typeof(ScriptGlobals), Factory);
Assert.Equal(1, calls);
}
/// <summary>A configured capacity bounds the cache and eviction is counted (01/SR-M2).</summary>
[Fact]
public void Eviction_HonoursConfiguredCapacity_AndCountsEvictions()
{
var cache = new SiteScriptCompileCache(16, logger: null);
for (var i = 0; i < 40; i++)
cache.GetOrAdd("return " + i + ";", typeof(ScriptGlobals), Ok);
Assert.Equal(16, cache.Capacity);
Assert.True(cache.Count <= 16);
Assert.True(cache.Evictions > 0);
}
}