From b9ed18987fe8b69c5d93993908a23d5df6fb98db Mon Sep 17 00:00:00 2001 From: "FOLIO3\\muhammadnoman" Date: Wed, 8 Feb 2023 21:05:12 +0500 Subject: [PATCH 1/9] Removing thread.sleep as it seems unnecessary --- OptimizelySDK/Odp/OdpEventManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index 13eef794..26857e1f 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -152,9 +152,9 @@ protected virtual void Run() FlushQueue(); } - if (!_eventQueue.TryTake(out object item, 50)) + if (!_eventQueue.TryTake(out object item, (int) + (_flushingIntervalDeadline - DateTime.Now.MillisecondsSince1970()))) { - Thread.Sleep(50); continue; } From 5b6bb5342c36a32b8813628163c32fb147d4584f Mon Sep 17 00:00:00 2001 From: "FOLIO3\\muhammadnoman" Date: Wed, 8 Feb 2023 22:00:22 +0500 Subject: [PATCH 2/9] Test --- OptimizelySDK/Odp/OdpEventManager.cs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index 26857e1f..a5e04645 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -146,15 +146,22 @@ protected virtual void Run() { while (true) { - if (DateTime.Now.MillisecondsSince1970() > _flushingIntervalDeadline) + Object item; + // If batch has events, set the timeout to remaining time for flush interval, + // otherwise wait for the new event indefinitely + if (_eventQueue.Count > 0) { - _logger.Log(LogLevel.DEBUG, $"Flushing queue."); - FlushQueue(); + _eventQueue.TryTake(out item, (int)(_flushingIntervalDeadline - DateTime.Now.MillisecondsSince1970())); } - - if (!_eventQueue.TryTake(out object item, (int) - (_flushingIntervalDeadline - DateTime.Now.MillisecondsSince1970()))) + else { + item = _eventQueue.Take(); + } + if (item == null) + { + // null means no new events received and flush interval is over, dispatch whatever is in the batch. + _logger.Log(LogLevel.DEBUG, $"Flushing queue."); + FlushQueue(); continue; } From de1fc0c23d7b2b64e0c23e4136ec7af79966e566 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 9 Feb 2023 17:43:52 +0500 Subject: [PATCH 3/9] Fixed the odpEventManager flush trigger. --- OptimizelySDK/Odp/OdpEventManager.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index a5e04645..e520dfbc 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -146,10 +146,10 @@ protected virtual void Run() { while (true) { - Object item; + object item; // If batch has events, set the timeout to remaining time for flush interval, // otherwise wait for the new event indefinitely - if (_eventQueue.Count > 0) + if (_currentBatch.Count > 0) { _eventQueue.TryTake(out item, (int)(_flushingIntervalDeadline - DateTime.Now.MillisecondsSince1970())); } @@ -160,8 +160,11 @@ protected virtual void Run() if (item == null) { // null means no new events received and flush interval is over, dispatch whatever is in the batch. - _logger.Log(LogLevel.DEBUG, $"Flushing queue."); - FlushQueue(); + if (_currentBatch.Count != 0) + { + _logger.Log(LogLevel.DEBUG, $"Flushing queue."); + FlushQueue(); + } continue; } From 8f0879c6588349c6f2bd8dba4e88e5d84ab38dc1 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 9 Feb 2023 18:02:23 +0500 Subject: [PATCH 4/9] Fixed the test as when there are no remaining items in queue then odpEventManager should not flush. --- OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs index 25c9be82..87ce608d 100644 --- a/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs +++ b/OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs @@ -311,7 +311,7 @@ public void ShouldAddAdditionalInformationToEachEvent() } [Test] - public void ShouldAttemptToFlushAnEmptyQueueAtFlushInterval() + public void ShouldNotAttemptToFlushAnEmptyQueueAtFlushInterval() { var eventManager = new OdpEventManager.Builder(). WithOdpEventApiManager(_mockApiManager.Object). @@ -327,7 +327,7 @@ public void ShouldAttemptToFlushAnEmptyQueueAtFlushInterval() eventManager.Stop(); _mockLogger.Verify(l => l.Log(LogLevel.DEBUG, "Flushing queue."), - Times.AtLeast(3)); + Times.Never); } [Test] From 64cdf80a9cc54fbd8e67188485bdcbb88662cbb7 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Fri, 10 Feb 2023 16:08:46 +0500 Subject: [PATCH 5/9] Removing mutex lock from notification registry instead using concurrentDictionary --- .../NotificationCenterRegistry.cs | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs index ecbbda84..2492c21f 100644 --- a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs +++ b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs @@ -15,6 +15,7 @@ */ using OptimizelySDK.Logger; +using System.Collections.Concurrent; using System.Collections.Generic; namespace OptimizelySDK.Notifications @@ -23,8 +24,7 @@ internal static class NotificationCenterRegistry { private static readonly object _mutex = new object(); - private static Dictionary _notificationCenters = - new Dictionary(); + private static ConcurrentDictionary _notificationCenters = new ConcurrentDictionary(); /// /// Thread-safe access to the NotificationCenter @@ -40,18 +40,16 @@ public static NotificationCenter GetNotificationCenter(string sdkKey, ILogger lo return default; } - NotificationCenter notificationCenter; - lock (_mutex) + NotificationCenter notificationCenter = null; + + if (_notificationCenters.ContainsKey(sdkKey)) { - if (_notificationCenters.ContainsKey(sdkKey)) - { - notificationCenter = _notificationCenters[sdkKey]; - } - else - { - notificationCenter = new NotificationCenter(logger); - _notificationCenters[sdkKey] = notificationCenter; - } + notificationCenter = _notificationCenters[sdkKey]; + } + else + { + notificationCenter = new NotificationCenter(logger); + _notificationCenters[sdkKey] = notificationCenter; } return notificationCenter; @@ -74,7 +72,7 @@ public static void RemoveNotificationCenter(string sdkKey) out NotificationCenter notificationCenter)) { notificationCenter.ClearAllNotifications(); - _notificationCenters.Remove(sdkKey); + _notificationCenters.TryRemove(sdkKey, out NotificationCenter obj); } } } From 331826b4e730434b4af6b40db793c25ea421320d Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Fri, 10 Feb 2023 16:53:00 +0500 Subject: [PATCH 6/9] Revert "Removing mutex lock from notification registry instead using concurrentDictionary" This reverts commit 64cdf80a9cc54fbd8e67188485bdcbb88662cbb7. --- .../NotificationCenterRegistry.cs | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs index 2492c21f..ecbbda84 100644 --- a/OptimizelySDK/Notifications/NotificationCenterRegistry.cs +++ b/OptimizelySDK/Notifications/NotificationCenterRegistry.cs @@ -15,7 +15,6 @@ */ using OptimizelySDK.Logger; -using System.Collections.Concurrent; using System.Collections.Generic; namespace OptimizelySDK.Notifications @@ -24,7 +23,8 @@ internal static class NotificationCenterRegistry { private static readonly object _mutex = new object(); - private static ConcurrentDictionary _notificationCenters = new ConcurrentDictionary(); + private static Dictionary _notificationCenters = + new Dictionary(); /// /// Thread-safe access to the NotificationCenter @@ -40,16 +40,18 @@ public static NotificationCenter GetNotificationCenter(string sdkKey, ILogger lo return default; } - NotificationCenter notificationCenter = null; - - if (_notificationCenters.ContainsKey(sdkKey)) - { - notificationCenter = _notificationCenters[sdkKey]; - } - else + NotificationCenter notificationCenter; + lock (_mutex) { - notificationCenter = new NotificationCenter(logger); - _notificationCenters[sdkKey] = notificationCenter; + if (_notificationCenters.ContainsKey(sdkKey)) + { + notificationCenter = _notificationCenters[sdkKey]; + } + else + { + notificationCenter = new NotificationCenter(logger); + _notificationCenters[sdkKey] = notificationCenter; + } } return notificationCenter; @@ -72,7 +74,7 @@ public static void RemoveNotificationCenter(string sdkKey) out NotificationCenter notificationCenter)) { notificationCenter.ClearAllNotifications(); - _notificationCenters.TryRemove(sdkKey, out NotificationCenter obj); + _notificationCenters.Remove(sdkKey); } } } From 80b3c333fcfc4ce5511be6280205719bcc8c5bb8 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Fri, 10 Feb 2023 18:35:35 +0500 Subject: [PATCH 7/9] Added Thread.sleep which somehow enabling thread to shutdown gracefully. --- OptimizelySDK/Odp/OdpEventManager.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index e520dfbc..2e9ca3d0 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -156,6 +156,7 @@ protected virtual void Run() else { item = _eventQueue.Take(); + Thread.Sleep(1); // TODO: need to figure out why this is allowing item to read shutdown signal. } if (item == null) { @@ -166,25 +167,24 @@ protected virtual void Run() FlushQueue(); } continue; - } - - if (item == _shutdownSignal) + } + else if (item == _shutdownSignal) { _logger.Log(LogLevel.INFO, "Received shutdown signal."); break; } - if (item == _flushSignal) + else if (item == _flushSignal) { _logger.Log(LogLevel.DEBUG, "Received flush signal."); FlushQueue(); continue; } - - if (item is OdpEvent odpEvent) + else if (item is OdpEvent odpEvent) { AddToBatch(odpEvent); } + } } catch (InvalidOperationException ioe) From 9c8e5dfb5474f5e34e2fae7bf33dc4f479b6ab15 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Fri, 10 Feb 2023 19:13:14 +0500 Subject: [PATCH 8/9] nit fix --- OptimizelySDK/Odp/OdpEventManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index 2e9ca3d0..6582fd63 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -158,6 +158,7 @@ protected virtual void Run() item = _eventQueue.Take(); Thread.Sleep(1); // TODO: need to figure out why this is allowing item to read shutdown signal. } + if (item == null) { // null means no new events received and flush interval is over, dispatch whatever is in the batch. @@ -173,7 +174,6 @@ protected virtual void Run() _logger.Log(LogLevel.INFO, "Received shutdown signal."); break; } - else if (item == _flushSignal) { _logger.Log(LogLevel.DEBUG, "Received flush signal."); From 31101f6b983dee7e43f19ddabab0dac8504afa7b Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Fri, 10 Feb 2023 19:33:56 +0500 Subject: [PATCH 9/9] nit fix --- OptimizelySDK/Odp/OdpEventManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OptimizelySDK/Odp/OdpEventManager.cs b/OptimizelySDK/Odp/OdpEventManager.cs index 6582fd63..3039e7f2 100644 --- a/OptimizelySDK/Odp/OdpEventManager.cs +++ b/OptimizelySDK/Odp/OdpEventManager.cs @@ -168,7 +168,7 @@ protected virtual void Run() FlushQueue(); } continue; - } + } else if (item == _shutdownSignal) { _logger.Log(LogLevel.INFO, "Received shutdown signal.");