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

perf(client): lazy child detection in BrowseAsync, NodeId-indexed UI notifications

Browse Source 
R08 3.6: BrowseAsync no longer issues one HasChildren Browse per Object child; an
Object reports HasChildren (may have children) and is confirmed when expanded or
recursed. ISessionAdapter.HasChildrenAsync is removed. The UI subscriptions panel
routes each notification through a NodeId index kept in step with
ActiveSubscriptions (add/remove/replace/reset) instead of scanning every row.

Refs #747
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 16:27:24 -04:00
parent a74a18e49a
commit 540b1632c5
8 changed files with 135 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/DefaultSessionAdapter.cs
-16
View File
@@ -115,22 +115,6 @@ internal sealed class DefaultSessionAdapter : ISessionAdapter
return (nextCp, nextRefs ?? []);
}
/// <inheritdoc />
public async Task<bool> HasChildrenAsync(NodeId nodeId, CancellationToken ct)
{
var (_, _, references) = await _session.BrowseAsync(
null,
null,
nodeId,
1u,
BrowseDirection.Forward,
ReferenceTypeIds.HierarchicalReferences,
true,
0u);
return references != null && references.Count > 0;
}
/// <inheritdoc />
public async Task<Models.HistoryUpdateOutcome> HistoryUpdateAsync(HistoryUpdateDetails details, CancellationToken ct)
{
src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/ISessionAdapter.cs
-8
View File
@@ -90,14 +90,6 @@ internal interface ISessionAdapter : IDisposable
Task<(byte[]? ContinuationPoint, ReferenceDescriptionCollection References)> BrowseNextAsync(
byte[] continuationPoint, CancellationToken ct = default);
/// <summary>
/// Checks whether a node has any forward hierarchical child references.
/// </summary>
/// <param name="nodeId">The node to inspect for child objects or variables.</param>
/// <param name="ct">The cancellation token that aborts the child lookup.</param>
/// <returns><c>true</c> when the node has at least one forward hierarchical child reference; otherwise <c>false</c>.</returns>
Task<bool> HasChildrenAsync(NodeId nodeId, CancellationToken ct = default);
/// <summary>
/// Reads raw historical data.
/// </summary>
src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Models/BrowseResult.cs
+3 -1
View File
@@ -34,7 +34,9 @@ public sealed class BrowseResult
public string NodeClass { get; }
/// <summary>
/// Whether the node has child references.
/// Whether the node MAY have child references: <see langword="true"/> for every Object, confirmed lazily when the
/// caller browses it (an expanded empty folder simply yields no children). Variables and Methods report
/// <see langword="false"/>.
/// </summary>
public bool HasChildren { get; }
}
src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs
+4 -7
View File
@@ -224,18 +224,15 @@ public sealed class OpcUaClientService : IOpcUaClientService
while (references.Count > 0)
{
// Child detection is lazy: an Object MAY have children and is confirmed when the caller expands or
// recurses into it. Probing each Object child with its own Browse made one level of N folders cost N + 1
// round trips, and a recursive subscribe over a 50k-node tree ~50k extra Browse calls.
foreach (var reference in references)
{
var childNodeId = ExpandedNodeId.ToNodeId(reference.NodeId, _session.NamespaceUris);
var hasChildren = reference.NodeClass == NodeClass.Object &&
await _session.HasChildrenAsync(childNodeId, ct);
results.Add(new BrowseResult(
reference.NodeId.ToString(),
reference.DisplayName?.Text ?? string.Empty,
reference.NodeClass.ToString(),
hasChildren));
}
reference.NodeClass == NodeClass.Object));
if (continuationPoint is not { Length: > 0 } pending) break;
(continuationPoint, references) = await _session.BrowseNextAsync(pending, ct);
src/Client/ZB.MOM.WW.OtOpcUa.Client.UI/ViewModels/SubscriptionsViewModel.cs
+44 -8
View File
@@ -1,4 +1,5 @@
using System.Collections.ObjectModel;
using System.Collections.Specialized;
using CommunityToolkit.Mvvm.ComponentModel;
using CommunityToolkit.Mvvm.Input;
using Opc.Ua;
@@ -19,6 +20,11 @@ public partial class SubscriptionsViewModel : ObservableObject
private readonly IUiDispatcher _dispatcher;
private readonly IOpcUaClientService _service;
// NodeId -> its rows, kept in step with ActiveSubscriptions by OnActiveSubscriptionsChanged, so a notification is
// one lookup instead of a scan of every row on the UI thread (2 000 items at 1 Hz was 4M string compares/s).
// Touched only on the UI thread, like the collection itself.
private readonly Dictionary<string, List<SubscriptionItemViewModel>> _rowsByNodeId = new(StringComparer.Ordinal);
/// <summary>
/// Last user-visible status message — set when a subscribe/unsubscribe operation fails so the
/// shell can surface the diagnostic instead of silently dropping the error. Cleared on success.
@@ -49,9 +55,13 @@ public partial class SubscriptionsViewModel : ObservableObject
{
_service = service;
_dispatcher = dispatcher;
ActiveSubscriptions.CollectionChanged += OnActiveSubscriptionsChanged;
_service.DataChanged += OnDataChanged;
}
/// <summary>The number of distinct NodeIds in the notification index (test seam).</summary>
internal int IndexedNodeCount => _rowsByNodeId.Count;
/// <summary>Currently active subscriptions.</summary>
public ObservableCollection<SubscriptionItemViewModel> ActiveSubscriptions { get; } = [];
@@ -62,16 +72,42 @@ public partial class SubscriptionsViewModel : ObservableObject
{
_dispatcher.Post(() =>
{
foreach (var item in ActiveSubscriptions)
if (item.NodeId == e.NodeId)
{
item.Value = Helpers.ValueFormatter.Format(e.Value.Value);
item.Status = Helpers.StatusCodeFormatter.Format(e.Value.StatusCode);
item.Timestamp = e.Value.SourceTimestamp.ToString("O");
}
if (!_rowsByNodeId.TryGetValue(e.NodeId, out var rows)) return;
foreach (var item in rows)
{
item.Value = Helpers.ValueFormatter.Format(e.Value.Value);
item.Status = Helpers.StatusCodeFormatter.Format(e.Value.StatusCode);
item.Timestamp = e.Value.SourceTimestamp.ToString("O");
}
});
}
private void OnActiveSubscriptionsChanged(object? sender, NotifyCollectionChangedEventArgs e)
{
if (e.Action == NotifyCollectionChangedAction.Reset)
{
_rowsByNodeId.Clear();
foreach (var item in ActiveSubscriptions) Index(item);
return;
}
if (e.OldItems != null)
foreach (SubscriptionItemViewModel item in e.OldItems)
if (_rowsByNodeId.TryGetValue(item.NodeId, out var rows) && rows.Remove(item) && rows.Count == 0)
_rowsByNodeId.Remove(item.NodeId);
if (e.NewItems != null)
foreach (SubscriptionItemViewModel item in e.NewItems)
Index(item);
}
private void Index(SubscriptionItemViewModel item)
{
if (!_rowsByNodeId.TryGetValue(item.NodeId, out var rows))
_rowsByNodeId[item.NodeId] = rows = [];
rows.Add(item);
}
private bool CanAddSubscription()
{
return IsConnected && !string.IsNullOrWhiteSpace(NewNodeIdText);
@@ -149,7 +185,7 @@ public partial class SubscriptionsViewModel : ObservableObject
if (!IsConnected || string.IsNullOrWhiteSpace(nodeIdStr)) return;
// Skip if already subscribed
if (ActiveSubscriptions.Any(s => s.NodeId == nodeIdStr)) return;
if (_rowsByNodeId.ContainsKey(nodeIdStr)) return;
try
{
tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/Fakes/FakeSessionAdapter.cs
-11
View File
@@ -28,8 +28,6 @@ internal sealed class FakeSessionAdapter : ISessionAdapter
public int BrowseCount { get; private set; }
/// <summary>Gets the number of times BrowseNextAsync has been called.</summary>
public int BrowseNextCount { get; private set; }
/// <summary>Gets the number of times HasChildrenAsync has been called.</summary>
public int HasChildrenCount { get; private set; }
/// <summary>Gets the number of times HistoryReadRawAsync has been called.</summary>
public int HistoryReadRawCount { get; private set; }
/// <summary>Gets the number of times HistoryReadAggregateAsync has been called.</summary>
@@ -57,8 +55,6 @@ internal sealed class FakeSessionAdapter : ISessionAdapter
public ReferenceDescriptionCollection BrowseNextResponse { get; set; } = [];
/// <summary>Gets or sets the continuation point for browse-next operations.</summary>
public byte[]? BrowseNextContinuationPoint { get; set; }
/// <summary>Gets or sets the result returned by has-children operations.</summary>
public bool HasChildrenResponse { get; set; } = false;
/// <summary>Gets or sets the historical values returned by raw history-read operations.</summary>
public List<DataValue> HistoryReadRawResponse { get; set; } = [];
@@ -150,13 +146,6 @@ internal sealed class FakeSessionAdapter : ISessionAdapter
return Task.FromResult((BrowseNextContinuationPoint, BrowseNextResponse));
}
/// <inheritdoc />
public Task<bool> HasChildrenAsync(NodeId nodeId, CancellationToken ct)
{
HasChildrenCount++;
return Task.FromResult(HasChildrenResponse);
}
/// <inheritdoc />
public Task<IReadOnlyList<DataValue>> HistoryReadRawAsync(
NodeId nodeId, DateTime startTime, DateTime endTime, int maxValues, CancellationToken ct)
tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs
+20 -6
View File
@@ -384,10 +384,12 @@ public class OpcUaClientServiceTests : IDisposable
}
/// <summary>
/// Verifies that object nodes trigger child-detection checks so the client can mark expandable branches.
/// R08 3.6: browsing a level issues ONE Browse (plus BrowseNext pages) however many Object children it holds. Child
/// detection is lazy: an Object may have children and is confirmed when it is expanded/recursed, instead of one
/// extra Browse per Object child (~50 000 calls for a recursive subscribe over a 50k-node raw tree).
/// </summary>
[Fact]
public async Task BrowseAsync_ObjectNode_ChecksHasChildren()
public async Task BrowseAsync_ObjectChildren_MayHaveChildren_WithoutAPerChildBrowse()
{
var session = new FakeSessionAdapter
{
@@ -398,17 +400,29 @@ public class OpcUaClientServiceTests : IDisposable
NodeId = new ExpandedNodeId("ns=2;s=Folder1"),
DisplayName = new LocalizedText("Folder1"),
NodeClass = NodeClass.Object
},
new ReferenceDescription
{
NodeId = new ExpandedNodeId("ns=2;s=Folder2"),
DisplayName = new LocalizedText("Folder2"),
NodeClass = NodeClass.Object
},
new ReferenceDescription
{
NodeId = new ExpandedNodeId("ns=2;s=Tag"),
DisplayName = new LocalizedText("Tag"),
NodeClass = NodeClass.Variable
}
],
HasChildrenResponse = true
]
};
_sessionFactory.EnqueueSession(session);
await _service.ConnectAsync(ValidSettings());
var results = await _service.BrowseAsync();
results[0].HasChildren.ShouldBeTrue();
session.HasChildrenCount.ShouldBe(1);
results.Select(r => r.HasChildren).ShouldBe([true, true, false]);
session.BrowseCount.ShouldBe(1);
session.BrowseNextCount.ShouldBe(0);
}
/// <summary>
tests/Client/ZB.MOM.WW.OtOpcUa.Client.UI.Tests/SubscriptionsViewModelTests.cs
+64
View File
@@ -111,6 +111,70 @@ public class SubscriptionsViewModelTests
_vm.ActiveSubscriptions[0].Value.ShouldBeNull();
}
/// <summary>
/// R08 3.6: notifications are routed through a NodeId index, not a scan of every row. The index follows every
/// mutation of <see cref="SubscriptionsViewModel.ActiveSubscriptions"/> (add, remove, replace, clear).
/// </summary>
[Fact]
public void NodeId_index_follows_every_collection_mutation()
{
var a = new SubscriptionItemViewModel("ns=2;s=A", 1000);
var b = new SubscriptionItemViewModel("ns=2;s=B", 1000);
var c = new SubscriptionItemViewModel("ns=2;s=C", 1000);
_vm.ActiveSubscriptions.Add(a);
_vm.ActiveSubscriptions.Add(b);
_vm.ActiveSubscriptions.Add(c);
_vm.IndexedNodeCount.ShouldBe(3);
_vm.ActiveSubscriptions.Remove(b);
_vm.IndexedNodeCount.ShouldBe(2);
_vm.ActiveSubscriptions[0] = new SubscriptionItemViewModel("ns=2;s=D", 1000); // replaces A
Raise("ns=2;s=A", 1);
Raise("ns=2;s=D", 2);
a.Value.ShouldBeNull();
_vm.ActiveSubscriptions[0].Value.ShouldBe("2");
_vm.ActiveSubscriptions.Clear();
_vm.IndexedNodeCount.ShouldBe(0);
}
/// <summary>A removed row stops receiving values while its sibling (the positive control) still does.</summary>
[Fact]
public void DataChanged_after_remove_updates_only_the_remaining_rows()
{
var a = new SubscriptionItemViewModel("ns=2;s=A", 1000);
var b = new SubscriptionItemViewModel("ns=2;s=B", 1000);
_vm.ActiveSubscriptions.Add(a);
_vm.ActiveSubscriptions.Add(b);
_vm.ActiveSubscriptions.Remove(a);
Raise("ns=2;s=A", 7);
Raise("ns=2;s=B", 42);
a.Value.ShouldBeNull();
b.Value.ShouldBe("42");
}
/// <summary>Two rows for the same node (no dedup on the manual-add path) both update, as before.</summary>
[Fact]
public void DataChanged_updates_every_row_for_the_node()
{
var first = new SubscriptionItemViewModel("ns=2;s=A", 1000);
var second = new SubscriptionItemViewModel("ns=2;s=A", 500);
_vm.ActiveSubscriptions.Add(first);
_vm.ActiveSubscriptions.Add(second);
Raise("ns=2;s=A", 5);
first.Value.ShouldBe("5");
second.Value.ShouldBe("5");
}
private void Raise(string nodeId, int value) =>
_service.RaiseDataChanged(new DataChangedEventArgs(nodeId,
new DataValue(new Variant(value), StatusCodes.Good, DateTime.UtcNow)));
/// <summary>Verifies that Clear removes all subscriptions.</summary>
[Fact]
public void Clear_RemovesAllSubscriptions()