Skip to content

Conversation

smitdylan2001
Copy link
Contributor

Purpose of this PR

I found some simple performance improvement, which do not change any functionality. Especially the position and rotation calls and LINQ call removals are very helpful for performance

  • Merge looped Add calls into AddRange
  • Optimize empty string checks
  • Merge position & rotation calls into one
  • Short circuit operators for bools
  • Optimize string generation
  • Use native Count instead of LINQ

Changelog

Improved runtime and editor performance

Documentation

  • No documentation changes or additions were necessary.

Testing & QA (How your changes can be verified during release Playtest)

These do not change functionality

Does the change require QA team to:

  • Review automated tests?
  • Execute manual tests?
  • Provide feedback about the PR?

If any boxes above are checked the QA team will be automatically added as a PR reviewer.

- Merge looped Add calls into AddRange
- Optimize empty string checks
- Merge position & rotation calls into one
- Short circuit operators for bools
- Optimize string generation
- Use native Count instead of LINQ
@smitdylan2001 smitdylan2001 requested a review from a team as a code owner September 17, 2025 07:33
Copy link
Collaborator

@EmandM EmandM left a comment

Choose a reason for hiding this comment

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

Hi! Thanks so much for this PR. It looks great!

We just have to ask for our internal tracking, why did you decide to create this PR? Also, did you encounter a specific issue that made this PR necessary?

@smitdylan2001
Copy link
Contributor Author

smitdylan2001 commented Sep 17, 2025 via email

@sentinel-u3d sentinel-u3d bot requested a review from EmandM September 17, 2025 18:30
Copy link
Collaborator

@EmandM EmandM left a comment

Choose a reason for hiding this comment

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

Thank you for the information! You are 100% correct that the small things can be missed over time. We really appreciate you taking this time and this level of attention to detail.

I'm approving this here, however we can't land these changes from your fork directly. Next steps is I will create an internal branch and merge your changes into it. I'll then close this issue and we will land the changes from the internal branch.

Thank you for your contribution!

@smitdylan2001
Copy link
Contributor Author

smitdylan2001 commented Sep 17, 2025 via email

@EmandM
Copy link
Collaborator

EmandM commented Sep 17, 2025

We don't use that tool but it looks like we should be using it! If you have any info or thoughts on how it can help us we'd be very happy to hear it

@smitdylan2001
Copy link
Contributor Author

smitdylan2001 commented Sep 18, 2025 via email

@michalChrobot
Copy link
Collaborator

I have project auditor tool on my list to figure out how to use it best in our workflow (or just run periodic checks) but I didn't have capacity yet

@smitdylan2001
Copy link
Contributor Author

smitdylan2001 commented Sep 18, 2025 via email

@EmandM
Copy link
Collaborator

EmandM commented Sep 18, 2025

We appreciate the recommendation!

I'll close this PR for now and we'll continue it in #3683.

If you'd like to make more performance improvements we will always welcome it. Our current team is doing our best to get around to it all, but it's a big project with lots of nuances. Thanks for paying attention to the small changes that somehow always fall to the bottom of the importance pile!

@EmandM EmandM closed this Sep 18, 2025
EmandM added a commit that referenced this pull request Oct 2, 2025
## Purpose of this PR

@smitdylan2001 found and fixed some small performance improvements,
which do not change any functionality. Especially the position and
rotation calls and LINQ call removals are very helpful for performance

1. Merge looped `Add` calls on `List<T>` variables into `AddRange`
    - `AddRange` will ensure the list is only extended once.
2. Optimize empty string checks
- Comparing strings using `String.Length` is faster than using `Equals`
([C# docs
reference](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1820?view=vs-2022#rule-description:~:text=Comparing%20strings%20using%20the%20String.Length%20property%20or%20the%20String.IsNullOrEmpty%20method%20is%20faster%20than%20using%20Equals))
3. Merge position & rotation calls into one
- `SetPositionAndRotation` has a small performance improvement
([docs](https://docs.unity3d.com/6000.2/Documentation/ScriptReference/Transform.SetPositionAndRotation.html#:~:text=When%20setting%20both%20the%20position%20and%20rotation%20of%20a%20transform%2C%20calling%20this%20method%20is%20more%20efficient%20than%20assigning%20to%20Transform.position%20and%20Transform.rotation%20individually.))
4. Short circuit operators for bools
- We had a couple of places that were using bytewise operations on
boolean properties. This fixes them.
5. Use native Count instead of LINQ
- `.Count()` uses LINQ and has a cost. `.Count` does the same thing
without the LINQ cost.
6. Use `StringBuilder.AppendJoin` rather than
`StringBuilder.Append(String.Join`

continues: #3680 

## Jira ticket

n/a: contribution from external user

### Changelog

 - Changed: made many very small performance improvements.

## Documentation

- Updated the scripting in the docs to use `SetPositionAndRotation`
rather than setting the two independently

## Testing & QA (How your changes can be verified during release
Playtest)

- These do not change functionality. Automated testing should catch any
compiler errors.

_Does the change require QA team to:_

- [ ] `Review automated tests`?
- [ ] `Execute manual tests`?
- [ ] `Provide feedback about the PR`?

If any boxes above are checked the QA team will be automatically added
as a PR reviewer.

## Backports

These are small performance improvements so they do not need to be
backported.

---------

Co-authored-by: Dylan Smit <[email protected]>
Co-authored-by: Unity Netcode CI <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants