perf(opcua): de-quadratic the lock-time teardown paths (B3)
Review 01 P1/P2/P3/P5 — work done under the manager-wide SDK Lock on the pinned publish thread, where every Read/Browse/Publish waits. - P1 RebuildAddressSpace detaches the shared root's top-level children once instead of every leaf from its parent (NodeState.RemoveChild is a linear scan + RemoveAt, ~k²/2 compares per k-child folder); root notifiers are cleared as a list instead of one O(F) RemoveRootNotifier each; ForgetPredefinedSubtree / TombstoneMonitoredSubtree are iterative with one child buffer. - P2 RemoveEquipmentSubtree walks a parent→children index maintained by EnsureFolder/EnsureVariable/MaterialiseAlarmCondition (keyed by the parent id the caller named) instead of prefix-scanning six maps per removed equipment; a surviving condition's wiring into a removed folder is found through the folder's own notifier list; descendants are not detached from a folder that is itself being discarded. - P3 LowerAllEventsMonitoring(condition) replaces the full O(C) reconcile on RemoveAlarmConditionNode / RemoveEquipmentSubtree / the kind-swap. - P5 AlarmNotifierWiring.Folders is a reference-identity HashSet and an already-wired pair skips the SDK's linear AddNotifier dedup. Benchmark (NodeManagerLockTimeBenchmarkTests, M-series Mac under load, three runs each): rebuild of a 10k-tag folder 75–150 ms → 5–6 ms; removing 200 equipment (250 signals each) from a 50k-node space 471–681 ms → 58–64 ms; all-events bookkeeping visits for 10 condition removals over 50 conditions 900 → 10. The two timing legs assert < 1 s; the P3 leg asserts the visit count (machine-independent). Refs #732 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
@@ -111,6 +111,13 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
/// against it (a miss ⇒ <c>BadHistoryOperationUnsupported</c>). Cleared on
|
||||
/// <see cref="RebuildAddressSpace"/>.</summary>
|
||||
private readonly ConcurrentDictionary<RealmKey, string> _eventNotifierSources = new();
|
||||
/// <summary>Review 01 P2 — parent key → the keys of the folders / variables / conditions materialised
|
||||
/// under it (by the parent id the CALLER named, so a node whose parent was unknown at creation and hangs
|
||||
/// under the shared root is still found under its intended parent). <see cref="RemoveEquipmentSubtree"/>
|
||||
/// walks this instead of prefix-scanning every map per removed equipment. Guarded by <c>Lock</c>.</summary>
|
||||
private readonly Dictionary<RealmKey, HashSet<RealmKey>> _childIndex = new();
|
||||
/// <summary>The reverse of <see cref="_childIndex"/>: child key → parent key. Guarded by <c>Lock</c>.</summary>
|
||||
private readonly Dictionary<RealmKey, RealmKey> _parentIndex = new();
|
||||
private FolderState? _root;
|
||||
private readonly ILogger<OtOpcUaNodeManager> _logger;
|
||||
|
||||
@@ -1400,8 +1407,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
ForgetPredefinedSubtree(existing);
|
||||
// The materialise tail raises only the new instance's unit (no full reconcile), so give the
|
||||
// dropped instance's back here rather than leave it tracked until the next full reconcile.
|
||||
if (_allEventsMonitoredConditions.Remove(existing))
|
||||
existing.SetAreEventsMonitored(SystemContext, false, includeChildren: true);
|
||||
LowerAllEventsMonitoring(existing);
|
||||
}
|
||||
|
||||
// H6a: re-materialising the same id as the OTHER kind (native↔scripted) must reflect the new
|
||||
@@ -1579,6 +1585,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
|
||||
AddPredefinedNode(SystemContext, alarm);
|
||||
_alarmConditions[conditionKey] = alarm;
|
||||
if (!string.IsNullOrEmpty(equipmentNodeId)) IndexChild(MapKey(realm, equipmentNodeId), conditionKey);
|
||||
// Only the new condition can be missing its unit here, so raise just that one — a full reconcile
|
||||
// walks every condition, which made a deploy of N alarms O(N²) under Lock whenever any Server-object
|
||||
// item was live (always, in production). Removal, rebuild and (un)subscribe keep the full reconcile.
|
||||
@@ -1690,11 +1697,12 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
}
|
||||
|
||||
/// <summary>One materialised native-alarm condition together with the equipment folders wired as extra
|
||||
/// event-notifier roots for it. The <see cref="Folders"/> list is mutated in place under <c>Lock</c> as
|
||||
/// notifiers are wired / torn down; <see cref="Alarm"/> is the concrete <see cref="AlarmConditionState"/>
|
||||
/// instance the notifiers were wired against (a rebuild recreates the instance, so the entry is reset when
|
||||
/// the instance changes).</summary>
|
||||
private sealed record AlarmNotifierWiring(AlarmConditionState Alarm, List<FolderState> Folders);
|
||||
/// event-notifier roots for it. The <see cref="Folders"/> set (reference identity — review 01 P5: the
|
||||
/// membership probe per wired folder was a linear <c>ReferenceEquals</c> walk) is mutated in place under
|
||||
/// <c>Lock</c> as notifiers are wired / torn down; <see cref="Alarm"/> is the concrete
|
||||
/// <see cref="AlarmConditionState"/> instance the notifiers were wired against (a rebuild recreates the
|
||||
/// instance, so the entry is reset when the instance changes).</summary>
|
||||
private sealed record AlarmNotifierWiring(AlarmConditionState Alarm, HashSet<FolderState> Folders);
|
||||
|
||||
// This is a RECONCILE, not additive-only: it wires the supplied set AND unwires any
|
||||
// previously-tracked folder whose id is no longer in notifierFolderNodeIds
|
||||
@@ -1735,7 +1743,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
// the entry (the old folder list referenced the discarded instance) when the instance differs.
|
||||
if (!_alarmNotifierWiring.TryGetValue(alarmKey, out var wiring) || !ReferenceEquals(wiring.Alarm, alarm))
|
||||
{
|
||||
wiring = new AlarmNotifierWiring(alarm, new List<FolderState>());
|
||||
wiring = new AlarmNotifierWiring(
|
||||
alarm, new HashSet<FolderState>(System.Collections.Generic.ReferenceEqualityComparer.Instance));
|
||||
_alarmNotifierWiring[alarmKey] = wiring;
|
||||
}
|
||||
|
||||
@@ -1772,13 +1781,13 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
// alarm.AddNotifier(isInverse:true, folder) — the upward bubble to the equipment folder
|
||||
// folder.AddNotifier(isInverse:false, alarm) — the downward AreEventsMonitored link
|
||||
// EnsureFolderIsEventNotifier(folder) — SubscribeToEvents + AddRootNotifier (idempotent)
|
||||
// AddNotifier dedups by ReferenceEquals(Node), so re-wiring the same pair (idempotent re-apply)
|
||||
// updates rather than duplicates.
|
||||
// A pair this wiring already holds is skipped outright (review 01 P5): the SDK's AddNotifier
|
||||
// dedups by a linear ReferenceEquals walk of the condition's notifier list, so re-wiring K
|
||||
// folders on an idempotent re-apply cost O(K²) for nothing.
|
||||
if (!wiring.Folders.Add(folder)) continue;
|
||||
alarm.AddNotifier(SystemContext, null, isInverse: true, folder);
|
||||
folder.AddNotifier(SystemContext, null, isInverse: false, alarm);
|
||||
EnsureFolderIsEventNotifier(folder);
|
||||
|
||||
if (!wiring.Folders.Any(f => ReferenceEquals(f, folder))) wiring.Folders.Add(folder);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2826,10 +2835,18 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
/// <param name="node">The node whose subtree is forgotten.</param>
|
||||
private void ForgetPredefinedSubtree(NodeState node)
|
||||
{
|
||||
PredefinedNodes?.Remove(node.NodeId);
|
||||
// Iterative with one reused child buffer — the recursive form allocated a list per node (review 01 P1).
|
||||
var pending = new Stack<NodeState>();
|
||||
pending.Push(node);
|
||||
var children = new List<BaseInstanceState>();
|
||||
node.GetChildren(SystemContext, children);
|
||||
foreach (var child in children) ForgetPredefinedSubtree(child);
|
||||
while (pending.Count > 0)
|
||||
{
|
||||
var current = pending.Pop();
|
||||
PredefinedNodes?.Remove(current.NodeId);
|
||||
children.Clear();
|
||||
current.GetChildren(SystemContext, children);
|
||||
foreach (var child in children) pending.Push(child);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Promote <paramref name="folder"/> to <see cref="EventNotifiers.SubscribeToEvents"/> and
|
||||
@@ -2922,9 +2939,33 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
parent.AddChild(folder);
|
||||
AddPredefinedNode(SystemContext, folder);
|
||||
_folders[key] = folder;
|
||||
if (!string.IsNullOrEmpty(parentNodeId)) IndexChild(MapKey(realm, parentNodeId), key);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Records <paramref name="child"/> under <paramref name="parent"/> in the P2 subtree index. MUST be
|
||||
/// called under <c>Lock</c>. Idempotent for the same pair; a child re-indexed under another parent moves.</summary>
|
||||
private void IndexChild(RealmKey parent, RealmKey child)
|
||||
{
|
||||
if (_parentIndex.TryGetValue(child, out var previous))
|
||||
{
|
||||
if (previous == parent) return;
|
||||
UnindexChild(child);
|
||||
}
|
||||
|
||||
if (!_childIndex.TryGetValue(parent, out var children)) _childIndex[parent] = children = new HashSet<RealmKey>();
|
||||
children.Add(child);
|
||||
_parentIndex[child] = parent;
|
||||
}
|
||||
|
||||
/// <summary>Drops <paramref name="child"/> from the P2 subtree index. MUST be called under <c>Lock</c>.</summary>
|
||||
private void UnindexChild(RealmKey child)
|
||||
{
|
||||
if (!_parentIndex.Remove(child, out var parent)) return;
|
||||
if (_childIndex.TryGetValue(parent, out var children) && children.Remove(child) && children.Count == 0)
|
||||
_childIndex.Remove(parent);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Update an EXISTING folder node's <see cref="FolderState.DisplayName"/> in place and notify
|
||||
/// subscribers, WITHOUT a rebuild — the in-place counterpart of <see cref="EnsureFolder"/> for a
|
||||
@@ -3036,6 +3077,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
parent.AddChild(variable);
|
||||
AddPredefinedNode(SystemContext, variable);
|
||||
_variables[key] = variable;
|
||||
if (!string.IsNullOrEmpty(parentFolderNodeId)) IndexChild(MapKey(realm, parentFolderNodeId), key);
|
||||
// Phase C: register the resolved historian tagname so the HistoryRead override can map this
|
||||
// NodeId back to its Aveva/historian source. A historized UNS reference node passes the SAME
|
||||
// tagname as its backing raw node — each NodeId keys its own entry, both mapping to one tagname.
|
||||
@@ -3500,16 +3542,6 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
// TombstoneMonitoredNode). The shared root survives the rebuild and keeps its items.
|
||||
TombstoneAllMonitoredNodes();
|
||||
|
||||
foreach (var v in _variables.Values)
|
||||
{
|
||||
v.Parent?.RemoveChild(v);
|
||||
PredefinedNodes?.Remove(v.NodeId);
|
||||
}
|
||||
_variables.Clear();
|
||||
// Phase C: drop the NodeId→historian-tagname registrations alongside the variables they map.
|
||||
_historizedTagnames.Clear();
|
||||
_arrayOnScalarRefused.Clear();
|
||||
|
||||
// Capture every NATIVE condition's live state before the conditions are discarded, so the
|
||||
// re-materialise that follows in this same apply can put it back (see CaptureCarryOver). Scripted
|
||||
// conditions are not captured — their engine re-asserts them after the reload.
|
||||
@@ -3520,38 +3552,56 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
if (_nativeAlarmNodeIds.Contains(key)) _rebuildCarryOver[key] = CaptureCarryOver(alarm);
|
||||
}
|
||||
|
||||
foreach (var alarm in _alarmConditions.Values)
|
||||
{
|
||||
alarm.Parent?.RemoveChild(alarm);
|
||||
ForgetPredefinedSubtree(alarm);
|
||||
}
|
||||
// tear down every native-alarm→equipment-folder notifier pair bidirectionally
|
||||
// BEFORE the conditions + folders are cleared below, so no inverse-notifier entry leaks across the
|
||||
// rebuild. The condition + folder NodeState objects still exist here (the loops above/below only
|
||||
// detach them from their parents + PredefinedNodes), so RemoveNotifier is valid; then clear the
|
||||
// tracking map so the re-materialise + re-wire on the next apply starts from a clean slate.
|
||||
// rebuild. The condition + folder NodeState objects still exist here, so RemoveNotifier is valid;
|
||||
// then clear the tracking map so the re-materialise + re-wire on the next apply starts clean.
|
||||
foreach (var alarmKey in _alarmNotifierWiring.Keys.ToList())
|
||||
UnwireAlarmNotifiers(alarmKey);
|
||||
|
||||
// Review 01 P1 — detach whole subtrees, not leaves. NodeState.RemoveChild is a linear scan +
|
||||
// RemoveAt, so detaching each of a folder's k children from it cost ~k²/2 compares under Lock
|
||||
// (a 10k-tag device folder ≈ 50M). Everything this manager minted hangs under the shared root,
|
||||
// which survives the rebuild, so detaching the root's own children once discards every subtree
|
||||
// beneath them; the SDK lookup dictionary is still cleared per id (O(1) each) because
|
||||
// PredefinedNodes indexes every node, condition children included.
|
||||
if (_root is { } root)
|
||||
{
|
||||
var topLevel = new List<BaseInstanceState>();
|
||||
root.GetChildren(SystemContext, topLevel);
|
||||
// Last-first: RemoveChild still scans for the reference, but RemoveAt on the tail shifts nothing.
|
||||
for (var i = topLevel.Count - 1; i >= 0; i--) root.RemoveChild(topLevel[i]);
|
||||
}
|
||||
|
||||
foreach (var v in _variables.Values) PredefinedNodes?.Remove(v.NodeId);
|
||||
_variables.Clear();
|
||||
// Phase C: drop the NodeId→historian-tagname registrations alongside the variables they map.
|
||||
_historizedTagnames.Clear();
|
||||
_arrayOnScalarRefused.Clear();
|
||||
|
||||
foreach (var alarm in _alarmConditions.Values) ForgetPredefinedSubtree(alarm);
|
||||
_alarmConditions.Clear();
|
||||
// H6a: drop the native-alarm flags in lock-step with the conditions they classify, so a
|
||||
// re-materialise on the next apply (possibly as the other kind) starts from a clean slate.
|
||||
_nativeAlarmNodeIds.Clear();
|
||||
|
||||
foreach (var f in _folders.Values)
|
||||
{
|
||||
f.Parent?.RemoveChild(f);
|
||||
PredefinedNodes?.Remove(f.NodeId);
|
||||
}
|
||||
foreach (var f in _folders.Values) PredefinedNodes?.Remove(f.NodeId);
|
||||
_folders.Clear();
|
||||
_childIndex.Clear();
|
||||
_parentIndex.Clear();
|
||||
|
||||
// Detach the Server↔folder HasNotifier ref for every promoted folder before dropping the
|
||||
// guard, otherwise the rebuild leaks an orphaned root-notifier reference on the Server
|
||||
// object. RemoveRootNotifier just severs that link, so its order relative to the folder
|
||||
// teardown above doesn't matter — but it must run under this same Lock.
|
||||
foreach (var folder in _notifierFolders.Values)
|
||||
// object. Done the way RemoveRootNotifier does it, but over the whole list at once: the SDK's
|
||||
// per-folder removal scans RootNotifiers each time (O(F²) for F promoted folders).
|
||||
if (RootNotifiers is { Count: > 0 } rootNotifiers)
|
||||
{
|
||||
RemoveRootNotifier(folder);
|
||||
foreach (var notifier in rootNotifiers)
|
||||
{
|
||||
notifier.OnReportEvent = null;
|
||||
notifier.RemoveReference(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server);
|
||||
}
|
||||
rootNotifiers.Clear();
|
||||
}
|
||||
|
||||
// Drop the notifier-folder guard so re-materialised alarms re-promote their (rebuilt)
|
||||
@@ -3611,6 +3661,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
// caller (AddressSpaceApplier) falls back to a full rebuild (resync). Mirrors the per-node
|
||||
// teardown inside RebuildAddressSpace, scoped to this one id.
|
||||
if (!_variables.TryRemove(key, out var variable)) return false;
|
||||
UnindexChild(key);
|
||||
TombstoneMonitoredNode(variable.NodeId, UtcNow);
|
||||
// 2026-08-15 review I3: sever the INVERSE half of every cross-tree Organizes edge this node holds
|
||||
// BEFORE detaching it — the same hazard (and the same fix shape) as UnwireAlarmNotifiers on a
|
||||
@@ -3678,6 +3729,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
// lingering notifier folder is harmless (RemoveEquipmentSubtree demotes it when the whole
|
||||
// equipment goes).
|
||||
if (!_alarmConditions.TryRemove(key, out var condition)) return false;
|
||||
UnindexChild(key);
|
||||
TombstoneMonitoredSubtree(condition, UtcNow);
|
||||
// unwire this condition's extra equipment-folder notifiers bidirectionally BEFORE
|
||||
// the condition drops — the folders (equipment, UNS realm) SURVIVE this scoped remove, so without
|
||||
@@ -3686,7 +3738,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
condition.Parent?.RemoveChild(condition);
|
||||
ForgetPredefinedSubtree(condition);
|
||||
_nativeAlarmNodeIds.Remove(key);
|
||||
ReconcileAllEventsMonitoring();
|
||||
LowerAllEventsMonitoring(condition);
|
||||
e = BuildNodesRemovedModelChange(alarmNodeId, realm);
|
||||
}
|
||||
ReportModelChangeOutsideLock(e, alarmNodeId);
|
||||
@@ -3706,84 +3758,15 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
// Unknown equipment id ⇒ map drift ⇒ false (caller rebuilds).
|
||||
if (!_folders.ContainsKey(equipKey)) return false;
|
||||
|
||||
// Maps key on the (realm, bare id) pair. Every descendant in the SAME realm shares that id exactly
|
||||
// or begins with "<equipmentNodeId>/" (sub-folders, variables, condition nodes); the explicit realm
|
||||
// compare is what makes the match realm-scoped — it replaces the old string key's leading "ns=N;s="
|
||||
// — so a node in the OTHER namespace that happens to share a bare id is never in scope. Snapshot
|
||||
// each map's in-scope keys first (ToList) so we don't mutate while enumerating.
|
||||
var prefix = equipmentNodeId + "/";
|
||||
bool InScope(RealmKey key) => key.Realm == realm
|
||||
&& (key.Id == equipmentNodeId || key.Id.StartsWith(prefix, StringComparison.Ordinal));
|
||||
|
||||
var now = UtcNow;
|
||||
// Value variables (+ their historized-tagname registrations).
|
||||
foreach (var id in _variables.Keys.Where(InScope).ToList())
|
||||
{
|
||||
if (_variables.TryRemove(id, out var v))
|
||||
{
|
||||
TombstoneMonitoredNode(v.NodeId, now);
|
||||
// Sever the inverse half of each cross-tree Organizes edge
|
||||
// first, exactly as RemoveVariableNode does — a UNS reference variable in scope points at a
|
||||
// RAW node that survives this UNS-scoped removal and would otherwise keep a dangling edge.
|
||||
SeverOutgoingReferences(v);
|
||||
v.Parent?.RemoveChild(v);
|
||||
PredefinedNodes?.Remove(v.NodeId);
|
||||
}
|
||||
_historizedTagnames.TryRemove(id, out _);
|
||||
_arrayOnScalarRefused.TryRemove(id, out _);
|
||||
}
|
||||
|
||||
// Part 9 condition nodes (+ their native flags).
|
||||
foreach (var id in _alarmConditions.Keys.Where(InScope).ToList())
|
||||
{
|
||||
if (_alarmConditions.TryRemove(id, out var c))
|
||||
{
|
||||
TombstoneMonitoredSubtree(c, now);
|
||||
c.Parent?.RemoveChild(c);
|
||||
ForgetPredefinedSubtree(c);
|
||||
}
|
||||
_nativeAlarmNodeIds.Remove(id);
|
||||
}
|
||||
ReconcileAllEventsMonitoring();
|
||||
|
||||
// a native alarm condition lives at its raw tag (Raw realm) and SURVIVES this
|
||||
// UNS-scoped subtree removal, but it may hold an in-scope equipment folder as an extra
|
||||
// event-notifier root. Unwire those condition↔folder pairs bidirectionally BEFORE the folders drop,
|
||||
// else the surviving condition keeps a dangling inverse-notifier entry to a removed folder. (The
|
||||
// condition's own scoped removal — if its raw tag is also removed this apply — runs later via
|
||||
// RemoveAlarmConditionNode; this only reaches surviving conditions.)
|
||||
foreach (var wiring in _alarmNotifierWiring.Values)
|
||||
{
|
||||
foreach (var folder in wiring.Folders.Where(f => InScope(MapKey(f.NodeId))).ToList())
|
||||
{
|
||||
wiring.Alarm.RemoveNotifier(SystemContext, folder, bidirectional: true);
|
||||
wiring.Folders.Remove(folder);
|
||||
}
|
||||
}
|
||||
|
||||
// Notifier demotion BEFORE dropping the folders: sever the Server↔folder HasNotifier ref for
|
||||
// every promoted folder in the subtree (an equipment folder, or a sub-folder that hosted an
|
||||
// alarm), else the removal leaks an orphaned root-notifier reference on the Server object.
|
||||
// _notifierFolders is keyed by the folder's NodeId; match InScope on its realm-qualified map key.
|
||||
foreach (var kvp in _notifierFolders.Where(kv => InScope(MapKey(kv.Key))).ToList())
|
||||
{
|
||||
RemoveRootNotifier(kvp.Value);
|
||||
_notifierFolders.Remove(kvp.Key);
|
||||
}
|
||||
// Drop the event-history source registrations for the subtree (keyed by the folder id string).
|
||||
foreach (var src in _eventNotifierSources.Keys.Where(InScope).ToList())
|
||||
_eventNotifierSources.TryRemove(src, out _);
|
||||
|
||||
// Sub-folders + the equipment folder itself (children already detached above).
|
||||
foreach (var id in _folders.Keys.Where(InScope).ToList())
|
||||
{
|
||||
if (_folders.TryRemove(id, out var f))
|
||||
{
|
||||
TombstoneMonitoredNode(f.NodeId, now);
|
||||
f.Parent?.RemoveChild(f);
|
||||
PredefinedNodes?.Remove(f.NodeId);
|
||||
}
|
||||
}
|
||||
// Review 01 P2 — walk the parent→children index the materialise calls maintain instead of
|
||||
// prefix-scanning every map (variables, conditions, folders, notifier folders, event sources and
|
||||
// every alarm's wiring) per removed equipment, which made a PureRemove of M equipment O(M·N)
|
||||
// under Lock. A node is detached from its SDK parent only when that parent SURVIVES (the
|
||||
// equipment folder from its line; a node hung under the shared root because its parent was
|
||||
// unknown at creation) — detaching k children from a folder that is itself being discarded cost
|
||||
// k²/2 compares for nothing (NodeState.RemoveChild is a linear scan).
|
||||
var removedParents = new HashSet<NodeState>(System.Collections.Generic.ReferenceEqualityComparer.Instance);
|
||||
RemoveSubtree(equipKey, removedParents, UtcNow);
|
||||
|
||||
e = BuildNodesRemovedModelChange(equipmentNodeId, realm);
|
||||
}
|
||||
@@ -3791,6 +3774,84 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
return true;
|
||||
}
|
||||
|
||||
/// <summary>Removes the node at <paramref name="key"/> and, when it is a folder, everything indexed beneath
|
||||
/// it (children first). MUST be called under <c>Lock</c>.</summary>
|
||||
/// <param name="key">The node to remove.</param>
|
||||
/// <param name="removedParents">The folders already discarded by this removal — a node under one of them is
|
||||
/// not detached from it.</param>
|
||||
/// <param name="now">The timestamp the removal statuses carry.</param>
|
||||
private void RemoveSubtree(RealmKey key, HashSet<NodeState> removedParents, DateTime now)
|
||||
{
|
||||
if (_folders.TryGetValue(key, out var folder))
|
||||
{
|
||||
removedParents.Add(folder);
|
||||
if (_childIndex.TryGetValue(key, out var children))
|
||||
{
|
||||
foreach (var child in children.ToArray()) RemoveSubtree(child, removedParents, now);
|
||||
_childIndex.Remove(key);
|
||||
}
|
||||
|
||||
_folders.TryRemove(key, out _);
|
||||
UnindexChild(key);
|
||||
TombstoneMonitoredNode(folder.NodeId, now);
|
||||
|
||||
// A native alarm condition lives at its raw tag (Raw realm) and SURVIVES this UNS-scoped
|
||||
// subtree removal, but it may hold this folder as an extra event-notifier root. Unwire those
|
||||
// condition↔folder pairs bidirectionally BEFORE the folder drops, else the surviving condition
|
||||
// keeps a dangling inverse-notifier entry to a removed folder. The folder's own notifier list
|
||||
// names exactly the conditions wired into it (the forward half of each pair).
|
||||
var notifiers = new List<NodeState.Notifier>();
|
||||
folder.GetNotifiers(SystemContext, notifiers);
|
||||
foreach (var entry in notifiers)
|
||||
{
|
||||
if (entry.IsInverse || entry.Node is not AlarmConditionState alarm) continue;
|
||||
if (!_alarmNotifierWiring.TryGetValue(MapKey(alarm.NodeId), out var wiring) || !ReferenceEquals(wiring.Alarm, alarm)) continue;
|
||||
alarm.RemoveNotifier(SystemContext, folder, bidirectional: true);
|
||||
wiring.Folders.Remove(folder);
|
||||
}
|
||||
|
||||
// Notifier demotion BEFORE dropping the folder: sever the Server↔folder HasNotifier ref for a
|
||||
// promoted folder (an equipment folder, or a sub-folder that hosted an alarm), else the removal
|
||||
// leaks an orphaned root-notifier reference on the Server object.
|
||||
if (_notifierFolders.Remove(folder.NodeId, out var promoted)) RemoveRootNotifier(promoted);
|
||||
_eventNotifierSources.TryRemove(key, out _);
|
||||
|
||||
if (folder.Parent is { } parent && !removedParents.Contains(parent)) parent.RemoveChild(folder);
|
||||
PredefinedNodes?.Remove(folder.NodeId);
|
||||
return;
|
||||
}
|
||||
|
||||
if (_variables.TryRemove(key, out var v))
|
||||
{
|
||||
UnindexChild(key);
|
||||
TombstoneMonitoredNode(v.NodeId, now);
|
||||
// Sever the inverse half of each cross-tree Organizes edge first, exactly as RemoveVariableNode
|
||||
// does — a UNS reference variable in scope points at a RAW node that survives this UNS-scoped
|
||||
// removal and would otherwise keep a dangling edge.
|
||||
SeverOutgoingReferences(v);
|
||||
if (v.Parent is { } parent && !removedParents.Contains(parent)) parent.RemoveChild(v);
|
||||
PredefinedNodes?.Remove(v.NodeId);
|
||||
_historizedTagnames.TryRemove(key, out _);
|
||||
_arrayOnScalarRefused.TryRemove(key, out _);
|
||||
return;
|
||||
}
|
||||
|
||||
if (_alarmConditions.TryRemove(key, out var c))
|
||||
{
|
||||
UnindexChild(key);
|
||||
TombstoneMonitoredSubtree(c, now);
|
||||
UnwireAlarmNotifiers(key);
|
||||
if (c.Parent is { } parent && !removedParents.Contains(parent)) parent.RemoveChild(c);
|
||||
ForgetPredefinedSubtree(c);
|
||||
_nativeAlarmNodeIds.Remove(key);
|
||||
LowerAllEventsMonitoring(c);
|
||||
return;
|
||||
}
|
||||
|
||||
// Indexed but already gone (a surgical remove dropped it first) — nothing to do.
|
||||
UnindexChild(key);
|
||||
}
|
||||
|
||||
private FolderState ResolveParentFolder(string? parentNodeId, AddressSpaceRealm realm)
|
||||
{
|
||||
EnsureAddressSpaceCreated();
|
||||
@@ -4296,10 +4357,18 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
/// children are monitorable in their own right). MUST be called under <c>Lock</c>.</summary>
|
||||
private void TombstoneMonitoredSubtree(NodeState node, DateTime now)
|
||||
{
|
||||
TombstoneMonitoredNode(node.NodeId, now);
|
||||
if (MonitoredNodes.Count == 0 && Volatile.Read(ref _suppressedItems) == 0) return;
|
||||
var pending = new Stack<NodeState>();
|
||||
pending.Push(node);
|
||||
var children = new List<BaseInstanceState>();
|
||||
node.GetChildren(SystemContext, children);
|
||||
foreach (var child in children) TombstoneMonitoredSubtree(child, now);
|
||||
while (pending.Count > 0)
|
||||
{
|
||||
var current = pending.Pop();
|
||||
TombstoneMonitoredNode(current.NodeId, now);
|
||||
children.Clear();
|
||||
current.GetChildren(SystemContext, children);
|
||||
foreach (var child in children) pending.Push(child);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>The rebuild form: every monitored node this manager serves except the shared root, which the
|
||||
@@ -4560,6 +4629,20 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
condition.SetAreEventsMonitored(SystemContext, true, includeChildren: true);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The removal-side mirror of <see cref="RaiseAllEventsMonitoring"/> (review 01 P3): gives back the one
|
||||
/// condition's unit, touching no other condition. Call under <c>Lock</c>, for a condition being removed
|
||||
/// or replaced. The surgical removal sites used to run the full reconcile — O(C) per removed condition
|
||||
/// while any Server-object item was live, i.e. always in production.
|
||||
/// </summary>
|
||||
/// <param name="condition">The condition leaving <c>_alarmConditions</c>.</param>
|
||||
private void LowerAllEventsMonitoring(AlarmConditionState condition)
|
||||
{
|
||||
AllEventsMonitoringConditionVisits++;
|
||||
if (_allEventsMonitoredConditions.Remove(condition))
|
||||
condition.SetAreEventsMonitored(SystemContext, false, includeChildren: true);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// How many conditions the Server-object monitored bookkeeping has visited since start — every
|
||||
/// condition a full <see cref="ReconcileAllEventsMonitoring"/> walks, plus one per incremental
|
||||
|
||||
+190
@@ -0,0 +1,190 @@
|
||||
using System.Diagnostics;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Opc.Ua;
|
||||
using Opc.Ua.Server;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Review 01 P1 / P2 / P3 — the lock-time budget of the three teardown paths that run under the
|
||||
/// manager-wide SDK <c>Lock</c> on the pinned publish thread, where every Read / Browse / Publish waits.
|
||||
/// <list type="bullet">
|
||||
/// <item>P1: a full rebuild of a 10k-tag device folder was O(k²) per folder because
|
||||
/// <c>NodeState.RemoveChild</c> is a linear scan + <c>RemoveAt</c> per leaf (~50M compares).</item>
|
||||
/// <item>P2: removing M equipment from an N-node space scanned every map per equipment (O(M·N)).</item>
|
||||
/// <item>P3: every surgical condition removal re-walked every condition for the Server-object
|
||||
/// monitoring bookkeeping (O(C) per removal) while any <c>i=2253</c> item existed.</item>
|
||||
/// </list>
|
||||
/// The wall-clock budgets are generous for CI hardware; the before/after numbers are in the commit
|
||||
/// message. P3 is pinned by the bookkeeping's own visit counter, which does not depend on the machine.
|
||||
/// </summary>
|
||||
[Trait("Category", "Benchmark")]
|
||||
public sealed class NodeManagerLockTimeBenchmarkTests(ITestOutputHelper output) : IDisposable
|
||||
{
|
||||
private static CancellationToken Ct => TestContext.Current.CancellationToken;
|
||||
private static readonly TimeSpan Budget = TimeSpan.FromSeconds(1);
|
||||
|
||||
private readonly string _pkiRoot = Path.Combine(Path.GetTempPath(), $"otopcua-bench-{Guid.NewGuid():N}");
|
||||
|
||||
/// <summary>P1 — a rebuild that discards one device folder holding 10k tags completes within the budget.</summary>
|
||||
[Fact]
|
||||
public async Task Rebuild_of_a_10k_tag_folder_stays_within_budget()
|
||||
{
|
||||
var (host, server) = await BootAsync();
|
||||
try
|
||||
{
|
||||
var nm = server.NodeManager!;
|
||||
const string device = "Plant/Modbus/dev1";
|
||||
nm.EnsureFolder(device, null, "dev1", AddressSpaceRealm.Raw);
|
||||
for (var i = 0; i < 10_000; i++)
|
||||
nm.EnsureVariable($"{device}/T{i}", device, $"T{i}", "Int32", writable: false, realm: AddressSpaceRealm.Raw);
|
||||
nm.VariableCount.ShouldBe(10_000);
|
||||
|
||||
var sw = Stopwatch.StartNew();
|
||||
nm.RebuildAddressSpace();
|
||||
sw.Stop();
|
||||
nm.CompleteAddressSpaceApply();
|
||||
|
||||
nm.VariableCount.ShouldBe(0);
|
||||
nm.FolderCount.ShouldBe(0);
|
||||
// Positive control: the space is really empty for the SDK too, and can be re-populated.
|
||||
nm.GetManagerHandle(new NodeId($"{device}/T0", nm.NamespaceIndexForRealm(AddressSpaceRealm.Raw))).ShouldBeNull();
|
||||
nm.EnsureFolder(device, null, "dev1", AddressSpaceRealm.Raw);
|
||||
nm.EnsureVariable($"{device}/T0", device, "T0", "Int32", writable: false, realm: AddressSpaceRealm.Raw);
|
||||
nm.TryGetVariable($"{device}/T0", AddressSpaceRealm.Raw)!.Parent.ShouldBeSameAs(nm.TryGetFolder(device, AddressSpaceRealm.Raw));
|
||||
|
||||
output.WriteLine($"BENCH rebuild 10k-tag folder: {sw.ElapsedMilliseconds} ms");
|
||||
sw.Elapsed.ShouldBeLessThan(Budget, $"rebuild of a 10k-tag folder took {sw.ElapsedMilliseconds} ms");
|
||||
}
|
||||
finally { await host.DisposeAsync(); }
|
||||
}
|
||||
|
||||
/// <summary>P2 — removing 200 equipment (250 signals each, 50k nodes) completes within the budget.</summary>
|
||||
[Fact]
|
||||
public async Task Removing_200_equipment_from_a_50k_node_space_stays_within_budget()
|
||||
{
|
||||
var (host, server) = await BootAsync();
|
||||
try
|
||||
{
|
||||
var nm = server.NodeManager!;
|
||||
const int equipmentCount = 200, perEquipment = 250;
|
||||
for (var e = 0; e < equipmentCount; e++)
|
||||
{
|
||||
var equip = $"area/line/eq{e}";
|
||||
nm.EnsureFolder(equip, null, $"eq{e}", AddressSpaceRealm.Uns);
|
||||
for (var i = 0; i < perEquipment; i++)
|
||||
nm.EnsureVariable($"{equip}/S{i}", equip, $"S{i}", "Float", writable: false, realm: AddressSpaceRealm.Uns);
|
||||
}
|
||||
// A survivor the removals must not touch.
|
||||
nm.EnsureFolder("area/line/keep", null, "keep", AddressSpaceRealm.Uns);
|
||||
nm.EnsureVariable("area/line/keep/S0", "area/line/keep", "S0", "Float", writable: false, realm: AddressSpaceRealm.Uns);
|
||||
nm.VariableCount.ShouldBe(equipmentCount * perEquipment + 1);
|
||||
|
||||
var sw = Stopwatch.StartNew();
|
||||
for (var e = 0; e < equipmentCount; e++)
|
||||
nm.RemoveEquipmentSubtree($"area/line/eq{e}", AddressSpaceRealm.Uns).ShouldBeTrue();
|
||||
sw.Stop();
|
||||
|
||||
nm.VariableCount.ShouldBe(1);
|
||||
nm.FolderCount.ShouldBe(1);
|
||||
nm.TryGetVariable("area/line/keep/S0").ShouldNotBeNull();
|
||||
nm.GetManagerHandle(new NodeId("area/line/eq0/S0", nm.NamespaceIndexForRealm(AddressSpaceRealm.Uns))).ShouldBeNull();
|
||||
|
||||
output.WriteLine($"BENCH remove 200 equipment / 50k nodes: {sw.ElapsedMilliseconds} ms");
|
||||
sw.Elapsed.ShouldBeLessThan(Budget, $"removing 200 equipment took {sw.ElapsedMilliseconds} ms");
|
||||
}
|
||||
finally { await host.DisposeAsync(); }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// P3 — with a Server-object item live, a surgical condition removal lowers only that condition's
|
||||
/// monitoring unit instead of re-walking every condition: the bookkeeping visits at most one
|
||||
/// condition per removal. (A full reconcile visits every condition twice — desired + held.)
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task A_surgical_condition_removal_visits_only_its_own_condition()
|
||||
{
|
||||
var (host, server) = await BootAsync();
|
||||
try
|
||||
{
|
||||
var nm = server.NodeManager!;
|
||||
nm.EnsureFolder("eq-1", null, "Equipment 1", AddressSpaceRealm.Uns);
|
||||
const int conditions = 50;
|
||||
for (var i = 0; i < conditions; i++)
|
||||
nm.MaterialiseAlarmCondition($"alm-{i}", "eq-1", $"Alarm {i}", "OffNormalAlarm", 500, AddressSpaceRealm.Uns);
|
||||
|
||||
// A Server-object (all-events) subscriber, so the per-condition monitoring unit is in play.
|
||||
nm.SubscribeToAllEvents(
|
||||
new OperationContext(new RequestHeader(), secureChannelContext: null, RequestType.CreateMonitoredItems, identity: null),
|
||||
1, new StubEventItem(), unsubscribe: false);
|
||||
nm.ConditionEventsMonitored("alm-0", AddressSpaceRealm.Uns).ShouldBeTrue("non-vacuity: the units really were raised");
|
||||
|
||||
var before = nm.AllEventsMonitoringConditionVisits;
|
||||
for (var i = 0; i < 10; i++)
|
||||
nm.RemoveAlarmConditionNode($"alm-{i}", AddressSpaceRealm.Uns).ShouldBeTrue();
|
||||
var visits = nm.AllEventsMonitoringConditionVisits - before;
|
||||
output.WriteLine($"BENCH all-events visits for 10 condition removals over {conditions} conditions: {visits}");
|
||||
|
||||
visits.ShouldBeLessThanOrEqualTo(10, $"ten removals visited {visits} conditions; a full reconcile per removal would visit ~{10 * 2 * conditions}");
|
||||
// The survivors keep their unit.
|
||||
nm.ConditionEventsMonitored("alm-49", AddressSpaceRealm.Uns).ShouldBeTrue();
|
||||
}
|
||||
finally { await host.DisposeAsync(); }
|
||||
}
|
||||
|
||||
private async Task<(OpcUaApplicationHost Host, OtOpcUaSdkServer Server)> BootAsync() =>
|
||||
await OpcUaHostStart.StartAsync(
|
||||
port => new OpcUaApplicationHost(
|
||||
new OpcUaApplicationHostOptions
|
||||
{
|
||||
ApplicationName = "OtOpcUa.BenchTest",
|
||||
ApplicationUri = $"urn:OtOpcUa.BenchTest:{Guid.NewGuid():N}",
|
||||
OpcUaPort = port,
|
||||
PublicHostname = "localhost",
|
||||
PkiStoreRoot = _pkiRoot,
|
||||
},
|
||||
NullLogger<OpcUaApplicationHost>.Instance),
|
||||
() => new OtOpcUaSdkServer(), Ct);
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
if (Directory.Exists(_pkiRoot)) Directory.Delete(_pkiRoot, recursive: true);
|
||||
}
|
||||
|
||||
/// <summary>The minimum of a Server-object event item the all-events bookkeeping touches: identity only.</summary>
|
||||
private sealed class StubEventItem : IEventMonitoredItem
|
||||
{
|
||||
public bool MonitoringAllEvents => true;
|
||||
public NodeId NodeId => ObjectIds.Server;
|
||||
public IUserIdentity EffectiveIdentity => null!;
|
||||
public object ManagerHandle => null!;
|
||||
public void QueueEvent(IFilterTarget instance) { }
|
||||
public void QueueEvent(IFilterTarget instance, bool bypassFilter) { }
|
||||
public void Dispose() { }
|
||||
|
||||
public INodeManager NodeManager => throw new NotSupportedException();
|
||||
public ISession Session => throw new NotSupportedException();
|
||||
public uint Id => 1;
|
||||
public uint SubscriptionId => 1;
|
||||
public bool IsDurable => false;
|
||||
public uint ClientHandle => 1;
|
||||
public ISubscription SubscriptionCallback { get => throw new NotSupportedException(); set => throw new NotSupportedException(); }
|
||||
public int MonitoredItemType => MonitoredItemTypeMask.Events;
|
||||
public bool IsReadyToPublish => false;
|
||||
public bool IsReadyToTrigger { get => false; set { } }
|
||||
public bool IsResendData => false;
|
||||
public MonitoringMode MonitoringMode => MonitoringMode.Reporting;
|
||||
public double SamplingInterval => 0;
|
||||
public EventFilter EventFilter => null!;
|
||||
public void SetupResendDataTrigger() { }
|
||||
public ServiceResult GetCreateResult(out MonitoredItemCreateResult result) => throw new NotSupportedException();
|
||||
public ServiceResult GetModifyResult(out MonitoredItemModifyResult result) => throw new NotSupportedException();
|
||||
public IStoredMonitoredItem ToStorableMonitoredItem() => throw new NotSupportedException();
|
||||
public MonitoringMode SetMonitoringMode(MonitoringMode monitoringMode) => throw new NotSupportedException();
|
||||
public bool Publish(OperationContext context, Queue<EventFieldList> notifications, uint maxNotificationsPerPublish) => throw new NotSupportedException();
|
||||
public ServiceResult ModifyAttributes(DiagnosticsMasks diagnosticsMasks, TimestampsToReturn timestampsToReturn, uint clientHandle, MonitoringFilter originalFilter, MonitoringFilter filterToUse, Opc.Ua.Range range, double samplingInterval, uint queueSize, bool discardOldest) => throw new NotSupportedException();
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user