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

[PATCH] SDL_WaitEventTimeout() delays much longer than timeout #3050

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 0 comments
Closed

[PATCH] SDL_WaitEventTimeout() delays much longer than timeout #3050

SDLBugzilla opened this issue Feb 11, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: 2.0.9
Reported for operating system, platform: All, All

Comments on the original bug report:

On 2018-11-03 23:00:14 +0000, Cameron Gutman wrote:

Created attachment 3443
Reduce delay in SDL_WaitEventTimeout

SDL_WaitEventTimeout() currently has a hardcoded SDL_Delay(10) that runs if the timeout hasn't yet expired. This is problematic because the caller of SDL_WaitEventTimeout() may want a wait significantly shorter than that. If the caller asks for a 1 ms timeout and no events are ready, the function will return in 10 ms.

In addition to the direct effect on SDL_WaitEventTimeout(), SDL_WaitEvent() uses SDL_WaitEventTimeout() with a timeout of -1. This means any application that doesn't want to poll (or has nothing useful to do without events available) can't use SDL_WaitEvent() unless they are willing to deal with the hidden 10 ms sleep inside it. This sleep causes dropped frames for apps with 6+ ms rendering times and caps the effective event loop polling rate using SDL_WaitEvent() or SDL_WaitEventTimeout() at about 100 Hz. To avoid these issues, these applications have to roll their own SDL_WaitEvent() loop with SDL_PollEvent() and SDL_Delay(1).

The attached patch fixes both issues by changing the SDL_Delay from 10 ms to 1 ms. The CPU usage effects aren't noticeable in my testing.

Alternative fix:
If you feel strongly that SDL_WaitEvent() should wait 10 ms between iterations, we can change SDL_WaitEventTimeout() to pass the remaining timeout to SDL_Delay() instead of always passing 10.

On 2018-11-03 23:03:23 +0000, Cameron Gutman wrote:

I forgot to mention: if you go with the alternative fix, please update the documentation for SDL_WaitEvent() to clearly indicate this potential performance issue. It took some non-trivial debugging/profiling for me to figure out where the mysterious 100 FPS rendering and 100 Hz input polling cap was coming from.

On 2019-07-30 17:49:36 +0000, Ryan C. Gordon wrote:

(Sorry if you get several emails like this, we're marking a bunch of bugs.)

We're hoping to ship SDL 2.0.11 on a much shorter timeframe than we have historically done releases, so I'm starting to tag bugs we hope to have closed in this release cycle.

Note that this tag means we just intend to scrutinize this bug for the 2.0.11 release: we may fix it, reject it, or even push it back to a later release for now, but this helps give us both a goal and a wishlist for the next release.

If this bug has been quiet for a few months and you have new information (such as, "this is definitely still broken" or "this got fixed at some point"), please feel free to retest and/or add more notes to the bug.

--ryan.

On 2019-09-20 20:47:39 +0000, Ryan C. Gordon wrote:

We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.

On 2019-09-20 20:48:42 +0000, Ryan C. Gordon wrote:

We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.

On 2019-10-15 05:15:13 +0000, Ryan C. Gordon wrote:

I'm putting in this change from a 10ms delay to 1ms, because it couldn't hurt (unless a platform treats a 1ms delay as a busy-loop, but we'll let them file separate bugs), but this function some day needs to stop looping and just do some gigantic equivalent of WaitForMultipleObjects()/select(), and let the OS put it to sleep until exactly when there's something new to report.

The delay was reduced in https://hg.libsdl.org/SDL/rev/a429ccdea379

--ryan.

On 2019-10-15 05:16:17 +0000, Ryan C. Gordon wrote:

(Also, my apologies, I just changed the value without using your actual patch, which had a much more detailed commit message, not to mention personal credit to you. Sorry about that!)

--ryan.

On 2019-10-15 05:42:07 +0000, Sam Lantinga wrote:

Better comment and attribution committed!
https://hg.libsdl.org/SDL/rev/830cadcdc1a2

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

No branches or pull requests

1 participant