From b0959f697b9c41ed792c7c46a88f887c2773c75e Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 17:43:01 -0400 Subject: [PATCH] perf(adminui): cache the modals' DynamicComponent parameter dictionaries (R04 H2) DriverConfigModal, DeviceModal and RawTagModal built a new parameter dictionary and new EventCallbacks every render. ParameterDictionaryCache reuses the dictionary while the values it carries are unchanged and rebuilds it with the current value when one changes (a build-once cache would strand edits); the callbacks are created once. Blazor still re-sets an EventCallback-bearing child's parameters per parent render, so the forms' _lastParsed guards stay. Refs #745 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../Shared/Drivers/DeviceModal.razor | 20 +++-- .../Shared/Drivers/DriverConfigModal.razor | 28 +++--- .../Shared/ParameterDictionaryCache.cs | 50 +++++++++++ .../Components/Shared/Raw/RawTagModal.razor | 26 ++++-- .../Raw/ParameterDictionaryCacheTests.cs | 87 +++++++++++++++++++ 5 files changed, 185 insertions(+), 26 deletions(-) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/ParameterDictionaryCache.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/ParameterDictionaryCacheTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DeviceModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DeviceModal.razor index aee2778e..eaebf206 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DeviceModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DeviceModal.razor @@ -108,13 +108,19 @@ private bool _enabled = true; private string _deviceConfigJson = "{}"; - /// Parameters handed to the DynamicComponent-rendered device form. Rebuilt on every render — - /// see DriverConfigModal for why this must not be cached. - private Dictionary _deviceFormParameters => new() - { - ["DeviceConfigJson"] = _deviceConfigJson, - ["DeviceConfigJsonChanged"] = EventCallback.Factory.Create(this, v => _deviceConfigJson = v), - }; + /// Parameters handed to the DynamicComponent-rendered device form. Cached on the value it + /// carries and rebuilt when that changes (never build-once: see DriverConfigModal), with the callback + /// created once (R04 H2). + private Dictionary _deviceFormParameters => _deviceFormParameterCache.Get( + [_deviceConfigJson], + () => new() + { + ["DeviceConfigJson"] = _deviceConfigJson, + ["DeviceConfigJsonChanged"] = _deviceConfigJsonChanged ??= + EventCallback.Factory.Create(this, v => _deviceConfigJson = v), + }); + private readonly ParameterDictionaryCache _deviceFormParameterCache = new(); + private EventCallback? _deviceConfigJsonChanged; private byte[] _rowVersion = []; private string _parentDriverConfig = "{}"; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverConfigModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverConfigModal.razor index d15c8a8c..8c3e7047 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverConfigModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverConfigModal.razor @@ -111,16 +111,24 @@ private string _name = ""; private string _driverConfigJson = "{}"; - /// Parameters handed to the DynamicComponent-rendered driver form. Rebuilt on every render so - /// the child receives the CURRENT json — a cached dictionary would pin the value captured at first - /// render and silently strand every subsequent edit. - private Dictionary _formParameters => new() - { - ["DriverConfigJson"] = _driverConfigJson, - ["DriverConfigJsonChanged"] = EventCallback.Factory.Create(this, v => _driverConfigJson = v), - ["ResilienceConfig"] = _resilienceConfig!, - ["ResilienceConfigChanged"] = EventCallback.Factory.Create(this, v => _resilienceConfig = v), - }; + /// Parameters handed to the DynamicComponent-rendered driver form. The child must receive the + /// CURRENT json — a build-once dictionary would pin the value captured at first render and silently + /// strand every subsequent edit — so the cache is keyed on the values it carries and rebuilt only when + /// one changes; the callbacks are created once and reused (R04 H2). + private Dictionary _formParameters => _formParameterCache.Get( + [_driverConfigJson, _resilienceConfig], + () => new() + { + ["DriverConfigJson"] = _driverConfigJson, + ["DriverConfigJsonChanged"] = _driverConfigJsonChanged ??= + EventCallback.Factory.Create(this, v => _driverConfigJson = v), + ["ResilienceConfig"] = _resilienceConfig!, + ["ResilienceConfigChanged"] = _resilienceConfigChanged ??= + EventCallback.Factory.Create(this, v => _resilienceConfig = v), + }); + private readonly ParameterDictionaryCache _formParameterCache = new(); + private EventCallback? _driverConfigJsonChanged; + private EventCallback? _resilienceConfigChanged; private string? _resilienceConfig; private byte[] _rowVersion = []; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/ParameterDictionaryCache.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/ParameterDictionaryCache.cs new file mode 100644 index 00000000..4a2e5f19 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/ParameterDictionaryCache.cs @@ -0,0 +1,50 @@ +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared; + +/// +/// Caches a DynamicComponent parameter dictionary (R04 H2) and rebuilds it only when one of the +/// values it carries changed. The modals used to build a new dictionary, and new +/// EventCallbacks, on every render. A plain build-once cache is wrong: it would pin the JSON +/// captured at first render and silently strand every later edit — so the cache is keyed on the +/// current values, and the callbacks it hands out are created once by the caller and reused. +/// Note the limit: Blazor's change detection treats any EventCallback parameter as +/// possibly changed, so a hosted form still sees SetParametersAsync on each parent render. The +/// per-form _lastParsed guards remain load-bearing; this removes the per-render allocations and +/// gives the child a stable dictionary and callback identity. +/// +public sealed class ParameterDictionaryCache +{ + private object?[]? _keyValues; + private Dictionary? _cached; + + /// Returns the cached dictionary when equal the previous call's, + /// otherwise builds, caches and returns a new one. + /// The values the dictionary carries (compared with ). + /// Builds the dictionary from the current state. + /// The parameter dictionary. + public Dictionary Get(object?[] keyValues, Func> build) + { + if (_cached is not null && _keyValues is not null && Same(_keyValues, keyValues)) return _cached; + + _cached = build(); + _keyValues = (object?[])keyValues.Clone(); + return _cached; + } + + /// Drops the cached dictionary (e.g. on modal close) so the next rebuilds. + public void Reset() + { + _cached = null; + _keyValues = null; + } + + private static bool Same(object?[] a, object?[] b) + { + if (a.Length != b.Length) return false; + for (var i = 0; i < a.Length; i++) + { + if (!Equals(a[i], b[i])) return false; + } + + return true; + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawTagModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawTagModal.razor index 474922f4..b9909392 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawTagModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawTagModal.razor @@ -217,15 +217,23 @@ } } - /// Parameters handed to the typed TagConfig editor. Internal so tests can assert them. - internal IDictionary BuildEditorParameters() => new Dictionary - { - ["ConfigJson"] = _form.TagConfig, - ["ConfigJsonChanged"] = EventCallback.Factory.Create(this, v => _form.TagConfig = v), - ["DriverType"] = DriverType ?? "", - // Live accessor, not a snapshot: the picker reads it when the operator clicks Browse. - ["GetDriverConfigJson"] = (Func)(() => _mergedConfigJson ?? "{}"), - }; + /// Parameters handed to the typed TagConfig editor. Internal so tests can assert them. Cached + /// on the values it carries and rebuilt when one changes (never build-once, which would pin the first + /// render's TagConfig); the callback and accessor are created once (R04 H2). + internal IDictionary BuildEditorParameters() => _editorParameterCache.Get( + [_form.TagConfig, DriverType ?? ""], + () => new Dictionary + { + ["ConfigJson"] = _form.TagConfig, + ["ConfigJsonChanged"] = _configJsonChanged ??= + EventCallback.Factory.Create(this, v => _form.TagConfig = v), + ["DriverType"] = DriverType ?? "", + // Live accessor, not a snapshot: the picker reads it when the operator clicks Browse. + ["GetDriverConfigJson"] = _getDriverConfigJson ??= () => _mergedConfigJson ?? "{}", + }); + private readonly ParameterDictionaryCache _editorParameterCache = new(); + private EventCallback? _configJsonChanged; + private Func? _getDriverConfigJson; private async Task SaveAsync() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/ParameterDictionaryCacheTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/ParameterDictionaryCacheTests.cs new file mode 100644 index 00000000..7bbe3c0b --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/ParameterDictionaryCacheTests.cs @@ -0,0 +1,87 @@ +using Microsoft.AspNetCore.Components; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Raw; + +/// +/// R04 H2: the modals' DynamicComponent parameter dictionaries are reused across renders while their +/// values are unchanged, and rebuilt — carrying the current value — the moment one changes. The second +/// half is the regression a naive build-once cache would introduce (stale JSON stranding every edit). +/// +[Trait("Category", "Unit")] +public sealed class ParameterDictionaryCacheTests +{ + [Fact] + public void Unchanged_values_return_the_same_dictionary() + { + var cache = new ParameterDictionaryCache(); + var builds = 0; + var json = "{}"; + Dictionary Build() { builds++; return new() { ["Json"] = json }; } + + var first = cache.Get([json], Build); + cache.Get([json], Build).ShouldBeSameAs(first); + builds.ShouldBe(1); + } + + [Fact] + public void A_changed_value_rebuilds_with_the_current_value() + { + var cache = new ParameterDictionaryCache(); + var json = "{}"; + Dictionary Build() => new() { ["Json"] = json }; + + cache.Get([json], Build)["Json"].ShouldBe("{}"); + json = "{\"a\":1}"; + cache.Get([json], Build)["Json"].ShouldBe("{\"a\":1}"); + } + + [Fact] + public void Null_values_compare_and_reset_forces_a_rebuild() + { + var cache = new ParameterDictionaryCache(); + var builds = 0; + Dictionary Build() { builds++; return new(); } + + cache.Get(["x", null], Build); + cache.Get(["x", null], Build); + builds.ShouldBe(1); + + cache.Reset(); + cache.Get(["x", null], Build); + builds.ShouldBe(2); + } + + [Fact] + public void Modals_build_their_parameters_through_the_cache_with_reused_callbacks() + { + foreach (var (path, cacheField) in new[] + { + (new[] { "Components", "Shared", "Drivers", "DriverConfigModal.razor" }, "_formParameterCache.Get("), + (new[] { "Components", "Shared", "Drivers", "DeviceModal.razor" }, "_deviceFormParameterCache.Get("), + (new[] { "Components", "Shared", "Raw", "RawTagModal.razor" }, "_editorParameterCache.Get("), + }) + { + var src = AdminUiSource.Read(path); + src.ShouldContain(cacheField, customMessage: path[^1]); + // Every EventCallback is created behind a ??= so it is made once per component. + var all = System.Text.RegularExpressions.Regex.Matches(src, @"EventCallback\.Factory\.Create").Count; + var once = System.Text.RegularExpressions.Regex.Matches(src, @"\?\?=\s*EventCallback\.Factory\.Create").Count; + all.ShouldBeGreaterThan(0, path[^1]); + once.ShouldBe(all, path[^1]); + } + } + + [Fact] + public void RawTagModal_reuses_its_callback_across_renders() + { + var modal = new ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Raw.RawTagModal(); + var first = modal.BuildEditorParameters(); + var second = modal.BuildEditorParameters(); + + second.ShouldBeSameAs(first); + ((EventCallback)first["ConfigJsonChanged"]).HasDelegate.ShouldBeTrue(); + } +}