-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Description
Many of our mutable collection types maintain version numbers. These version numbers are incremented for many (not necessarily all) of the mutating operations performed on the type, e.g.
runtime/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/List.cs
Lines 196 to 198 in 2ba2396
| public void Add(T item) | |
| { | |
| _version++; |
The version number is then snapshot as part of calling GetEnumerator, and every MoveNext on the resulting enumerator compares the version number that was snapshot against the current version number.
runtime/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/List.cs
Lines 1166 to 1170 in 2ba2396
| public bool MoveNext() | |
| { | |
| List<T> localList = _list; | |
| if (_version == localList._version && ((uint)_index < (uint)localList._size)) |
This serves as a way to flag some misuse, where mutation is performed during enumeration.
The motivation here is good: help developers find misuse. There are a few problems, though:
- It adds a little overhead. It's minimal, and this alone is not worth a change for, but it requires an extra Int32 field on the type as well as on the enumerator, it requires an increment operation as part of each mutation, and it requires an extra branch as part of each MoveNext.
- It doesn't cover all forms of enumeration. If, for example, you enumerate a
List<T>viaforeach (var item in list) Use(item);, then it applies, but if you change to iterating viafor (int i = 0; i < list.Count; i++) Use(list[i]);, then it doesn't. - We're considering proposals for enabling the compiler to lower foreach usage down to such for-based iteration, and at that point we'd need to either forego the compiler optimization or give up on many places where this versioning would apply.
- The inconsistency of if/where it kicks in is visible in higher-level APIs. For example, LINQ's Select operator special-cases
IList<T>implementations to use indexing in some situations... usinglist.Select(Transform).ToArray()will not use the version checks even though the Transform could mutate the list. We also end up avoiding optimizing in this way in other places because it'd be visible.
I'm opening this issue to gauge folks' appetite for removing these versions. Do they still provide good enough value that they're worth keeping? Have they outlived their usefulness?