From afec56d03b16f933a5973400821f7b2bda980e71 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 15 Aug 2026 16:58:02 -0400 Subject: [PATCH] perf(worker): reverse tag index + memoized views + indexed removals in the handle registry --- .../MxAccess/MxAccessHandleRegistryTests.cs | 272 ++++++++++++++++ .../MxAccess/MxAccessHandleRegistry.cs | 291 ++++++++++++++++-- .../MxAccess/MxAccessSession.cs | 31 +- 3 files changed, 555 insertions(+), 39 deletions(-) create mode 100644 src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessHandleRegistryTests.cs diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessHandleRegistryTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessHandleRegistryTests.cs new file mode 100644 index 0000000..14814d7 --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessHandleRegistryTests.cs @@ -0,0 +1,272 @@ +using System; +using System.Collections.Generic; +using ZB.MOM.WW.MxGateway.Worker.MxAccess; + +namespace ZB.MOM.WW.MxGateway.Worker.Tests.MxAccess; + +/// +/// Unit tests for . The registry gained a +/// reverse (server handle, item definition) index, memoized sorted views, and +/// secondary removal indexes so bulk reads and bulk teardown stop rescanning +/// the whole handle table. These tests pin the observable behaviour those +/// structures must preserve: the same lookup semantics the old linear scan +/// had, snapshot views that only rebuild after a mutation, and removals that +/// leave every index consistent. +/// +public sealed class MxAccessHandleRegistryTests +{ + /// + /// Verifies the reverse index answers by server handle and ordinal item + /// definition, returns every duplicate registration in ascending item + /// handle order (the order a scan of + /// would have visited them), and misses on the wrong server, a different + /// tag, and a case-differing tag. + /// + [Fact] + public void GetItemHandlesForDefinition_MatchesServerAndOrdinalTag_InAscendingHandleOrder() + { + MxAccessHandleRegistry registry = new(); + registry.RegisterServerHandle(serverHandle: 1, clientName: "client"); + registry.RegisterServerHandle(serverHandle: 2, clientName: "client"); + + // Same tag added twice under server 1 — MXAccess hands back a distinct + // item handle per AddItem, so the index must keep both, lowest first. + registry.RegisterItemHandle(1, itemHandle: 40, "Tank1.PV", string.Empty, hasItemContext: false); + registry.RegisterItemHandle(1, itemHandle: 10, "Tank1.PV", string.Empty, hasItemContext: false); + registry.RegisterItemHandle(1, itemHandle: 11, "Tank1.SP", string.Empty, hasItemContext: false); + registry.RegisterItemHandle(2, itemHandle: 12, "Tank1.PV", string.Empty, hasItemContext: false); + + Assert.Equal(new[] { 10, 40 }, registry.GetItemHandlesForDefinition(1, "Tank1.PV")); + Assert.Equal(new[] { 11 }, registry.GetItemHandlesForDefinition(1, "Tank1.SP")); + Assert.Equal(new[] { 12 }, registry.GetItemHandlesForDefinition(2, "Tank1.PV")); + + // Misses: unknown server, unknown tag, and a case-differing tag — the + // read path compares tag addresses ordinally, so casing must not match. + Assert.Empty(registry.GetItemHandlesForDefinition(3, "Tank1.PV")); + Assert.Empty(registry.GetItemHandlesForDefinition(1, "Tank9.PV")); + Assert.Empty(registry.GetItemHandlesForDefinition(1, "tank1.pv")); + } + + /// + /// Verifies re-registering an item handle under a new item definition + /// retires the old reverse-index entry instead of leaving a stale hit. + /// + [Fact] + public void RegisterItemHandle_ReusedHandleWithNewDefinition_RetiresStaleIndexEntry() + { + MxAccessHandleRegistry registry = new(); + registry.RegisterItemHandle(1, itemHandle: 10, "Tank1.PV", string.Empty, hasItemContext: false); + registry.RegisterItemHandle(1, itemHandle: 10, "Tank2.PV", string.Empty, hasItemContext: false); + + Assert.Empty(registry.GetItemHandlesForDefinition(1, "Tank1.PV")); + Assert.Equal(new[] { 10 }, registry.GetItemHandlesForDefinition(1, "Tank2.PV")); + Assert.Single(registry.ItemHandles); + } + + /// + /// Verifies the reverse index drops handles as they are removed, both for + /// a single item removal and for a whole-server teardown. + /// + [Fact] + public void GetItemHandlesForDefinition_AfterRemoval_MissesRemovedHandles() + { + MxAccessHandleRegistry registry = new(); + registry.RegisterItemHandle(1, itemHandle: 10, "Tank1.PV", string.Empty, hasItemContext: false); + registry.RegisterItemHandle(1, itemHandle: 11, "Tank1.PV", string.Empty, hasItemContext: false); + + registry.RemoveItemHandle(1, itemHandle: 10); + Assert.Equal(new[] { 11 }, registry.GetItemHandlesForDefinition(1, "Tank1.PV")); + + registry.UnregisterServerHandle(1); + Assert.Empty(registry.GetItemHandlesForDefinition(1, "Tank1.PV")); + } + + /// + /// Verifies each sorted view is memoized: repeated reads hand back the + /// same instance until a mutation of that table invalidates it, and the + /// rebuilt view reflects the mutation while keeping its sort order. + /// + [Fact] + public void Views_AreMemoizedUntilTheirTableMutates() + { + MxAccessHandleRegistry registry = new(); + registry.RegisterServerHandle(serverHandle: 2, clientName: "client"); + registry.RegisterServerHandle(serverHandle: 1, clientName: "client"); + registry.RegisterItemHandle(1, itemHandle: 11, "Tank1.PV", string.Empty, hasItemContext: false); + registry.RegisterItemHandle(1, itemHandle: 10, "Tank1.SP", string.Empty, hasItemContext: false); + registry.RegisterAdviceHandle(1, itemHandle: 10, MxAccessAdviceKind.Supervisory); + registry.RegisterAdviceHandle(1, itemHandle: 10, MxAccessAdviceKind.Plain); + + IReadOnlyList servers = registry.ServerHandles; + IReadOnlyList items = registry.ItemHandles; + IReadOnlyList advices = registry.AdviceHandles; + + // No mutation in between: the memoized arrays are handed back as-is. + Assert.Same(servers, registry.ServerHandles); + Assert.Same(items, registry.ItemHandles); + Assert.Same(advices, registry.AdviceHandles); + + // Sort orders are unchanged by memoization. + Assert.Equal(new[] { 1, 2 }, Map(servers, handle => handle.ServerHandle)); + Assert.Equal(new[] { 10, 11 }, Map(items, handle => handle.ItemHandle)); + Assert.Equal( + new[] { MxAccessAdviceKind.Plain, MxAccessAdviceKind.Supervisory }, + Map(advices, handle => handle.AdviceKind)); + + // A mutation of one table invalidates that view only. + registry.RegisterServerHandle(serverHandle: 3, clientName: "client"); + Assert.NotSame(servers, registry.ServerHandles); + Assert.Equal(3, registry.ServerHandles.Count); + Assert.Same(items, registry.ItemHandles); + Assert.Same(advices, registry.AdviceHandles); + + registry.RemoveItemHandle(1, itemHandle: 11); + Assert.NotSame(items, registry.ItemHandles); + Assert.Single(registry.ItemHandles); + + registry.RemoveAdviceHandles(1, itemHandle: 10); + Assert.NotSame(advices, registry.AdviceHandles); + Assert.Empty(registry.AdviceHandles); + } + + /// + /// Verifies removing an item also removes every advice recorded for it and + /// nothing recorded for a sibling item — the bulk unadvise/remove path + /// leans on this to leave a consistent table. + /// + [Fact] + public void RemoveItemHandle_RemovesOnlyThatItemsAdvices() + { + MxAccessHandleRegistry registry = new(); + registry.RegisterItemHandle(1, itemHandle: 10, "Tank1.PV", string.Empty, hasItemContext: false); + registry.RegisterItemHandle(1, itemHandle: 11, "Tank1.SP", string.Empty, hasItemContext: false); + registry.RegisterAdviceHandle(1, itemHandle: 10, MxAccessAdviceKind.Plain); + registry.RegisterAdviceHandle(1, itemHandle: 10, MxAccessAdviceKind.Supervisory); + registry.RegisterAdviceHandle(1, itemHandle: 11, MxAccessAdviceKind.Plain); + + registry.RemoveItemHandle(1, itemHandle: 10); + + Assert.False(registry.ContainsItemHandle(1, 10)); + Assert.False(registry.ContainsAdviceHandle(1, 10, MxAccessAdviceKind.Plain)); + Assert.False(registry.ContainsAdviceHandle(1, 10, MxAccessAdviceKind.Supervisory)); + Assert.True(registry.ContainsItemHandle(1, 11)); + Assert.True(registry.ContainsAdviceHandle(1, 11, MxAccessAdviceKind.Plain)); + Assert.Single(registry.AdviceHandles); + } + + /// + /// Verifies a bulk unadvise followed by a bulk remove drains the registry + /// completely, so the indexed removals cannot leave orphaned entries that + /// a later lookup would resurrect. + /// + [Fact] + public void BulkUnadviseThenRemove_LeavesRegistryEmpty() + { + MxAccessHandleRegistry registry = new(); + registry.RegisterServerHandle(serverHandle: 1, clientName: "client"); + + for (int itemHandle = 1; itemHandle <= 50; itemHandle++) + { + registry.RegisterItemHandle(1, itemHandle, "Tank." + itemHandle, string.Empty, hasItemContext: false); + registry.RegisterAdviceHandle(1, itemHandle, MxAccessAdviceKind.Plain); + } + + for (int itemHandle = 1; itemHandle <= 50; itemHandle++) + { + registry.RemoveAdviceHandles(1, itemHandle); + } + + Assert.Empty(registry.AdviceHandles); + Assert.Equal(50, registry.ItemHandles.Count); + + for (int itemHandle = 1; itemHandle <= 50; itemHandle++) + { + registry.RemoveItemHandle(1, itemHandle); + } + + Assert.Empty(registry.ItemHandles); + Assert.Empty(registry.GetItemHandlesForDefinition(1, "Tank.25")); + Assert.True(registry.ContainsServerHandle(1)); + } + + /// + /// Verifies unregistering a server drops its items and advices and leaves + /// every other server's handles untouched, including an advice registered + /// for an item that was never added to the item table. + /// + [Fact] + public void UnregisterServerHandle_RemovesOnlyThatServersHandles() + { + MxAccessHandleRegistry registry = new(); + registry.RegisterServerHandle(serverHandle: 1, clientName: "client-one"); + registry.RegisterServerHandle(serverHandle: 2, clientName: "client-two"); + + registry.RegisterItemHandle(1, itemHandle: 10, "Tank1.PV", string.Empty, hasItemContext: false); + registry.RegisterItemHandle(1, itemHandle: 11, "Tank1.SP", string.Empty, hasItemContext: false); + registry.RegisterAdviceHandle(1, itemHandle: 10, MxAccessAdviceKind.Plain); + + // Advice without a matching item registration: the old scan removed it by + // server handle, so the per-server index must reach it too. + registry.RegisterAdviceHandle(1, itemHandle: 99, MxAccessAdviceKind.Supervisory); + + // Server 2 reuses the same item handle values — packing must keep them apart. + registry.RegisterItemHandle(2, itemHandle: 10, "Tank1.PV", string.Empty, hasItemContext: false); + registry.RegisterAdviceHandle(2, itemHandle: 10, MxAccessAdviceKind.Plain); + + registry.UnregisterServerHandle(1); + + Assert.False(registry.ContainsServerHandle(1)); + Assert.False(registry.ContainsItemHandle(1, 10)); + Assert.False(registry.ContainsItemHandle(1, 11)); + Assert.False(registry.ContainsAdviceHandle(1, 10, MxAccessAdviceKind.Plain)); + Assert.False(registry.ContainsAdviceHandle(1, 99, MxAccessAdviceKind.Supervisory)); + Assert.Empty(registry.GetItemHandlesForDefinition(1, "Tank1.PV")); + + Assert.True(registry.ContainsServerHandle(2)); + Assert.True(registry.ContainsItemHandle(2, 10)); + Assert.True(registry.ContainsAdviceHandle(2, 10, MxAccessAdviceKind.Plain)); + Assert.Equal(new[] { 10 }, registry.GetItemHandlesForDefinition(2, "Tank1.PV")); + Assert.Single(registry.ItemHandles); + Assert.Single(registry.AdviceHandles); + Assert.Single(registry.ServerHandles); + } + + /// + /// Verifies re-registering an advice that is already present does not + /// duplicate it in the per-item removal index — a duplicate would survive + /// the removal that drains that index. + /// + [Fact] + public void RegisterAdviceHandle_RegisteredTwice_StillRemovedByOneCall() + { + MxAccessHandleRegistry registry = new(); + registry.RegisterAdviceHandle(1, itemHandle: 10, MxAccessAdviceKind.Plain); + registry.RegisterAdviceHandle(1, itemHandle: 10, MxAccessAdviceKind.Plain); + + Assert.Single(registry.AdviceHandles); + + registry.RemoveAdviceHandles(1, itemHandle: 10); + + Assert.Empty(registry.AdviceHandles); + Assert.False(registry.ContainsAdviceHandle(1, 10, MxAccessAdviceKind.Plain)); + + // Re-advising after the removal must work off a clean index. + registry.RegisterAdviceHandle(1, itemHandle: 10, MxAccessAdviceKind.Plain); + Assert.Single(registry.AdviceHandles); + Assert.True(registry.ContainsAdviceHandle(1, 10, MxAccessAdviceKind.Plain)); + } + + private static List Map( + IReadOnlyList source, + Func selector) + { + List mapped = new(source.Count); + + for (int index = 0; index < source.Count; index++) + { + mapped.Add(selector(source[index])); + } + + return mapped; + } +} diff --git a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessHandleRegistry.cs b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessHandleRegistry.cs index e761594..31531e6 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessHandleRegistry.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessHandleRegistry.cs @@ -4,27 +4,52 @@ using System.Linq; namespace ZB.MOM.WW.MxGateway.Worker.MxAccess; +/// +/// Tracks the server, item, and advice handles owned by one MXAccess session. +/// +/// +/// The registry is STA-confined by contract: every mutation and every query +/// runs on the session's dedicated STA thread (see +/// , which marshals all registry access +/// through that thread), so there is no locking here and the memoized views +/// and secondary indexes below need none either. +/// public sealed class MxAccessHandleRegistry { private readonly Dictionary serverHandles = new(); private readonly Dictionary itemHandles = new(); private readonly Dictionary adviceHandles = new(); + // Secondary indexes. Every register/remove method below maintains these so + // lookups by tag and removals stay proportional to what is actually removed + // instead of scanning a whole primary table. + private readonly Dictionary> itemHandlesByDefinition = new(); + private readonly Dictionary> itemKeysByServer = new(); + private readonly Dictionary> adviceKeysByItem = new(); + private readonly Dictionary> advisedItemKeysByServer = new(); + + // Memoized materializations of the three sorted views. Each is dropped by + // the mutation that invalidates it, so repeated reads between mutations + // hand back the same array instead of re-sorting and re-copying the table. + private RegisteredServerHandle[]? serverHandlesView; + private RegisteredItemHandle[]? itemHandlesView; + private RegisteredAdviceHandle[]? adviceHandlesView; + /// Gets a read-only list of registered server handles ordered by handle value. - public IReadOnlyList ServerHandles => serverHandles + public IReadOnlyList ServerHandles => serverHandlesView ??= serverHandles .Values .OrderBy(handle => handle.ServerHandle) .ToArray(); /// Gets a read-only list of registered item handles ordered by server handle then item handle. - public IReadOnlyList ItemHandles => itemHandles + public IReadOnlyList ItemHandles => itemHandlesView ??= itemHandles .Values .OrderBy(handle => handle.ServerHandle) .ThenBy(handle => handle.ItemHandle) .ToArray(); /// Gets a read-only list of registered advice handles ordered by server handle, item handle, and advice kind. - public IReadOnlyList AdviceHandles => adviceHandles + public IReadOnlyList AdviceHandles => adviceHandlesView ??= adviceHandles .Values .OrderBy(handle => handle.ServerHandle) .ThenBy(handle => handle.ItemHandle) @@ -39,28 +64,38 @@ public sealed class MxAccessHandleRegistry string clientName) { serverHandles[serverHandle] = new RegisteredServerHandle(serverHandle, clientName); + serverHandlesView = null; } /// Unregisters a server handle and all associated item and advice handles from the registry. /// Handle returned by the worker. public void UnregisterServerHandle(int serverHandle) { - serverHandles.Remove(serverHandle); - - foreach (long key in itemHandles - .Where(pair => pair.Value.ServerHandle == serverHandle) - .Select(pair => pair.Key) - .ToArray()) + if (serverHandles.Remove(serverHandle)) { - itemHandles.Remove(key); + serverHandlesView = null; } - foreach (AdviceHandleKey key in adviceHandles - .Where(pair => pair.Value.ServerHandle == serverHandle) - .Select(pair => pair.Key) - .ToArray()) + // Per-server indexes: the removal cost is proportional to the handles + // this server actually owns, not to the size of the whole table. + if (itemKeysByServer.TryGetValue(serverHandle, out HashSet? ownedItemKeys)) { - adviceHandles.Remove(key); + itemKeysByServer.Remove(serverHandle); + + foreach (long itemKey in ownedItemKeys) + { + RemoveItemEntry(itemKey); + } + } + + if (advisedItemKeysByServer.TryGetValue(serverHandle, out HashSet? advisedItemKeys)) + { + advisedItemKeysByServer.Remove(serverHandle); + + foreach (long itemKey in advisedItemKeys) + { + RemoveAdviceEntries(itemKey); + } } } @@ -85,12 +120,32 @@ public sealed class MxAccessHandleRegistry string itemContext, bool hasItemContext) { - itemHandles[CreateItemKey(serverHandle, itemHandle)] = new RegisteredItemHandle( + long itemKey = CreateItemKey(serverHandle, itemHandle); + + if (itemHandles.TryGetValue(itemKey, out RegisteredItemHandle? existing)) + { + // Re-registering the same handle can carry a different item + // definition; drop the stale reverse-index entry first. + RemoveDefinitionIndexEntry(existing); + } + + itemHandles[itemKey] = new RegisteredItemHandle( serverHandle, itemHandle, itemDefinition, itemContext, hasItemContext); + + AddDefinitionIndexEntry(serverHandle, itemHandle, itemDefinition); + + if (!itemKeysByServer.TryGetValue(serverHandle, out HashSet? ownedItemKeys)) + { + ownedItemKeys = new HashSet(); + itemKeysByServer[serverHandle] = ownedItemKeys; + } + + ownedItemKeys.Add(itemKey); + itemHandlesView = null; } /// Removes an item handle and all associated advice handles from the registry. @@ -100,7 +155,19 @@ public sealed class MxAccessHandleRegistry int serverHandle, int itemHandle) { - itemHandles.Remove(CreateItemKey(serverHandle, itemHandle)); + long itemKey = CreateItemKey(serverHandle, itemHandle); + + if (RemoveItemEntry(itemKey) + && itemKeysByServer.TryGetValue(serverHandle, out HashSet? ownedItemKeys)) + { + ownedItemKeys.Remove(itemKey); + + if (ownedItemKeys.Count == 0) + { + itemKeysByServer.Remove(serverHandle); + } + } + RemoveAdviceHandles(serverHandle, itemHandle); } @@ -115,6 +182,34 @@ public sealed class MxAccessHandleRegistry return itemHandles.ContainsKey(CreateItemKey(serverHandle, itemHandle)); } + /// + /// Gets the item handles registered under the specified server handle for + /// the specified item definition, ordered by item handle. + /// + /// Handle returned by the worker. + /// Item definition name, compared ordinally. + /// + /// The matching item handles in ascending order — the same order a scan of + /// would visit them — or an empty list when the + /// definition is not registered for that server. MXAccess allows the same + /// tag to be added more than once under one server handle, so this is a + /// list rather than a single handle. The returned list is a live view of + /// the index and is only valid until the next registry mutation. + /// + public IReadOnlyList GetItemHandlesForDefinition( + int serverHandle, + string itemDefinition) + { + if (itemHandlesByDefinition.TryGetValue( + new ItemDefinitionKey(serverHandle, itemDefinition), + out List? handles)) + { + return handles; + } + + return Array.Empty(); + } + /// Registers an advice handle with the registry. /// Handle returned by the worker. /// Handle returned by the worker. @@ -125,10 +220,36 @@ public sealed class MxAccessHandleRegistry MxAccessAdviceKind adviceKind) { AdviceHandleKey key = new(serverHandle, itemHandle, adviceKind); + + if (!adviceHandles.ContainsKey(key)) + { + // Only index a key the primary table does not already hold, so the + // per-item list never accumulates duplicates. + long itemKey = CreateItemKey(serverHandle, itemHandle); + + if (!adviceKeysByItem.TryGetValue(itemKey, out List? itemAdviceKeys)) + { + itemAdviceKeys = new List(2); + adviceKeysByItem[itemKey] = itemAdviceKeys; + } + + itemAdviceKeys.Add(key); + + if (!advisedItemKeysByServer.TryGetValue(serverHandle, out HashSet? advisedItemKeys)) + { + advisedItemKeys = new HashSet(); + advisedItemKeysByServer[serverHandle] = advisedItemKeys; + } + + advisedItemKeys.Add(itemKey); + } + adviceHandles[key] = new RegisteredAdviceHandle( serverHandle, itemHandle, adviceKind); + + adviceHandlesView = null; } /// Removes all advice handles for the specified server and item handles from the registry. @@ -138,12 +259,17 @@ public sealed class MxAccessHandleRegistry int serverHandle, int itemHandle) { - foreach (AdviceHandleKey key in adviceHandles - .Where(pair => pair.Value.ServerHandle == serverHandle && pair.Value.ItemHandle == itemHandle) - .Select(pair => pair.Key) - .ToArray()) + long itemKey = CreateItemKey(serverHandle, itemHandle); + RemoveAdviceEntries(itemKey); + + if (advisedItemKeysByServer.TryGetValue(serverHandle, out HashSet? advisedItemKeys)) { - adviceHandles.Remove(key); + advisedItemKeys.Remove(itemKey); + + if (advisedItemKeys.Count == 0) + { + advisedItemKeysByServer.Remove(serverHandle); + } } } @@ -167,6 +293,81 @@ public sealed class MxAccessHandleRegistry return ((long)serverHandle << 32) | (uint)itemHandle; } + /// Removes one item from the primary table and the definition index. + /// Packed server/item key produced by . + /// if an item was removed; otherwise, . + private bool RemoveItemEntry(long itemKey) + { + if (!itemHandles.TryGetValue(itemKey, out RegisteredItemHandle? registered)) + { + return false; + } + + itemHandles.Remove(itemKey); + RemoveDefinitionIndexEntry(registered); + itemHandlesView = null; + + return true; + } + + /// Removes every advice handle recorded for one item from the primary table and the per-item index. + /// Packed server/item key produced by . + private void RemoveAdviceEntries(long itemKey) + { + if (!adviceKeysByItem.TryGetValue(itemKey, out List? itemAdviceKeys)) + { + return; + } + + adviceKeysByItem.Remove(itemKey); + + foreach (AdviceHandleKey key in itemAdviceKeys) + { + adviceHandles.Remove(key); + } + + adviceHandlesView = null; + } + + private void AddDefinitionIndexEntry( + int serverHandle, + int itemHandle, + string itemDefinition) + { + ItemDefinitionKey definitionKey = new(serverHandle, itemDefinition); + + if (!itemHandlesByDefinition.TryGetValue(definitionKey, out List? handles)) + { + handles = new List(1); + itemHandlesByDefinition[definitionKey] = handles; + } + + // Kept sorted so callers see the same order a scan of ItemHandles would. + int index = handles.BinarySearch(itemHandle); + + if (index < 0) + { + handles.Insert(~index, itemHandle); + } + } + + private void RemoveDefinitionIndexEntry(RegisteredItemHandle registered) + { + ItemDefinitionKey definitionKey = new(registered.ServerHandle, registered.ItemDefinition); + + if (!itemHandlesByDefinition.TryGetValue(definitionKey, out List? handles)) + { + return; + } + + handles.Remove(registered.ItemHandle); + + if (handles.Count == 0) + { + itemHandlesByDefinition.Remove(definitionKey); + } + } + private readonly struct AdviceHandleKey : IEquatable { private readonly int serverHandle; @@ -214,4 +415,50 @@ public sealed class MxAccessHandleRegistry } } } + + /// + /// Reverse-index key: a server handle plus an item definition, compared + /// ordinally to match the tag comparison the read path performs. + /// + private readonly struct ItemDefinitionKey : IEquatable + { + private readonly int serverHandle; + private readonly string itemDefinition; + + /// Initializes a new instance of the struct. + /// Handle returned by the worker. + /// Item definition name from MXAccess. + public ItemDefinitionKey( + int serverHandle, + string itemDefinition) + { + this.serverHandle = serverHandle; + this.itemDefinition = itemDefinition; + } + + /// + public bool Equals(ItemDefinitionKey other) + { + return serverHandle == other.serverHandle + && string.Equals(itemDefinition, other.itemDefinition, StringComparison.Ordinal); + } + + /// + public override bool Equals(object? obj) + { + return obj is ItemDefinitionKey other && Equals(other); + } + + /// + public override int GetHashCode() + { + unchecked + { + int hashCode = serverHandle; + hashCode = (hashCode * 397) ^ (itemDefinition ?? string.Empty).GetHashCode(); + + return hashCode; + } + } + } } diff --git a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessSession.cs b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessSession.cs index 78f1c0d..ee394ab 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessSession.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessSession.cs @@ -990,23 +990,20 @@ public sealed class MxAccessSession : IDisposable out int itemHandle, out MxAccessValueCache.CachedValue cachedValue) { - // Linear scan — bulk-read sizes are small in practice and the registry - // is keyed by handle, not by tag. If profiling ever shows this hot, a - // reverse tag→handle map can be added on the registry side. - foreach (RegisteredItemHandle registered in handleRegistry.ItemHandles) + // Dictionary probe, not a scan: the registry keeps a reverse + // (server handle, item definition) → item handle index, so a bulk read + // no longer walks the whole item table once per tag. The index matches + // the tag ordinally and hands the handles back in ascending order — the + // same order the previous linear scan over the sorted ItemHandles view + // visited them — so the first advised-and-cached candidate still wins. + IReadOnlyList candidates = handleRegistry.GetItemHandlesForDefinition(serverHandle, tagAddress); + + for (int index = 0; index < candidates.Count; index++) { - if (registered.ServerHandle != serverHandle) - { - continue; - } + int candidate = candidates[index]; - if (!string.Equals(registered.ItemDefinition, tagAddress, StringComparison.Ordinal)) - { - continue; - } - - if (!handleRegistry.ContainsAdviceHandle(serverHandle, registered.ItemHandle, MxAccessAdviceKind.Plain) - && !handleRegistry.ContainsAdviceHandle(serverHandle, registered.ItemHandle, MxAccessAdviceKind.Supervisory)) + if (!handleRegistry.ContainsAdviceHandle(serverHandle, candidate, MxAccessAdviceKind.Plain) + && !handleRegistry.ContainsAdviceHandle(serverHandle, candidate, MxAccessAdviceKind.Supervisory)) { // Tag is added but not advised — no fresh OnDataChange will // arrive without us advising. Fall through to the snapshot @@ -1014,9 +1011,9 @@ public sealed class MxAccessSession : IDisposable continue; } - if (valueCache.TryGet(serverHandle, registered.ItemHandle, out cachedValue)) + if (valueCache.TryGet(serverHandle, candidate, out cachedValue)) { - itemHandle = registered.ItemHandle; + itemHandle = candidate; return true; } }