From e714d3caa84e80abeb17ece4ae614b13d7055759 Mon Sep 17 00:00:00 2001 From: MCCshreyas Date: Thu, 12 Nov 2020 14:31:05 +0530 Subject: [PATCH 1/3] Added condition of empty for concurrent dictionary Collection is already empty why to initialize it again with new emtpy tables. --- .../Concurrent/ConcurrentDictionary.cs | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs b/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs index aa79092e64064..125560477f2bd 100644 --- a/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs +++ b/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs @@ -637,19 +637,22 @@ nullableHashcode is null || /// public void Clear() { - int locksAcquired = 0; - try + if (!IsEmpty) { - AcquireAllLocks(ref locksAcquired); + int locksAcquired = 0; + try + { + AcquireAllLocks(ref locksAcquired); - Tables tables = _tables; - var newTables = new Tables(new Node[DefaultCapacity], tables._locks, new int[tables._countPerLock.Length]); - _tables = newTables; - _budget = Math.Max(1, newTables._buckets.Length / newTables._locks.Length); - } - finally - { - ReleaseLocks(0, locksAcquired); + Tables tables = _tables; + var newTables = new Tables(new Node[DefaultCapacity], tables._locks, new int[tables._countPerLock.Length]); + _tables = newTables; + _budget = Math.Max(1, newTables._buckets.Length / newTables._locks.Length); + } + finally + { + ReleaseLocks(0, locksAcquired); + } } } From a4dc03d144781b0c08fac79784e541f3800e1999 Mon Sep 17 00:00:00 2001 From: MCCshreyas Date: Sun, 15 Nov 2020 11:55:31 +0530 Subject: [PATCH 2/3] Made AreAllBucketsEmpty method private from local function. Addressed PR feedback. --- .../Concurrent/ConcurrentDictionary.cs | 56 ++++++++++--------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs b/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs index 125560477f2bd..1a575cc257775 100644 --- a/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs +++ b/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs @@ -637,22 +637,25 @@ nullableHashcode is null || /// public void Clear() { - if (!IsEmpty) + int locksAcquired = 0; + try { - int locksAcquired = 0; - try - { - AcquireAllLocks(ref locksAcquired); + AcquireAllLocks(ref locksAcquired); - Tables tables = _tables; - var newTables = new Tables(new Node[DefaultCapacity], tables._locks, new int[tables._countPerLock.Length]); - _tables = newTables; - _budget = Math.Max(1, newTables._buckets.Length / newTables._locks.Length); - } - finally + // If underlying bucket is already empty then just return + if (AreAllBucketsEmpty()) { - ReleaseLocks(0, locksAcquired); + return; } + + Tables tables = _tables; + var newTables = new Tables(new Node[DefaultCapacity], tables._locks, new int[tables._countPerLock.Length]); + _tables = newTables; + _budget = Math.Max(1, newTables._buckets.Length / newTables._locks.Length); + } + finally + { + ReleaseLocks(0, locksAcquired); } } @@ -1424,20 +1427,7 @@ public bool IsEmpty ReleaseLocks(0, acquiredLocks); } - bool AreAllBucketsEmpty() - { - int[] countPerLock = _tables._countPerLock; - for (int i = 0; i < countPerLock.Length; i++) - { - if (countPerLock[i] != 0) - { - return false; - } - } - - return true; - } } } @@ -1877,6 +1867,22 @@ void ICollection.CopyTo(Array array, int index) #endregion + + private bool AreAllBucketsEmpty() + { + int[] countPerLock = _tables._countPerLock; + + for (int i = 0; i < countPerLock.Length; i++) + { + if (countPerLock[i] != 0) + { + return false; + } + } + + return true; + } + /// /// Replaces the bucket table with a larger one. To prevent multiple threads from resizing the /// table as a result of races, the Tables instance that holds the table of buckets deemed too From 483962132f472fe4fd013b1ad8181795ddbb3112 Mon Sep 17 00:00:00 2001 From: Shreyas Jejurkar Date: Sun, 15 Nov 2020 20:53:13 +0530 Subject: [PATCH 3/3] Addressed PR feedback. Comment corrected. Co-authored-by: Stephen Toub --- .../src/System/Collections/Concurrent/ConcurrentDictionary.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs b/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs index 1a575cc257775..fb49255cfe655 100644 --- a/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs +++ b/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs @@ -642,7 +642,7 @@ public void Clear() { AcquireAllLocks(ref locksAcquired); - // If underlying bucket is already empty then just return + // If the dictionary is already empty, then there's nothing to clear. if (AreAllBucketsEmpty()) { return;