perf(site-runtime): warm the next startup batch during the stagger delay and compile a batch's distinct units in parallel (review 2026-10-21 01/SR-P3)

DeployCompileValidator.WarmCompileCache dedupes script bodies/trigger expressions across a batch and compiles them with Parallel.ForEach (half the cores). CreateInstanceActorBatch starts the next batch's warm as it arms the stagger timer; creation order and delay are unchanged. The warm's parsed configs are reused at creation.
2026-10-22 08:22:26 -04:00
parent dde22a3dd1
commit 999653a6b9
5 changed files with 256 additions and 29 deletions
@@ -138,7 +138,7 @@ flowchart TD
 >
 > **Residuals still open**, recorded precisely in the decision memo: a removal is not durable across a restart; and a delete is undetectable on a node that never adopted the row. See `docs/plans/2026-08-17-dcl-connection-arbitration-decision.md`.

-> **Per-instance compilation during staggered startup — CLOSED (WP3.1)**: this was a deferred optimization (P6) affecting failover time-to-recover; each Instance Actor Roslyn-compiled its own scripts inside its own start, serialising a site's whole recovery behind compilation. Each staggered startup batch now runs a **pre-warm step**: before the batch's Instance Actors are created, the Deployment Manager compiles that batch's distinct script bodies and trigger expressions on a background task and pipes `BatchCompileWarmed` back, so every `PreStart` compile is a cache hit. Instance Actor start still calls the compilation service — it remains the correctness backstop — but in every warmed path those calls are memoised lookups. Cost: one extra message per batch.
+> **Per-instance compilation during staggered startup — CLOSED (WP3.1)**: this was a deferred optimization (P6) affecting failover time-to-recover; each Instance Actor Roslyn-compiled its own scripts inside its own start, serialising a site's whole recovery behind compilation. Each staggered startup batch now runs a **pre-warm step**: before the batch's Instance Actors are created, the Deployment Manager compiles that batch's distinct script bodies and trigger expressions on a background task and pipes `BatchCompileWarmed` back, so every `PreStart` compile is a cache hit. Instance Actor start still calls the compilation service — it remains the correctness backstop — but in every warmed path those calls are memoised lookups. Cost: one extra message per batch. **Pipelined and parallel since review 2026-10-21 01/SR-P3:** the warm used to be one serial task per batch, and batch k+1's warm started only after batch k's actors existed *and* the stagger delay had elapsed, so recovery time was the sum of every distinct compile plus every delay, on one thread. Now (a) when a batch is created the **next** batch's warm starts immediately, running during the stagger delay, and (b) a batch's distinct compile units (script bodies + Expression triggers, de-duplicated by code first so N instances of one template do not race N identical cold compiles) are compiled with `Parallel.ForEach` at up to half the cores (`DeployCompileValidator.WarmCompileCache`; `ScriptCompilationService`/`SiteScriptCompileCache` are thread-safe — `ConcurrentDictionary` with a double-checked eviction lock). Creation order and the stagger delay are unchanged — a batch is still created only once its timer has fired **and** its warm has landed — so the reconnection-storm protection is exactly as before. The warm also returns each config parsed, which creation reuses (01/SR-P5).

 > **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.

@@ -396,7 +396,7 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
        Receive<DeployCompileWarmed>(HandleDeployCompileWarmed);
        // A staggered startup batch's compiles have been pre-warmed off-thread;
        // create the batch's Instance Actors now that their PreStart compiles are hits.
-        Receive<BatchCompileWarmed>(msg => CreateInstanceActorBatch(msg.Batch));
+        Receive<BatchCompileWarmed>(CreateInstanceActorBatch);

        // Notify-and-fetch: central sends a small RefreshDeploymentCommand;
        // the active singleton fetches the flattened config over HTTP, then reuses the
@@ -830,32 +830,72 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
        var startIdx = state.NextIndex;
        var endIdx = Math.Min(startIdx + batchSize, state.Configs.Count);

