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

Performance and garbage optimizations to Joystick.SDL #6829

Merged
merged 1 commit into from Jul 27, 2019

Conversation

kimimaru4000
Copy link
Contributor

@kimimaru4000 kimimaru4000 commented Jul 27, 2019

Relevant issue: #6820

Summary of changes in the initial commit:

  • Added a default JoystickState that's returned if the joystick isn't found to bypass allocating several zero-length arrays
  • Replaced HasFlag with bitwise operators to avoid boxing
  • Reduced dictionary lookups if the joystick exists by replacing ContainsKey followed by a dictionary indexer with TryGetValue

I'd like to figure out a way to let users bypass allocating the arrays involved in creating each JoystickState, as they can generate a lot of garbage over time.

-Added a default JoystickState that's returned if the joystick isn't found to bypass allocating additional arrays
-Replaced HasFlag with bitwise operators to avoid boxing
-Reduced dictionary lookups if the joystick exists by replacing ContainsKey followed by a dictionary indexer with TryGetValue
@Jjagg
Copy link
Contributor

Jjagg commented Jul 27, 2019

You can use Array<T>.Empty instead of new T[0]. No need to cache the default instance. Otherwise, this looks good :)

@kimimaru4000
Copy link
Contributor Author

kimimaru4000 commented Jul 27, 2019

I'm still using Protobuild to generate the project files so I'm seeing the project as .NET Framework 4.5, which doesn't have Array<T>.Empty. Should I be running the CAKE script?

@Jjagg
Copy link
Contributor

Jjagg commented Jul 27, 2019

Oh wow, I thought it existed way back. Nevermind then. Merging! Thanks @tdeeb

@Jjagg Jjagg merged commit ac217ce into MonoGame:develop Jul 27, 2019
@kimimaru4000
Copy link
Contributor Author

kimimaru4000 commented Jul 27, 2019

@Jjagg I appreciate the merge; this is definitely an improvement over before. I should have mentioned that it isn't complete yet since I wanted to discuss if we can make any further improvements when a joystick does exist - for example, an API to allow the user to supply their own arrays, or an alternative that can reduce or eliminate garbage if the user needs to.

@Jjagg
Copy link
Contributor

Jjagg commented Jul 27, 2019

We can discuss it further in #6820 :)

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.

None yet

3 participants