dohertj2/lmxopcua
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(opcua): #855 perf review 1 — groups evaluate their own filter clone; lazy fields; tighter equivalence tests

Browse Source 
- A filter group owns a deep clone of its first filter, validated once and
  evaluated under the group's lock: no item-owned filter is mutated or raced
  (Validate mutates a select clause).
- The projection is computed in the SDK's order: where clause first, fields
  only for an item that will queue.
- Unambiguous locale key; counter for filters given a private group past the
  registry cap; DeliveryCache getter returns its own allocation.
- UseProjectedEventDelivery=false also asks the role and grant checks per
  item, so the equivalence test compares against the pre-optimisation path.
- Equivalence test: same select list under three where clauses, a filter
  modified mid-run, a session with other locales, an ungranted anonymous
  Server-object item, per-rig counter; plus a small overflowing queue with
  DiscardOldest true and false.

Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk
This commit is contained in:
Joseph Doherty
2026-11-11 13:59:22 -05:00
parent 98d962b24b
commit ca0e73fbca
5 changed files with 317 additions and 120 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs
+26 -9
View File
@@ -1711,19 +1711,31 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
/// </summary>
private void QueueToItem(IEventMonitoredItem item, IFilterTarget e)
{
if (e is ConditionEventSnapshot snapshot) snapshot.DeliveryCache.Queue(Server, item, e, UseProjectedEventDelivery);
else item.QueueEvent(e);
if (e is ConditionEventSnapshot snapshot)
{
if (snapshot.DeliveryCache.Queue(Server, item, e, UseProjectedEventDelivery)) Interlocked.Increment(ref _projectedDeliveries);
}
else
{
item.QueueEvent(e);
}
}
private long _projectedDeliveries;
/// <summary>Items this node manager served through the projected path (test observation: it is really taken).</summary>
internal long ProjectedDeliveries => Interlocked.Read(ref _projectedDeliveries);
/// <summary>
/// Whether condition events are queued through the report's shared projection (#855 perf, default) or through the
/// SDK's own <c>QueueEvent(IFilterTarget)</c> per item. A test switch: the two must deliver identical events.
/// SDK's own <c>QueueEvent(IFilterTarget)</c> per item, with the role and grant checks asked per item (the
/// pre-optimisation path). A test switch: the two must deliver identical events.
/// </summary>
internal bool UseProjectedEventDelivery { get; set; } = true;
/// <summary>The SDK's role-permission verdict for one item, shared per (session, identity) within a report (#855 perf).</summary>
private bool RoleAllows(IEventMonitoredItem item, IFilterTarget e) =>
e is ConditionEventSnapshot snapshot
UseProjectedEventDelivery && e is ConditionEventSnapshot snapshot
? snapshot.DeliveryCache.RoleAllows(item, i => !ServiceResult.IsBad(ValidateEventRolePermissions(i, e)))
: !ServiceResult.IsBad(ValidateEventRolePermissions(item, e));
@@ -1734,7 +1746,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
private bool IsConditionVisibleTo(
IEventMonitoredItem item, List<NodeId> scopes, Dictionary<object, bool> perIdentity)
{
if (item.EffectiveIdentity is not { } identity) return IsConditionVisibleTo(item, scopes);
if (!UseProjectedEventDelivery || item.EffectiveIdentity is not { } identity) return IsConditionVisibleTo(item, scopes);
if (perIdentity.TryGetValue(identity, out var cached)) return cached;
var visible = IsConditionVisibleTo(item, scopes);
perIdentity[identity] = visible;
@@ -1938,10 +1950,15 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
/// the report's own walk uses it (sequentially, even when part of the walk runs off <c>Lock</c>); dropped when the
/// walk ends.
/// </summary>
public EventDeliveryCache DeliveryCache =>
Volatile.Read(ref _deliveryCache)
?? Interlocked.CompareExchange(ref _deliveryCache, new EventDeliveryCache(), null)
?? _deliveryCache!;
public EventDeliveryCache DeliveryCache
{
get
{
if (Volatile.Read(ref _deliveryCache) is { } existing) return existing;
var created = new EventDeliveryCache();
return Interlocked.CompareExchange(ref _deliveryCache, created, null) ?? created;
}
}
/// <summary>Drops the delivery cache once the walk is over (the snapshot stays referenced from the item queues).</summary>
public void ReleaseDeliveryCache() => Volatile.Write(ref _deliveryCache, null);
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ProjectedEventDelivery.cs
+130 -55
View File
@@ -57,7 +57,8 @@ internal sealed class EventDeliveryCache
/// <remarks>
/// The SDK check builds an <c>OperationContext</c> from the item and asks <c>ReceiveEvents</c> on the event's type
/// and source for that context's identity and session — nothing else of the item takes part, so two items of one
/// session and identity always get the same answer for one event.
/// session and identity always get the same answer for one event. Held for the report's walk: on a walk that
/// suspends (none of ours does) a <c>RolePermissions</c> change between two hops lands one event later.
/// </remarks>
public bool RoleAllows(IEventMonitoredItem item, Func<IEventMonitoredItem, bool> compute)
{
@@ -80,16 +81,20 @@ internal sealed class EventDeliveryCache
/// <param name="item">The event item.</param>
/// <param name="e">The condition event.</param>
/// <param name="projected">False routes it through the SDK's own <c>QueueEvent</c> (the node manager's test switch).</param>
public void Queue(IServerInternal server, IEventMonitoredItem item, IFilterTarget e, bool projected)
/// <returns>True when the projected path handled the item (queued, or decided not to queue exactly as the SDK would).</returns>
public bool Queue(IServerInternal server, IEventMonitoredItem item, IFilterTarget e, bool projected)
{
if (!projected
|| e is not InstanceStateSnapshot { Handle: ConditionState condition }
|| condition.SupportsFilteredRetain?.Value == true
|| item.GetType() != typeof(SdkMonitoredItem)
|| !ProjectedEventDelivery.TryQueue(server, (SdkMonitoredItem)item, e, this))
if (projected
&& e is InstanceStateSnapshot { Handle: ConditionState condition }
&& condition.SupportsFilteredRetain?.Value != true
&& item.GetType() == typeof(SdkMonitoredItem)
&& ProjectedEventDelivery.TryQueue(server, (SdkMonitoredItem)item, e, this))
{
item.QueueEvent(e);
return true;
}
item.QueueEvent(e);
return false;
}
/// <summary>Marks the item as queued for this report; false when it already was (the SDK's duplicate check).</summary>
@@ -98,28 +103,67 @@ internal sealed class EventDeliveryCache
lock (_sync) return _queued.Add(item);
}
/// <summary>The projection of the event for a filter group and locale list, computed on first use.</summary>
internal Projection ProjectionFor(FilterGroup group, string locales, Func<Projection> compute)
/// <summary>The projection of the event for a filter group and locale list, created on first use.</summary>
internal Projection ProjectionFor(FilterGroup group, string locales, Func<Projection> create)
{
lock (_sync)
{
if (_projections.TryGetValue((group, locales), out var cached)) return cached;
var projection = create();
_projections[(group, locales)] = projection;
return projection;
}
var projection = compute();
lock (_sync) _projections[(group, locales)] = projection;
return projection;
}
/// <summary>The event as one filter sees it: whether the where clause passes, and the selected field values.</summary>
/// <remarks>The fields are shared by every item queued from this projection: <see cref="ArrayOf{T}"/> is a read-only view, and the array behind it is never written after it is built.</remarks>
internal sealed record Projection(bool Passes, ArrayOf<Variant> Fields);
/// <summary>
/// The event as one filter group sees it, each part computed once and only when needed, in the SDK's order: the
/// where clause first, the select-clause fields only for an item that will actually queue the event.
/// </summary>
/// <remarks>
/// The fields are shared by every item queued from this projection: <see cref="ArrayOf{T}"/> is a read-only view,
/// and the array behind it is never written after it is built. Only the report's own walk uses a projection, one
/// step at a time; the lazies are thread-safe regardless.
/// </remarks>
internal sealed class Projection(Func<bool> passes, Func<ArrayOf<Variant>> fields)
{
private readonly Lazy<bool> _passes = new(passes, LazyThreadSafetyMode.ExecutionAndPublication);
private readonly Lazy<ArrayOf<Variant>> _fields = new(fields, LazyThreadSafetyMode.ExecutionAndPublication);
public bool Passes => _passes.Value;
public ArrayOf<Variant> Fields => _fields.Value;
}
}
/// <summary>A set of value-equal <see cref="EventFilter"/>s (one canonical representative), so they can share a projection.</summary>
internal sealed class FilterGroup(EventFilter representative)
/// <summary>
/// A set of value-equal <see cref="EventFilter"/>s sharing one projection. The group owns a deep CLONE of the first
/// member (validated once, here), so evaluating it never touches — or races — a filter object an item owns (#855 perf
/// review: <c>Validate</c> mutates a select clause).
/// </summary>
internal sealed class FilterGroup
{
public EventFilter Representative { get; } = representative;
public FilterGroup(EventFilter filter, IServerInternal server)
{
Representative = (EventFilter)CoreUtils.Clone(filter)!;
// The SDK's GetRejectedSelectClauses: validation depends on the namespaces and type tree, never on the locales.
var context = new FilterContext(server.NamespaceUris, server.TypeTree, server.Telemetry);
var clauses = Representative.SelectClauses;
Rejected = new bool[clauses.Count];
for (var i = 0; i < Rejected.Length; i++)
{
var clause = clauses[i];
Rejected[i] = clause is null || (!clause.Validated && ServiceResult.IsBad(clause.Validate(context, 0)));
}
}
/// <summary>The group's own copy of the filter; evaluated only under <see cref="Gate"/>.</summary>
public EventFilter Representative { get; }
/// <summary>Per select clause: rejected (always projected as null), as the SDK decides once per filter.</summary>
public bool[] Rejected { get; }
/// <summary>Serialises evaluation of <see cref="Representative"/> (two reports may project it at once).</summary>
public object Gate { get; } = new();
}
/// <summary>The SDK internals the projected fast path reads, resolved once by reflection — see <see cref="EventDeliveryCache"/>.</summary>
@@ -165,7 +209,9 @@ internal static class ProjectedEventDelivery
if (!cache.TryMarkQueued(item)) return true; // the SDK's duplicate check (see EventDeliveryCache)
var locales = item.Session?.PreferredLocales; // string[]?, exactly what the SDK hands its FilterContext and TranslateValue
var projection = cache.ProjectionFor(GroupOf(filter), LocalesKey(locales), () => Project(server, filter, locales, e));
var group = GroupOf(filter, server);
var projection = cache.ProjectionFor(group, LocalesKey(locales), () => new EventDeliveryCache.Projection(
() => Passes(server, group, locales, e), () => Project(server, group, locales, e)));
if (!projection.Passes || handler.SetQueueOverflowIfFull()) return true;
Interlocked.Increment(ref s_projectedQueued);
@@ -179,42 +225,55 @@ internal static class ProjectedEventDelivery
}
}
/// <summary>The SDK's where-clause evaluation and <c>GetEventFields</c> projection, for one filter and locale list.</summary>
private static EventDeliveryCache.Projection Project(
IServerInternal server, EventFilter filter, string[]? locales, IFilterTarget e)
/// <summary>The SDK's <c>CanSendFilteredAlarm</c> without filtered retain: the where clause, for one group and locale list.</summary>
private static bool Passes(IServerInternal server, FilterGroup group, string[]? locales, IFilterTarget e)
{
var context = new FilterContext(
server.NamespaceUris, server.TypeTree, locales!, server.Telemetry); // as the SDK: Session?.PreferredLocales, null included
var passes = filter.WhereClause.Evaluate(context, e);
var clauses = filter.SelectClauses;
var fields = new Variant[clauses.Count];
for (var i = 0; i < fields.Length; i++)
lock (group.Gate)
{
var clause = clauses[i];
if (clause is null || (!clause.Validated && ServiceResult.IsBad(clause.Validate(context, 0))))
{
fields[i] = Variant.Null; // a rejected select clause
continue;
}
var value = e.GetAttributeValue(context, clause.TypeDefinitionId, clause.BrowsePath, clause.AttributeId, clause.ParsedIndexRange);
if (value.IsNull)
{
fields[i] = Variant.Null;
continue;
}
if (value.TypeInfo.BuiltInType == BuiltInType.LocalizedText)
value = server.ResourceManager.TranslateValue(locales!, value);
fields[i] = value;
return group.Representative.WhereClause.Evaluate(ContextFor(server, locales), e);
}
return new EventDeliveryCache.Projection(passes, (ArrayOf<Variant>)fields);
}
/// <summary>The SDK's <c>GetEventFields</c> values, for one group and locale list.</summary>
private static ArrayOf<Variant> Project(IServerInternal server, FilterGroup group, string[]? locales, IFilterTarget e)
{
lock (group.Gate)
{
var context = ContextFor(server, locales);
var clauses = group.Representative.SelectClauses;
var fields = new Variant[clauses.Count];
for (var i = 0; i < fields.Length; i++)
{
if (group.Rejected[i])
{
fields[i] = Variant.Null;
continue;
}
var clause = clauses[i];
var value = e.GetAttributeValue(context, clause.TypeDefinitionId, clause.BrowsePath, clause.AttributeId, clause.ParsedIndexRange);
if (value.IsNull)
{
fields[i] = Variant.Null;
continue;
}
if (value.TypeInfo.BuiltInType == BuiltInType.LocalizedText)
value = server.ResourceManager.TranslateValue(locales!, value);
fields[i] = value;
}
return fields;
}
}
/// <summary>The SDK's per-item <c>FilterContext</c>: the server's namespaces and type tree, the session's locales (null included).</summary>
private static FilterContext ContextFor(IServerInternal server, string[]? locales) =>
new(server.NamespaceUris, server.TypeTree, locales!, server.Telemetry);
/// <summary>The canonical group of a filter object: value-equal filters share one (decided once per filter object).</summary>
private static FilterGroup GroupOf(EventFilter filter) =>
s_groups.GetValue(filter, static f =>
private static FilterGroup GroupOf(EventFilter filter, IServerInternal server) =>
s_groups.GetValue(filter, f =>
{
var key = SelectKey(f);
lock (s_registryLock)
@@ -227,14 +286,24 @@ internal static class ProjectedEventDelivery
}
}
var created = new FilterGroup(f);
if (s_registered >= MaxSharedFilterGroups) return created;
var created = new FilterGroup(f, server);
if (s_registered >= MaxSharedFilterGroups)
{
Interlocked.Increment(ref s_unsharedGroups);
return created;
}
s_registered++;
(bucket ??= s_registry[key] = []).Add(created);
return created;
}
});
private static long s_unsharedGroups;
/// <summary>Filters given a private group because the shared registry is full (observability; sharing stops, delivery is unchanged).</summary>
internal static long UnsharedFilterGroups => Interlocked.Read(ref s_unsharedGroups);
/// <summary>A cheap bucket key over the select clauses (equality within a bucket is decided by <c>Utils.IsEqual</c>).</summary>
private static string SelectKey(EventFilter filter)
{
@@ -255,8 +324,14 @@ internal static class ProjectedEventDelivery
return string.Join("|", parts);
}
private static string LocalesKey(string[]? locales) =>
locales is null ? "\u0002" : string.Join("\u0001", locales);
/// <summary>An unambiguous key for a locale list (length-prefixed entries; null distinct from empty).</summary>
private static string LocalesKey(string[]? locales)
{
if (locales is null) return "null";
var key = new System.Text.StringBuilder().Append(locales.Length).Append(':');
foreach (var locale in locales) key.Append(locale?.Length ?? -1).Append(':').Append(locale).Append(';');
return key.ToString();
}
private static Func<SdkMonitoredItem, T?>? FieldGetter<T>(string name) where T : class
{