Make some Dictionary code smaller (#10993)

mikedn · jkotas · commit 6cb9a6fa3d09 · 2017-04-15T12:11:51.000-07:00
Store a reference to the relevant entry at the start of the loop to avoid indexing `entries` multiple times.
This avoids some redundant range checks in `Remove` and allows the elimination of a duplicate `index++` in `Enumerator.MoveNext`.

Saves ~7KB of code.
diff --git a/src/mscorlib/src/System/Collections/Generic/Dictionary.cs b/src/mscorlib/src/System/Collections/Generic/Dictionary.cs
@@ -428,8 +428,7 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior)
 
             if (buckets == null) Initialize(0);
             int hashCode = comparer.GetHashCode(key) & 0x7FFFFFFF;
-            int targetBucket = hashCode % buckets.Length;
-
+            int targetBucket = hashCode % buckets.Length;            
 #if FEATURE_RANDOMIZED_STRING_HASHING
             int collisionCount = 0;
 #endif
@@ -452,7 +451,6 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior)
 
                     return false;
                 }
-
 #if FEATURE_RANDOMIZED_STRING_HASHING
                 collisionCount++;
 #endif
@@ -599,27 +597,33 @@ public bool Remove(TKey key)
                 int hashCode = comparer.GetHashCode(key) & 0x7FFFFFFF;
                 int bucket = hashCode % buckets.Length;
                 int last = -1;
-                for (int i = buckets[bucket]; i >= 0; last = i, i = entries[i].next)
+                int i = buckets[bucket];
+                while (i >= 0)
                 {
-                    if (entries[i].hashCode == hashCode && comparer.Equals(entries[i].key, key))
+                    ref Entry entry = ref entries[i];
+
+                    if (entry.hashCode == hashCode && comparer.Equals(entry.key, key))
                     {
                         if (last < 0)
                         {
-                            buckets[bucket] = entries[i].next;
+                            buckets[bucket] = entry.next;
                         }
                         else
                         {
-                            entries[last].next = entries[i].next;
+                            entries[last].next = entry.next;
                         }
-                        entries[i].hashCode = -1;
-                        entries[i].next = freeList;
-                        entries[i].key = default(TKey);
-                        entries[i].value = default(TValue);
+                        entry.hashCode = -1;
+                        entry.next = freeList;
+                        entry.key = default(TKey);
+                        entry.value = default(TValue);
                         freeList = i;
                         freeCount++;
                         version++;
                         return true;
                     }
+
+                    last = i;
+                    i = entry.next;
                 }
             }
             return false;
@@ -640,30 +644,36 @@ public bool Remove(TKey key, out TValue value)
                 int hashCode = comparer.GetHashCode(key) & 0x7FFFFFFF;
                 int bucket = hashCode % buckets.Length;
                 int last = -1;
-                for (int i = buckets[bucket]; i >= 0; last = i, i = entries[i].next)
+                int i = buckets[bucket];
+                while (i >= 0)
                 {
-                    if (entries[i].hashCode == hashCode && comparer.Equals(entries[i].key, key))
+                    ref Entry entry = ref entries[i];
+
+                    if (entry.hashCode == hashCode && comparer.Equals(entry.key, key))
                     {
                         if (last < 0)
                         {
-                            buckets[bucket] = entries[i].next;
+                            buckets[bucket] = entry.next;
                         }
                         else
                         {
-                            entries[last].next = entries[i].next;
+                            entries[last].next = entry.next;
                         }
 
-                        value = entries[i].value;
+                        value = entry.value;
 
-                        entries[i].hashCode = -1;
-                        entries[i].next = freeList;
-                        entries[i].key = default(TKey);
-                        entries[i].value = default(TValue);
+                        entry.hashCode = -1;
+                        entry.next = freeList;
+                        entry.key = default(TKey);
+                        entry.value = default(TValue);
                         freeList = i;
                         freeCount++;
                         version++;
                         return true;
                     }
+
+                    last = i;
+                    i = entry.next;
                 }
             }
             value = default(TValue);
@@ -955,13 +965,13 @@ public bool MoveNext()
                 // dictionary.count+1 could be negative if dictionary.count is Int32.MaxValue
                 while ((uint)index < (uint)dictionary.count)
                 {
-                    if (dictionary.entries[index].hashCode >= 0)
+                    ref Entry entry = ref dictionary.entries[index++];
+
+                    if (entry.hashCode >= 0)
                     {
-                        current = new KeyValuePair<TKey, TValue>(dictionary.entries[index].key, dictionary.entries[index].value);
-                        index++;
+                        current = new KeyValuePair<TKey, TValue>(entry.key, entry.value);
                         return true;
                     }
-                    index++;
                 }
 
                 index = dictionary.count + 1;