-        var validator = _deployCompileValidator;
-        var batchConfigs = new string[endIdx - startIdx];
-        for (var i = startIdx; i < endIdx; i++)
-            batchConfigs[i - startIdx] = state.Configs[i].ConfigJson;
+        // Review 2026-10-21 01/SR-P3: the previous batch normally started THIS batch's warm when
+        // it scheduled the stagger delay, so by now it is usually done. Only the first batch (or
+        // a batch whose prewarm belongs to a different boot load) warms from here.
+        var warm = _nextBatchWarm is { } prewarm &&
+                   ReferenceEquals(prewarm.Configs, state.Configs) &&
+                   prewarm.StartIndex == startIdx
+            ? prewarm.Warm
+            : StartBatchWarm(state.Configs, startIdx, endIdx);
+        _nextBatchWarm = null;

-        Task.Run(() =>
+        warm.ContinueWith(t => new BatchCompileWarmed(msg, t.IsCompletedSuccessfully ? t.Result : null))
+            .PipeTo(Self);
+    }
+
+    /// <summary>
+    /// Upper bound on concurrent Roslyn compiles while warming a startup batch (01/SR-P3). Half
+    /// the cores: boot is a burst, but the node is also re-establishing DCL connections and
+    /// starting the batch already warmed, so the warm must not take every core.
+    /// </summary>
+    private static int StartupWarmParallelism => Math.Max(1, Environment.ProcessorCount / 2);
+
+    /// <summary>
+    /// The last startup batch's look-ahead warm: the boot load it belongs to (compared by
+    /// reference), the batch's first index, and the task that parses + warms it (01/SR-P3).
+    /// </summary>
+    private (List<DeployedInstance> Configs, int StartIndex, Task<FlattenedConfiguration?[]> Warm)? _nextBatchWarm;
+
+    /// <summary>
+    /// Parses one staggered startup batch's configs and warms their distinct compile units in
+    /// parallel, off the actor thread (review 2026-10-21 01/SR-P3). Returns the parsed configs
+    /// index-aligned with the batch (a null slot = did not parse) so creation reuses them instead
+    /// of deserializing again (01/SR-P5). Never faults: a failure just yields null slots, and the
+    /// Instance Actor's own compile/parse is the backstop exactly as before the warm existed.
+    /// </summary>
+    private Task<FlattenedConfiguration?[]> StartBatchWarm(List<DeployedInstance> configs, int startIdx, int endIdx)
+    {
+        var validator = _deployCompileValidator;
+        var batchJson = new string[endIdx - startIdx];
+        for (var i = startIdx; i < endIdx; i++)
+            batchJson[i - startIdx] = configs[i].ConfigJson;
+        var parallelism = StartupWarmParallelism;
+
+        return Task.Run(() =>
        {
-            foreach (var configJson in batchConfigs)
+            var parsed = new FlattenedConfiguration?[batchJson.Length];
+            for (var i = 0; i < batchJson.Length; i++)
            {
-                try
-                {
-                    // Verdict discarded: startup does not gate on compile failures (a failing
-                    // script is logged by the Instance Actor and leaves the rest running).
-                    // This call exists only to populate the compile cache owned by the
-                    // shared ScriptCompilationService (SiteScriptCompileCache).
-                    validator.Validate(configJson);
-                }
-                catch
-                {
-                    // Best-effort warm — a throw here just means that instance's PreStart
-                    // compiles the hard way, exactly as it did before the warm-then-gate change.
-                }
+                if (DeployCompileValidator.TryParse(batchJson[i], out var config, out _))
+                    parsed[i] = config;
            }

-            return new BatchCompileWarmed(msg);
-        }).PipeTo(Self);
+            try
+            {
+                // Verdicts discarded: startup does not gate on compile failures (a failing
+                // script is logged by the Instance Actor and leaves the rest running). This
+                // exists only to populate the compile cache owned by the shared
+                // ScriptCompilationService (SiteScriptCompileCache).
+                validator.WarmCompileCache(parsed.OfType<FlattenedConfiguration>().ToList(), parallelism);
+            }
+            catch (Exception ex)
+            {
+                _logger.LogDebug(ex, "Startup batch compile warm threw; the Instance Actors will compile instead");
+            }
+
+            return parsed;
+        });
    }

    /// <summary>
