docs(plans): script-compile metadata resolver cache plan
This commit is contained in:
Joseph Doherty
@@ -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<object?>`'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;
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
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<PortableExecutableReference> 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;
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
///
|
||||
/// <para>
|
||||
/// <b>Why this exists.</b> The shared <c>ScriptOptions</c> on every compile
|
||||
/// surface carry only the direct API-surface references; each
|
||||
/// <c>script.Compile()</c> binds their transitive closure, and every
|
||||
/// transitively-referenced assembly is resolved through the options'
|
||||
/// <see cref="MetadataReferenceResolver"/>. Roslyn's
|
||||
/// <c>RuntimeMetadataReferenceResolver</c> has NO cross-compilation cache: each
|
||||
/// resolution calls <c>MetadataReference.CreateFromFile</c>, which eagerly
|
||||
/// copies the whole assembly into native memory (<c>AssemblyMetadata</c> →
|
||||
/// <c>PEReader</c> → <c>NativeHeapMemoryBlock</c>). 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.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// <b>Trust model unaffected.</b> 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.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// <b>Cache-correctness assumptions.</b> The missing-assembly cache keys on the
|
||||
/// assembly identity display name and deliberately ignores the requesting
|
||||
/// <c>definition</c> (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
|
||||
/// <c>GetOrAdd</c> 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 <see cref="ScriptTrustPolicy.AnalysisReferences"/>.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public sealed class CachingScriptMetadataResolver : MetadataReferenceResolver
|
||||
{
|
||||
/// <summary>
|
||||
/// The shared process-wide instance every script-compile surface attaches via
|
||||
/// <c>ScriptOptions.WithMetadataResolver</c>. Decorates
|
||||
/// <see cref="ScriptOptions.Default"/>'s resolver — the exact resolver those
|
||||
/// surfaces used implicitly before this fix.
|
||||
/// </summary>
|
||||
public static readonly CachingScriptMetadataResolver Instance =
|
||||
new(ScriptOptions.Default.MetadataResolver);
|
||||
|
||||
private readonly MetadataReferenceResolver _inner;
|
||||
|
||||
private readonly ConcurrentDictionary<string, PortableExecutableReference?> _missingByIdentity =
|
||||
new(StringComparer.OrdinalIgnoreCase);
|
||||
|
||||
private readonly ConcurrentDictionary<(string Reference, string? BaseFilePath, MetadataReferenceProperties Properties),
|
||||
ImmutableArray<PortableExecutableReference>> _referencesByPath = new();
|
||||
|
||||
/// <summary>Creates a decorator over the given inner resolver. Exposed for tests; production uses <see cref="Instance"/>.</summary>
|
||||
/// <param name="inner">The resolver whose results are memoized.</param>
|
||||
public CachingScriptMetadataResolver(MetadataReferenceResolver inner) => _inner = inner;
|
||||
|
||||
/// <inheritdoc />
|
||||
public override bool ResolveMissingAssemblies => _inner.ResolveMissingAssemblies;
|
||||
|
||||
/// <inheritdoc />
|
||||
public override PortableExecutableReference? ResolveMissingAssembly(
|
||||
MetadataReference definition, AssemblyIdentity referenceIdentity)
|
||||
=> _missingByIdentity.GetOrAdd(
|
||||
referenceIdentity.GetDisplayName(),
|
||||
_ => _inner.ResolveMissingAssembly(definition, referenceIdentity));
|
||||
|
||||
/// <inheritdoc />
|
||||
public override ImmutableArray<PortableExecutableReference> ResolveReference(
|
||||
string reference, string? baseFilePath, MetadataReferenceProperties properties)
|
||||
=> _referencesByPath.GetOrAdd(
|
||||
(reference, baseFilePath, properties),
|
||||
_ => _inner.ResolveReference(reference, baseFilePath, properties));
|
||||
|
||||
/// <inheritdoc />
|
||||
public override bool Equals(object? other) => ReferenceEquals(this, other);
|
||||
|
||||
/// <inheritdoc />
|
||||
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
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
[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<object?>(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<PortableExecutableReference> 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
|
||||
/// <para>
|
||||
/// Static options alone are NOT enough: every <c>script.Compile()</c> binds the
|
||||
/// transitive closure of these references, and Roslyn's default resolver
|
||||
/// re-resolves that closure through <c>MetadataReference.CreateFromFile</c> 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
|
||||
/// <see cref="CachingScriptMetadataResolver"/> memoizes those resolutions
|
||||
/// process-wide; see its doc for the full mechanism.
|
||||
/// </para>
|
||||
```
|
||||
|
||||
**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.
|
||||
@@ -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"
|
||||
}
|
||||
Reference in New Issue
Block a user