Performance issue because of GamepadManager._checkGamepadsStatus()

Bugs
1

Hi all,

I discovered a performance issue in my project.

When a gamepad is connected and used, FPS drops slowly but inexorably. I’ve highlighted the offending line: https://github.com/BabylonJS/Babylon.js/blob/master/packages/dev/core/src/Gamepads/gamepadManager.ts#L230

_checkGamepadsStatus() can be called in a loop with the creation of a frame, leaving no room for the execution of parallel tasks and create a huge stack. It’s hard to show this in a simple project, but I think one sees an obvious problem simply by looking at the code.

Simply replacing this with a timeout that doesn’t create a frame already solves the problem:

      if (this._isMonitoring) {
        // Engine.QueueNewFrame(() => {
        //   this._checkGamepadsStatus();
        // });
        setTimeout(() => { this._checkGamepadsStatus() }, 1);
      }

I also tried to replace it by this._scene.onBeforeRenderObservable.addOnce(() => {this._checkGamepadsStatus()}); but surprisingly it freeze the app.

To summarize, I think that the creation of a frame here is useless and problematic. Could you confirm this?

Cheers!

2

cc @PolygonalSun

4

Lemme take a look and see what’s going on.

5

From what I’m seeing, there isn’t much difference in perf with respect to time but the heap memory for Engine.QueueNewFrame looks proportionately higher, as compared to setTimeout at least if I’m reading the profiler correctly.

Since, the GamepadManager doesn’t require a reference to the Scene or hold a reference to the Engine, I imagine that Engine.QueueNewFrame is a compromise (because we can’t guarantee that there’s a Scene to use an observable off of).

With using setTimeout though, I am seeing a couple of things. First, with a timer of 1 ms, it’s executing more often than the frame rate which could be a bit much, given that a frame may take 16-17 ms (assuming 60 FPS) and an average adult can press a button repeatedly 6-7 times a second (~147-167 ms per press). The other thing that I’m seeing is that with setTimeout there’s an increase in garbage collection, due to the frequency of functions created and executed for a timeout. Using something like setInterval outside of the function (maybe in _startMonitoringGamepads) could avoid this issue.

In any case, I do believe that there’s room to improve for the _checkGamepadsStatus function.

6

Thank you for your investigation. It seems to be very rational and I like that.

An alternative would be to use scene.onAfterRenderObservable.addOnce(() => {}). This would solve the heap issue, and would have no arbitrary timeout to setup.
What do you think about this?