fix: Removed unnecessary GC Allocation #3527
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Do you know if this is only 2.X specific or should also be backported to 1.X ( |
Portions of this PR could be back ported to v1.x. |
NoelStephensUnity
added
the
port:1.x-needed
NoelStephensUnity
added a commit
that referenced
this pull request
Aug 14, 2025
5 tasks
NoelStephensUnity
added
port:1.x-completed
NoelStephensUnity
approved these changes
Aug 14, 2025
There was a problem hiding this comment.
@samlucas-unity Looks good and thank you for the fix! Sorry for the delay on getting this back ported.
NoelStephensUnity
added a commit
that referenced
this pull request
Aug 15, 2025
Fix for [MTTB-85](https://jira.unity3d.com/browse/MTTB-85) removed the use of a foreach causing an unnecessary GC Allocation. Did a full sweep of internal `NetworkManager.ConnectedClientsIds` and replaced with either `for` loop or directly referencing the `NetworkConnectionManager.ConnectedClientIds`. ## Changelog - Fixed: Issue with unnecessary internal GC Allocations when using the `IReadOnlyList` `NetworkManager.ConnectedClientsIds` within a `foreach` statement by either replacing with a `for` loop or directly referencing the `NetworkConnectionManager.ConnectedClientIds`. ## Documentation - No documentation updates were required. ## Testing & QA [//]: # ( This section is REQUIRED and should describe how the changes were tested and how should they be tested when Playtesting for the release. It can range from "edge case covered by unit tests" to "manual testing required and new sample was added". Expectation is that PR creator does some manual testing and provides a summary of it here.) ### Functional Testing [//]: # (If checked, List manual tests that have been performed.) _Manual testing :_ - [X] `Manual testing done` - Tested manually before and after fix to confirm the allocation was present and then absent. _Automated tests:_ - [ ] `Covered by existing automated tests` - [ ] `Covered by new automated tests` _Does the change require QA team to:_ - [ ] `Review automated tests`? - [ ] `Execute manual tests`? If any boxes above are checked, please add QA as a PR reviewer. ## Backport This is the backport for #3527
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix for MTTB-85 removed the use of a foreach causing an unnecessary GC Allocation.
Did a full sweep of internal
NetworkManager.ConnectedClientsIdsand replaced with eitherforloop or directly referencing theNetworkConnectionManager.ConnectedClientIds.Changelog
IReadOnlyListNetworkManager.ConnectedClientsIdswithin aforeachstatement by either replacing with aforloop or directly referencing theNetworkConnectionManager.ConnectedClientIds.Documentation
Testing & QA
Functional Testing
Manual testing :
Manual testing doneAutomated tests:
Covered by existing automated testsCovered by new automated testsDoes the change require QA team to:
Review automated tests?Execute manual tests?If any boxes above are checked, please add QA as a PR reviewer.
Backport
The backport of this PR is #3601