perf(template-engine): cache script compile verdicts by code hash — unchanged scripts compile once per process
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
This commit is contained in:
@@ -0,0 +1,74 @@
|
||||
using System.Collections.Concurrent;
|
||||
using System.Security.Cryptography;
|
||||
using System.Text;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.TemplateEngine.Validation;
|
||||
|
||||
/// <summary>
|
||||
/// Process-wide cache of script compile verdicts, keyed on a SHA-256 hash of the
|
||||
/// script code. The verdict (clean/compiles, or the name-free error text) is a pure
|
||||
/// function of the code plus the compile-time-static <c>ScriptTrustPolicy</c> and
|
||||
/// globals surface, so it is sound to memoise it across the whole process: an
|
||||
/// unchanged script runs the forbidden-API analysis and the Roslyn compile exactly
|
||||
/// once, removing both the repeat CPU cost and the repeat compiled-assembly load.
|
||||
///
|
||||
/// <para>
|
||||
/// The stored error is deliberately <em>name-free</em> — the caller's script name is
|
||||
/// formatted into the returned message at read time, so two callers submitting the
|
||||
/// same bad code each see their own name in the failure text while sharing one verdict.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// Bounded at <see cref="MaxEntries"/> entries; on overflow the cache is simply
|
||||
/// cleared (the verdict is cheap to recompute, so a coarse reset is safe and avoids
|
||||
/// eviction bookkeeping). <see cref="Hits"/>/<see cref="Count"/>/<see cref="Clear"/>
|
||||
/// are exposed for tests and diagnostics.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public static class ScriptCompileVerdictCache
|
||||
{
|
||||
/// <summary>Upper bound on cached entries; the cache is cleared wholesale on overflow.</summary>
|
||||
private const int MaxEntries = 4096;
|
||||
|
||||
private static readonly ConcurrentDictionary<string, (bool Ok, string? Error)> Cache = new();
|
||||
private static long _hits;
|
||||
|
||||
/// <summary>Number of cache hits observed since the last <see cref="Clear"/>.</summary>
|
||||
public static long Hits => Interlocked.Read(ref _hits);
|
||||
|
||||
/// <summary>Current number of cached verdicts.</summary>
|
||||
public static int Count => Cache.Count;
|
||||
|
||||
/// <summary>
|
||||
/// Returns the cached verdict for <paramref name="code"/>, or computes it via
|
||||
/// <paramref name="factory"/> and caches it on a miss. A hit increments
|
||||
/// <see cref="Hits"/>. The verdict's error text must be name-free — the caller
|
||||
/// formats the script name in on return.
|
||||
/// </summary>
|
||||
public static (bool Ok, string? Error) GetOrAdd(string code, Func<(bool Ok, string? Error)> factory)
|
||||
{
|
||||
var key = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(code)));
|
||||
|
||||
if (Cache.TryGetValue(key, out var verdict))
|
||||
{
|
||||
Interlocked.Increment(ref _hits);
|
||||
return verdict;
|
||||
}
|
||||
|
||||
verdict = factory();
|
||||
|
||||
// Coarse bound: on overflow drop everything rather than track evictions.
|
||||
if (Cache.Count >= MaxEntries)
|
||||
Cache.Clear();
|
||||
|
||||
Cache[key] = verdict;
|
||||
return verdict;
|
||||
}
|
||||
|
||||
/// <summary>Clears all cached verdicts and resets the hit counter.</summary>
|
||||
public static void Clear()
|
||||
{
|
||||
Cache.Clear();
|
||||
Interlocked.Exchange(ref _hits, 0);
|
||||
}
|
||||
}
|
||||
@@ -35,17 +35,28 @@ public class ScriptCompiler
|
||||
if (string.IsNullOrWhiteSpace(code))
|
||||
return Result<bool>.Failure($"Script '{scriptName}' has empty code.");
|
||||
|
||||
// Authoritative forbidden-API verdict first — a security violation must
|
||||
// gate the script regardless of whether it otherwise compiles.
|
||||
var violations = ScriptTrustValidator.FindViolations(code);
|
||||
if (violations.Count > 0)
|
||||
return Result<bool>.Failure($"Script '{scriptName}' uses forbidden API: {violations[0]}");
|
||||
// The verdict is a pure function of the code (the forbidden-API policy and
|
||||
// globals surface are compile-time static), so memoise it process-wide: an
|
||||
// unchanged script compiles exactly once per process. The cached error is
|
||||
// name-free — the script name is formatted in below at read time.
|
||||
var (ok, error) = ScriptCompileVerdictCache.GetOrAdd(code, () =>
|
||||
{
|
||||
// Authoritative forbidden-API verdict first — a security violation must
|
||||
// gate the script regardless of whether it otherwise compiles.
|
||||
var violations = ScriptTrustValidator.FindViolations(code);
|
||||
if (violations.Count > 0)
|
||||
return (false, $"uses forbidden API: {violations[0]}");
|
||||
|
||||
// Real CSharpScript compile against the runtime-mirroring globals surface.
|
||||
var errors = RoslynScriptCompiler.Compile(code, typeof(ScriptCompileSurface));
|
||||
if (errors.Count > 0)
|
||||
return Result<bool>.Failure($"Script '{scriptName}' failed to compile: {errors[0]}");
|
||||
// Real CSharpScript compile against the runtime-mirroring globals surface.
|
||||
var errors = RoslynScriptCompiler.Compile(code, typeof(ScriptCompileSurface));
|
||||
if (errors.Count > 0)
|
||||
return (false, $"failed to compile: {errors[0]}");
|
||||
|
||||
return Result<bool>.Success(true);
|
||||
return (true, (string?)null);
|
||||
});
|
||||
|
||||
return ok
|
||||
? Result<bool>.Success(true)
|
||||
: Result<bool>.Failure($"Script '{scriptName}' {error}");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -135,4 +135,35 @@ public class ScriptCompilerTests
|
||||
Assert.NotNull(error);
|
||||
Assert.Contains("forbidden", error, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
// --- Compile-verdict cache: unchanged code compiles once per process ---
|
||||
|
||||
[Fact]
|
||||
public void TryCompile_SameCodeTwice_SecondCallIsCacheHit()
|
||||
{
|
||||
ScriptCompileVerdictCache.Clear();
|
||||
var c = new ScriptCompiler();
|
||||
c.TryCompile("return 1;", "A");
|
||||
var hitsBefore = ScriptCompileVerdictCache.Hits;
|
||||
var r = c.TryCompile("return 1;", "B"); // same code, different name
|
||||
Assert.True(r.IsSuccess);
|
||||
Assert.Equal(hitsBefore + 1, ScriptCompileVerdictCache.Hits);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TryCompile_CachedFailure_FormatsNameForEachCaller()
|
||||
{
|
||||
// The stored verdict is name-free; the script name is formatted in at read
|
||||
// time, so two callers of the same bad code get their own name in the message.
|
||||
ScriptCompileVerdictCache.Clear();
|
||||
var c = new ScriptCompiler();
|
||||
var first = c.TryCompile("var x = NoSuchThing.Foo();", "Alpha");
|
||||
var second = c.TryCompile("var x = NoSuchThing.Foo();", "Beta"); // cache hit
|
||||
|
||||
Assert.True(first.IsFailure);
|
||||
Assert.True(second.IsFailure);
|
||||
Assert.Contains("'Alpha'", first.Error);
|
||||
Assert.Contains("'Beta'", second.Error);
|
||||
Assert.Contains("compile", second.Error, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user