-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Optimize Queue<T>.Enumerator #117341
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
Optimize Queue<T>.Enumerator #117341
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Optimizes Queue<T>.Enumerator by rewriting internal fields and the MoveNext loop for performance, and updates related test properties to reflect changed behavior.
- Renames internal enumerator fields (
_q→_queue,_index→_i) and refactorsMoveNextto eliminate expensive modulo operations. - Updates
CurrentandResetimplementations accordingly. - Renames test properties in both generic and non-generic
Queuetests to include “Empty” in the undefined-operation checks.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Queue.cs | Refactored enumerator internals, field renames, and loop logic |
| src/libraries/System.Collections/tests/Generic/Queue/Queue.Tests.cs | Renamed test property for undefined Current operation |
| src/libraries/System.Collections/tests/Generic/Queue/Queue.Generic.Tests.cs | Renamed generic test property for undefined Current operation |
Comments suppressed due to low confidence (2)
src/libraries/System.Collections/tests/Generic/Queue/Queue.Tests.cs:30
- [nitpick] The renamed test property
Enumerator_Empty_Current_UndefinedOperation_Throwis inconsistent with its original plural suffix and the naming in base classes. Rename it toEnumerator_Empty_Current_UndefinedOperation_Throwsto match the established pattern.
protected override bool Enumerator_Empty_Current_UndefinedOperation_Throw => true;
src/libraries/System.Collections/tests/Generic/Queue/Queue.Generic.Tests.cs:54
- [nitpick] Ensure the test property naming matches across both generic and non-generic fixtures; if
Enumerator_Empty_Current_UndefinedOperation_Throwsis intended here, update the non-generic test to use the same name for consistency.
protected override bool Enumerator_Empty_Current_UndefinedOperation_Throws => true;
src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Queue.cs
Show resolved
Hide resolved
|
Tagging subscribers to this area: @dotnet/area-system-collections |
src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Queue.cs
Outdated
Show resolved
Hide resolved
|
/ba-g wasm infra failure |
Related to #117328