From efa2e617796f166aa726a70f198d8aa03f22fb1a Mon Sep 17 00:00:00 2001 From: MarkusSintonen Date: Tue, 30 Jun 2015 21:42:29 +0300 Subject: [PATCH 1/6] ConcurrentBag performance improvements and new functions -Improved ToList() performance by initializing the list with initial capacity -Use initial capacity for bag version lists (slightly better performance when large number of threads access the bag) -Added Clear function -Added Contains/Find/FindAll/Exists/TrueForAll functions that avoid List allocations. Functions represent moment-in-time state of the bag -Added GetEnumerableSlim to avoid list allocation. Returned enumerable does not represent moment-in-time state of the bag (which can be used when moment-in-time representation is not enforced) New functions can be used to avoid the list allocations which is problematic with very large bags. --- .../Collections/Concurrent/ConcurrentBag.cs | 281 +++++++++++++++++- 1 file changed, 276 insertions(+), 5 deletions(-) diff --git a/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs b/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs index 4873c9ca5e17..29d6bc33cbe3 100644 --- a/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs +++ b/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs @@ -48,6 +48,13 @@ public class ConcurrentBag : IProducerConsumerCollection, IReadOnlyCollect // GlobalListsLock lock private bool _needSync; + // Approximate item count inside the bag. This is used to determine initial capacity for list copying operations. + // Note that this might not represent totally correct item count inside the bag. + private volatile int _approxCount; + + // Count of the thread lists. This is used to determine initial capacity for the version lists + private volatile int _threadListCount; + /// /// Initializes a new instance of the /// class. @@ -84,14 +91,18 @@ private void Initialize(IEnumerable collection) _locals = new ThreadLocal(); // Copy the collection to the bag + int count = 0; if (collection != null) { ThreadLocalList list = GetThreadList(true); foreach (T item in collection) { list.Add(item, false); + count++; } } + + _approxCount = count; } /// @@ -130,6 +141,9 @@ private void AddInternal(ThreadLocalList list, T item) Monitor.Enter(list, ref lockTaken); } list.Add(item, lockTaken); +#pragma warning disable 0420 + Interlocked.Increment(ref _approxCount); +#pragma warning restore 0420 } finally { @@ -230,6 +244,10 @@ private bool TryTakeOrPeek(out T result, bool take) } } list.Remove(out result); +#pragma warning disable 0420 + Interlocked.Decrement(ref _approxCount); +#pragma warning restore 0420 + return true; } else { @@ -274,6 +292,7 @@ private ThreadLocalList GetThreadList(bool forceCreate) list = new ThreadLocalList(Environment.CurrentManagedThreadId); _headList = list; _tailList = list; + _threadListCount++; } else { @@ -283,6 +302,7 @@ private ThreadLocalList GetThreadList(bool forceCreate) list = new ThreadLocalList(Environment.CurrentManagedThreadId); _tailList._nextList = list; _tailList = list; + _threadListCount++; } } _locals.Value = list; @@ -339,7 +359,7 @@ private bool Steal(out T result, bool take) #endif bool loop; - List versionsList = new List(); // save the lists version + List versionsList = new List(_threadListCount); // save the lists version do { versionsList.Clear(); //clear the list from the previous iteration @@ -386,6 +406,12 @@ private bool TrySteal(ThreadLocalList list, out T result, bool take) if (CanSteal(list)) { list.Steal(out result, take); + if (take) + { +#pragma warning disable 0420 + Interlocked.Decrement(ref _approxCount); +#pragma warning restore 0420 + } return true; } result = default(T); @@ -573,14 +599,246 @@ IEnumerator IEnumerable.GetEnumerator() return ((ConcurrentBag)this).GetEnumerator(); } + /// + /// Returns an enumerator that iterates through the . + /// + /// An enumerator for the contents of the . + /// + /// The enumerator returned from the function is safe to use concurrently with + /// reads and writes to the bag, however it does not represent a moment-in-time snapshot + /// of the bag. The contents exposed through the enumerator may contain modifications + /// made to the bag after was called. + /// does not copy the bag contents to a list as does. + /// + public IEnumerable GetEnumerableSlim() + { + ThreadLocalList currentList = _headList; + while (currentList != null) + { + Node currentNode = currentList._head; + while (currentNode != null) + { + yield return currentNode._value; + currentNode = currentNode._next; + } + currentList = currentList._nextList; + } + } + + /// + /// Determines whether the contains the specified item. + /// + /// The item to locate in the . + /// true if the contains the specified item; + /// otherwise, false. + /// + /// The returned value represents a moment-in-time snapshot of the contents of the bag. + /// + public bool Contains(T item) + { + // Short path if the bag is empty + if (_headList == null) + return false; + + bool lockTaken = false; + try + { + FreezeBag(ref lockTaken); + + EqualityComparer comparer = EqualityComparer.Default; + foreach (T bagItem in GetEnumerableSlim()) + { + if (comparer.Equals(bagItem, item)) + return true; // Item found + } + return false; + } + finally + { + UnfreezeBag(lockTaken); + } + } + + /// + /// Searches for an element from that matches the conditions defined by + /// the specified predicate, and returns the first occurrence that matches the predicate. + /// + /// The delegate that defines the conditions of the element to search for. + /// When this method returns, contains the matched object from the + /// or the default value of if there was no match. + /// true if the contains the matched item; + /// otherwise, false. + /// + /// The returned value represents a moment-in-time snapshot of the contents of the bag. + /// + public bool Find(Predicate match, out T item) + { + if (match == null) + { + throw new ArgumentNullException("match"); + } + + // Short path if the bag is empty + if (_headList == null) + { + item = default(T); + return false; + } + + bool lockTaken = false; + try + { + FreezeBag(ref lockTaken); + + foreach (T bagItem in GetEnumerableSlim()) + { + if (match(bagItem)) + { + item = bagItem; + return true; // Item found + } + } + item = default(T); + return false; + } + finally + { + UnfreezeBag(lockTaken); + } + } + + /// + /// Retrieves all the elements from that match + /// the conditions defined by the specified predicate. + /// + /// The delegate that defines the conditions of the elements to search for. + /// A containing all the elements that match the conditions defined by the specified predicate, + /// if found; otherwise, an empty . + /// + /// The returned value represents a moment-in-time snapshot of the contents of the bag. + /// + public List FindAll(Predicate match) + { + if (match == null) + { + throw new ArgumentNullException("match"); + } + + // Short path if the bag is empty + if (_headList == null) + return new List(); + + bool lockTaken = false; + try + { + FreezeBag(ref lockTaken); + + List matches = new List(); + foreach (T bagItem in GetEnumerableSlim()) + { + if (match(bagItem)) + matches.Add(bagItem); + } + return matches; + } + finally + { + UnfreezeBag(lockTaken); + } + } + + /// + /// Searches for an element from that matches the conditions defined by + /// the specified predicate, and returns true if the match is found, otherwise false. + /// + /// The delegate that defines the conditions of the element to search for. + /// true if the contains the matched item; + /// otherwise, false. + /// + /// The returned value represents a moment-in-time snapshot of the contents of the bag. + /// + public bool Exists(Predicate match) + { + T dummy = default(T); + return Find(match, out dummy); + } + + /// + /// Determines whether every element in the matches the + /// conditions defined by the specified predicate. + /// + /// The delegate that defines the conditions to check against the elements. + /// true if every element in the matches the conditions defined by the specified predicate; + /// otherwise, false. If the list has no elements, the return value is true. + /// + /// The returned value represents a moment-in-time snapshot of the contents of the bag. + /// + public bool TrueForAll(Predicate match) + { + if (match == null) + { + throw new ArgumentNullException("match"); + } + + // Short path if the bag is empty + if (_headList == null) + return true; + + bool lockTaken = false; + try + { + FreezeBag(ref lockTaken); + + foreach (T bagItem in GetEnumerableSlim()) + { + if (!match(bagItem)) + return false; + } + return true; + } + finally + { + UnfreezeBag(lockTaken); + } + } + + /// + /// Removes all items from the . + /// + public void Clear() + { + // Short path if the bag is empty + if (_headList == null) + return; + + bool lockTaken = false; + try + { + FreezeBag(ref lockTaken); + + ThreadLocalList currentList = _headList; + while (currentList != null) + { + currentList.Reset(); + currentList = currentList._nextList; + } + + _approxCount = 0; + } + finally + { + UnfreezeBag(lockTaken); + } + } + /// /// Gets the number of elements contained in the . /// /// The number of elements contained in the . /// /// The count returned represents a moment-in-time snapshot of the contents - /// of the bag. It does not reflect any updates to the collection after - /// was called. + /// of the bag. It does not reflect any updates to the collection after + /// was called. /// public int Count { @@ -666,7 +924,7 @@ object ICollection.SyncRoot /// /// A global lock object, used in two cases: /// 1- To maintain the _tailList pointer for each new list addition process ( first time a thread called Add ) - /// 2- To freeze the bag in GetEnumerator, CopyTo, ToArray and Count members + /// 2- To freeze the bag in GetEnumerator, CopyTo, ToArray, Contains, Clear and Count members /// private object GlobalListsLock { @@ -815,7 +1073,11 @@ private List ToList() { Debug.Assert(Monitor.IsEntered(GlobalListsLock)); - List list = new List(); + int capacity = _approxCount; + if (capacity < 0) // Ensure capacity + capacity = 0; + + List list = new List(capacity); ThreadLocalList currentList = _headList; while (currentList != null) { @@ -986,6 +1248,15 @@ internal void Steal(out T result, bool remove) result = tail._value; } + internal void Reset() + { + _head = null; + _tail = null; + _count = 0; + _stealCount = 0; + _version = 0; // Reset version since the local list is empty again + } + /// /// Gets the total list count, it's not thread safe, may provide incorrect count if it is called concurrently From 1cf4334ed9277ffaae53fc6969355cb1b82b1cd4 Mon Sep 17 00:00:00 2001 From: MarkusSintonen Date: Tue, 30 Jun 2015 22:05:39 +0300 Subject: [PATCH 2/6] Added comment --- .../src/System/Collections/Concurrent/ConcurrentBag.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs b/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs index 29d6bc33cbe3..6ff5b4b4166f 100644 --- a/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs +++ b/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs @@ -1248,6 +1248,9 @@ internal void Steal(out T result, bool remove) result = tail._value; } + /// + /// Resets the local list + /// internal void Reset() { _head = null; From 0eb1c0a44281a9a0b30f389e9318c31a0f3cc63f Mon Sep 17 00:00:00 2001 From: MarkusSintonen Date: Fri, 10 Jul 2015 00:14:37 +0300 Subject: [PATCH 3/6] Fixed wrong commit in the wrong place --- .../Collections/Concurrent/ConcurrentBag.cs | 284 +----------------- 1 file changed, 5 insertions(+), 279 deletions(-) diff --git a/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs b/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs index 6ff5b4b4166f..4873c9ca5e17 100644 --- a/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs +++ b/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs @@ -48,13 +48,6 @@ public class ConcurrentBag : IProducerConsumerCollection, IReadOnlyCollect // GlobalListsLock lock private bool _needSync; - // Approximate item count inside the bag. This is used to determine initial capacity for list copying operations. - // Note that this might not represent totally correct item count inside the bag. - private volatile int _approxCount; - - // Count of the thread lists. This is used to determine initial capacity for the version lists - private volatile int _threadListCount; - /// /// Initializes a new instance of the /// class. @@ -91,18 +84,14 @@ private void Initialize(IEnumerable collection) _locals = new ThreadLocal(); // Copy the collection to the bag - int count = 0; if (collection != null) { ThreadLocalList list = GetThreadList(true); foreach (T item in collection) { list.Add(item, false); - count++; } } - - _approxCount = count; } /// @@ -141,9 +130,6 @@ private void AddInternal(ThreadLocalList list, T item) Monitor.Enter(list, ref lockTaken); } list.Add(item, lockTaken); -#pragma warning disable 0420 - Interlocked.Increment(ref _approxCount); -#pragma warning restore 0420 } finally { @@ -244,10 +230,6 @@ private bool TryTakeOrPeek(out T result, bool take) } } list.Remove(out result); -#pragma warning disable 0420 - Interlocked.Decrement(ref _approxCount); -#pragma warning restore 0420 - return true; } else { @@ -292,7 +274,6 @@ private ThreadLocalList GetThreadList(bool forceCreate) list = new ThreadLocalList(Environment.CurrentManagedThreadId); _headList = list; _tailList = list; - _threadListCount++; } else { @@ -302,7 +283,6 @@ private ThreadLocalList GetThreadList(bool forceCreate) list = new ThreadLocalList(Environment.CurrentManagedThreadId); _tailList._nextList = list; _tailList = list; - _threadListCount++; } } _locals.Value = list; @@ -359,7 +339,7 @@ private bool Steal(out T result, bool take) #endif bool loop; - List versionsList = new List(_threadListCount); // save the lists version + List versionsList = new List(); // save the lists version do { versionsList.Clear(); //clear the list from the previous iteration @@ -406,12 +386,6 @@ private bool TrySteal(ThreadLocalList list, out T result, bool take) if (CanSteal(list)) { list.Steal(out result, take); - if (take) - { -#pragma warning disable 0420 - Interlocked.Decrement(ref _approxCount); -#pragma warning restore 0420 - } return true; } result = default(T); @@ -599,246 +573,14 @@ IEnumerator IEnumerable.GetEnumerator() return ((ConcurrentBag)this).GetEnumerator(); } - /// - /// Returns an enumerator that iterates through the . - /// - /// An enumerator for the contents of the . - /// - /// The enumerator returned from the function is safe to use concurrently with - /// reads and writes to the bag, however it does not represent a moment-in-time snapshot - /// of the bag. The contents exposed through the enumerator may contain modifications - /// made to the bag after was called. - /// does not copy the bag contents to a list as does. - /// - public IEnumerable GetEnumerableSlim() - { - ThreadLocalList currentList = _headList; - while (currentList != null) - { - Node currentNode = currentList._head; - while (currentNode != null) - { - yield return currentNode._value; - currentNode = currentNode._next; - } - currentList = currentList._nextList; - } - } - - /// - /// Determines whether the contains the specified item. - /// - /// The item to locate in the . - /// true if the contains the specified item; - /// otherwise, false. - /// - /// The returned value represents a moment-in-time snapshot of the contents of the bag. - /// - public bool Contains(T item) - { - // Short path if the bag is empty - if (_headList == null) - return false; - - bool lockTaken = false; - try - { - FreezeBag(ref lockTaken); - - EqualityComparer comparer = EqualityComparer.Default; - foreach (T bagItem in GetEnumerableSlim()) - { - if (comparer.Equals(bagItem, item)) - return true; // Item found - } - return false; - } - finally - { - UnfreezeBag(lockTaken); - } - } - - /// - /// Searches for an element from that matches the conditions defined by - /// the specified predicate, and returns the first occurrence that matches the predicate. - /// - /// The delegate that defines the conditions of the element to search for. - /// When this method returns, contains the matched object from the - /// or the default value of if there was no match. - /// true if the contains the matched item; - /// otherwise, false. - /// - /// The returned value represents a moment-in-time snapshot of the contents of the bag. - /// - public bool Find(Predicate match, out T item) - { - if (match == null) - { - throw new ArgumentNullException("match"); - } - - // Short path if the bag is empty - if (_headList == null) - { - item = default(T); - return false; - } - - bool lockTaken = false; - try - { - FreezeBag(ref lockTaken); - - foreach (T bagItem in GetEnumerableSlim()) - { - if (match(bagItem)) - { - item = bagItem; - return true; // Item found - } - } - item = default(T); - return false; - } - finally - { - UnfreezeBag(lockTaken); - } - } - - /// - /// Retrieves all the elements from that match - /// the conditions defined by the specified predicate. - /// - /// The delegate that defines the conditions of the elements to search for. - /// A containing all the elements that match the conditions defined by the specified predicate, - /// if found; otherwise, an empty . - /// - /// The returned value represents a moment-in-time snapshot of the contents of the bag. - /// - public List FindAll(Predicate match) - { - if (match == null) - { - throw new ArgumentNullException("match"); - } - - // Short path if the bag is empty - if (_headList == null) - return new List(); - - bool lockTaken = false; - try - { - FreezeBag(ref lockTaken); - - List matches = new List(); - foreach (T bagItem in GetEnumerableSlim()) - { - if (match(bagItem)) - matches.Add(bagItem); - } - return matches; - } - finally - { - UnfreezeBag(lockTaken); - } - } - - /// - /// Searches for an element from that matches the conditions defined by - /// the specified predicate, and returns true if the match is found, otherwise false. - /// - /// The delegate that defines the conditions of the element to search for. - /// true if the contains the matched item; - /// otherwise, false. - /// - /// The returned value represents a moment-in-time snapshot of the contents of the bag. - /// - public bool Exists(Predicate match) - { - T dummy = default(T); - return Find(match, out dummy); - } - - /// - /// Determines whether every element in the matches the - /// conditions defined by the specified predicate. - /// - /// The delegate that defines the conditions to check against the elements. - /// true if every element in the matches the conditions defined by the specified predicate; - /// otherwise, false. If the list has no elements, the return value is true. - /// - /// The returned value represents a moment-in-time snapshot of the contents of the bag. - /// - public bool TrueForAll(Predicate match) - { - if (match == null) - { - throw new ArgumentNullException("match"); - } - - // Short path if the bag is empty - if (_headList == null) - return true; - - bool lockTaken = false; - try - { - FreezeBag(ref lockTaken); - - foreach (T bagItem in GetEnumerableSlim()) - { - if (!match(bagItem)) - return false; - } - return true; - } - finally - { - UnfreezeBag(lockTaken); - } - } - - /// - /// Removes all items from the . - /// - public void Clear() - { - // Short path if the bag is empty - if (_headList == null) - return; - - bool lockTaken = false; - try - { - FreezeBag(ref lockTaken); - - ThreadLocalList currentList = _headList; - while (currentList != null) - { - currentList.Reset(); - currentList = currentList._nextList; - } - - _approxCount = 0; - } - finally - { - UnfreezeBag(lockTaken); - } - } - /// /// Gets the number of elements contained in the . /// /// The number of elements contained in the . /// /// The count returned represents a moment-in-time snapshot of the contents - /// of the bag. It does not reflect any updates to the collection after - /// was called. + /// of the bag. It does not reflect any updates to the collection after + /// was called. /// public int Count { @@ -924,7 +666,7 @@ object ICollection.SyncRoot /// /// A global lock object, used in two cases: /// 1- To maintain the _tailList pointer for each new list addition process ( first time a thread called Add ) - /// 2- To freeze the bag in GetEnumerator, CopyTo, ToArray, Contains, Clear and Count members + /// 2- To freeze the bag in GetEnumerator, CopyTo, ToArray and Count members /// private object GlobalListsLock { @@ -1073,11 +815,7 @@ private List ToList() { Debug.Assert(Monitor.IsEntered(GlobalListsLock)); - int capacity = _approxCount; - if (capacity < 0) // Ensure capacity - capacity = 0; - - List list = new List(capacity); + List list = new List(); ThreadLocalList currentList = _headList; while (currentList != null) { @@ -1248,18 +986,6 @@ internal void Steal(out T result, bool remove) result = tail._value; } - /// - /// Resets the local list - /// - internal void Reset() - { - _head = null; - _tail = null; - _count = 0; - _stealCount = 0; - _version = 0; // Reset version since the local list is empty again - } - /// /// Gets the total list count, it's not thread safe, may provide incorrect count if it is called concurrently From fe9ec04336f17c473e8e13024ed373d5f99936ae Mon Sep 17 00:00:00 2001 From: MarkusSintonen Date: Fri, 10 Jul 2015 00:17:13 +0300 Subject: [PATCH 4/6] Restored original commit back --- .../src/System/Collections/Generic/HashSet.cs | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/src/System.Collections/src/System/Collections/Generic/HashSet.cs b/src/System.Collections/src/System/Collections/Generic/HashSet.cs index e966a6c8b2cd..3466ac7ec34b 100644 --- a/src/System.Collections/src/System/Collections/Generic/HashSet.cs +++ b/src/System.Collections/src/System/Collections/Generic/HashSet.cs @@ -75,22 +75,16 @@ public class HashSet : ICollection, ISet, IReadOnlyCollection #region Constructors public HashSet() - : this(EqualityComparer.Default) + : this(0, EqualityComparer.Default) { } public HashSet(IEqualityComparer comparer) - { - if (comparer == null) - { - comparer = EqualityComparer.Default; - } + : this(0, comparer) + { } - _comparer = comparer; - _lastIndex = 0; - _count = 0; - _freeList = -1; - _version = 0; - } + public HashSet(int capacity) + : this(capacity, EqualityComparer.Default) + { } public HashSet(IEnumerable collection) : this(collection, EqualityComparer.Default) @@ -131,6 +125,30 @@ public HashSet(IEnumerable collection, IEqualityComparer comparer) } } + public HashSet(int capacity, IEqualityComparer comparer) + { + if (capacity < 0) + { + throw new ArgumentOutOfRangeException("capacity"); + } + + if (comparer == null) + { + comparer = EqualityComparer.Default; + } + + _comparer = comparer; + _lastIndex = 0; + _count = 0; + _freeList = -1; + _version = 0; + + if (capacity > 0) + { + Initialize(capacity); + } + } + #endregion #region ICollection methods From 53fba0bc002e16c089c5ae44b0d1ecf8c68efb5a Mon Sep 17 00:00:00 2001 From: MarkusSintonen Date: Fri, 10 Jul 2015 00:48:01 +0300 Subject: [PATCH 5/6] -Removed ToList() initial capacity determining due to silghtly lower performance when adding/removing items to bag -Removed few new bag iterating functions and added more versatile ForEach function to make it possible to avoid list allocations --- .../Collections/Concurrent/ConcurrentBag.cs | 138 +++++++++++++++++- 1 file changed, 135 insertions(+), 3 deletions(-) diff --git a/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs b/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs index 4873c9ca5e17..48e04d43a8d7 100644 --- a/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs +++ b/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs @@ -48,6 +48,9 @@ public class ConcurrentBag : IProducerConsumerCollection, IReadOnlyCollect // GlobalListsLock lock private bool _needSync; + // Count of the thread lists. This is used to determine initial capacity for the version lists + private volatile int _threadListCount; + /// /// Initializes a new instance of the /// class. @@ -274,6 +277,7 @@ private ThreadLocalList GetThreadList(bool forceCreate) list = new ThreadLocalList(Environment.CurrentManagedThreadId); _headList = list; _tailList = list; + _threadListCount++; } else { @@ -283,6 +287,7 @@ private ThreadLocalList GetThreadList(bool forceCreate) list = new ThreadLocalList(Environment.CurrentManagedThreadId); _tailList._nextList = list; _tailList = list; + _threadListCount++; } } _locals.Value = list; @@ -339,7 +344,7 @@ private bool Steal(out T result, bool take) #endif bool loop; - List versionsList = new List(); // save the lists version + List versionsList = new List(_threadListCount); // save the lists version do { versionsList.Clear(); //clear the list from the previous iteration @@ -573,6 +578,121 @@ IEnumerator IEnumerable.GetEnumerator() return ((ConcurrentBag)this).GetEnumerator(); } + /// + /// Returns an enumerator that iterates through the . + /// + /// An enumerator for the contents of the . + /// + /// The enumerator returned from the function is safe to use concurrently with + /// reads and writes to the bag, however it does not represent a moment-in-time snapshot + /// of the bag. The contents exposed through the enumerator may contain modifications + /// made to the bag after was called. + /// does not copy the bag contents to a list as does. + /// + public IEnumerable GetEnumerableSlim() + { + ThreadLocalList currentList = _headList; + while (currentList != null) + { + Node currentNode = currentList._head; + while (currentNode != null) + { + yield return currentNode._value; + currentNode = currentNode._next; + } + currentList = currentList._nextList; + } + } + + /// + /// Determines whether the contains the specified item. + /// + /// The item to locate in the . + /// true if the contains the specified item; + /// otherwise, false. + /// + /// The returned value represents a moment-in-time snapshot of the contents of the bag. + /// + public bool Contains(T item) + { + // Short path if the bag is empty + if (_headList == null) + return false; + + bool lockTaken = false; + try + { + FreezeBag(ref lockTaken); + + EqualityComparer comparer = EqualityComparer.Default; + foreach (T bagItem in GetEnumerableSlim()) + { + if (comparer.Equals(bagItem, item)) + return true; // Item found + } + return false; + } + finally + { + UnfreezeBag(lockTaken); + } + } + + /// + /// Performs the specified action on each element of the . + /// + /// The delegate to perform on each element of the . + /// Break the action by returning false from the specified delegate. + public void ForEach(Func action) + { + // Short path if the bag is empty + if (_headList == null) + return; + + bool lockTaken = false; + try + { + FreezeBag(ref lockTaken); + + foreach (T bagItem in GetEnumerableSlim()) + { + if (!action(bagItem)) + return; + } + } + finally + { + UnfreezeBag(lockTaken); + } + } + + /// + /// Removes all items from the . + /// + public void Clear() + { + // Short path if the bag is empty + if (_headList == null) + return; + + bool lockTaken = false; + try + { + FreezeBag(ref lockTaken); + + ThreadLocalList currentList = _headList; + while (currentList != null) + { + currentList.Reset(); + currentList = currentList._nextList; + } + } + finally + { + UnfreezeBag(lockTaken); + } + } + /// /// Gets the number of elements contained in the . /// @@ -580,7 +700,7 @@ IEnumerator IEnumerable.GetEnumerator() /// /// The count returned represents a moment-in-time snapshot of the contents /// of the bag. It does not reflect any updates to the collection after - /// was called. + /// was called. /// public int Count { @@ -666,7 +786,7 @@ object ICollection.SyncRoot /// /// A global lock object, used in two cases: /// 1- To maintain the _tailList pointer for each new list addition process ( first time a thread called Add ) - /// 2- To freeze the bag in GetEnumerator, CopyTo, ToArray and Count members + /// 2- To freeze the bag in GetEnumerator, CopyTo, ToArray, Contains, Clear and Count members /// private object GlobalListsLock { @@ -986,6 +1106,18 @@ internal void Steal(out T result, bool remove) result = tail._value; } + /// + /// Resets the local list + /// + internal void Reset() + { + _head = null; + _tail = null; + _count = 0; + _stealCount = 0; + _version = 0; // Reset version since the local list is empty again + } + /// /// Gets the total list count, it's not thread safe, may provide incorrect count if it is called concurrently From 4a01108ecc7f6808332f7147ded4fa286e18408c Mon Sep 17 00:00:00 2001 From: MarkusSintonen Date: Fri, 10 Jul 2015 00:51:28 +0300 Subject: [PATCH 6/6] -Ooops, branch was still commiting to wrong place --- .../Collections/Concurrent/ConcurrentBag.cs | 138 +----------------- 1 file changed, 3 insertions(+), 135 deletions(-) diff --git a/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs b/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs index 48e04d43a8d7..4873c9ca5e17 100644 --- a/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs +++ b/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs @@ -48,9 +48,6 @@ public class ConcurrentBag : IProducerConsumerCollection, IReadOnlyCollect // GlobalListsLock lock private bool _needSync; - // Count of the thread lists. This is used to determine initial capacity for the version lists - private volatile int _threadListCount; - /// /// Initializes a new instance of the /// class. @@ -277,7 +274,6 @@ private ThreadLocalList GetThreadList(bool forceCreate) list = new ThreadLocalList(Environment.CurrentManagedThreadId); _headList = list; _tailList = list; - _threadListCount++; } else { @@ -287,7 +283,6 @@ private ThreadLocalList GetThreadList(bool forceCreate) list = new ThreadLocalList(Environment.CurrentManagedThreadId); _tailList._nextList = list; _tailList = list; - _threadListCount++; } } _locals.Value = list; @@ -344,7 +339,7 @@ private bool Steal(out T result, bool take) #endif bool loop; - List versionsList = new List(_threadListCount); // save the lists version + List versionsList = new List(); // save the lists version do { versionsList.Clear(); //clear the list from the previous iteration @@ -578,121 +573,6 @@ IEnumerator IEnumerable.GetEnumerator() return ((ConcurrentBag)this).GetEnumerator(); } - /// - /// Returns an enumerator that iterates through the . - /// - /// An enumerator for the contents of the . - /// - /// The enumerator returned from the function is safe to use concurrently with - /// reads and writes to the bag, however it does not represent a moment-in-time snapshot - /// of the bag. The contents exposed through the enumerator may contain modifications - /// made to the bag after was called. - /// does not copy the bag contents to a list as does. - /// - public IEnumerable GetEnumerableSlim() - { - ThreadLocalList currentList = _headList; - while (currentList != null) - { - Node currentNode = currentList._head; - while (currentNode != null) - { - yield return currentNode._value; - currentNode = currentNode._next; - } - currentList = currentList._nextList; - } - } - - /// - /// Determines whether the contains the specified item. - /// - /// The item to locate in the . - /// true if the contains the specified item; - /// otherwise, false. - /// - /// The returned value represents a moment-in-time snapshot of the contents of the bag. - /// - public bool Contains(T item) - { - // Short path if the bag is empty - if (_headList == null) - return false; - - bool lockTaken = false; - try - { - FreezeBag(ref lockTaken); - - EqualityComparer comparer = EqualityComparer.Default; - foreach (T bagItem in GetEnumerableSlim()) - { - if (comparer.Equals(bagItem, item)) - return true; // Item found - } - return false; - } - finally - { - UnfreezeBag(lockTaken); - } - } - - /// - /// Performs the specified action on each element of the . - /// - /// The delegate to perform on each element of the . - /// Break the action by returning false from the specified delegate. - public void ForEach(Func action) - { - // Short path if the bag is empty - if (_headList == null) - return; - - bool lockTaken = false; - try - { - FreezeBag(ref lockTaken); - - foreach (T bagItem in GetEnumerableSlim()) - { - if (!action(bagItem)) - return; - } - } - finally - { - UnfreezeBag(lockTaken); - } - } - - /// - /// Removes all items from the . - /// - public void Clear() - { - // Short path if the bag is empty - if (_headList == null) - return; - - bool lockTaken = false; - try - { - FreezeBag(ref lockTaken); - - ThreadLocalList currentList = _headList; - while (currentList != null) - { - currentList.Reset(); - currentList = currentList._nextList; - } - } - finally - { - UnfreezeBag(lockTaken); - } - } - /// /// Gets the number of elements contained in the . /// @@ -700,7 +580,7 @@ public void Clear() /// /// The count returned represents a moment-in-time snapshot of the contents /// of the bag. It does not reflect any updates to the collection after - /// was called. + /// was called. /// public int Count { @@ -786,7 +666,7 @@ object ICollection.SyncRoot /// /// A global lock object, used in two cases: /// 1- To maintain the _tailList pointer for each new list addition process ( first time a thread called Add ) - /// 2- To freeze the bag in GetEnumerator, CopyTo, ToArray, Contains, Clear and Count members + /// 2- To freeze the bag in GetEnumerator, CopyTo, ToArray and Count members /// private object GlobalListsLock { @@ -1106,18 +986,6 @@ internal void Steal(out T result, bool remove) result = tail._value; } - /// - /// Resets the local list - /// - internal void Reset() - { - _head = null; - _tail = null; - _count = 0; - _stealCount = 0; - _version = 0; // Reset version since the local list is empty again - } - /// /// Gets the total list count, it's not thread safe, may provide incorrect count if it is called concurrently