Improve performance for GetViewBetween and enumerating TreeSubSet (dotnet/corefx...

* Improve performance for GetViewBetween and enumerating TreeSubSet

* VersionCheckImpl is not called within the constructor

* count does not get updated by VersionCheckImpl

* count is now updated within VersionCheckCount which is
  only called when retrieving Count property. As a result
  count has been replaced with Count inside TreeSubSet/SortedSet
  where nessessary.

* For TreeSubSet the enumerator now uses the Count of the
  parent set (entire tree) when creating the stack.
  This results in a larger stack size, however it eliminates the
  need to get the count of TreeSubSet, which drastically
  increases the performance.

* Cleaned up code and added comments

* Fixed logic error with updateCount

* Removed unnecessary blank line

* Changed VersionCheckCount -> VersionCheck

* Cleaning code

Commit migrated from https://github.com/dotnet/corefx/commit/cc4d621fc57b5b515d6601c623337ce552ad7f86

diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.TreeSubSet.cs b/src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.TreeSubSet.cs
index cf86629..4083142 100644 (file)
--- a/src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.TreeSubSet.cs
+++ b/src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.TreeSubSet.cs
@@ -17,6 +17,10 @@ namespace System.Collections.Generic
         {
             private SortedSet<T> _underlying;
             private T _min, _max;
+            // keeps track of whether the count variable is up to date
+            // up to date -> _countVersion = _underlying.version
+            // not up to date -> _countVersion < _underlying.version
+            private int _countVersion;
             // these exist for unbounded collections
             // for instance, you could allow this subset to be defined for i > 10. The set will throw if
             // anything <= 10 is added, but there is no upper bound. These features Head(), Tail(), were punted
@@ -42,7 +46,7 @@ namespace System.Collections.Generic
                 root = _underlying.FindRange(_min, _max, _lBoundActive, _uBoundActive); // root is first element within range
                 count = 0;
                 version = -1;
-                VersionCheckImpl();
+                _countVersion = -1;
             }
 
             internal override bool AddIfNotPresent(T item)
@@ -87,7 +91,7 @@ namespace System.Collections.Generic
 
             public override void Clear()
             {
-                if (count == 0)
+                if (Count == 0)
                 {
                     return;
                 }
@@ -300,21 +304,36 @@ namespace System.Collections.Generic
 
             /// <summary>
             /// Checks whether this subset is out of date, and updates it if necessary.
+            /// <param name="updateCount">Updates the count variable if necessary.</param>
             /// </summary>
-            internal override void VersionCheck() => VersionCheckImpl();
+            internal override void VersionCheck(bool updateCount = false) => VersionCheckImpl(updateCount);
 
-            private void VersionCheckImpl()
+            private void VersionCheckImpl(bool updateCount)
             {
                 Debug.Assert(_underlying != null);
                 if (version != _underlying.version)
                 {
                     root = _underlying.FindRange(_min, _max, _lBoundActive, _uBoundActive);
                     version = _underlying.version;
+                }
+
+                if (updateCount && _countVersion != _underlying.version)
+                {
                     count = 0;
                     InOrderTreeWalk(n => { count++; return true; });
+                    _countVersion = _underlying.version;
                 }
             }
 
+            /// <summary>
+            /// Returns the number of elements <c>count</c> of the parent set.
+            /// </summary>
+            internal override int TotalCount()
+            {
+                Debug.Assert(_underlying != null);
+                return _underlying.Count;
+            }
+
             // This passes functionality down to the underlying tree, clipping edges if necessary
             // There's nothing gained by having a nested subset. May as well draw it from the base
             // Cannot increase the bounds of the subset, can only decrease it

diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.cs b/src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.cs
index 7f7375b..6ec7b1b 100644 (file)
--- a/src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.cs
+++ b/src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.cs
@@ -281,7 +281,7 @@ namespace System.Collections.Generic
         {
             get
             {
-                VersionCheck();
+                VersionCheck(updateCount: true);
                 return count;
             }
         }
@@ -310,7 +310,9 @@ namespace System.Collections.Generic
         #region Subclass helpers
 
         // Virtual function for TreeSubSet, which may need to update its count.
-        internal virtual void VersionCheck() { }
+        internal virtual void VersionCheck(bool updateCount = false) { }
+        // Virtual function for TreeSubSet, which may need the count variable of the parent set.
+        internal virtual int TotalCount() { return Count; }
 
         // Virtual function for TreeSubSet, which may need to do range checks.
         internal virtual bool IsWithinRange(T item) => true;
@@ -895,7 +897,7 @@ namespace System.Collections.Generic
             if (treeSubset != null)
                 VersionCheck();
 
-            if (asSorted != null && treeSubset == null && count == 0)
+            if (asSorted != null && treeSubset == null && Count == 0)
             {
                 SortedSet<T> dummy = new SortedSet<T>(asSorted, comparer);
                 root = dummy.root;
@@ -1950,7 +1952,7 @@ namespace System.Collections.Generic
                 _version = set.version;
 
                 // 2 log(n + 1) is the maximum height.
-                _stack = new Stack<Node>(2 * (int)Log2(set.Count + 1));
+                _stack = new Stack<Node>(2 * (int)Log2(set.TotalCount() + 1));
                 _current = null;
                 _reverse = reverse;