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

Fix slow desktop controller iteration #11

Merged
merged 1 commit into from
Jun 22, 2021

Conversation

DylanFrese
Copy link
Contributor

@DylanFrese DylanFrese commented Jun 21, 2021

In order to update the list of plugged-in controllers on desktop, the JamepadControllerMonitor iterates through Jamepad's internal list of controllers.

This works fine, but in order to get a bound in order to index that list, ControllerManager.getNumControllers() is called. This has performance implications: rather than returning the size of the list, getNumControllers probes each input device to verify it's a controller (rather than some other input device, such as a tablet pen).

On Linux, for example, this probing takes the form of an openat(), followed by several ioctl()s and a close(), for each non-contoller input device. This has a very high overhead, going back and forth with the kernel potentially many times each frame, and can tank a game's performance if these peripherals are present.

This fix simply uses maxNumControllers as a bounds to index into ControllerManager, which I think is the behavior that was intended anyway.

ControllerManager should also probably expose its own bounds to allow consumers to call getControllerIndex() safely.

In order to update the list of plugged-in controllers on desktop, the
JamepadControllerMonitor iterates through Jamepad's internal list of
controllers.

This works fine, but in order to get a bound in order to index that
list, ControllerManager.getNumControllers() is called. This has
performance implications: rather than returning the size of the list,
getNumControllers probes each input device to verify it's a controller
(rather than some other input device, such as a tablet pen).

On Linux, for example, this probing takes the form of an openat(),
followed by several ioctl()s and a close(), for each non-contoller input
device. This has a very high overhead, going back and forth with the
kernel potentially many times each frame, and can tank a game's
performance if these peripherals are present.

This fix simply uses maxNumControllers as a bounds to index into
ControllerManager, which I think is the behavior that was intended
anyway.

ControllerManager should also probably expose its own bounds to allow
consumers to call getControllerIndex() safely.
Copy link
Member

@MrStahlfelge MrStahlfelge left a comment

Choose a reason for hiding this comment

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

I agree that his is better and it works well this way on Linux. Which OS did you test?

@DylanFrese
Copy link
Contributor Author

I'm on Debian Bullseye with Linux amd64 5.10.0-6, but I'm not sure how much that matters here. I think the root cause is SDL_IsGameController has to query those devices every time. gdx + lwjgl seems to run it every frame. An overhead of a few ms between kernel and userspace matters a lot, then. I was going from 144+ FPS to 17, so that's a 50ms overhead for each scan.

I arrived here after a debugging session that started with me playing Wildermyth and wondering why it was performing so badly: https://steamcommunity.com/app/763890/discussions/0/3077629031367169536

@MrStahlfelge
Copy link
Member

On Windows SDL's behaviour is sometimes different because the driver stack is completely different (it even differs when using useRawInput or not). I don't see a way how this change could lead to crashes, but better safe than sorry.

@MrStahlfelge
Copy link
Member

BTW, Jamepad is not deprecated but maintained under libgdx organisation as well.

@DylanFrese
Copy link
Contributor Author

I realised that after; I had been looking at https://github.com/williamahartman/Jamepad/ which had the 1.x releases.

@WinterAlexander
Copy link
Contributor

This is great and explains a lot about the slowdown!

@MrStahlfelge MrStahlfelge merged commit 4a1a3d8 into libgdx:master Jun 22, 2021
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