diff --git a/src/Opc.Ua.Server/Subscription/MonitoredItem/MonitoredItem.cs b/src/Opc.Ua.Server/Subscription/MonitoredItem/MonitoredItem.cs index b0b4412d99..dc2be30b45 100644 --- a/src/Opc.Ua.Server/Subscription/MonitoredItem/MonitoredItem.cs +++ b/src/Opc.Ua.Server/Subscription/MonitoredItem/MonitoredItem.cs @@ -2362,11 +2362,15 @@ namespace Opc.Ua.Server value = value.WithSourceTimestamp(DateTimeUtc.MinValue); } - // the notification is pooled; it is returned to the pool when the message that - // carries it was acknowledged and sent (NotificationMessageLease). - var item = (MonitoredItemNotification)MonitoredItemNotificationActivator.Instance.CreateInstance(); - item.ClientHandle = ClientHandle; - item.Value = value; + // not pooled: a notification lives until its message was sent and acknowledged, so + // a publishing cycle holds all notifications of the cycle at once. Recycling them + // through the shared activator pool cost more CPU (pool contention, touching cold + // instances again) than allocating them. + var item = new MonitoredItemNotification + { + ClientHandle = ClientHandle, + Value = value + }; ServerUtils.ReportPublishValue(NodeId, Id, value); notifications.Enqueue(item); diff --git a/src/Opc.Ua.Server/Subscription/NotificationMessageLease.cs b/src/Opc.Ua.Server/Subscription/NotificationMessageLease.cs index c4055a37e0..2f6c17af7e 100644 --- a/src/Opc.Ua.Server/Subscription/NotificationMessageLease.cs +++ b/src/Opc.Ua.Server/Subscription/NotificationMessageLease.cs @@ -36,8 +36,7 @@ namespace Opc.Ua.Server /// /// Counts the owners of a notification message whose payload objects (the /// , its notification lists and their - /// / entries) came - /// from their activator pools. The payload is returned to the pools only when the last + /// entries) came from their activator pools. The payload is returned to the pools only when the last /// owner releases it. /// /// @@ -126,12 +125,8 @@ namespace Opc.Ua.Server { if (data[i].TryGetValue(out DataChangeNotification? dcn)) { - ReadOnlySpan items = - dcn.MonitoredItems.Span; - for (int j = 0; j < items.Length; j++) - { - (items[j] as IPooledEncodeable)?.Reuse(); - } + // the MonitoredItemNotification entries are not pooled (see + // MonitoredItem.Publish) and are left to the garbage collector. (dcn as IPooledEncodeable)?.Reuse(); } else if (data[i].TryGetValue(out EventNotificationList? enl)) diff --git a/src/Opc.Ua.Server/Subscription/Subscription.cs b/src/Opc.Ua.Server/Subscription/Subscription.cs index bfa3ed2a1d..fb78321493 100644 --- a/src/Opc.Ua.Server/Subscription/Subscription.cs +++ b/src/Opc.Ua.Server/Subscription/Subscription.cs @@ -1113,15 +1113,16 @@ namespace Opc.Ua.Server /// /// Adds the monitored items of this subscription to the process-wide count that sizes - /// the notification pools, and raises the pool bounds when the count grew. + /// the event field list pool, and raises the pool bound when the count grew. /// /// - /// The activator pools are process-wide and retain 1024 instances by default. A - /// notification is recycled after its message was sent and acknowledged, so about two - /// publishing cycles of notifications are in flight; with more items than the bound - /// most notifications would be allocated again. The bound follows twice the number of + /// The activator pool is process-wide and retains 1024 instances by default. An + /// event field list is recycled after its message was sent and acknowledged, so about + /// two publishing cycles of event field lists are in flight; with more items than the + /// bound most of them would be allocated again. The bound follows twice the number of /// monitored items of all subscriptions in the process, capped at - /// , and is never lowered. + /// , and is never lowered. Data change + /// notifications are not pooled (see ). /// private void CountPooledItems(int delta) { @@ -1137,7 +1138,6 @@ namespace Opc.Ua.Server int bound = (int)Math.Min( kMaxPooledNotifications, Math.Max(PooledEncodeableType.DefaultMaximumRetained, total * 2)); - MonitoredItemNotificationActivator.Instance.EnsureMaximumRetained(bound); EventFieldListActivator.Instance.EnsureMaximumRetained(bound); } } diff --git a/tests/Opc.Ua.Server.Tests/Subscription/NotificationEncodingLifetimeRegressionTests.cs b/tests/Opc.Ua.Server.Tests/Subscription/NotificationEncodingLifetimeRegressionTests.cs index 172d6459bf..9bad0a3b0d 100644 --- a/tests/Opc.Ua.Server.Tests/Subscription/NotificationEncodingLifetimeRegressionTests.cs +++ b/tests/Opc.Ua.Server.Tests/Subscription/NotificationEncodingLifetimeRegressionTests.cs @@ -178,7 +178,10 @@ namespace Opc.Ua.Server.Tests Is.True); Assert.That(value!.MonitoredItems[0].ClientHandle, Is.EqualTo(77u)); Assert.That(value.MonitoredItems[0].Value.WrappedValue.GetInt32(), Is.EqualTo(42)); - Assert.That(item.ClientHandle, Is.Zero); + + // data change notifications are not pooled by the server; they are left to the GC. + Assert.That(item.ClientHandle, Is.EqualTo(77u)); + item.Reuse(); eventItem.Reuse(); } queue.Clear(); @@ -212,7 +215,9 @@ namespace Opc.Ua.Server.Tests Assert.Multiple(() => { Assert.That(message.ReuseCount, Is.EqualTo(1)); - Assert.That(item.ReuseCount, Is.EqualTo(1)); + + // data change notifications are not pooled by the server and never reused. + Assert.That(item.ReuseCount, Is.Zero); }); } diff --git a/tests/Opc.Ua.Server.Tests/Subscription/ReadyListRegressionTests.cs b/tests/Opc.Ua.Server.Tests/Subscription/ReadyListRegressionTests.cs index 8815613ebe..c6a5f2bb0e 100644 --- a/tests/Opc.Ua.Server.Tests/Subscription/ReadyListRegressionTests.cs +++ b/tests/Opc.Ua.Server.Tests/Subscription/ReadyListRegressionTests.cs @@ -198,16 +198,12 @@ namespace Opc.Ua.Server.Tests } [Test] - public async Task CreatingMonitoredItemsRaisesTheNotificationPoolBoundAsync() + public async Task CreatingMonitoredItemsRaisesTheEventFieldListPoolBoundAsync() { // the pools are process-wide and other tests may only raise them further. using var scenario = new Scenario(withScanItem: false, maxNotificationsPerPublish: 0, maxMessageCount: 100); await scenario.CreateAsync(itemCount: 1200).ConfigureAwait(false); - Assert.Multiple(() => - { - Assert.That(MonitoredItemNotificationActivator.Instance.MaximumRetained, Is.GreaterThanOrEqualTo(2400)); - Assert.That(EventFieldListActivator.Instance.MaximumRetained, Is.GreaterThanOrEqualTo(2400)); - }); + Assert.That(EventFieldListActivator.Instance.MaximumRetained, Is.GreaterThanOrEqualTo(2400)); } private static async Task AssertSameAsScanAsync(