@@ -1231,13 +1241,13 @@ public bool MoveNext()
 
                     while ((uint)index < (uint)dictionary.count)
                     {
-                        if (dictionary.entries[index].hashCode >= 0)
+                        ref Entry entry = ref dictionary.entries[index++];
+
+                        if (entry.hashCode >= 0)
                         {
-                            currentKey = dictionary.entries[index].key;
-                            index++;
+                            currentKey = entry.key;
                             return true;
                         }
-                        index++;
                     }
 
                     index = dictionary.count + 1;
@@ -1459,13 +1469,13 @@ public bool MoveNext()
 
                     while ((uint)index < (uint)dictionary.count)
                     {
-                        if (dictionary.entries[index].hashCode >= 0)
+                        ref Entry entry = ref dictionary.entries[index++];
+
+                        if (entry.hashCode >= 0)
                         {
-                            currentValue = dictionary.entries[index].value;
-                            index++;
+                            currentValue = entry.value;
                             return true;
                         }
-                        index++;
                     }
                     index = dictionary.count + 1;
                     currentValue = default(TValue);

Original file line number	Diff line number	Diff line change
`@@ -428,8 +428,7 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior)`
`428`	`428`
`429`	`429`	`if (buckets == null) Initialize(0);`
`430`	`430`	`int hashCode = comparer.GetHashCode(key) & 0x7FFFFFFF;`
`431`		`- int targetBucket = hashCode % buckets.Length;`
`432`		`-`
	`431`	`+ int targetBucket = hashCode % buckets.Length;`
`433`	`432`	`#if FEATURE_RANDOMIZED_STRING_HASHING`
`434`	`433`	`int collisionCount = 0;`
`435`	`434`	`#endif`
`@@ -452,7 +451,6 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior)`
`452`	`451`
`453`	`452`	`return false;`
`454`	`453`	`}`
`455`		`-`
`456`	`454`	`#if FEATURE_RANDOMIZED_STRING_HASHING`
`457`	`455`	`collisionCount++;`
`458`	`456`	`#endif`
`@@ -599,27 +597,33 @@ public bool Remove(TKey key)`
`599`	`597`	`int hashCode = comparer.GetHashCode(key) & 0x7FFFFFFF;`
`600`	`598`	`int bucket = hashCode % buckets.Length;`
`601`	`599`	`int last = -1;`
`602`		`- for (int i = buckets[bucket]; i >= 0; last = i, i = entries[i].next)`
	`600`	`+ int i = buckets[bucket];`
	`601`	`+ while (i >= 0)`
`603`	`602`	`{`
`604`		`- if (entries[i].hashCode == hashCode && comparer.Equals(entries[i].key, key))`
	`603`	`+ ref Entry entry = ref entries[i];`
	`604`	`+`
	`605`	`+ if (entry.hashCode == hashCode && comparer.Equals(entry.key, key))`
`605`	`606`	`{`
`606`	`607`	`if (last < 0)`
`607`	`608`	`{`
`608`		`- buckets[bucket] = entries[i].next;`
	`609`	`+ buckets[bucket] = entry.next;`
`609`	`610`	`}`
`610`	`611`	`else`
`611`	`612`	`{`
`612`		`- entries[last].next = entries[i].next;`
	`613`	`+ entries[last].next = entry.next;`
`613`	`614`	`}`
`614`		`- entries[i].hashCode = -1;`
`615`		`- entries[i].next = freeList;`
`616`		`- entries[i].key = default(TKey);`
`617`		`- entries[i].value = default(TValue);`
	`615`	`+ entry.hashCode = -1;`
	`616`	`+ entry.next = freeList;`
	`617`	`+ entry.key = default(TKey);`
	`618`	`+ entry.value = default(TValue);`
`618`	`619`	`freeList = i;`
`619`	`620`	`freeCount++;`
`620`	`621`	`version++;`
`621`	`622`	`return true;`
`622`	`623`	`}`
	`624`	`+`
	`625`	`+ last = i;`
	`626`	`+ i = entry.next;`
`623`	`627`	`}`
`624`	`628`	`}`
`625`	`629`	`return false;`
`@@ -640,30 +644,36 @@ public bool Remove(TKey key, out TValue value)`
`640`	`644`	`int hashCode = comparer.GetHashCode(key) & 0x7FFFFFFF;`
`641`	`645`	`int bucket = hashCode % buckets.Length;`
`642`	`646`	`int last = -1;`
`643`		`- for (int i = buckets[bucket]; i >= 0; last = i, i = entries[i].next)`
	`647`	`+ int i = buckets[bucket];`
	`648`	`+ while (i >= 0)`
