-
Notifications
You must be signed in to change notification settings - Fork 14
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
Reduce garbage produced every update #38
base: master
Are you sure you want to change the base?
Conversation
cache ControllerButton.values() and ControllerAxis.values() to prevent creating garbage also axis values are no longer boxed into Float objects every update
@@ -31,7 +33,7 @@ public class JamepadController implements Controller { | |||
|
|||
private final CompositeControllerListener compositeControllerListener = new CompositeControllerListener(); | |||
private final IntMap<Boolean> buttonState = new IntMap<>(); | |||
private final IntMap<Float> axisState = new IntMap<>(); |
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.
Libgdx has IntFloatMap which you can use to achieve this
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.
Nice, that's what I was looking for. Thx!
@@ -18,6 +19,8 @@ public class JamepadController implements Controller { | |||
private static final IntMap<ControllerButton> CODE_TO_BUTTON = new IntMap<>(ControllerButton.values().length); | |||
private static final IntMap<ControllerAxis> CODE_TO_AXIS = new IntMap<>(ControllerAxis.values().length); | |||
private static final Logger logger = new Logger(JamepadController.class.getSimpleName()); | |||
private static final ControllerButton[] CONTROLLER_BUTTON_VALUES = ControllerButton.values(); |
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.
Please add a comment why this is done so people only reading the code know there were performance issues
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.
Yes, good idea, can do!
@@ -164,7 +167,7 @@ private void updateButtonsState() { | |||
|
|||
private void initializeState() { | |||
for (ControllerAxis axis : ControllerAxis.values()) { |
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.
I know this is only called once, but is there a reason this does not use CONTROLLER_AXIS_VALUES
?
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.
Exactly, I was only concerned with the update method. Since this is only called once, it doesn't matter here. I'll happily change it to what you prefer.
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.
I think it is better to use the new constant, otherwise code readers may ask why it is used only at some places.
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.
Alright. Won't get to it this weekend, but will implement the changes after the weekend.
also use the constants instead of ControllerButton.values() consistently throughout the file
Forgot about this one, sorry, but finally made the changes. |
Hi, after some profiling found that this library produces quite some garbage every frame/update.
ControllerButton.values()
andControllerAxis.values()
for the update methods, since those were creating new arrays all the time. (some JVMs might be able to correctly optimize this, but the one I profiled did not)axisState.put(id, value);
was causing the values to be boxed to newFloat
s every update, so I introduced a simple wrapper class with a mutable float, which does not create any garbageThis only covers the desktop version, similar optimizations could be made for other platforms probably.
IDK if this can be merged as-is, but would love to hear about any suggestions or doubts.