Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary _version++ from a few places #82500

Merged
merged 3 commits into from
Feb 23, 2023

Conversation

stephentoub
Copy link
Member

In some collection types, resizing the backing store can affect in-flight enumerations, e.g. growing a queue will cause the implementation to relayout the elements contiguously from the beginning, which will invalidate indices stored in enumerators. But for List<T> and Stack<T>, changing the backing array doesn't impact the index stored in an enumerator, nor does it impact what the enumerator will enumerate, so these operations needn't invalidate the enumerators. List<T>'s set_Capacity and TrimExcess already didn't update the version, but its EnsureCapacity did, and Stack<T>'s TrimExcess and EnsureCapacity both updated the version. This change just removes those unnecessary increments.

Fixes #82455
Related to #81523

In some collection types, resizing the backing store can affect in-flight enumerations, e.g. growing a queue will cause the implementation to relayout the elements contiguously from the beginning, which will invalidate indices stored in enumerators.  But for `List<T>` and `Stack<T>`, changing the backing array doesn't impact the index stored in an enumerator, nor does it impact what the enumerator will enumerate, so these operations needn't invalidate the enumerators.  `List<T>`'s `set_Capacity` and `TrimExcess` already didn't update the version, but its `EnsureCapacity` did, and `Stack<T>`'s `TrimExcess` and `EnsureCapacity` both updated the version.  This change just removes those unnecessary increments.
@stephentoub stephentoub added this to the 8.0.0 milestone Feb 22, 2023
@ghost ghost assigned stephentoub Feb 22, 2023
@ghost
Copy link

ghost commented Feb 22, 2023

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

Issue Details

In some collection types, resizing the backing store can affect in-flight enumerations, e.g. growing a queue will cause the implementation to relayout the elements contiguously from the beginning, which will invalidate indices stored in enumerators. But for List<T> and Stack<T>, changing the backing array doesn't impact the index stored in an enumerator, nor does it impact what the enumerator will enumerate, so these operations needn't invalidate the enumerators. List<T>'s set_Capacity and TrimExcess already didn't update the version, but its EnsureCapacity did, and Stack<T>'s TrimExcess and EnsureCapacity both updated the version. This change just removes those unnecessary increments.

Fixes #82455
Related to #81523

Author: stephentoub
Assignees: -
Labels:

area-System.Collections

Milestone: 8.0.0

@stephentoub stephentoub changed the title Remove unnecessarion _version++ from a few places Remove unnecessary _version++ from a few places Feb 23, 2023
Copy link
Member

@eiriktsarpalis eiriktsarpalis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@stephentoub stephentoub merged commit b04d321 into dotnet:main Feb 23, 2023
@stephentoub stephentoub deleted the versioninc branch February 23, 2023 22:03
@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EnsureCapacity() on List<> increments the version
4 participants