From 7153a1974e8aa477de1d18e40a3814ba105a0269 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 9 Nov 2026 13:29:51 -0500 Subject: [PATCH] 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 --- docs/requirements/Component-Transport.md | 2 +- .../Import/BundleImporter.cs | 104 +++++++-- .../Import/ImportApplyScalingTests.cs | 213 ++++++++++++++++++ 3 files changed, 297 insertions(+), 22 deletions(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/ImportApplyScalingTests.cs diff --git a/docs/requirements/Component-Transport.md b/docs/requirements/Component-Transport.md index 579a40b4..39272d11 100644 --- a/docs/requirements/Component-Transport.md +++ b/docs/requirements/Component-Transport.md @@ -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). diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs index 3ecf8159..82726e6e 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs @@ -3055,10 +3055,18 @@ public sealed class BundleImporter : IBundleImporter { if (dtos.Count == 0) return; + // Gitea #47: only a template whose bundled alarms name an on-trigger script + // has a link to rewire, so only those are loaded — and in one batched read, + // not one children query per template. The tracked-template name index is + // built once (each DbSet.Local access runs DetectChanges over the whole + // tracker, which made this pass quadratic in the bundle's size). + var localByName = SnapshotLocalTemplatesByName(); + var targets = new List<(TemplateDto Dto, Template Stub)>(); foreach (var dto in dtos) { var resolution = ResolveOrDefault(resolutionMap, "Template", dto.Name); if (resolution.Action == ResolutionAction.Skip) continue; + if (!dto.Alarms.Any(a => !string.IsNullOrEmpty(a.OnTriggerScriptName))) continue; // Resolve the imported template by its post-rename name. Skip- // resolved DTOs were already filtered out above; everything else @@ -3066,16 +3074,19 @@ public sealed class BundleImporter : IBundleImporter var importedName = resolution.Action == ResolutionAction.Rename ? (resolution.RenameTo ?? dto.Name) : dto.Name; + if (localByName.TryGetValue(importedName, out var stub)) + targets.Add((dto, stub)); + } + if (targets.Count == 0) return; - // Use the children-loaded variant so the alarm + script collections - // are populated — the unfiltered GetAllTemplatesAsync returns the - // tracked entities EF assigned ids to during the flush, but the - // navigation collections aren't necessarily included. - var stub = _dbContext.Templates.Local.FirstOrDefault(t => - string.Equals(t.Name, importedName, StringComparison.Ordinal)); - if (stub is null) continue; - var template = await _templateRepo.GetTemplateWithChildrenAsync(stub.Id, ct).ConfigureAwait(false); - if (template is null) continue; + // Use the children-loaded read so the alarm + script collections are + // populated — the tracked entities EF assigned ids to during the flush + // don't necessarily have their navigation collections included. + var loadedById = await LoadTemplatesWithChildrenAsync(targets.Select(t => t.Stub), ct).ConfigureAwait(false); + + foreach (var (dto, stub) in targets) + { + if (!loadedById.TryGetValue(stub.Id, out var template)) continue; foreach (var alarmDto in dto.Alarms) { @@ -3142,6 +3153,12 @@ public sealed class BundleImporter : IBundleImporter { if (dtos.Count == 0) return; + // Gitea #47: one tracked-template name index for the pass (this pass stages + // composition rows only, never a template, so the index stays exact) and one + // batched children read for every parent, instead of a DbSet.Local scan + // (= a full DetectChanges) and a children query per template + per edge. + var localByName = SnapshotLocalTemplatesByName(); + var targets = new List<(TemplateDto Dto, Template Stub)>(); foreach (var dto in dtos) { var resolution = ResolveOrDefault(resolutionMap, "Template", dto.Name); @@ -3152,11 +3169,16 @@ public sealed class BundleImporter : IBundleImporter ? (resolution.RenameTo ?? dto.Name) : dto.Name; - var parentStub = _dbContext.Templates.Local.FirstOrDefault(t => - string.Equals(t.Name, importedName, StringComparison.Ordinal)); - if (parentStub is null) continue; - var parent = await _templateRepo.GetTemplateWithChildrenAsync(parentStub.Id, ct).ConfigureAwait(false); - if (parent is null) continue; + if (localByName.TryGetValue(importedName, out var parentStub)) + targets.Add((dto, parentStub)); + } + if (targets.Count == 0) return; + + var loadedById = await LoadTemplatesWithChildrenAsync(targets.Select(t => t.Stub), ct).ConfigureAwait(false); + + foreach (var (dto, parentStub) in targets) + { + if (!loadedById.TryGetValue(parentStub.Id, out var parent)) continue; // Clear existing rows (Overwrite-truly-overwrites). EF Core tracks // the deletes via the navigation; explicit DeleteAsync ensures the @@ -3200,8 +3222,7 @@ public sealed class BundleImporter : IBundleImporter // The fallback is a dictionary lookup into the per-apply library // snapshot (04/P4) — a name not in Local is a pre-existing target // template this apply has not touched, so its pre-write id is current. - var composedStub = _dbContext.Templates.Local.FirstOrDefault(t => - string.Equals(t.Name, compDto.ComposedTemplateName, StringComparison.Ordinal)); + localByName.TryGetValue(compDto.ComposedTemplateName, out var composedStub); int? composedId = composedStub?.Id; if (composedId is null && (await library.GetIdByNameAsync(ct).ConfigureAwait(false)) @@ -3258,6 +3279,10 @@ public sealed class BundleImporter : IBundleImporter { if (dtos.Count == 0) return; + // Gitea #47: one tracked-template name index for the pass (it only rewrites + // ParentTemplateId, never a name) instead of two DbSet.Local scans per DTO. + var localByName = SnapshotLocalTemplatesByName(); + foreach (var dto in dtos) { var resolution = ResolveOrDefault(resolutionMap, "Template", dto.Name); @@ -3268,9 +3293,7 @@ public sealed class BundleImporter : IBundleImporter : dto.Name; // The just-imported/updated template is tracked in the Local set. - var template = _dbContext.Templates.Local.FirstOrDefault(t => - string.Equals(t.Name, importedName, StringComparison.Ordinal)); - if (template is null) continue; + if (!localByName.TryGetValue(importedName, out var template)) continue; // A root-template bundle — or an Overwrite that dropped the base — // must clear any stale parent edge on the target row. @@ -3289,8 +3312,7 @@ public sealed class BundleImporter : IBundleImporter ? (baseResolution.RenameTo ?? dto.BaseTemplateName) : dto.BaseTemplateName; - var baseStub = _dbContext.Templates.Local.FirstOrDefault(t => - string.Equals(t.Name, baseName, StringComparison.Ordinal)); + localByName.TryGetValue(baseName, out var baseStub); int? baseId = baseStub?.Id; if (baseId is null && (await library.GetIdByNameAsync(ct).ConfigureAwait(false)).TryGetValue(baseName, out var libraryId)) @@ -3324,6 +3346,46 @@ public sealed class BundleImporter : IBundleImporter } } + /// + /// Indexes the change tracker's templates by name (ordinal; the first tracked + /// template wins a duplicate, matching the FirstOrDefault scans this + /// replaces). Gitea #47: every DbSet.Local access runs a full + /// DetectChanges over the tracker — every template, member and instance + /// the import has staged — so the rewire passes take one snapshot per pass + /// instead of one scan per bundled template (and per composition edge), which + /// made a large import's apply quadratic in its size. + /// + private Dictionary SnapshotLocalTemplatesByName() + { + var byName = new Dictionary(StringComparer.Ordinal); + foreach (var template in _dbContext.Templates.Local) + { + byName.TryAdd(template.Name, template); + } + return byName; + } + + /// + /// Loads the given tracked templates with their child collections in one batched + /// read (Gitea #47 — previously one GetTemplateWithChildrenAsync per + /// template), keyed by id. The read is by name, so the result is keyed by id and + /// looked up by each stub's id: a database collation that matches names + /// case-insensitively can only add rows, never swap the exact one out. + /// + private async Task> LoadTemplatesWithChildrenAsync( + IEnumerable