From 901cec9026a11df44985016ad9efc23f2033ca2a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 12 Aug 2026 16:30:53 -0400 Subject: [PATCH] docs(plans): script-compile metadata resolver cache plan --- ...-script-compile-metadata-resolver-cache.md | 572 ++++++++++++++++++ ...pile-metadata-resolver-cache.md.tasks.json | 15 + 2 files changed, 587 insertions(+) create mode 100644 docs/plans/2026-08-12-script-compile-metadata-resolver-cache.md create mode 100644 docs/plans/2026-08-12-script-compile-metadata-resolver-cache.md.tasks.json diff --git a/docs/plans/2026-08-12-script-compile-metadata-resolver-cache.md b/docs/plans/2026-08-12-script-compile-metadata-resolver-cache.md new file mode 100644 index 00000000..a3ffc986 --- /dev/null +++ b/docs/plans/2026-08-12-script-compile-metadata-resolver-cache.md @@ -0,0 +1,572 @@ +# Script-Compile Metadata Resolver Cache Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans to implement this plan task-by-task. + +**Goal:** Stop Roslyn from minting a fresh `AssemblyMetadata` → `PEReader` → `NativeHeapMemoryBlock` set (~74–150 assemblies' worth) on **every** script compile by installing one process-wide memoizing `MetadataReferenceResolver` on all four script-compile surfaces. + +**Architecture:** A new `CachingScriptMetadataResolver` in the ScriptAnalysis component (the script-compilation single-source-of-truth home) decorates Roslyn's default `ScriptMetadataResolver` and memoizes `ResolveMissingAssembly`/`ResolveReference` results in process-wide concurrent dictionaries. Each of the four compile surfaces attaches the shared instance via `ScriptOptions.WithMetadataResolver(...)`. Resolution *results* are unchanged — only their identity is: one `PortableExecutableReference` per distinct assembly per process instead of one per assembly **per compile**. + +**Tech Stack:** Microsoft.CodeAnalysis.CSharp.Scripting 5.0.0 (Roslyn), net10.0, xUnit. + +--- + +## Root cause (confirmed 2026-08-12, post-`5a781c70`) + +`5a781c70` made all four `ScriptOptions`/`MetadataReference.CreateFromFile` construction sites static, and that fix is correct and live — but it was not the dominant allocator. The dominant allocator is **inside Roslyn**: + +- The static options carry only the *direct* API-surface references (e.g. SiteRuntime's `ScriptAssemblies` is 5 assemblies). Every `script.Compile()` must bind their **transitive closure**. +- Each transitively-referenced assembly identity not in the explicit reference list is resolved through `ScriptOptions.MetadataResolver` (`ScriptMetadataResolver` → `RuntimeMetadataReferenceResolver`), whose `ResolveMissingAssembly` calls `MetadataReference.CreateFromFile` — eagerly copying the whole file into native memory (`PEStreamOptions.PrefetchEntireImage` → `NativeHeapMemoryBlock`) — **with no cross-compilation cache**. +- Those fresh references are retained by the compiled `Script`'s `Compilation`, which `SiteScriptCompileCache` (site) / `_scriptHandlers` (inbound API) hold for the life of the process. + +**Empirical proof** (scratch repro, Roslyn 5.0.0, options mirroring the site's): a 5-assembly explicit set has a 74-assembly transitive closure; compiling the *same code* three times drove `ResolveMissingAssembly` 74 times **per compile** (222 total) and returned 222 **distinct** `PortableExecutableReference` instances — i.e. 74 fresh `AssemblyMetadata`+`PEReader`+native-block sets per compile. With a memoizing decorator: compile #1 = 74 inner resolutions, compiles #2/#3 = **0**, and the script still evaluates correctly. + +This matches the production gcdump signature exactly: +- Per-script scaling, untouched by the `5a781c70` fix: Site (21 scripts) 6,640 triple-objects ≈ 21 × ~105 refs × 3 counted types; Central (6 scripts) 2,699 ≈ 6 × ~150 × 3. +- Bit-identical counts pre/post fix on an identical workload (`NativeHeapMemoryBlock` 2,681 both times): the minting is deterministic per compiled script, and the explicit sets the fix de-duplicated were a small fraction of the closure. +- Invisible to source greps: no ScadaBridge code calls anything per compile — the `CreateFromFile` calls happen inside `RuntimeMetadataReferenceResolver`. + +**Impact today:** ~21 duplicate native copies of a ~105-assembly closure on a site node (plausibly the bulk of its 2.8 GB working set), growing further on every *changed-code* recompile (script edit + redeploy, inbound method revision change) with no reclamation path, since the compile caches pin the references. + +**Explicitly NOT the cause (verified):** `ScriptTrustValidator.FindViolations` — its `CSharpCompilation.CreateScriptCompilation` gets no resolver in its `CSharpCompilationOptions`, and a plain compilation performs no missing-assembly resolution without one; its `AnalysisReferences` are static TPA-wide objects. `ScriptTrustPolicy.BuildMinimalFallbackReferences()` mints per call but has **no production callers** (test-only, by design). Leave both alone. + +## Design + +### Why a caching resolver decorator (and not the alternatives) + +- **Widening the explicit reference sets to the transitive closure** would also stop the resolver calls, but it changes what the compile gate *sees* (`DefaultReferences` is deliberately minimal so forbidden types die as undefined symbols — see the `ScriptTrustPolicy` remark that anchors are added "ONLY here, never to DefaultReferences"). Even though resolver-resolved assemblies end up bound anyway, changing the explicit set risks semantic drift in a security gate for no extra benefit. Rejected. +- **Disabling missing-assembly resolution** would change compile results (unresolved dependency symbols). Rejected. +- **The decorator** preserves resolution semantics *exactly* — same inner resolver, same answers — and only de-duplicates the result objects. Trust model unaffected: the resolver can only resolve what the inner resolver would have resolved anyway. + +### Known accepted residual + +Each `CSharpScript.Create` also mints ~2–3 references internally via `MetadataReference.CreateFromAssemblyInternal` (corlib + globals-type assembly); that path is not reachable from `ScriptOptions` and is bounded by compile count at ~3 objects per compile — noise (~200× smaller than the fixed leak). Post-fix measurements should expect the metadata triple to grow by ≤ ~10 objects per *new* compile, not ~300. + +### Cache-correctness assumptions (document in code) + +- Keying `ResolveMissingAssembly` on `AssemblyIdentity.GetDisplayName()` ignores the `definition` parameter (the requesting assembly's directory is a search path in the inner resolver). Safe here: all ScadaBridge nodes run from a single publish directory + shared framework, so identity→path is stable process-wide. +- `ConcurrentDictionary.GetOrAdd` may race two factory invocations for the same key; one extra mint, bounded, benign. +- The cache never disposes: entries live for the process lifetime, bounded by distinct assemblies on disk (~473 worst case ≈ one native copy each) — the same order as the already-static `AnalysisReferences`, and strictly better than one copy per assembly per compiled script. + +--- + +### Task 1: `CachingScriptMetadataResolver` (TDD) + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** none (everything else builds on it) + +**Files:** +- Create: `src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/CachingScriptMetadataResolver.cs` +- Test: `tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/CachingScriptMetadataResolverTests.cs` + +**Step 1: Write the failing tests** + +```csharp +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Scripting; +using ZB.MOM.WW.ScadaBridge.ScriptAnalysis; + +namespace ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests; + +/// +/// Pins the per-compile native-metadata fix (2026-08-12 follow-up to 5a781c70): +/// Roslyn's default ScriptMetadataResolver re-resolves the transitive assembly +/// closure through MetadataReference.CreateFromFile on EVERY script compile — +/// each call minting a fresh AssemblyMetadata → PEReader → NativeHeapMemoryBlock. +/// The decorator memoizes so each distinct assembly is resolved once per process. +/// +public class CachingScriptMetadataResolverTests +{ + private sealed class CountingResolver : MetadataReferenceResolver + { + public int MissingCalls; + public int ReferenceCalls; + public override bool ResolveMissingAssemblies => true; + + public override PortableExecutableReference? ResolveMissingAssembly( + MetadataReference definition, AssemblyIdentity referenceIdentity) + { + Interlocked.Increment(ref MissingCalls); + return MetadataReference.CreateFromFile(typeof(object).Assembly.Location); + } + + public override ImmutableArray ResolveReference( + string reference, string? baseFilePath, MetadataReferenceProperties properties) + { + Interlocked.Increment(ref ReferenceCalls); + return [MetadataReference.CreateFromFile(typeof(object).Assembly.Location)]; + } + + public override bool Equals(object? other) => ReferenceEquals(this, other); + public override int GetHashCode() => 0; + } + + private static readonly AssemblyIdentity SomeIdentity = new("System.Fake", new Version(1, 0, 0, 0)); + private static readonly MetadataReference SomeDefinition = + MetadataReference.CreateFromFile(typeof(object).Assembly.Location); + + [Fact] + public void ResolveMissingAssembly_SameIdentityTwice_ResolvesOnceAndSharesInstance() + { + var inner = new CountingResolver(); + var sut = new CachingScriptMetadataResolver(inner); + + var first = sut.ResolveMissingAssembly(SomeDefinition, SomeIdentity); + var second = sut.ResolveMissingAssembly(SomeDefinition, SomeIdentity); + + Assert.Equal(1, inner.MissingCalls); + Assert.Same(first, second); // one AssemblyMetadata/PEReader/native block, not two + } + + [Fact] + public void ResolveMissingAssembly_DistinctIdentities_ResolveIndependently() + { + var inner = new CountingResolver(); + var sut = new CachingScriptMetadataResolver(inner); + + sut.ResolveMissingAssembly(SomeDefinition, SomeIdentity); + sut.ResolveMissingAssembly(SomeDefinition, new AssemblyIdentity("System.Other", new Version(1, 0, 0, 0))); + + Assert.Equal(2, inner.MissingCalls); + } + + [Fact] + public void ResolveReference_SameArgsTwice_ResolvesOnceAndSharesInstances() + { + var inner = new CountingResolver(); + var sut = new CachingScriptMetadataResolver(inner); + + var first = sut.ResolveReference("System.Xml", baseFilePath: null, MetadataReferenceProperties.Assembly); + var second = sut.ResolveReference("System.Xml", baseFilePath: null, MetadataReferenceProperties.Assembly); + + Assert.Equal(1, inner.ReferenceCalls); + Assert.Same(first[0], second[0]); + } + + [Fact] + public void Instance_IsProcessWideSingleton() + => Assert.Same(CachingScriptMetadataResolver.Instance, CachingScriptMetadataResolver.Instance); + + [Fact] + public void ResolveMissingAssemblies_DelegatesToInner() + { + var sut = new CachingScriptMetadataResolver(new CountingResolver()); + Assert.True(sut.ResolveMissingAssemblies); + } +} +``` + +**Step 2: Run tests to verify they fail** + +Run: `dotnet test tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests --filter CachingScriptMetadataResolverTests 2>&1 | tail -5` +Expected: build FAILS — `CachingScriptMetadataResolver` does not exist. + +**Step 3: Write the implementation** + +```csharp +using System.Collections.Concurrent; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Scripting; + +namespace ZB.MOM.WW.ScadaBridge.ScriptAnalysis; + +/// +/// Process-wide memoizing decorator over Roslyn's default script metadata +/// resolver, closing the second (dominant) half of the script-compile native +/// memory leak fixed in part by 5a781c70. +/// +/// +/// Why this exists. The shared ScriptOptions on every compile +/// surface carry only the direct API-surface references; each +/// script.Compile() binds their transitive closure, and every +/// transitively-referenced assembly is resolved through the options' +/// . Roslyn's +/// RuntimeMetadataReferenceResolver has NO cross-compilation cache: each +/// resolution calls MetadataReference.CreateFromFile, which eagerly +/// copies the whole assembly into native memory (AssemblyMetadata → +/// PEReader → NativeHeapMemoryBlock). Measured on Roslyn 5.0.0: +/// a 5-assembly explicit set resolves a 74-assembly closure afresh on EVERY +/// compile — ~74 fresh native metadata copies per compiled script, pinned for +/// the process lifetime by the compile caches. This decorator memoizes, so each +/// distinct assembly is materialized once per process regardless of compile count. +/// +/// +/// +/// Trust model unaffected. Resolution RESULTS are identical to the inner +/// resolver's — only object identity is de-duplicated. The resolver can never +/// resolve anything the undecorated options would not have resolved. +/// +/// +/// +/// Cache-correctness assumptions. The missing-assembly cache keys on the +/// assembly identity display name and deliberately ignores the requesting +/// definition (whose directory is a search path in the inner resolver): +/// every ScadaBridge node runs from a single publish directory plus the shared +/// framework, so identity → path is stable process-wide. A +/// GetOrAdd factory race can mint one duplicate — bounded, benign. +/// Entries are never disposed; the cache is bounded by the distinct assemblies +/// on disk, the same order as the static . +/// +/// +public sealed class CachingScriptMetadataResolver : MetadataReferenceResolver +{ + /// + /// The shared process-wide instance every script-compile surface attaches via + /// ScriptOptions.WithMetadataResolver. Decorates + /// 's resolver — the exact resolver those + /// surfaces used implicitly before this fix. + /// + public static readonly CachingScriptMetadataResolver Instance = + new(ScriptOptions.Default.MetadataResolver); + + private readonly MetadataReferenceResolver _inner; + + private readonly ConcurrentDictionary _missingByIdentity = + new(StringComparer.OrdinalIgnoreCase); + + private readonly ConcurrentDictionary<(string Reference, string? BaseFilePath, MetadataReferenceProperties Properties), + ImmutableArray> _referencesByPath = new(); + + /// Creates a decorator over the given inner resolver. Exposed for tests; production uses . + /// The resolver whose results are memoized. + public CachingScriptMetadataResolver(MetadataReferenceResolver inner) => _inner = inner; + + /// + public override bool ResolveMissingAssemblies => _inner.ResolveMissingAssemblies; + + /// + public override PortableExecutableReference? ResolveMissingAssembly( + MetadataReference definition, AssemblyIdentity referenceIdentity) + => _missingByIdentity.GetOrAdd( + referenceIdentity.GetDisplayName(), + _ => _inner.ResolveMissingAssembly(definition, referenceIdentity)); + + /// + public override ImmutableArray ResolveReference( + string reference, string? baseFilePath, MetadataReferenceProperties properties) + => _referencesByPath.GetOrAdd( + (reference, baseFilePath, properties), + _ => _inner.ResolveReference(reference, baseFilePath, properties)); + + /// + public override bool Equals(object? other) => ReferenceEquals(this, other); + + /// + public override int GetHashCode() => System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(this); +} +``` + +**Step 4: Run tests to verify they pass** + +Run: `dotnet test tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests --filter CachingScriptMetadataResolverTests 2>&1 | tail -5` +Expected: 5 PASS. + +**Step 5: Commit** + +```bash +git add src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/CachingScriptMetadataResolver.cs tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/CachingScriptMetadataResolverTests.cs +git commit -m "feat(scripts): add process-wide caching metadata resolver for script compiles" +``` + +### Task 2: End-to-end proof — repeat compiles resolve zero + +**Classification:** small +**Estimated implement time:** ~4 min +**Parallelizable with:** none (needs Task 1) + +**Files:** +- Test: `tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/CachingScriptMetadataResolverTests.cs` (append) + +**Step 1: Write the test** (this is the compile-count-keyed regression the gcdump measurements could not express) + +```csharp + /// + /// The decisive regression: with options shaped like the real compile + /// surfaces (small explicit set, large transitive closure), the FIRST + /// compile resolves the closure through the resolver and every subsequent + /// compile — same or different code — resolves NOTHING. Before the fix, + /// every compile re-resolved the full closure (measured: 74 fresh + /// PortableExecutableReferences per compile on Roslyn 5.0.0). + /// A probe wraps the DEFAULT resolver so the test also proves resolution + /// actually flows through this path at all (first > 0) — guarding against + /// the whole mechanism silently changing in a Roslyn upgrade. + /// + [Fact] + public void Compile_RepeatAndDistinctScripts_ResolveClosureOnlyOnce() + { + var probe = new ProbeResolver(Microsoft.CodeAnalysis.Scripting.ScriptOptions.Default.MetadataResolver); + var caching = new CachingScriptMetadataResolver(probe); + + var options = Microsoft.CodeAnalysis.Scripting.ScriptOptions.Default + .WithReferences( + typeof(object).Assembly, + typeof(Enumerable).Assembly, + typeof(Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo).Assembly) + .WithImports("System", "System.Linq") + .WithMetadataResolver(caching); + + long CompileAndCount(string code) + { + long before = probe.MissingCalls; + var script = Microsoft.CodeAnalysis.CSharp.Scripting.CSharpScript.Create(code, options); + var errors = script.Compile().Count(d => d.Severity == DiagnosticSeverity.Error); + Assert.Equal(0, errors); + return probe.MissingCalls - before; + } + + var first = CompileAndCount("return Enumerable.Range(1, 3).Sum();"); + var repeat = CompileAndCount("return Enumerable.Range(1, 3).Sum();"); + var distinct = CompileAndCount("return string.Join(\",\", Enumerable.Range(1, 2)).Length;"); + + Assert.True(first > 0, "expected the first compile to resolve the transitive closure through the resolver"); + Assert.Equal(0, repeat); // pre-fix: == first (~74) — one fresh native metadata copy per assembly per compile + Assert.Equal(0, distinct); + } + + private sealed class ProbeResolver : MetadataReferenceResolver + { + private readonly MetadataReferenceResolver _inner; + public long MissingCalls; + public ProbeResolver(MetadataReferenceResolver inner) => _inner = inner; + public override bool ResolveMissingAssemblies => _inner.ResolveMissingAssemblies; + public override PortableExecutableReference? ResolveMissingAssembly( + MetadataReference definition, AssemblyIdentity referenceIdentity) + { + Interlocked.Increment(ref MissingCalls); + return _inner.ResolveMissingAssembly(definition, referenceIdentity); + } + public override ImmutableArray ResolveReference( + string reference, string? baseFilePath, MetadataReferenceProperties properties) + => _inner.ResolveReference(reference, baseFilePath, properties); + public override bool Equals(object? other) => ReferenceEquals(this, other); + public override int GetHashCode() => 0; + } +``` + +(Add `using Microsoft.CodeAnalysis;` / `System.Collections.Immutable` if not already present.) + +**Step 2: Run and verify pass** + +Run: `dotnet test tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests --filter CachingScriptMetadataResolverTests 2>&1 | tail -5` +Expected: all PASS (first > 0, repeat/distinct == 0). + +**Step 3: Commit** + +```bash +git add tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/CachingScriptMetadataResolverTests.cs +git commit -m "test(scripts): pin that repeat compiles resolve the assembly closure zero times" +``` + +### Task 3: Attach resolver — SiteRuntime `ScriptCompilationService` + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 4, Task 5, Task 6 + +**Files:** +- Modify: `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs:93-99` +- Test: `tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptCompilationServiceTests.cs` + +**Step 1: Write the failing pin test** (append to `ScriptCompilationServiceTests`; the compiled `Script.Options` is public, so no internals needed) + +```csharp + [Fact] + public void Compile_UsesProcessWideCachingMetadataResolver() + { + SiteScriptCompileCache.Clear(); + var result = _service.Compile("resolver-pin", "return 41 + 1;"); + + Assert.True(result.IsSuccess); + // Without the shared caching resolver, EVERY compile re-resolves the + // transitive assembly closure via MetadataReference.CreateFromFile — + // ~74+ fresh native metadata copies per compiled script (2026-08-12 dump). + Assert.Same( + ZB.MOM.WW.ScadaBridge.ScriptAnalysis.CachingScriptMetadataResolver.Instance, + result.CompiledScript!.Options.MetadataResolver); + } +``` + +**Step 2: Run to verify it fails** + +Run: `dotnet test tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests --filter Compile_UsesProcessWideCachingMetadataResolver 2>&1 | tail -5` +Expected: FAIL — resolver is `ScriptMetadataResolver`, not the shared instance. + +**Step 3: Attach the resolver** — in `SharedScriptOptions`, and extend the doc comment: + +```csharp + private static readonly ScriptOptions SharedScriptOptions = ScriptOptions.Default + .WithReferences(ScriptAssemblies) + .WithMetadataResolver(CachingScriptMetadataResolver.Instance) + .WithImports( + "System", + "System.Collections.Generic", + "System.Linq", + "System.Threading.Tasks"); +``` + +Append to the existing `SharedScriptOptions` XML doc (after the 473-DLLs paragraph): + +```csharp + /// + /// Static options alone are NOT enough: every script.Compile() binds the + /// transitive closure of these references, and Roslyn's default resolver + /// re-resolves that closure through MetadataReference.CreateFromFile on + /// EVERY compile (~74–105 fresh native metadata copies per compiled script, + /// confirmed live post-5a781c70 on 2026-08-12: 21 scripts held 6,640 + /// AssemblyMetadata/PEReader/MetadataImageReference objects). The shared + /// memoizes those resolutions + /// process-wide; see its doc for the full mechanism. + /// +``` + +**Step 4: Run tests** + +Run: `dotnet test tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests --filter ScriptCompilationServiceTests 2>&1 | tail -5` +Expected: all PASS (including the pre-existing reference-set reference-equality test). + +**Step 5: Commit** + +```bash +git add src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptCompilationService.cs tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/ScriptCompilationServiceTests.cs +git commit -m "fix(scripts): memoize missing-assembly resolution on the site compile path" +``` + +### Task 4: Attach resolver — InboundAPI `InboundScriptExecutor` + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 3, Task 5, Task 6 + +**Files:** +- Modify: `src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs:224-236` + +**Step 1: Attach the resolver** to its `SharedScriptOptions` (same one-line `.WithMetadataResolver(CachingScriptMetadataResolver.Instance)` after `.WithReferences(...)`), add `using ZB.MOM.WW.ScadaBridge.ScriptAnalysis;` if absent (the project already references ScriptAnalysis for `ForbiddenApiChecker`; if not, add the project reference), and append the same style of doc-comment paragraph as Task 3 (central variant: "6 inbound methods held 2,699 objects; method revisions recompile, so growth was unbounded"). + +**Step 2: Build + run the existing suite** + +Run: `dotnet test tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests 2>&1 | tail -5` +Expected: PASS. (No public seam exposes the compiled script here; Task 2's resolver test plus this wiring is the coverage — do not add reflection-based tests.) + +**Step 3: Commit** + +```bash +git add src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs +git commit -m "fix(scripts): memoize missing-assembly resolution on inbound API method compiles" +``` + +### Task 5: Attach resolver — CentralUI `ScriptAnalysisService` + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 3, Task 4, Task 6 + +**Files:** +- Modify: `src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs:44-58` + +**Step 1: Attach the resolver** to `DefaultOptions` (`.WithMetadataResolver(ZB.MOM.WW.ScadaBridge.ScriptAnalysis.CachingScriptMetadataResolver.Instance)` after `.AddReferences(...)`). `SandboxOptions` and every per-request `options` derived via `With*`/`Add*` inherit it automatically — `ScriptOptions` is immutable-with-copy. Note in the doc comment that editor diagnostics compile on every analysis request, so this path churned a full closure per keystroke-debounce before the fix. + +**Step 2: Build + run** + +Run: `dotnet test tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests 2>&1 | tail -5` +Expected: PASS. + +**Step 3: Commit** + +```bash +git add src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs +git commit -m "fix(scripts): memoize missing-assembly resolution in the UI script editor/sandbox" +``` + +### Task 6: Attach resolver — `RoslynScriptCompiler` (design-time deploy gate) + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 3, Task 4, Task 5 + +**Files:** +- Modify: `src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/RoslynScriptCompiler.cs:66-68` +- Test: `tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/RoslynScriptCompilerTests.cs` + +**Step 1: Attach the resolver** in `Compile`: + +```csharp + var options = ScriptOptions.Default + .WithReferences(references) + .WithMetadataResolver(CachingScriptMetadataResolver.Instance) + .WithImports(imports); +``` + +The per-call `ScriptOptions` object itself is cheap managed garbage; the references list already reuses the static `DefaultReferences` objects — only the resolver was missing. Every template/deploy validation compile previously minted the closure afresh. + +**Step 2: Add a behavior test** (append to `RoslynScriptCompilerTests`): compile the same trivial script twice via `RoslynScriptCompiler.Compile("return 1;")` and assert both return empty errors — then assert the shared cache took effect indirectly by pinning `CachingScriptMetadataResolver.Instance`'s process-wide identity is what `ScriptCompilationService` also uses (already covered by Task 3's `Assert.Same`); here just guard the happy path still compiles: + +```csharp + [Fact] + public void Compile_TwiceWithSharedResolver_StillCompilesCleanly() + { + Assert.Empty(RoslynScriptCompiler.Compile("return 1 + 1;")); + Assert.Empty(RoslynScriptCompiler.Compile("return 2 + 2;")); + } +``` + +**Step 3: Run** + +Run: `dotnet test tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests 2>&1 | tail -5` +Expected: PASS. + +**Step 4: Commit** + +```bash +git add src/ZB.MOM.WW.ScadaBridge.ScriptAnalysis/RoslynScriptCompiler.cs tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests/RoslynScriptCompilerTests.cs +git commit -m "fix(scripts): memoize missing-assembly resolution on the design-time compile gate" +``` + +### Task 7: Documentation sync + +**Classification:** trivial +**Estimated implement time:** ~4 min +**Parallelizable with:** none (write after code lands so line references are right) + +**Files:** +- Modify: `docs/requirements/Component-ScriptAnalysis.md` — add the resolver to the component's design: process-wide memoizing `MetadataReferenceResolver`, owned here, attached by all four compile surfaces; trust-model-neutral by construction. +- Modify: `docs/requirements/Component-SiteRuntime.md` — extend the note `5a781c70` added: static options fixed the per-compile *options* leak; the resolver cache fixes the per-compile *closure re-resolution* (the dominant term). +- Modify: `CLAUDE.md` — in *Akka.NET Conventions → Script trust model* bullet (or a nearby scripts note), one sentence: all four compile surfaces must attach `CachingScriptMetadataResolver.Instance`; new compile surfaces that skip it reintroduce a per-compile native metadata leak. + +**Step: Commit** + +```bash +git add docs/requirements/Component-ScriptAnalysis.md docs/requirements/Component-SiteRuntime.md CLAUDE.md +git commit -m "docs(scripts): record the shared caching metadata resolver invariant" +``` + +### Task 8: Milestone verification — build + targeted suites + +**Classification:** trivial +**Estimated implement time:** ~5 min (mostly wall-clock) +**Parallelizable with:** none + +**Steps:** + +1. `dotnet build ZB.MOM.WW.ScadaBridge.slnx 2>&1 | tail -3` — expect 0 errors. +2. `dotnet test tests/ZB.MOM.WW.ScadaBridge.ScriptAnalysis.Tests tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests 2>&1 | tail -8` — expect all green (TemplateEngine included because its validation path consumes `RoslynScriptCompiler`). +3. `git diff main --stat` review per Editing Rules, then done — **do not push or deploy**; live-gate is a separate operator step below. + +### Task 9 (operator-gated): Live verification, keyed to compile count — not elapsed time + +**Classification:** high-risk (production measurement; no code) +**Estimated implement time:** operator session + +Run on the docker rig first, then wonder-app-vd03 after deploy (tooling at `E:\ApiInstall\_tools\diag\`, baselines at `E:\ApiInstall\_leakcheck-post5a781c70\`): + +1. Baseline: `dotnet-gcdump collect` on a Site node; record the `AssemblyMetadata`/`PEReader`/`MetadataImageReference` triple (the reliable proxy — `NativeHeapMemoryBlock` gets truncated out of large reports). +2. Force **N real compiles**: edit a script's *body* and redeploy, N times. **A no-op redeploy compiles nothing** — `SiteScriptCompileCache` keys on code hash, so unchanged bodies are cache hits; the body must actually change each round. +3. Re-dump. Pass gate: triple grows ≤ ~10 objects per forced compile (the accepted `CreateFromAssemblyInternal` residual). Pre-fix behavior: ~300+ per compile. +4. Also expect the *steady-state* count to collapse on a restarted post-fix node: the 21-script site should hold ~1 closure set (+ statics) instead of 21 — thousands → hundreds, with a corresponding working-set drop. + +## Out of scope / follow-ups + +- `SiteScriptCompileCache` is unbounded across code revisions (every edited script body adds a permanent entry). After this fix each entry is small, but an LRU cap is a reasonable hygiene follow-up. +- The ~2–3 refs/compile `CreateFromAssemblyInternal` residual is inside Roslyn and not reachable from `ScriptOptions`; revisit only if a future measurement shows it matters. +- TLS/upstream Roslyn issue: consider filing upstream — `RuntimeMetadataReferenceResolver`'s lack of caching is a general scripting-host footgun. diff --git a/docs/plans/2026-08-12-script-compile-metadata-resolver-cache.md.tasks.json b/docs/plans/2026-08-12-script-compile-metadata-resolver-cache.md.tasks.json new file mode 100644 index 00000000..eabc3ab5 --- /dev/null +++ b/docs/plans/2026-08-12-script-compile-metadata-resolver-cache.md.tasks.json @@ -0,0 +1,15 @@ +{ + "planPath": "docs/plans/2026-08-12-script-compile-metadata-resolver-cache.md", + "tasks": [ + {"id": 1, "subject": "Task 1: CachingScriptMetadataResolver (TDD)", "status": "pending"}, + {"id": 2, "subject": "Task 2: End-to-end compile-count regression test", "status": "pending", "blockedBy": [1]}, + {"id": 3, "subject": "Task 3: Attach resolver — SiteRuntime ScriptCompilationService", "status": "pending", "blockedBy": [1]}, + {"id": 4, "subject": "Task 4: Attach resolver — InboundAPI InboundScriptExecutor", "status": "pending", "blockedBy": [1]}, + {"id": 5, "subject": "Task 5: Attach resolver — CentralUI ScriptAnalysisService", "status": "pending", "blockedBy": [1]}, + {"id": 6, "subject": "Task 6: Attach resolver — RoslynScriptCompiler deploy gate", "status": "pending", "blockedBy": [1]}, + {"id": 7, "subject": "Task 7: Documentation sync", "status": "pending", "blockedBy": [3, 4, 5, 6]}, + {"id": 8, "subject": "Task 8: Milestone verification — build + targeted suites", "status": "pending", "blockedBy": [2, 7]}, + {"id": 9, "subject": "Task 9 (operator-gated): Live compile-count-keyed gcdump gate", "status": "pending", "blockedBy": [8]} + ], + "lastUpdated": "2026-08-12" +}