Skip to content

Commit

Permalink
Make m_rootNotifiers a concurrent NodeIdDictionary
Browse files Browse the repository at this point in the history
  • Loading branch information
romanett committed Dec 17, 2024
1 parent 72c549b commit 50ef0bf
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 71 deletions.
3 changes: 2 additions & 1 deletion Applications/Quickstarts.Servers/Alarms/AlarmNodeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Threading;
using Opc.Ua;
using Opc.Ua.Server;
Expand Down Expand Up @@ -854,7 +855,7 @@ public override ServiceResult ConditionRefresh(
{
if (RootNotifiers != null)
{
nodesToRefresh.AddRange(RootNotifiers);
nodesToRefresh.AddRange(RootNotifiers.Values.ToList());
}
}
else
Expand Down
116 changes: 46 additions & 70 deletions Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ public string AliasRoot
/// <summary>
/// The root notifiers for the node manager.
/// </summary>
protected List<NodeState> RootNotifiers => m_rootNotifiers;
protected NodeIdDictionary<NodeState> RootNotifiers => m_rootNotifiers;

Check warning on line 230 in Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs#L230

Added line #L230 was not covered by tests

/// <summary>
/// Gets the table of nodes being monitored.
Expand Down Expand Up @@ -572,33 +572,29 @@ protected virtual void AddPredefinedNode(ISystemContext context, NodeState node)
{
AddTypesToTypeTree(type);
}
lock (Lock)

// update the root notifiers.
NodeState rootNotifier = null;
if (m_rootNotifiers?.TryGetValue(activeNode.NodeId, out rootNotifier) == true)
{
// update the root notifiers.
if (m_rootNotifiers != null)
//update entry only if it was not changed or deleted by another thread in an atomic operation
if (m_rootNotifiers.TryUpdate(activeNode.NodeId, activeNode, rootNotifier))
{
for (int ii = 0; ii < m_rootNotifiers.Count; ii++)
// need to prevent recursion with the server object.
if (activeNode.NodeId != ObjectIds.Server)
{
if (m_rootNotifiers[ii].NodeId == activeNode.NodeId)
{
m_rootNotifiers[ii] = activeNode;
activeNode.OnReportEvent = OnReportEvent;

// need to prevent recursion with the server object.
if (activeNode.NodeId != ObjectIds.Server)
{
activeNode.OnReportEvent = OnReportEvent;

if (!activeNode.ReferenceExists(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server))
{
activeNode.AddReference(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server);
}
}
break;
if (!activeNode.ReferenceExists(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server))
{
activeNode.AddReference(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server);

Check warning on line 590 in Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs#L590

Added line #L590 was not covered by tests
}
}
}

}


List<BaseInstanceState> children = new List<BaseInstanceState>();
activeNode.GetChildren(context, children);

Expand Down Expand Up @@ -3112,21 +3108,20 @@ public virtual ServiceResult SubscribeToAllEvents(
{
ServerSystemContext systemContext = SystemContext.Copy(context);

lock (Lock)
// A client has subscribed to the Server object which means all events produced
// by this manager must be reported. This is done by incrementing the monitoring
// reference count for all root notifiers.
// No Lock needed for the loop a concurrent dictionary takes a snapshot for enumerating values
foreach (var rootNotifier in m_rootNotifiers?.Values)
{
// A client has subscribed to the Server object which means all events produced
// by this manager must be reported. This is done by incrementing the monitoring
// reference count for all root notifiers.
if (m_rootNotifiers != null)
lock (Lock)
{
for (int ii = 0; ii < m_rootNotifiers.Count; ii++)
{
SubscribeToEvents(systemContext, m_rootNotifiers[ii], monitoredItem, unsubscribe);
}
SubscribeToEvents(systemContext, rootNotifier, monitoredItem, unsubscribe);
}

return ServiceResult.Good;
}

return ServiceResult.Good;

Check warning on line 3124 in Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs#L3124

Added line #L3124 was not covered by tests
}

#region SubscribeToEvents Support Functions
Expand All @@ -3140,34 +3135,19 @@ public virtual ServiceResult SubscribeToAllEvents(
/// </remarks>
protected virtual void AddRootNotifier(NodeState notifier)
{
lock (Lock)
if (m_rootNotifiers == null)
{
if (m_rootNotifiers == null)
{
m_rootNotifiers = new List<NodeState>();
}

bool mustAdd = true;

for (int ii = 0; ii < m_rootNotifiers.Count; ii++)
{
if (Object.ReferenceEquals(notifier, m_rootNotifiers[ii]))
{
return;
}

if (m_rootNotifiers[ii].NodeId == notifier.NodeId)
{
m_rootNotifiers[ii] = notifier;
mustAdd = false;
break;
}
}
m_rootNotifiers = new NodeIdDictionary<NodeState>();
}
bool notifierAlreadyExists = false;
m_rootNotifiers.AddOrUpdate(notifier.NodeId, notifier, (key, oldNotifier) => {
notifierAlreadyExists = oldNotifier == notifier;
return notifier;
});

Check warning on line 3146 in Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs#L3145-L3146

Added lines #L3145 - L3146 were not covered by tests

if (mustAdd)
{
m_rootNotifiers.Add(notifier);
}
if (notifierAlreadyExists)
{
return;
}

// need to prevent recursion with the server object.
Expand All @@ -3190,11 +3170,14 @@ protected virtual void AddRootNotifier(NodeState notifier)
{
if (monitoredItems[ii].MonitoringAllEvents)
{
SubscribeToEvents(
lock (Lock)
{

Check warning on line 3174 in Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs#L3174

Added line #L3174 was not covered by tests
SubscribeToEvents(
SystemContext,
notifier,
monitoredItems[ii],
true);
}

Check warning on line 3180 in Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs#L3180

Added line #L3180 was not covered by tests
}
}
}
Expand All @@ -3206,17 +3189,10 @@ protected virtual void AddRootNotifier(NodeState notifier)
/// <param name="notifier">The notifier.</param>
protected virtual void RemoveRootNotifier(NodeState notifier)
{
lock (Lock)
{
if (m_rootNotifiers == null)
{
return;
}
if (m_rootNotifiers.Remove(notifier))
{
notifier.OnReportEvent = null;
notifier.RemoveReference(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server);
}
NodeState removedNotifier = null;
if (m_rootNotifiers?.TryRemove(notifier.NodeId, out removedNotifier) == true){
removedNotifier.OnReportEvent = null;
removedNotifier.RemoveReference(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server);

Check warning on line 3195 in Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs#L3195

Added line #L3195 was not covered by tests
}
}

Expand Down Expand Up @@ -3364,7 +3340,7 @@ public virtual ServiceResult ConditionRefresh(
{
if (m_rootNotifiers != null)
{
nodesToRefresh.AddRange(m_rootNotifiers);
nodesToRefresh.AddRange(m_rootNotifiers.Values.ToList());
}
}
else
Expand Down Expand Up @@ -4738,7 +4714,7 @@ protected NodeState AddNodeToComponentCache(ISystemContext context, NodeHandle h
private NodeIdDictionary<MonitoredNode2> m_monitoredNodes;
private NodeIdDictionary<CacheEntry> m_componentCache;
private NodeIdDictionary<NodeState> m_predefinedNodes;
private List<NodeState> m_rootNotifiers;
private NodeIdDictionary<NodeState> m_rootNotifiers;
private uint m_maxQueueSize;
private string m_aliasRoot;
#endregion
Expand Down

0 comments on commit 50ef0bf

Please sign in to comment.