merge(agent-c): row 100 — SiteScriptCompileCache becomes an instance-scoped sealed class owned by the ScriptCompilationService DI singleton (zero production Clear() callers existed, so Clear() is deleted outright); row-91 serialized collection relaxed and delta assertions restored to exact counts; new registration test pins the load-bearing singleton lifetime

This commit is contained in:
Joseph Doherty
2026-08-17 06:20:17 -04:00
10 changed files with 263 additions and 181 deletions
+3 -1
View File
@@ -128,7 +128,9 @@ flowchart TD
> **The Roslyn `ScriptOptions` are process-static, and must stay that way**: the reference set backing every site-side compile is built once per process, never per compile. `ScriptOptions.WithReferences(Assembly[])` resolves each assembly through `MetadataReference.CreateFromFile`, which does **not** cache — each call mints a fresh `MetadataReference` owning an `AssemblyMetadata` → `PEReader` → `NativeHeapMemoryBlock`, an unmanaged copy of the assembly metadata that nothing disposes. Building the options per compile leaks native memory permanently: no GC reclaims it, and it is invisible to gcdump and to the managed allocation counters, so the node's working set grows without the GC heap growing. This was a live defect (a Site node at 2,885 MB working set with 150 MB of live GC heap, ~2,469 MB of it on the default process heap across ~6,700 undisposed `AssemblyMetadata` instances). Note this is orthogonal to the compile cache above — the cache dedupes *identical* script bodies, so it bounds nothing when the bodies differ. (It no longer clears wholesale on overflow: WP3.1 replaced that latency cliff with approximate-LRU batch eviction, see **Compile cache eviction** below.) Pinned by a reference-equality regression test rather than by watching memory. > **The Roslyn `ScriptOptions` are process-static, and must stay that way**: the reference set backing every site-side compile is built once per process, never per compile. `ScriptOptions.WithReferences(Assembly[])` resolves each assembly through `MetadataReference.CreateFromFile`, which does **not** cache — each call mints a fresh `MetadataReference` owning an `AssemblyMetadata` → `PEReader` → `NativeHeapMemoryBlock`, an unmanaged copy of the assembly metadata that nothing disposes. Building the options per compile leaks native memory permanently: no GC reclaims it, and it is invisible to gcdump and to the managed allocation counters, so the node's working set grows without the GC heap growing. This was a live defect (a Site node at 2,885 MB working set with 150 MB of live GC heap, ~2,469 MB of it on the default process heap across ~6,700 undisposed `AssemblyMetadata` instances). Note this is orthogonal to the compile cache above — the cache dedupes *identical* script bodies, so it bounds nothing when the bodies differ. (It no longer clears wholesale on overflow: WP3.1 replaced that latency cliff with approximate-LRU batch eviction, see **Compile cache eviction** below.) Pinned by a reference-equality regression test rather than by watching memory.
> **Compile cache eviction is approximate LRU, not a wholesale clear (WP3.1)**: `SiteScriptCompileCache` is bounded at 1024 entries — deliberately smaller than the TemplateEngine verdict cache's 4096, because entries pin compiled assemblies rather than verdict strings. It used to `Clear()` **everything** on overflow. Instance scripts *and* trigger expressions share this cache, so on a site large enough to cross the bound every overflow discarded up to 1023 live compiled scripts and the next deploy or Instance-Actor start paid a full recompile storm **on actor threads**. Overflow now evicts only the oldest ⅛ (128 entries) by last-access stamp: one 1024-element scan instead of 1023 future recompiles, and hot entries survive. Recency is an `Interlocked` access sequence rather than a clock — deterministic for tests and immune to clock steps; hits update it lock-free, and only the (rare) eviction sweep takes a lock, double-checked so concurrent inserts do not stampede it. The sweep snapshots through `ConcurrentDictionary.ToArray()` and **not** LINQ over the dictionary: LINQ's `ToArray` picks the `ICollection.CopyTo` fast path, which throws when a concurrent insert lands between its Count read and its copy, and inserts are deliberately lock-free. > **The compile cache is owned by the `ScriptCompilationService` singleton, not static (register row 100)**: `SiteScriptCompileCache` is an ordinary instance held by `ScriptCompilationService`, and "one Roslyn compile per script body per process" (N4) is exactly the statement that `AddSiteRuntime` registers that service as a **DI singleton** — the deploy gate (via `DeployCompileValidator`), every Instance Actor's `PreStart`, and `SharedScriptLibrary` all reach the one cache through the one service the Deployment Manager threads to them. The singleton lifetime is therefore load-bearing rather than incidental, and is pinned by a registration test (`SiteRuntimeServiceRegistrationTests`); downgrading it to Scoped/Transient would hand every consumer an empty cache and silently reintroduce WP3.1's recompile storms with no behavioural test failing. The class was previously `static`, which made its entry table and its `Hits`/`Count` counters one process-global blob every test class in the assembly wrote into; the `Clear()` method that existed only so tests could reset that blob is **removed** — it never had a production caller, and constructing a cache is the honest reset.
> **Compile cache eviction is approximate LRU, not a wholesale clear (WP3.1)**: `SiteScriptCompileCache` is bounded at 1024 entries — deliberately smaller than the TemplateEngine verdict cache's 4096, because entries pin compiled assemblies rather than verdict strings. It used to clear **everything** on overflow. Instance scripts *and* trigger expressions share this cache, so on a site large enough to cross the bound every overflow discarded up to 1023 live compiled scripts and the next deploy or Instance-Actor start paid a full recompile storm **on actor threads**. Overflow now evicts only the oldest ⅛ (128 entries) by last-access stamp: one 1024-element scan instead of 1023 future recompiles, and hot entries survive. Recency is an `Interlocked` access sequence rather than a clock — deterministic for tests and immune to clock steps; hits update it lock-free, and only the (rare) eviction sweep takes a lock, double-checked so concurrent inserts do not stampede it. The sweep snapshots through `ConcurrentDictionary.ToArray()` and **not** LINQ over the dictionary: LINQ's `ToArray` picks the `ICollection.CopyTo` fast path, which throws when a concurrent insert lands between its Count read and its copy, and inserts are deliberately lock-free.
> **Static options were only half the fix — the compile must also use the shared caching metadata resolver**: the static `ScriptOptions` carry only the *direct* API-surface references (5 assemblies here), and each `script.Compile()` still binds their **transitive closure** (~105 assemblies on a site node). Every transitively-referenced assembly is resolved through the options' `MetadataReferenceResolver`, and Roslyn's default one has no cross-compilation cache — it calls `MetadataReference.CreateFromFile` afresh on **every compile**, minting the same undisposed `AssemblyMetadata` → `PEReader` → `NativeHeapMemoryBlock` triple per assembly per compiled script, pinned for the process lifetime by the compile cache. That per-compile *re-resolution* — not the per-compile options construction — was the dominant term (measured live: 21 site scripts holding 6,640 metadata objects, counts bit-identical before and after the static-options fix). `ScriptCompilationService` therefore also attaches the process-wide `CachingScriptMetadataResolver.Instance` (owned by the Script Analysis component, see `Component-ScriptAnalysis.md`) via `ScriptOptions.WithMetadataResolver`, so each distinct assembly is materialized once per process. The decorator returns exactly what the undecorated resolver returned — only object identity is de-duplicated — so compile results and the trust gate are unchanged. > **Static options were only half the fix — the compile must also use the shared caching metadata resolver**: the static `ScriptOptions` carry only the *direct* API-surface references (5 assemblies here), and each `script.Compile()` still binds their **transitive closure** (~105 assemblies on a site node). Every transitively-referenced assembly is resolved through the options' `MetadataReferenceResolver`, and Roslyn's default one has no cross-compilation cache — it calls `MetadataReference.CreateFromFile` afresh on **every compile**, minting the same undisposed `AssemblyMetadata` → `PEReader` → `NativeHeapMemoryBlock` triple per assembly per compiled script, pinned for the process lifetime by the compile cache. That per-compile *re-resolution* — not the per-compile options construction — was the dominant term (measured live: 21 site scripts holding 6,640 metadata objects, counts bit-identical before and after the static-options fix). `ScriptCompilationService` therefore also attaches the process-wide `CachingScriptMetadataResolver.Instance` (owned by the Script Analysis component, see `Component-ScriptAnalysis.md`) via `ScriptOptions.WithMetadataResolver`, so each distinct assembly is materialized once per process. The decorator returns exactly what the undecorated resolver returned — only object identity is de-duplicated — so compile results and the trust gate are unchanged.
@@ -677,7 +677,8 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
{ {
// Verdict discarded: startup does not gate on compile failures (a failing // Verdict discarded: startup does not gate on compile failures (a failing
// script is logged by the Instance Actor and leaves the rest running). // script is logged by the Instance Actor and leaves the rest running).
// This call exists only to populate SiteScriptCompileCache. // This call exists only to populate the compile cache owned by the
// shared ScriptCompilationService (SiteScriptCompileCache).
validator.Validate(configJson); validator.Validate(configJson);
} }
catch catch
@@ -798,8 +799,8 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
} }
/// <summary> /// <summary>
/// Compiles the deployment's scripts off the actor thread purely to populate the /// Compiles the deployment's scripts off the actor thread purely to populate the compile
/// process-wide <see cref="SiteScriptCompileCache"/>, then pipes /// cache owned by the shared <c>ScriptCompilationService</c> singleton, then pipes
/// <see cref="DeployCompileWarmed"/> back so the authoritative gate can run on the actor /// <see cref="DeployCompileWarmed"/> back so the authoritative gate can run on the actor
/// thread against a warm cache. The warm is best-effort: its verdict is discarded and any /// thread against a warm cache. The warm is best-effort: its verdict is discarded and any
/// exception swallowed, because <see cref="RunDeployGateAndProceed"/> re-derives the /// exception swallowed, because <see cref="RunDeployGateAndProceed"/> re-derives the
@@ -910,9 +911,10 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
// already been warmed off-thread (StartDeployCompileWarm), so this call is // already been warmed off-thread (StartDeployCompileWarm), so this call is
// pure cache hits and no longer holds the singleton — while the per-instance // pure cache hits and no longer holds the singleton — while the per-instance
// warm guard keeps this instance's own command ordering intact. Redeploys and // warm guard keeps this instance's own command ordering intact. Redeploys and
// multi-instance deploys of unchanged scripts hit the process-wide compile // multi-instance deploys of unchanged scripts hit the compile cache
// cache (SiteScriptCompileCache) too, and the Instance Actor's PreStart reuses // (SiteScriptCompileCache, owned by the ScriptCompilationService singleton this
// the gate's compile (N4). // actor shares with every Instance Actor) too, and the Instance Actor's PreStart
// reuses the gate's compile (N4).
var errors = _deployCompileValidator.Validate(command.FlattenedConfigurationJson); var errors = _deployCompileValidator.Validate(command.FlattenedConfigurationJson);
if (errors.Count > 0) if (errors.Count > 0)
{ {
@@ -13,12 +13,13 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Deployment;
/// do not compile on the site (design spec: site-side compile failure rejects the /// do not compile on the site (design spec: site-side compile failure rejects the
/// deployment; no partial state is applied). /// deployment; no partial state is applied).
/// ///
/// Because it holds no actor state and only calls the (stateless) /// Because it holds no actor state and only calls <see cref="ScriptCompilationService"/>
/// <see cref="ScriptCompilationService"/>, it is pure and thread-safe; the Deployment /// (whose only state is a thread-safe compile cache), it is pure and thread-safe; the
/// Manager nevertheless invokes it <em>synchronously on the singleton's actor thread</em> /// Deployment Manager nevertheless invokes it <em>synchronously on the singleton's actor
/// as a pure prefix step, preserving mailbox-FIFO deploy ordering (see /// thread</em> as a pure prefix step, preserving mailbox-FIFO deploy ordering (see
/// <see cref="Actors.DeploymentManagerActor.HandleDeploy"/>). Compiles of unchanged /// <see cref="Actors.DeploymentManagerActor.HandleDeploy"/>). Compiles of unchanged
/// script bodies are deduped by the process-wide compile cache. /// script bodies are deduped by that cache, which is shared across the whole site because
/// the compilation service is a DI singleton.
/// </summary> /// </summary>
public sealed class DeployCompileValidator public sealed class DeployCompileValidator
{ {
@@ -14,22 +14,48 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
/// execution-path compile of the script against <see cref="ScriptGlobals"/> / /// execution-path compile of the script against <see cref="ScriptGlobals"/> /
/// <see cref="TriggerExpressionGlobals"/>. /// <see cref="TriggerExpressionGlobals"/>.
/// ///
/// Compile results — success AND failure — are memoised process-wide in /// Compile results — success AND failure — are memoised in this service's
/// <see cref="SiteScriptCompileCache"/> (keyed on code hash + globals type), so an /// <see cref="SiteScriptCompileCache"/> (keyed on code hash + globals type), so an
/// unchanged script body compiles once per process lifetime and the deploy gate, /// unchanged script body compiles once per process lifetime and the deploy gate,
/// the Instance Actor's PreStart recompile, and SharedScriptLibrary all reuse one /// the Instance Actor's PreStart recompile, and SharedScriptLibrary all reuse one
/// compiled <c>Script&lt;object?&gt;</c>. Error text is name-free by construction, so a /// compiled <c>Script&lt;object?&gt;</c>. Error text is name-free by construction, so a
/// cached failure renders correctly for every caller. /// cached failure renders correctly for every caller.
///
/// <para><b>The cache is owned per service instance, not static (register row 100).</b>
/// "Once per process" is a property of this service being registered as a DI
/// <b>singleton</b> (<see cref="ServiceCollectionExtensions.AddSiteRuntime"/>, pinned by
/// <c>SiteRuntimeServiceRegistrationTests</c>) — the whole site reaches the cache through
/// that one instance, which the Deployment Manager threads into its
/// <see cref="Deployment.DeployCompileValidator"/> and into every Instance Actor. Making the
/// cache an instance field rather than static state means a test (or a second host in one
/// process) that constructs its own service gets its own cache instead of mutating everyone
/// else's; that shared-static cross-talk is what row 100 closed.</para>
/// </summary> /// </summary>
public class ScriptCompilationService public class ScriptCompilationService
{ {
private readonly ILogger<ScriptCompilationService> _logger; private readonly ILogger<ScriptCompilationService> _logger;
private readonly SiteScriptCompileCache _compileCache;
/// <summary>Initializes a new instance of the ScriptCompilationService class.</summary> /// <summary>Initializes a new instance of the ScriptCompilationService class.</summary>
/// <param name="logger">Logger instance.</param> /// <param name="logger">Logger instance.</param>
public ScriptCompilationService(ILogger<ScriptCompilationService> logger) public ScriptCompilationService(ILogger<ScriptCompilationService> logger)
: this(logger, new SiteScriptCompileCache())
{
}
/// <summary>
/// Test seam: constructs the service over a caller-supplied compile cache, so a test can
/// observe <see cref="SiteScriptCompileCache.Hits"/> / <see cref="SiteScriptCompileCache.Count"/>
/// for exactly the compiles it caused. Production always uses the public constructor.
/// </summary>
/// <param name="logger">Logger instance.</param>
/// <param name="compileCache">The compile cache this service memoises into.</param>
internal ScriptCompilationService(
ILogger<ScriptCompilationService> logger,
SiteScriptCompileCache compileCache)
{ {
_logger = logger; _logger = logger;
_compileCache = compileCache;
} }
/// <summary> /// <summary>
@@ -136,16 +162,16 @@ public class ScriptCompilationService
/// <summary> /// <summary>
/// Shared compilation funnel for every site-side compile (the deploy gate, the /// Shared compilation funnel for every site-side compile (the deploy gate, the
/// Instance Actor's PreStart recompile, and SharedScriptLibrary). Results — success /// Instance Actor's PreStart recompile, and SharedScriptLibrary). Results — success
/// AND failure — are memoised process-wide in <see cref="SiteScriptCompileCache"/>, /// AND failure — are memoised in this service's <see cref="SiteScriptCompileCache"/>,
/// keyed on code hash + globals type: an unchanged script body compiles once per /// keyed on code hash + globals type: an unchanged script body compiles once per
/// process lifetime and the compiled <c>Script&lt;object?&gt;</c> is shared across all /// process lifetime (this service is a DI singleton) and the compiled
/// call sites (N4). Cached error text is name-free by construction (Roslyn diagnostics / /// <c>Script&lt;object?&gt;</c> is shared across all call sites (N4). Cached error text is
/// the compile-exception message never embed the caller's script name), so a cached /// name-free by construction (Roslyn diagnostics / the compile-exception message never
/// failure renders correctly for every caller. /// embed the caller's script name), so a cached failure renders correctly for every caller.
/// </summary> /// </summary>
private ScriptCompilationResult CompileCore(string name, string code, Type globalsType) private ScriptCompilationResult CompileCore(string name, string code, Type globalsType)
{ {
var result = SiteScriptCompileCache.GetOrAdd(code, globalsType, var result = _compileCache.GetOrAdd(code, globalsType,
() => CompileUncached(name, code, globalsType)); () => CompileUncached(name, code, globalsType));
if (!result.IsSuccess) if (!result.IsSuccess)
_logger.LogDebug( _logger.LogDebug(
@@ -5,13 +5,25 @@ using System.Text;
namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
/// <summary> /// <summary>
/// Process-wide cache of site-side script <see cref="ScriptCompilationResult"/>s, keyed on /// Cache of site-side script <see cref="ScriptCompilationResult"/>s, keyed on
/// the globals type plus a SHA-256 hash of the script code. Unlike the TemplateEngine's /// the globals type plus a SHA-256 hash of the script code. Unlike the TemplateEngine's
/// <c>ScriptCompileVerdictCache</c> (which caches central-validation <em>verdicts</em>), this /// <c>ScriptCompileVerdictCache</c> (which caches central-validation <em>verdicts</em>), this
/// caches the full compiled result — the immutable Roslyn <c>Script&lt;object?&gt;</c> itself — /// caches the full compiled result — the immutable Roslyn <c>Script&lt;object?&gt;</c> itself —
/// so the synchronous deploy gate and the <c>InstanceActor.PreStart</c> recompile share one /// so the synchronous deploy gate and the <c>InstanceActor.PreStart</c> recompile share one
/// Roslyn compile per script body per process lifetime (N4). /// Roslyn compile per script body per process lifetime (N4).
/// ///
/// <para><b>Owned by <see cref="ScriptCompilationService"/>, not static (register row 100).</b>
/// This used to be a <c>static</c> class, which made its <c>Hits</c>/<c>Count</c> counters and
/// its entry table one process-global blob that every test class in the assembly shared. The
/// process-wide N4 sharing property is unchanged in production — it now comes from
/// <c>ScriptCompilationService</c> being a DI <b>singleton</b> (<c>ServiceCollectionExtensions</c>,
/// pinned by <c>SiteRuntimeServiceRegistrationTests</c>), so exactly one cache exists per host and
/// the deploy gate, the Instance Actor's PreStart recompile and SharedScriptLibrary all reach it
/// through that one service. What changed is that a test constructing its own
/// <c>ScriptCompilationService</c> now gets its own cache instead of writing into everyone else's.
/// The old <c>Clear()</c> escape hatch is deliberately GONE: it had no production caller — it
/// existed only so tests could reset the global — and a fresh instance is the honest reset.</para>
///
/// <para>Soundness rests on three invariants:</para> /// <para>Soundness rests on three invariants:</para>
/// <list type="number"> /// <list type="number">
/// <item>A compile result is a pure function of (code, globals type, compile-time-static /// <item>A compile result is a pure function of (code, globals type, compile-time-static
@@ -30,7 +42,7 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
/// </para> /// </para>
/// ///
/// <para><b>WP3.1 — approximate LRU replaced the overflow cliff.</b> The cache used to /// <para><b>WP3.1 — approximate LRU replaced the overflow cliff.</b> The cache used to
/// <c>Clear()</c> wholesale on overflow. Instance scripts AND trigger expressions share this /// clear itself wholesale on overflow. Instance scripts AND trigger expressions share this
/// cache, so on a site large enough to cross 1024 entries every overflow discarded up to 1023 /// cache, so on a site large enough to cross 1024 entries every overflow discarded up to 1023
/// live compiled scripts and the next deploy or Instance-Actor start paid a full recompile /// live compiled scripts and the next deploy or Instance-Actor start paid a full recompile
/// storm — on actor threads. Now an insert at the bound evicts only the oldest /// storm — on actor threads. Now an insert at the bound evicts only the oldest
@@ -42,7 +54,7 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
/// only lock is the (rare) eviction sweep, double-checked so concurrent inserts do not /// only lock is the (rare) eviction sweep, double-checked so concurrent inserts do not
/// stampede it.</para> /// stampede it.</para>
/// </summary> /// </summary>
internal static class SiteScriptCompileCache internal sealed class SiteScriptCompileCache
{ {
/// <summary>Upper bound on cached entries; crossing it evicts the oldest batch.</summary> /// <summary>Upper bound on cached entries; crossing it evicts the oldest batch.</summary>
internal const int MaxEntries = 1024; internal const int MaxEntries = 1024;
@@ -54,18 +66,18 @@ internal static class SiteScriptCompileCache
/// </summary> /// </summary>
private const int EvictionBatchDivisor = 8; private const int EvictionBatchDivisor = 8;
private static readonly ConcurrentDictionary<string, CacheEntry> Cache = new(); private readonly ConcurrentDictionary<string, CacheEntry> _cache = new();
private static readonly object EvictionLock = new(); private readonly object _evictionLock = new();
private static long _hits; private long _hits;
/// <summary>Monotonic access sequence; the recency stamp written onto entries.</summary> /// <summary>Monotonic access sequence; the recency stamp written onto entries.</summary>
private static long _accessSequence; private long _accessSequence;
/// <summary>Number of cache hits observed since the last <see cref="Clear"/>.</summary> /// <summary>Number of cache hits observed by this cache instance.</summary>
public static long Hits => Interlocked.Read(ref _hits); public long Hits => Interlocked.Read(ref _hits);
/// <summary>Current number of cached compile results.</summary> /// <summary>Current number of cached compile results.</summary>
public static int Count => Cache.Count; public int Count => _cache.Count;
/// <summary> /// <summary>
/// Returns the cached compile result for <paramref name="code"/> against /// Returns the cached compile result for <paramref name="code"/> against
@@ -77,11 +89,11 @@ internal static class SiteScriptCompileCache
/// <param name="globalsType">The Roslyn globals surface the script compiles against; part of the key so identical source under different globals stays distinct.</param> /// <param name="globalsType">The Roslyn globals surface the script compiles against; part of the key so identical source under different globals stays distinct.</param>
/// <param name="factory">Computes the compile result on a cache miss.</param> /// <param name="factory">Computes the compile result on a cache miss.</param>
/// <returns>The cached or newly computed compile result.</returns> /// <returns>The cached or newly computed compile result.</returns>
public static ScriptCompilationResult GetOrAdd(string code, Type globalsType, Func<ScriptCompilationResult> factory) public ScriptCompilationResult GetOrAdd(string code, Type globalsType, Func<ScriptCompilationResult> factory)
{ {
var key = globalsType.FullName + ":" + Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(code))); var key = globalsType.FullName + ":" + Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(code)));
if (Cache.TryGetValue(key, out var entry)) if (_cache.TryGetValue(key, out var entry))
{ {
Interlocked.Increment(ref _hits); Interlocked.Increment(ref _hits);
Touch(entry); Touch(entry);
@@ -90,35 +102,28 @@ internal static class SiteScriptCompileCache
var result = factory(); var result = factory();
if (Cache.Count >= MaxEntries) if (_cache.Count >= MaxEntries)
EvictOldestBatch(); EvictOldestBatch();
Cache[key] = new CacheEntry(result) { LastAccess = NextStamp() }; _cache[key] = new CacheEntry(result) { LastAccess = NextStamp() };
return result; return result;
} }
/// <summary>Clears all cached compile results and resets the hit counter.</summary>
public static void Clear()
{
Cache.Clear();
Interlocked.Exchange(ref _hits, 0);
}
/// <summary>Refreshes an entry's recency stamp. Lock-free — a lost race only costs accuracy, never correctness.</summary> /// <summary>Refreshes an entry's recency stamp. Lock-free — a lost race only costs accuracy, never correctness.</summary>
private static void Touch(CacheEntry entry) => Volatile.Write(ref entry.LastAccess, NextStamp()); private void Touch(CacheEntry entry) => Volatile.Write(ref entry.LastAccess, NextStamp());
private static long NextStamp() => Interlocked.Increment(ref _accessSequence); private long NextStamp() => Interlocked.Increment(ref _accessSequence);
/// <summary> /// <summary>
/// Evicts the oldest <see cref="MaxEntries"/> / <see cref="EvictionBatchDivisor"/> entries /// Evicts the oldest <see cref="MaxEntries"/> / <see cref="EvictionBatchDivisor"/> entries
/// by recency stamp. Double-checked under <see cref="EvictionLock"/> so several concurrent /// by recency stamp. Double-checked under <see cref="_evictionLock"/> so several concurrent
/// inserts crossing the bound together perform ONE sweep rather than one each. /// inserts crossing the bound together perform ONE sweep rather than one each.
/// </summary> /// </summary>
private static void EvictOldestBatch() private void EvictOldestBatch()
{ {
lock (EvictionLock) lock (_evictionLock)
{ {
if (Cache.Count < MaxEntries) return; // another thread already swept if (_cache.Count < MaxEntries) return; // another thread already swept
var batch = Math.Max(1, MaxEntries / EvictionBatchDivisor); var batch = Math.Max(1, MaxEntries / EvictionBatchDivisor);
@@ -128,14 +133,14 @@ internal static class SiteScriptCompileCache
// copies, and throws ArgumentException when a concurrent insert lands between // copies, and throws ArgumentException when a concurrent insert lands between
// the two. The eviction lock only excludes other EVICTORS — inserts run // the two. The eviction lock only excludes other EVICTORS — inserts run
// lock-free by design — so the snapshot must be the thread-safe one. // lock-free by design — so the snapshot must be the thread-safe one.
var victims = Cache.ToArray() var victims = _cache.ToArray()
.OrderBy(kvp => Volatile.Read(ref kvp.Value.LastAccess)) .OrderBy(kvp => Volatile.Read(ref kvp.Value.LastAccess))
.Take(batch) .Take(batch)
.Select(kvp => kvp.Key) .Select(kvp => kvp.Key)
.ToArray(); .ToArray();
foreach (var victim in victims) foreach (var victim in victims)
Cache.TryRemove(victim, out _); _cache.TryRemove(victim, out _);
} }
} }
@@ -32,7 +32,14 @@ public static class ServiceCollectionExtensions
services.AddHostedService<SiteStorageInitializer>(); services.AddHostedService<SiteStorageInitializer>();
// Script compilation service // Script compilation service.
//
// SINGLETON IS LOAD-BEARING (register row 100): this service owns the
// SiteScriptCompileCache, so "one Roslyn compile per script body per process"
// (N4) is exactly the statement that there is one of these per host. Downgrading
// it to Scoped/Transient would silently give every consumer its own cache and
// reintroduce the recompile storms WP3.1 removed — no test would fail on
// behaviour, only the registration pin in SiteRuntimeServiceRegistrationTests.
services.AddSingleton<ScriptCompilationService>(); services.AddSingleton<ScriptCompilationService>();
// Shared script library // Shared script library
@@ -29,13 +29,17 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors;
/// answer its deployer instead of leaving it to Ask-timeout, and commands for DIFFERENT /// answer its deployer instead of leaving it to Ask-timeout, and commands for DIFFERENT
/// instances must not block each other.</para> /// instances must not block each other.</para>
/// ///
/// <para>Shares the <c>SiteScriptCompileCache</c> collection because the batch pre-warm test /// <para>The batch pre-warm test asserts on the compile cache's hit counter. As of register
/// asserts on that process-wide cache's hit counter.</para> /// row 100 that cache is owned by the <c>ScriptCompilationService</c> instance this fixture
/// builds — the one it threads into the Deployment Manager and, through it, into every Instance
/// Actor — so the counter is private to the test. The old
/// <c>[Collection("SiteScriptCompileCache")]</c> serialisation against the two
/// <c>Scripts/</c> test classes is therefore gone: nothing here is shared any more.</para>
/// </summary> /// </summary>
[Collection("SiteScriptCompileCache")]
public class DeploymentWarmThenGateTests : TestKit, IDisposable public class DeploymentWarmThenGateTests : TestKit, IDisposable
{ {
private readonly SiteStorageService _storage; private readonly SiteStorageService _storage;
private readonly SiteScriptCompileCache _compileCache;
private readonly ScriptCompilationService _compilationService; private readonly ScriptCompilationService _compilationService;
private readonly SharedScriptLibrary _sharedScriptLibrary; private readonly SharedScriptLibrary _sharedScriptLibrary;
private readonly TestLocalDb _localDb; private readonly TestLocalDb _localDb;
@@ -45,8 +49,9 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
_localDb = TestLocalDb.CreateTemp("dm-warm-gate-test"); _localDb = TestLocalDb.CreateTemp("dm-warm-gate-test");
_storage = new SiteStorageService(_localDb.Db, NullLogger<SiteStorageService>.Instance); _storage = new SiteStorageService(_localDb.Db, NullLogger<SiteStorageService>.Instance);
_storage.InitializeAsync().GetAwaiter().GetResult(); _storage.InitializeAsync().GetAwaiter().GetResult();
_compileCache = new SiteScriptCompileCache();
_compilationService = new ScriptCompilationService( _compilationService = new ScriptCompilationService(
NullLogger<ScriptCompilationService>.Instance); NullLogger<ScriptCompilationService>.Instance, _compileCache);
_sharedScriptLibrary = new SharedScriptLibrary( _sharedScriptLibrary = new SharedScriptLibrary(
_compilationService, NullLogger<SharedScriptLibrary>.Instance); _compilationService, NullLogger<SharedScriptLibrary>.Instance);
} }
@@ -247,21 +252,23 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
await _storage.StoreDeployedConfigAsync( await _storage.StoreDeployedConfigAsync(
"BatchTwo", ConfigJson("BatchTwo", sharedCode), "d2", "h2", true); "BatchTwo", ConfigJson("BatchTwo", sharedCode), "d2", "h2", true);
SiteScriptCompileCache.Clear(); // No Clear() and no baseline subtraction: this cache is the fixture's own and nothing
// Baseline rather than Assert.Equal(0, Hits): Hits is a process-global counter and test // has compiled into it yet (register rows 91/100 — the counter used to be a
// classes outside this collection compile scripts in parallel, so a foreign hit can land // process-global that other test classes bumped in parallel, forcing a delta-over-
// between the Clear() and the read (deferred-work register row 91). Everything below is // baseline reading). Only the Deployment Manager below writes to it.
// asserted as a delta over this baseline. Assert.Equal(0, _compileCache.Hits);
var hitsBefore = SiteScriptCompileCache.Hits;
CreateDeploymentManager(); CreateDeploymentManager();
AwaitAssert(() => AwaitAssert(() =>
{ {
// One compile, then repeated hits: the second pre-warm plus both PreStarts. // One compile, then repeated hits: the second pre-warm plus both PreStarts.
Assert.True(SiteScriptCompileCache.Hits - hitsBefore >= 3, Assert.True(_compileCache.Hits >= 3,
$"expected the pre-warmed body to be served from cache, saw " + $"expected the pre-warmed body to be served from cache, saw {_compileCache.Hits} hits");
$"{SiteScriptCompileCache.Hits - hitsBefore} hits over baseline {hitsBefore}");
}, TimeSpan.FromSeconds(20)); }, TimeSpan.FromSeconds(20));
// …and exactly one entry for the one shared body: the hits above were the SAME compiled
// script being reused, not several bodies each compiled once.
Assert.Equal(1, _compileCache.Count);
} }
} }
@@ -15,15 +15,20 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Scripts;
/// <c>System.Diagnostics.Process</c> is blocked under System.Diagnostics — /// <c>System.Diagnostics.Process</c> is blocked under System.Diagnostics —
/// <c>Stopwatch</c> stays allowed. The real execution-path compile against /// <c>Stopwatch</c> stays allowed. The real execution-path compile against
/// <c>ScriptGlobals</c> / <c>TriggerExpressionGlobals</c> is unchanged. /// <c>ScriptGlobals</c> / <c>TriggerExpressionGlobals</c> is unchanged.
///
/// <para><b>Register row 100:</b> the compile cache is owned by the service instance, not
/// static, so this class's service — and therefore its cache — is private to each test. The
/// <c>[Collection("SiteScriptCompileCache")]</c> serialisation and the <c>Clear()</c> calls that
/// used to open the cache-sensitive tests are gone; <c>Hits</c> is now exact.</para>
/// </summary> /// </summary>
[Collection("SiteScriptCompileCache")]
public class ScriptCompilationServiceTests public class ScriptCompilationServiceTests
{ {
private readonly SiteScriptCompileCache _cache = new();
private readonly ScriptCompilationService _service; private readonly ScriptCompilationService _service;
public ScriptCompilationServiceTests() public ScriptCompilationServiceTests()
{ {
_service = new ScriptCompilationService(NullLogger<ScriptCompilationService>.Instance); _service = new ScriptCompilationService(NullLogger<ScriptCompilationService>.Instance, _cache);
} }
[Fact] [Fact]
@@ -38,28 +43,27 @@ public class ScriptCompilationServiceTests
[Fact] [Fact]
public void Compile_SameCodeTwice_SharesOneRoslynCompile() public void Compile_SameCodeTwice_SharesOneRoslynCompile()
{ {
SiteScriptCompileCache.Clear();
var r1 = _service.Compile("deploy-gate-copy", "return 1 + 1;"); var r1 = _service.Compile("deploy-gate-copy", "return 1 + 1;");
var r2 = _service.Compile("prestart-copy", "return 1 + 1;"); var r2 = _service.Compile("prestart-copy", "return 1 + 1;");
Assert.True(r1.IsSuccess); Assert.True(r1.IsSuccess);
Assert.Same(r1.CompiledScript, r2.CompiledScript); // one compile, shared Script<T> (N4) — the definitive proof Assert.Same(r1.CompiledScript, r2.CompiledScript); // one compile, shared Script<T> (N4) — the definitive proof
// Hits is a process-global counter; other test classes in this assembly compile scripts // The cache belongs to this service instance (register row 100), so the hit count is
// concurrently (they are not in this serialized collection), so the exact post-Clear count // exactly the one the second Compile caused — it used to be a >= 1 lower bound on a
// is not deterministic under a full-assembly parallel run. The shared-Script<T> assertion // process-global counter other test classes were bumping in parallel.
// above is the real proof of cache reuse; here we only require the second lookup registered Assert.Equal(1, _cache.Hits);
// a hit (>= 1) rather than pinning an exact global count. Assert.Equal(1, _cache.Count);
Assert.True(SiteScriptCompileCache.Hits >= 1);
} }
[Fact] [Fact]
public void Compile_ScriptAndTriggerExpression_DoNotCrossContaminate() public void Compile_ScriptAndTriggerExpression_DoNotCrossContaminate()
{ {
SiteScriptCompileCache.Clear();
var script = _service.Compile("s", "1 > 0"); var script = _service.Compile("s", "1 > 0");
var trigger = _service.CompileTriggerExpression("t", "1 > 0"); var trigger = _service.CompileTriggerExpression("t", "1 > 0");
Assert.NotSame(script.CompiledScript, trigger.CompiledScript); // different globals surfaces Assert.NotSame(script.CompiledScript, trigger.CompiledScript); // different globals surfaces
Assert.Equal(2, _cache.Count); // two entries, no cross-globals hit
Assert.Equal(0, _cache.Hits);
} }
[Fact] [Fact]
@@ -231,7 +235,6 @@ public class ScriptCompilationServiceTests
[Fact] [Fact]
public void Compile_DistinctScripts_ShareOneMetadataReferenceSet_SoNativeMemoryDoesNotGrow() public void Compile_DistinctScripts_ShareOneMetadataReferenceSet_SoNativeMemoryDoesNotGrow()
{ {
SiteScriptCompileCache.Clear();
var first = _service.Compile("first", "return 1 + 1;"); var first = _service.Compile("first", "return 1 + 1;");
var second = _service.Compile("second", "return 2 + 2;"); var second = _service.Compile("second", "return 2 + 2;");
@@ -255,7 +258,6 @@ public class ScriptCompilationServiceTests
[Fact] [Fact]
public void Compile_UsesProcessWideCachingMetadataResolver() public void Compile_UsesProcessWideCachingMetadataResolver()
{ {
SiteScriptCompileCache.Clear();
var result = _service.Compile("resolver-pin", "return 41 + 1;"); var result = _service.Compile("resolver-pin", "return 41 + 1;");
Assert.True(result.IsSuccess); Assert.True(result.IsSuccess);
@@ -5,48 +5,34 @@ using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Scripts; namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Scripts;
/// <summary> /// <summary>
/// Tests for the process-wide <see cref="SiteScriptCompileCache"/>. The cache is /// Tests for <see cref="SiteScriptCompileCache"/>.
/// static/process-wide state, so this class shares the "SiteScriptCompileCache" xunit
/// collection with <c>ScriptCompilationServiceTests</c> and <c>DeploymentWarmThenGateTests</c>
/// (no cross-class parallelism between those three, so no concurrent
/// <see cref="SiteScriptCompileCache.Clear"/>).
/// </summary> /// </summary>
/// <remarks> /// <remarks>
/// <para> /// <para>
/// <b>Deferred-work register row 91.</b> That collection does NOT stop the rest of the /// <b>Deferred-work register row 100 — the cache is no longer static.</b> Every test here builds
/// assembly: any test class that compiles a script — Instance/Script actor tests, deployment /// its OWN <see cref="SiteScriptCompileCache"/>, so <see cref="SiteScriptCompileCache.Hits"/> and
/// tests — goes through <see cref="SiteScriptCompileCache.GetOrAdd"/> too, in parallel, and /// <see cref="SiteScriptCompileCache.Count"/> count this test's compiles and nothing else. That
/// bumps the same process-global <see cref="SiteScriptCompileCache.Hits"/> / /// retires three workarounds this file used to carry:
/// <see cref="SiteScriptCompileCache.Count"/>. Absolute assertions such as
/// <c>Assert.Equal(1, Hits)</c> after a <c>Clear()</c> therefore only held while nothing else
/// happened to be compiling; they failed roughly 1 run in 4 of a full parallel solution run and
/// were always green in isolation.
/// </para> /// </para>
/// <list type="bullet">
/// <item>the <c>[Collection("SiteScriptCompileCache")]</c> serialisation with
/// <c>ScriptCompilationServiceTests</c> and <c>DeploymentWarmThenGateTests</c> — those three
/// classes shared a mutable global and had to be kept off each other; they no longer touch any
/// shared state and run fully in parallel again;</item>
/// <item>the per-test GUID key token (register row 91) — no other class can reach this cache at
/// all, so keys need not be globally unique;</item>
/// <item>the one-sided counter bounds and the <c>ForeignEntrySlack = 512</c> allowance on
/// <c>Count</c> — foreign inserts are impossible now, so every assertion below is exact.</item>
/// </list>
/// <para> /// <para>
/// The fix follows the register's row-74 pattern: assert the DELTA attributable to this test's /// The old escape hatch, <c>Clear()</c>, is gone from the type: it had no production caller and
/// OWN keys across its own scoped action, never an absolute global value. Two habits carry it: /// existed only to reset the global between tests. Constructing a cache is the reset.
/// every test keys its cache entries on a per-test-instance unique token (<see cref="_uniq"/>),
/// so no other class can collide with or serve them; and the global counters are only ever
/// asserted as one-sided bounds (<c>&gt;= baseline + n</c>), since foreign traffic can only add.
/// A collection fixture spanning every script-compiling class in the assembly would fix it too,
/// but would serialise most of the suite for the benefit of five facts.
/// </para> /// </para>
/// </remarks> /// </remarks>
[Collection("SiteScriptCompileCache")]
public class SiteScriptCompileCacheTests public class SiteScriptCompileCacheTests
{ {
/// <summary> /// <summary>The cache under test. Private to this test instance — xunit builds one per test.</summary>
/// Slack allowed on <see cref="SiteScriptCompileCache.Count"/> bounds for entries inserted private readonly SiteScriptCompileCache _cache = new();
/// by test classes outside this collection while a bound test runs (register row 91). The
/// exactness of the eviction claims comes from the per-key probes, not from this bound.
/// </summary>
private const int ForeignEntrySlack = 512;
/// <summary>Per-test-instance token making every cache key in this test uniquely ours.</summary>
private readonly string _uniq = Guid.NewGuid().ToString("N");
/// <summary>Builds a cache key body that no other test class in the assembly can produce.</summary>
private string Code(string body) => $"/*{_uniq}*/ {body}";
private static ScriptCompilationResult Ok() => private static ScriptCompilationResult Ok() =>
ScriptCompilationResult.Succeeded(CSharpScript.Create<object?>("1", ScriptOptions.Default, typeof(ScriptGlobals))); ScriptCompilationResult.Succeeded(CSharpScript.Create<object?>("1", ScriptOptions.Default, typeof(ScriptGlobals)));
@@ -54,72 +40,94 @@ public class SiteScriptCompileCacheTests
[Fact] [Fact]
public void GetOrAdd_SameCodeAndGlobals_ComputesOnce() public void GetOrAdd_SameCodeAndGlobals_ComputesOnce()
{ {
var code = Code("return 1;");
var hitsBefore = SiteScriptCompileCache.Hits;
var factoryCalls = 0; var factoryCalls = 0;
ScriptCompilationResult Factory() { factoryCalls++; return Ok(); } ScriptCompilationResult Factory() { factoryCalls++; return Ok(); }
var r1 = SiteScriptCompileCache.GetOrAdd(code, typeof(ScriptGlobals), Factory); var r1 = _cache.GetOrAdd("return 1;", typeof(ScriptGlobals), Factory);
var r2 = SiteScriptCompileCache.GetOrAdd(code, typeof(ScriptGlobals), Factory); var r2 = _cache.GetOrAdd("return 1;", typeof(ScriptGlobals), Factory);
// Scoped to our own key, so these two are exact and are the real claim: one compile,
// and the second lookup returned that very instance rather than a re-computation.
Assert.Equal(1, factoryCalls); Assert.Equal(1, factoryCalls);
Assert.Same(r1, r2); Assert.Same(r1, r2);
Assert.Equal(1, _cache.Hits);
// Hits is process-global; a parallel class's compile can only push the delta up, so the Assert.Equal(1, _cache.Count);
// claim "the second lookup was counted as a hit" is a lower bound, not an equality.
Assert.True(SiteScriptCompileCache.Hits >= hitsBefore + 1,
$"expected the second GetOrAdd to register a hit; hits went {hitsBefore} -> {SiteScriptCompileCache.Hits}");
} }
[Fact] [Fact]
public void GetOrAdd_SameCode_DifferentGlobals_AreSeparateEntries() public void GetOrAdd_SameCode_DifferentGlobals_AreSeparateEntries()
{ {
var code = Code("1 > 0");
var scriptCompiles = 0; var scriptCompiles = 0;
var triggerCompiles = 0; var triggerCompiles = 0;
var script = SiteScriptCompileCache.GetOrAdd(code, typeof(ScriptGlobals), var script = _cache.GetOrAdd("1 > 0", typeof(ScriptGlobals),
() => { scriptCompiles++; return Ok(); }); () => { scriptCompiles++; return Ok(); });
var trigger = SiteScriptCompileCache.GetOrAdd(code, typeof(TriggerExpressionGlobals), var trigger = _cache.GetOrAdd("1 > 0", typeof(TriggerExpressionGlobals),
() => { triggerCompiles++; return Ok(); }); () => { triggerCompiles++; return Ok(); });
// "No cross-globals hit" asserted where it is actually observable and ours: the second
// lookup had to compute, and the two globals surfaces hold distinct entries. (The old
// form — Count == 2 and Hits == 0 on the process-global counters — was only true while
// no other class was compiling.)
Assert.Equal(1, scriptCompiles); Assert.Equal(1, scriptCompiles);
Assert.Equal(1, triggerCompiles); Assert.Equal(1, triggerCompiles);
Assert.NotSame(script, trigger); Assert.NotSame(script, trigger);
// Each entry is independently addressable afterwards: re-getting either must serve the // Two distinct entries and no cross-globals hit — exact, because this cache is ours.
// matching cached result, never the other globals surface's. Assert.Equal(2, _cache.Count);
Assert.Same(script, SiteScriptCompileCache.GetOrAdd(code, typeof(ScriptGlobals), Assert.Equal(0, _cache.Hits);
// Each entry stays independently addressable afterwards.
Assert.Same(script, _cache.GetOrAdd("1 > 0", typeof(ScriptGlobals),
() => throw new InvalidOperationException("script entry was not cached"))); () => throw new InvalidOperationException("script entry was not cached")));
Assert.Same(trigger, SiteScriptCompileCache.GetOrAdd(code, typeof(TriggerExpressionGlobals), Assert.Same(trigger, _cache.GetOrAdd("1 > 0", typeof(TriggerExpressionGlobals),
() => throw new InvalidOperationException("trigger entry was not cached"))); () => throw new InvalidOperationException("trigger entry was not cached")));
} }
/// <summary>
/// Register row 100: two caches are genuinely independent stores. This is the property the
/// whole change exists for — one test class's cache activity can no longer evict, serve, or
/// be counted by another's. It is the direct replacement for the old
/// <c>Clear()</c>-under-a-serialized-collection arrangement.
/// </summary>
[Fact]
public void SeparateCaches_DoNotShareEntriesOrCounters()
{
var other = new SiteScriptCompileCache();
var mine = _cache.GetOrAdd("return 1;", typeof(ScriptGlobals), Ok);
_cache.GetOrAdd("return 1;", typeof(ScriptGlobals), Ok); // a hit on OUR cache
// The other cache has neither the entry nor the hit.
Assert.Equal(0, other.Count);
Assert.Equal(0, other.Hits);
// Same key, other store: it must compute its own value, and get a distinct instance.
var theirCompiles = 0;
var theirs = other.GetOrAdd("return 1;", typeof(ScriptGlobals),
() => { theirCompiles++; return Ok(); });
Assert.Equal(1, theirCompiles);
Assert.NotSame(mine, theirs);
// …and filling the other cache past its bound — the closest thing left to the old
// wholesale Clear() — leaves ours completely untouched.
for (var i = 0; i <= SiteScriptCompileCache.MaxEntries; i++)
other.GetOrAdd($"return {i};", typeof(ScriptGlobals), Ok);
Assert.Equal(1, _cache.Count);
Assert.Equal(1, _cache.Hits);
Assert.Same(mine, _cache.GetOrAdd("return 1;", typeof(ScriptGlobals),
() => throw new InvalidOperationException("our entry was evicted by another cache")));
}
[Fact] [Fact]
public void Overflow_StaysWithinTheBound() public void Overflow_StaysWithinTheBound()
{ {
SiteScriptCompileCache.Clear();
for (var i = 0; i <= SiteScriptCompileCache.MaxEntries; i++) for (var i = 0; i <= SiteScriptCompileCache.MaxEntries; i++)
SiteScriptCompileCache.GetOrAdd(Code($"return {i};"), typeof(ScriptGlobals), Ok); _cache.GetOrAdd($"return {i};", typeof(ScriptGlobals), Ok);
// MaxEntries + 1 distinct inserts must not all be resident — the sweep ran. The bound // MaxEntries + 1 distinct inserts, single-threaded: the sweep ran and the ceiling holds
// carries slack for foreign inserts (register row 91); the sharp claim is the eviction // exactly (no foreign inserts can land here any more — register row 100).
// probe below, which is scoped to keys only this test can produce. Assert.True(_cache.Count <= SiteScriptCompileCache.MaxEntries,
Assert.True(SiteScriptCompileCache.Count <= SiteScriptCompileCache.MaxEntries + ForeignEntrySlack, $"cache grew past its bound: {_cache.Count} vs max {SiteScriptCompileCache.MaxEntries}");
$"cache grew past its bound: {SiteScriptCompileCache.Count} vs max {SiteScriptCompileCache.MaxEntries}");
var recomputed = 0; var recomputed = 0;
for (var i = 0; i < 16; i++) for (var i = 0; i < 16; i++)
{ _cache.GetOrAdd($"return {i};", typeof(ScriptGlobals), () => { recomputed++; return Ok(); });
var code = Code($"return {i};");
SiteScriptCompileCache.GetOrAdd(code, typeof(ScriptGlobals), () => { recomputed++; return Ok(); });
}
Assert.True(recomputed > 0, "no oldest-batch entry was evicted — the overflow sweep did not run"); Assert.True(recomputed > 0, "no oldest-batch entry was evicted — the overflow sweep did not run");
} }
@@ -134,50 +142,37 @@ public class SiteScriptCompileCacheTests
[Fact] [Fact]
public void Overflow_EvictsOldestBatch_AndKeepsRecentlyTouchedEntries() public void Overflow_EvictsOldestBatch_AndKeepsRecentlyTouchedEntries()
{ {
// Note: the cache is process-wide static and other test classes compile into it
// concurrently, so the assertions below are phrased as properties of the eviction
// policy (what survives, what does not, nothing is wiped) rather than as exact
// counts, which no test can own here.
SiteScriptCompileCache.Clear();
// Fill to the bound. Entries 1..N are inserted oldest-first. // Fill to the bound. Entries 1..N are inserted oldest-first.
for (var i = 0; i < SiteScriptCompileCache.MaxEntries; i++) for (var i = 0; i < SiteScriptCompileCache.MaxEntries; i++)
SiteScriptCompileCache.GetOrAdd(Code($"return {i};"), typeof(ScriptGlobals), Ok); _cache.GetOrAdd($"return {i};", typeof(ScriptGlobals), Ok);
// Touch entry 0 so it carries the NEWEST access stamp despite being inserted first. // Touch entry 0 so it carries the NEWEST access stamp despite being inserted first.
var hot = SiteScriptCompileCache.GetOrAdd(Code("return 0;"), typeof(ScriptGlobals), Ok); var hot = _cache.GetOrAdd("return 0;", typeof(ScriptGlobals), Ok);
var hitsAfterTouch = SiteScriptCompileCache.Hits; var hitsAfterTouch = _cache.Hits;
Assert.Equal(1, hitsAfterTouch); // exactly one hit so far: the touch
// Push well past the bound so at least one eviction sweep definitely runs. // Push well past the bound so at least one eviction sweep definitely runs.
for (var i = 0; i < 200; i++) for (var i = 0; i < 200; i++)
SiteScriptCompileCache.GetOrAdd(Code($"return overflow{i};"), typeof(ScriptGlobals), Ok); _cache.GetOrAdd($"return overflow{i};", typeof(ScriptGlobals), Ok);
// Nothing was cleared wholesale: the cache is still most of the way full and the hit // Nothing was cleared wholesale: the cache is still most of the way full and the hit
// counter survived (Clear() would have reset it to 0). // counter survived (a wholesale clear used to reset it to 0).
Assert.True(SiteScriptCompileCache.Count > SiteScriptCompileCache.MaxEntries / 2, Assert.True(_cache.Count > SiteScriptCompileCache.MaxEntries / 2,
$"cache collapsed to {SiteScriptCompileCache.Count} entries — this looks like a wholesale clear"); $"cache collapsed to {_cache.Count} entries — this looks like a wholesale clear");
Assert.True(SiteScriptCompileCache.Hits >= hitsAfterTouch); Assert.Equal(hitsAfterTouch, _cache.Hits); // 200 distinct inserts, none of them hits
// The touched entry survived the sweep — a hit, not a recompile. This is the LRU // The touched entry survived the sweep — a hit, not a recompile. This is the LRU
// property: recency, not insertion order, decides what stays. // property: recency, not insertion order, decides what stays.
var hitsBefore = SiteScriptCompileCache.Hits; var again = _cache.GetOrAdd("return 0;", typeof(ScriptGlobals),
var again = SiteScriptCompileCache.GetOrAdd(Code("return 0;"), typeof(ScriptGlobals),
() => throw new InvalidOperationException("hot entry was evicted — LRU is not keeping recently-used entries")); () => throw new InvalidOperationException("hot entry was evicted — LRU is not keeping recently-used entries"));
// Assert.Same is the sharp claim (our own key, our own instance). The counter check is a
// lower bound: Hits is process-global and a parallel class's hit lands in the same window
// (register row 91 — this was `Assert.Equal(hitsBefore + 1, …)`).
Assert.Same(hot, again); Assert.Same(hot, again);
Assert.True(SiteScriptCompileCache.Hits >= hitsBefore + 1, Assert.Equal(hitsAfterTouch + 1, _cache.Hits);
$"expected the surviving hot entry to register a hit; hits went {hitsBefore} -> {SiteScriptCompileCache.Hits}");
// …and untouched old entries genuinely were evicted, so the sweep really ran. // …and untouched old entries genuinely were evicted, so the sweep really ran.
var recomputed = 0; var recomputed = 0;
for (var i = 1; i <= 16; i++) for (var i = 1; i <= 16; i++)
{ _cache.GetOrAdd($"return {i};", typeof(ScriptGlobals), () => { recomputed++; return Ok(); });
var code = Code($"return {i};");
SiteScriptCompileCache.GetOrAdd(code, typeof(ScriptGlobals),
() => { recomputed++; return Ok(); });
}
Assert.True(recomputed > 0, "no old entry was evicted — the overflow sweep did not run"); Assert.True(recomputed > 0, "no old entry was evicted — the overflow sweep did not run");
} }
@@ -189,26 +184,25 @@ public class SiteScriptCompileCacheTests
[Fact] [Fact]
public async Task ConcurrentGetOrAddStorm_KeepsTheBound() public async Task ConcurrentGetOrAddStorm_KeepsTheBound()
{ {
SiteScriptCompileCache.Clear();
const int perTask = 400; const int perTask = 400;
var tasks = Enumerable.Range(0, 8).Select(t => Task.Run(() => const int tasks = 8;
await Task.WhenAll(Enumerable.Range(0, tasks).Select(t => Task.Run(() =>
{ {
for (var i = 0; i < perTask; i++) for (var i = 0; i < perTask; i++)
SiteScriptCompileCache.GetOrAdd(Code($"return {t}_{i};"), typeof(ScriptGlobals), Ok); _cache.GetOrAdd($"return {t}_{i};", typeof(ScriptGlobals), Ok);
})); })));
await Task.WhenAll(tasks);
// A small transient overshoot is by design: threads that lose the double-checked // A small transient overshoot is by design: threads that lose the double-checked
// eviction race still insert their own entry afterwards, so the ceiling is "bounded", // eviction race still insert their own entry afterwards, so the ceiling is "bounded",
// not "never exceeded by one". The property under test is that 3200 distinct inserts // not "never exceeded by one". The slack is bounded by the number of racing threads
// do not accumulate — before WP3.1 this was a wholesale Clear(), and a per-entry // (register row 100 — it used to be 512, to absorb inserts from other test classes
// eviction bug would show up here as unbounded growth, not as a handful of extra rows. // sharing the process-global cache; nothing foreign can reach this instance).
// (Other test classes compile into this process-wide cache concurrently, which is the // The property under test is that 3200 distinct inserts do not accumulate — before
// other reason the bound is asserted with slack rather than exactly.) // WP3.1 this was a wholesale clear, and a per-entry eviction bug would show up here as
Assert.True(SiteScriptCompileCache.Count <= SiteScriptCompileCache.MaxEntries + ForeignEntrySlack, // unbounded growth, not as a handful of extra rows.
$"cache grew past its bound: {SiteScriptCompileCache.Count} vs max {SiteScriptCompileCache.MaxEntries} " + Assert.True(_cache.Count <= SiteScriptCompileCache.MaxEntries + tasks,
$"after {8 * perTask} distinct inserts"); $"cache grew past its bound: {_cache.Count} vs max {SiteScriptCompileCache.MaxEntries} " +
$"(+{tasks} racing-insert slack) after {tasks * perTask} distinct inserts");
} }
} }
@@ -0,0 +1,36 @@
using Microsoft.Extensions.DependencyInjection;
using ZB.MOM.WW.ScadaBridge.SiteRuntime;
using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests;
/// <summary>
/// Pins the DI lifetimes that carry a behavioural contract rather than a convention.
/// </summary>
public class SiteRuntimeServiceRegistrationTests
{
/// <summary>
/// Register row 100. <c>ScriptCompilationService</c> owns the
/// <c>SiteScriptCompileCache</c>, so "one Roslyn compile per script body per process"
/// (N4 / WP3.1's warm-then-gate deploy path) is precisely the statement that DI hands
/// out ONE of these per host. The cache stopped being <c>static</c> when row 100 removed
/// the process-global state tests were clobbering; this registration is what replaced
/// the static as the guarantee, so it is pinned here.
///
/// <para>Downgrading this to Scoped or Transient would give every consumer — the deploy
/// gate, each Instance Actor's PreStart, SharedScriptLibrary — its own empty cache and
/// silently reintroduce the recompile storms WP3.1 removed, with no functional test
/// failing. This assertion is the only thing that would notice.</para>
/// </summary>
[Fact]
public void AddSiteRuntime_RegistersScriptCompilationServiceAsSingleton_SoOneCompileCacheExistsPerProcess()
{
var services = new ServiceCollection();
services.AddSiteRuntime();
var descriptor = Assert.Single(
services, d => d.ServiceType == typeof(ScriptCompilationService));
Assert.Equal(ServiceLifetime.Singleton, descriptor.Lifetime);
}
}