`644`	`649`	`{`
`645`		`- if (entries[i].hashCode == hashCode && comparer.Equals(entries[i].key, key))`
	`650`	`+ ref Entry entry = ref entries[i];`
	`651`	`+`
	`652`	`+ if (entry.hashCode == hashCode && comparer.Equals(entry.key, key))`
`646`	`653`	`{`
`647`	`654`	`if (last < 0)`
`648`	`655`	`{`
`649`		`- buckets[bucket] = entries[i].next;`
	`656`	`+ buckets[bucket] = entry.next;`
`650`	`657`	`}`
`651`	`658`	`else`
`652`	`659`	`{`
`653`		`- entries[last].next = entries[i].next;`
	`660`	`+ entries[last].next = entry.next;`
`654`	`661`	`}`
`655`	`662`
`656`		`- value = entries[i].value;`
	`663`	`+ value = entry.value;`
`657`	`664`
`658`		`- entries[i].hashCode = -1;`
`659`		`- entries[i].next = freeList;`
`660`		`- entries[i].key = default(TKey);`
`661`		`- entries[i].value = default(TValue);`
	`665`	`+ entry.hashCode = -1;`
	`666`	`+ entry.next = freeList;`
	`667`	`+ entry.key = default(TKey);`
	`668`	`+ entry.value = default(TValue);`
`662`	`669`	`freeList = i;`
`663`	`670`	`freeCount++;`
`664`	`671`	`version++;`
`665`	`672`	`return true;`
`666`	`673`	`}`
	`674`	`+`
	`675`	`+ last = i;`
	`676`	`+ i = entry.next;`
`667`	`677`	`}`
`668`	`678`	`}`
`669`	`679`	`value = default(TValue);`
`@@ -955,13 +965,13 @@ public bool MoveNext()`
`955`	`965`	`// dictionary.count+1 could be negative if dictionary.count is Int32.MaxValue`
`956`	`966`	`while ((uint)index < (uint)dictionary.count)`
`957`	`967`	`{`
`958`		`- if (dictionary.entries[index].hashCode >= 0)`
	`968`	`+ ref Entry entry = ref dictionary.entries[index++];`
	`969`	`+`
	`970`	`+ if (entry.hashCode >= 0)`
`959`	`971`	`{`
`960`		`- current = new KeyValuePair<TKey, TValue>(dictionary.entries[index].key, dictionary.entries[index].value);`
`961`		`- index++;`
	`972`	`+ current = new KeyValuePair<TKey, TValue>(entry.key, entry.value);`
`962`	`973`	`return true;`
`963`	`974`	`}`
`964`		`- index++;`
`965`	`975`	`}`
`966`	`976`
`967`	`977`	`index = dictionary.count + 1;`
`@@ -1231,13 +1241,13 @@ public bool MoveNext()`
`1231`	`1241`
`1232`	`1242`	`while ((uint)index < (uint)dictionary.count)`
`1233`	`1243`	`{`
`1234`		`- if (dictionary.entries[index].hashCode >= 0)`
	`1244`	`+ ref Entry entry = ref dictionary.entries[index++];`
	`1245`	`+`
	`1246`	`+ if (entry.hashCode >= 0)`
`1235`	`1247`	`{`
`1236`		`- currentKey = dictionary.entries[index].key;`
`1237`		`- index++;`
	`1248`	`+ currentKey = entry.key;`
`1238`	`1249`	`return true;`
`1239`	`1250`	`}`
`1240`		`- index++;`
`1241`	`1251`	`}`
`1242`	`1252`
`1243`	`1253`	`index = dictionary.count + 1;`
`@@ -1459,13 +1469,13 @@ public bool MoveNext()`
`1459`	`1469`
`1460`	`1470`	`while ((uint)index < (uint)dictionary.count)`
`1461`	`1471`	`{`
`1462`		`- if (dictionary.entries[index].hashCode >= 0)`
	`1472`	`+ ref Entry entry = ref dictionary.entries[index++];`
	`1473`	`+`
	`1474`	`+ if (entry.hashCode >= 0)`
`1463`	`1475`	`{`
`1464`		`- currentValue = dictionary.entries[index].value;`
`1465`		`- index++;`
	`1476`	`+ currentValue = entry.value;`
`1466`	`1477`	`return true;`
`1467`	`1478`	`}`
`1468`		`- index++;`
`1469`	`1479`	`}`
`1470`	`1480`	`index = dictionary.count + 1;`
`1471`	`1481`	`currentValue = default(TValue);`