@@ -863,8 +903,9 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
    /// schedules the next batch, with a configurable delay between batches to prevent
    /// reconnection storms on failover.
    /// </summary>
-    private void CreateInstanceActorBatch(StartNextBatch msg)
+    private void CreateInstanceActorBatch(BatchCompileWarmed warmed)
    {
+        var msg = warmed.Batch;
        var state = msg.State;
        var batchSize = _options.StartupBatchSize;
        var startIdx = state.NextIndex;
@@ -878,8 +919,12 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
        {
            var config = state.Configs[i];
            // CreateInstanceActor ensures this config's DCL connections (register row 85). A
-            // refused actor name is logged inside and skips just that instance (01/SR-C1).
-            CreateInstanceActor(config.InstanceUniqueName, config.ConfigJson, config.RevisionHash, out _);
+            // refused actor name is logged inside and skips just that instance (01/SR-C1). The
+            // warm's parsed config is reused rather than deserialized again (01/SR-P5).
+            var parsed = warmed.Parsed is { } all && i - startIdx < all.Length ? all[i - startIdx] : null;
+            CreateInstanceActor(
+                config.InstanceUniqueName, config.ConfigJson, config.RevisionHash, out _,
+                parsed: parsed);
        }

        UpdateInstanceCounts();
@@ -888,6 +933,14 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
        if (endIdx < state.Configs.Count)
        {
            var nextState = new BatchState(state.Configs, endIdx);
+
+            // Review 2026-10-21 01/SR-P3: warm the NEXT batch now, while the stagger delay runs,
+            // instead of after it. Actor creation order and the delay are unchanged — the next
+            // batch is still created only once the timer fires AND its warm has landed — so the
+            // reconnection-storm protection is exactly as before; only the compiles moved earlier.
+            _nextBatchWarm = (state.Configs, endIdx, StartBatchWarm(
+                state.Configs, endIdx, Math.Min(endIdx + batchSize, state.Configs.Count)));
+
            Timers.StartSingleTimer(
                "startup-batch",
                new StartNextBatch(nextState),
@@ -4058,7 +4111,12 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
    /// Piped back to self once a staggered startup batch's compiles have been
    /// pre-warmed, carrying the original batch message so actor creation resumes unchanged.
    /// </summary>
-    internal sealed record BatchCompileWarmed(StartNextBatch Batch);
+    /// <param name="Batch">The batch message the warm ran for.</param>
+    /// <param name="Parsed">
+    /// The batch's configs parsed by the warm, index-aligned (01/SR-P5); null when the warm task
+    /// itself failed, in which case each config is parsed at creation instead.
+    /// </param>
+    internal sealed record BatchCompileWarmed(StartNextBatch Batch, FlattenedConfiguration?[]? Parsed = null);

    /// <summary>
    /// A lifecycle command queued because its instance's deploy was mid-compile-warm, with
@@ -154,6 +154,69 @@ public sealed class DeployCompileValidator
        return errors;
    }

+    /// <summary>
+    /// Populates the shared compile cache for a whole startup batch (review 2026-10-21 01/SR-P3):
+    /// every DISTINCT script body and trigger expression across <paramref name="configs"/> is
+    /// compiled once, up to <paramref name="maxDegreeOfParallelism"/> at a time. Same compile units
+    /// as <see cref="Validate(FlattenedConfiguration, string?)"/> — scripts and alarm on-trigger
+    /// scripts against <c>ScriptGlobals</c>, Expression triggers against
+    /// <c>TriggerExpressionGlobals</c> — but deduplicated first, because the cache is keyed on
+    /// code: N instances of one template would otherwise race N identical cold compiles in
+    /// parallel. Best-effort: verdicts are discarded and a throwing unit is skipped, since every
+    /// Instance Actor's own compile remains the correctness backstop.
+    /// </summary>
+    /// <param name="configs">The batch's parsed configurations.</param>
+    /// <param name="maxDegreeOfParallelism">Upper bound on concurrent Roslyn compiles.</param>
+    public void WarmCompileCache(IReadOnlyList<FlattenedConfiguration> configs, int maxDegreeOfParallelism)
+    {
+        var units = new Dictionary<(bool IsTrigger, string Code), string>();
+        foreach (var config in configs)
+        {
+            foreach (var script in config.Scripts)
+            {
+                units.TryAdd((false, script.Code), script.CanonicalName);
+                AddTriggerUnit(units, script.TriggerType, script.TriggerConfiguration,
+                    $"script-trigger-{script.CanonicalName}");
+            }
+
+            // Alarm on-trigger scripts compile the referenced SCRIPT's body against ScriptGlobals,
+            // i.e. the same cache key as that script — already a unit above.
+            foreach (var alarm in config.Alarms)
+                AddTriggerUnit(units, alarm.TriggerType, alarm.TriggerConfiguration,
+                    $"alarm-trigger-expr-{alarm.CanonicalName}");
+        }
+
+        Parallel.ForEach(
+            units,
+            new ParallelOptions { MaxDegreeOfParallelism = Math.Max(1, maxDegreeOfParallelism) },
+            unit =>
+            {
+                try
+                {
+                    if (unit.Key.IsTrigger)
+                        _compilationService.CompileTriggerExpression(unit.Value, unit.Key.Code);
+                    else
+                        _compilationService.Compile(unit.Value, unit.Key.Code);
+                }
+                catch
+                {
+                    // Best-effort warm — the Instance Actor's PreStart compiles it the hard way.
+                }
+            });
+    }
+
+    private static void AddTriggerUnit(
+        Dictionary<(bool IsTrigger, string Code), string> units,
+        string? triggerType, string? triggerConfigJson, string compileName)
+    {
+        if (!string.Equals(triggerType, "Expression", StringComparison.OrdinalIgnoreCase))
+            return;
+
+        var expression = TriggerExpressionGlobals.ExtractExpression(triggerConfigJson);
+        if (expression != null)
+            units.TryAdd((true, expression), compileName);
+    }
+
    /// <summary>
    /// Review 2026-10-21 01/SR-C1: every name that becomes an Akka actor name on the site — the
    /// Instance Actor itself (<c>DeploymentManagerActor.CreateInstanceActor</c>) and its
@@ -292,8 +292,10 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable

        AwaitAssert(() =>
        {
-            // One compile, then repeated hits: the second pre-warm plus both PreStarts.
-            Assert.True(_compileCache.Hits >= 3,
+            // One compile, then repeated hits from both PreStarts. (The batch warm dedupes
+            // distinct bodies before compiling — review 2026-10-21 01/SR-P3 — so the second
+            // config no longer costs a warm-side lookup of its own.)
+            Assert.True(_compileCache.Hits >= 2,
                $"expected the pre-warmed body to be served from cache, saw {_compileCache.Hits} hits");
        }, TimeSpan.FromSeconds(20));

@@ -0,0 +1,104 @@
+using System.Text.Json;
+using Akka.Actor;
+using Akka.TestKit.Xunit2;
+using Microsoft.Extensions.Logging.Abstractions;
+using ZB.MOM.WW.ScadaBridge.Commons.Messages.DebugView;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening;
+using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors;
+using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence;
+using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
+using ZB.MOM.WW.ScadaBridge.TestSupport;
+
+namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors;
+
+/// <summary>
+/// Review 2026-10-21 01/SR-P3: boot/failover compile warm was serial within a batch and
+/// serialised across batches — batch k+1's compiles did not start until batch k's actors existed
+/// AND the stagger delay had elapsed, so recovery time was the sum of every distinct compile plus
+/// every delay, on one thread. The next batch is now warmed while the current batch's delay runs,
+/// and a batch's distinct compile units are warmed in parallel.
+/// </summary>
+public class StartupBatchPrewarmTests : TestKit, IDisposable
+{
+    private readonly SiteStorageService _storage;
+    private readonly SiteScriptCompileCache _compileCache = new();
+    private readonly ScriptCompilationService _compilationService;
+    private readonly SharedScriptLibrary _sharedScriptLibrary;
+    private readonly TestLocalDb _localDb;
+
+    /// <summary>Creates a temporary LocalDb-backed site store and an observable compile cache.</summary>
+    public StartupBatchPrewarmTests()
+    {
+        _localDb = TestLocalDb.CreateTemp("startup-prewarm");
+        _storage = new SiteStorageService(_localDb.Db, NullLogger<SiteStorageService>.Instance);
+        _storage.InitializeAsync().GetAwaiter().GetResult();
+        _compilationService = new ScriptCompilationService(
+            NullLogger<ScriptCompilationService>.Instance, _compileCache);
+        _sharedScriptLibrary = new SharedScriptLibrary(
+            _compilationService, NullLogger<SharedScriptLibrary>.Instance);
+    }
+
+    /// <inheritdoc />
+    void IDisposable.Dispose()
+    {
+        Shutdown();
+        var path = _localDb.Path;
+        _localDb.Dispose();
+        TestLocalDb.DeleteFiles(path);
+    }
+
+    private static string ConfigWithScript(string name, int distinct) => JsonSerializer.Serialize(new FlattenedConfiguration
+    {
+        InstanceUniqueName = name,
+        Attributes = [new ResolvedAttribute { CanonicalName = "A", Value = "1", DataType = "Int32" }],
+        Scripts = [new ResolvedScript { CanonicalName = "S", Code = $"return {distinct} * 2;" }]
+    });
+
+    private async Task<bool> IsRunningAsync(IActorRef dm, string name)
+    {
+        var snapshot = await dm.Ask<DebugViewSnapshot>(
+            new DebugSnapshotRequest(name, Guid.NewGuid().ToString("N")), TimeSpan.FromSeconds(5));
+        return !snapshot.InstanceNotFound;
+    }
+
+    /// <summary>
+    /// Two one-instance batches with a long stagger delay: batch 2's script is compiled while the
+    /// delay after batch 1 is still running, i.e. BEFORE batch 2's Instance Actor exists. Before
+    /// the fix its compile only started once the delay had elapsed.
+    /// </summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task NextBatch_IsWarmedDuringTheStaggerDelay()
+    {
+        await _storage.StoreDeployedConfigAsync("B0", ConfigWithScript("B0", 1001), "d0", "r0", true);
+        await _storage.StoreDeployedConfigAsync("B1", ConfigWithScript("B1", 1002), "d1", "r1", true);
+
+        var dm = ActorOf(Props.Create(() => new DeploymentManagerActor(
+            _storage, _compilationService, _sharedScriptLibrary, null,
+            new SiteRuntimeOptions
+            {
+                StartupBatchSize = 1, StartupBatchDelayMs = 6000, InstanceSweepIntervalSeconds = 0
+            },
+            NullLogger<DeploymentManagerActor>.Instance,
+            dclManager: null, healthCollector: null, serviceProvider: null,
+            loggerFactory: null, configFetcher: null)));
+
+        // Batch 1 is up...
+        await AwaitAssertAsync(async () => Assert.True(await IsRunningAsync(dm, "B0") || await IsRunningAsync(dm, "B1")),
+            TimeSpan.FromSeconds(20), TimeSpan.FromMilliseconds(50));
+
+        // ...and both scripts are compiled while exactly one instance is running.
+        await AwaitAssertAsync(() =>
+        {
+            Assert.True(_compileCache.Count >= 2, $"compile cache holds {_compileCache.Count} entries");
+            return Task.CompletedTask;
+        }, TimeSpan.FromSeconds(3), TimeSpan.FromMilliseconds(50));
+        Assert.False(await IsRunningAsync(dm, "B0") && await IsRunningAsync(dm, "B1"),
+            "the second batch was created early — the stagger delay must be unchanged");
+
+        // The stagger itself is unchanged: batch 2 still starts after the delay.
+        await AwaitAssertAsync(async () =>
+            Assert.True(await IsRunningAsync(dm, "B0") && await IsRunningAsync(dm, "B1")),
+            TimeSpan.FromSeconds(20), TimeSpan.FromMilliseconds(100));
+    }
+}