From 5321dc044e6ff54494c83695e4f39b06bf5e7b23 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 27 Mar 2020 17:03:43 -0700 Subject: [PATCH 1/2] Hoist volatile variable's access outside the loop `_tables` is a volatile object and it has `_countPerLock` volatile field. On ARM64, JIT generates memory barrier instruction "dmb" for every volatile variable access which could be expensive. This PR caches the volatile variables outside the loop and use cached local variables instead. Fixes: https://github.com/dotnet/runtime/issues/34198 --- .../Concurrent/ConcurrentDictionary.cs | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 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 4ef5ecdf2927a3..eaaa981bd739f5 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 @@ -636,10 +636,11 @@ void ICollection>.CopyTo(KeyValuePair[] AcquireAllLocks(ref locksAcquired); int count = 0; - - for (int i = 0; i < _tables._locks.Length && count >= 0; i++) + int noOfLocks = _tables._locks.Length; + int[] countPerLock = _tables._countPerLock; + for (int i = 0; i < noOfLocks && count >= 0; i++) { - count += _tables._countPerLock[i]; + count += countPerLock[i]; } if (array.Length - count < index || count < 0) //"count" itself or "count + index" can overflow @@ -670,9 +671,11 @@ public KeyValuePair[] ToArray() int count = 0; checked { - for (int i = 0; i < _tables._locks.Length; i++) + int numOfLocks = _tables._locks.Length; + int[] countPerLock = _tables._countPerLock; + for (int i = 0; i < numOfLocks; i++) { - count += _tables._countPerLock[i]; + count += countPerLock[i]; } } @@ -998,11 +1001,12 @@ public int Count private int GetCountInternal() { int count = 0; + int[] countPerLocks = _tables._countPerLock; // Compute the count, we allow overflow - for (int i = 0; i < _tables._countPerLock.Length; i++) + for (int i = 0; i < countPerLocks.Length; i++) { - count += _tables._countPerLock[i]; + count += countPerLocks[i]; } return count; @@ -1668,10 +1672,11 @@ void ICollection.CopyTo(Array array, int index) Tables tables = _tables; int count = 0; - - for (int i = 0; i < tables._locks.Length && count >= 0; i++) + int[] countPerLock = tables._countPerLock; + int numOfLocks = tables._locks.Length; + for (int i = 0; i < numOfLocks && count >= 0; i++) { - count += tables._countPerLock[i]; + count += countPerLock[i]; } if (array.Length - count < index || count < 0) //"count" itself or "count + index" can overflow @@ -1985,9 +1990,10 @@ private ReadOnlyCollection GetKeys() if (count < 0) throw new OutOfMemoryException(); List keys = new List(count); - for (int i = 0; i < _tables._buckets.Length; i++) + Node[] buckets = _tables._buckets; + for (int i = 0; i < buckets.Length; i++) { - Node current = _tables._buckets[i]; + Node current = buckets[i]; while (current != null) { keys.Add(current._key); @@ -2017,9 +2023,10 @@ private ReadOnlyCollection GetValues() if (count < 0) throw new OutOfMemoryException(); List values = new List(count); - for (int i = 0; i < _tables._buckets.Length; i++) + Node[] buckets = _tables._buckets; + for (int i = 0; i < buckets.Length; i++) { - Node current = _tables._buckets[i]; + Node current = buckets[i]; while (current != null) { values.Add(current._value); From de1b2e75b7b84d15a9da565613d06784653767c5 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 30 Mar 2020 07:40:22 -0700 Subject: [PATCH 2/2] Adderessed review comments --- .../Collections/Concurrent/ConcurrentDictionary.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 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 eaaa981bd739f5..e8a10a5d788eb2 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 @@ -636,9 +636,8 @@ void ICollection>.CopyTo(KeyValuePair[] AcquireAllLocks(ref locksAcquired); int count = 0; - int noOfLocks = _tables._locks.Length; int[] countPerLock = _tables._countPerLock; - for (int i = 0; i < noOfLocks && count >= 0; i++) + for (int i = 0; i < countPerLock.Length && count >= 0; i++) { count += countPerLock[i]; } @@ -671,9 +670,8 @@ public KeyValuePair[] ToArray() int count = 0; checked { - int numOfLocks = _tables._locks.Length; int[] countPerLock = _tables._countPerLock; - for (int i = 0; i < numOfLocks; i++) + for (int i = 0; i < countPerLock.Length; i++) { count += countPerLock[i]; } @@ -1673,8 +1671,7 @@ void ICollection.CopyTo(Array array, int index) int count = 0; int[] countPerLock = tables._countPerLock; - int numOfLocks = tables._locks.Length; - for (int i = 0; i < numOfLocks && count >= 0; i++) + for (int i = 0; i < countPerLock.Length && count >= 0; i++) { count += countPerLock[i]; }