perf(transport): make large-bundle apply linear in the template rewire passes (ScadaBridge#47)

Measured first: a scratch scaling probe (seed -> export -> load/preview/apply
into a fresh target and over itself, in-memory provider, every repository
call counted, DetectChanges counted via ChangeTracker.DetectingAllChanges)
showed load, preview and the instance pass scale linearly (500 / 1000 / 2000
instances apply in 0.13 / 0.27 / 0.35 s), but the post-flush template
rewire passes were quadratic: 100 / 400 / 800 composing, derived templates
applied in 0.5 / 8.1 / 26.5 s, with DetectChanges running 5N+10 times and
GetTemplateWithChildrenAsync 2N+2 times.

Cause: ResolveAlarmScriptLinksAsync, ResolveCompositionEdgesAsync and
ResolveInheritanceEdgesAsync found each template with
_dbContext.Templates.Local.FirstOrDefault(...) - per template, and per
composition edge / base lookup. Every DbSet.Local access runs a full
DetectChanges over everything the import has staged. The first two passes
also re-read every template's children one query at a time (a split query
per call on SQL Server).

Fix (no bundle-format or semantic change):
- each pass indexes the tracked templates by name once
  (SnapshotLocalTemplatesByName; ordinal, first wins like FirstOrDefault).
  These passes never stage a template or rename one, so the index stays
  exact for the pass;
- the alarm and composition passes read their templates' children in one
  batched GetTemplatesWithChildrenAsync, matched back by the stub's id;
- the alarm pass only loads templates whose bundled alarms name an
  on-trigger script (the only ones it can rewire).

Result on the same probe: 100 / 400 / 800 templates apply in
0.13 / 0.62 / 0.64 s; DetectChanges is a constant 9; per-template children
reads 0, batched reads 1 (add) / 2 (overwrite).

Evidence: ImportApplyScalingTests (Transport.IntegrationTests) exports 5 and
40 composing, derived templates with alarm -> script links, imports each
into an empty target (Add) and again (Overwrite), and asserts the imported
inheritance / composition / alarm-script links, zero per-template children
reads, at most 3 batched reads, and an identical DetectChanges count for 5
and 40 templates. Fails on the old importer (12 per-template reads), and
with a per-DTO Local snapshot reintroduced in one pass (DetectChanges 51
vs 16). Transport unit (198) and integration (138) suites pass.

Not changed: preview's per-name lookups for database connections,
notification lists and API methods stay per-name - moving them to an
in-memory ordinal match could change name matching under a case-insensitive
SQL collation.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
2026-11-09 13:29:51 -05:00
parent 51d2f6c906
commit 7153a1974e
3 changed files with 297 additions and 22 deletions
+1 -1
View File
@@ -303,7 +303,7 @@ Site identifiers and connection names are environment-specific, so a bundle expo
**Template library reads (review 2026-10-21 04/P4 + 08/P1).** A preview and each apply attempt read the target template library **once**, through body-free projections (`GetTemplateStubsAsync` for id/name maps, `GetAllTemplatesForAnalysisAsync` — untracked, members and lock flags but no script bodies — where members or edges are needed), held on a per-session `TemplateLibrary` and threaded to every consumer: preview diff names, blocker and locked-override scans, apply-time validation, the composition/inheritance rewire fallbacks (formerly a full library reload *inside* the per-template loop), the instance pass's name→id map (overlaid with the change tracker's just-flushed templates), the acyclicity guard and the stale-instance expansion. The full tracked graph is loaded only for the templates the bundle **Overwrites** (`GetTemplatesWithChildrenAsync` by name), since their child syncs mutate it. The acyclicity guard reads the change tracker's staged templates overlaid on that snapshot — the untouched part of the graph has not changed during the apply. Instance lookups are batched by name set (`ITemplateEngineRepository.GetInstancesByUniqueNamesAsync`, one tracked + hydrated read for preview and one for the instance pass, Rename targets included), and the preview resolves an existing instance's template / site / area names from the library, one site list and one area list per site, and reads shared scripts and external systems once each.
**Template second-pass rewires.** Bundles carry template-to-template references by **name**, not id (ids are environment-specific). After the templates are flushed, the importer runs three name-resolution passes over the imported templates so every cross-template FK points at the freshly-persisted target row: the **alarm on-trigger-script** links (`TemplateAlarm.OnTriggerScriptId`), the **composition** edges (`TemplateComposition`), and the **inheritance** edges (`Template.ParentTemplateId`, resolved from `BaseTemplateName` through the same rename/skip resolution map — a `null` base clears a stale edge on Overwrite). Each pass is best-effort: an unresolvable reference leaves the FK null and emits a warning audit row (`BundleImportAlarmScriptUnresolved` / `BundleImportCompositionUnresolved` / `BundleImportBaseTemplateUnresolved`) rather than aborting the import.
**Template second-pass rewires.** Bundles carry template-to-template references by **name**, not id (ids are environment-specific). After the templates are flushed, the importer runs three name-resolution passes over the imported templates so every cross-template FK points at the freshly-persisted target row: the **alarm on-trigger-script** links (`TemplateAlarm.OnTriggerScriptId`), the **composition** edges (`TemplateComposition`), and the **inheritance** edges (`Template.ParentTemplateId`, resolved from `BaseTemplateName` through the same rename/skip resolution map — a `null` base clears a stale edge on Overwrite). Each pass is best-effort: an unresolvable reference leaves the FK null and emits a warning audit row (`BundleImportAlarmScriptUnresolved` / `BundleImportCompositionUnresolved` / `BundleImportBaseTemplateUnresolved`) rather than aborting the import. Each pass indexes the change tracker's templates by name **once** (a `DbSet.Local` access runs a full `DetectChanges` over everything the import has staged, so the former per-template and per-edge scans made apply time quadratic in bundle size) and reads the children it rewires in **one** batched `GetTemplatesWithChildrenAsync`; the alarm pass loads only templates whose bundled alarms name an on-trigger script (Gitea #47 — measured on the in-memory provider, 800 composing, derived templates applied in 0.6 s instead of 26.5 s; `ImportApplyScalingTests` pins the cost as independent of bundle size).
**Rename call-site limitation (load-bearing).** A `Rename` resolution renames the entity being imported (e.g. a SharedScript `Foo` → `Bar`), but it does **not** rewrite call sites inside importing scripts — a script body that calls `CallShared("Foo")` still says `Foo` after import. So the target DB can end up with scripts referencing the old name. This is deliberate: Pass 1 name resolution registers **both** the original and renamed names as resolvable (so the import isn't blocked), and the authoritative check is the target's own **deploy-time validation** (`ScriptCompiler.TryCompile` in the Template Engine deploy gate), which recompiles against the actual persisted catalog. Operators renaming a shared dependency must fix the calling scripts on the target (or re-export with the dependency included under its original name).