SDL_WaitEvent causes high cpu usage

[SDLBugzilla]

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• franko's patch as of 13 jan 2021 (sdl2-blocking-franko.patch, text/plain, 2021-01-13 20:08:02 +0000, 36047 bytes)
• Patch for blocking wait call version 2 (0001-Implement-blocking-WaitEvent-call.patch, text/plain, 2021-01-15 16:43:23 +0000, 26741 bytes)
• Patch for blocking wait call version 3 (0001-Implement-blocking-WaitEvent-call.patch, text/plain, 2021-01-17 15:09:39 +0000, 27951 bytes)
• Additional patch to fix build without threads enabled (0001-Fix-error-with-wake-up-mutex-when-threads-are-disabl.patch, text/plain, 2021-01-18 10:27:49 +0000, 2970 bytes)
• Patch for blocking wait call version 4 (0001-Implement-blocking-WaitEvent-call.patch, text/plain, 2021-01-23 18:54:25 +0000, 28342 bytes)
• Patch for blocking wait call version 5 (0001-Implement-blocking-WaitEvent-call.patch, text/plain, 2021-01-26 17:32:38 +0000, 27794 bytes)

Reported in version: 2.0.14
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2021-01-09 18:06:42 +0000, wrote:

while SDL2 v 2.0.9 used SDL_Delay(10) in the polling loop in SDL_events.c,
which caused a CPU usage of about 0.5%, in 2.0.14 the delay has been changed to 1 ms, which causes an idle cpu usage of any SDL application of about 5% on my 800Mhz system. this is inacceptable if SDL isn't used for gaming but for a platform-independent way to produce GUI applications as i do.

i used 2.0.9 until today and even the 0.5% permanent cpu usage appeared high enough to warrant a bug report, but i wanted to first test whether latest release fixes the issue. unfortunately it got worse rather than better.

what SDL_WaitEvent() really should do is run system's poll() on the set of fds it uses (input, X11 window).
alternatively, a new API could be introduced for this purpose (maybe SDL_WaitEventBlock).

On 2021-01-09 19:10:51 +0000, Cameron Gutman wrote:

As a workaround, you can always sleep yourself (since that's what SDL_WaitEvent() is doing internally) for as long as you want, like:

for (;;) {
if (!SDL_PollEvent(&event)) {
SDL_Delay(15);
}

// Do important stuff...

}

That's obviously not a fix, but it may allow you to use SDL until this issue is fixed.

BTW, there are other SDL defaults that aren't too friendly for normal apps, like SDL_HINT_TIMER_RESOLUTION=1 and SDL_HINT_VIDEO_ALLOW_SCREENSAVER=0 that you'll want to override too for power and UX reasons.

On 2021-01-09 23:22:18 +0000, wrote:

thanks cameron for these helpful tips.

i also want to add that bug 323 https://bugzilla.libsdl.org/show_bug.cgi?id=323 has a patch of ryan c gordon implementing the desired behaviour for SDL 1.2, however it was then later closed as WONTFIX with the reasoning that SDL 1.2 is pretty much EOL and a big change like this would be inappropriate.

these reasons do however (fortunately) not match the situation for SDL2, so i hope this can be implemented/upstreamed for good this time - having a patch for SDL1.2 certainly simplifies the development. it would be a pity if SDL2 couldn't be used for serious cross-platform GUI apps due to this battery-leeching bug.

On 2021-01-10 18:11:27 +0000, wrote:

thinking about it some more, there doesn't need to be a separate name for this function with permanent blocking added. any FPS-sensitive app will already call SDL_PollEvent rather than SDL_WaitEvent so they can control the frame rate.
those that call SDL_WaitEvent already use the kind of application where the screen doesn't need to be redrawn until the next event happens, in which case the previous polling loop with 10 ms delay would cause notable input lag (that's probably the reason why it was changed to 1ms).
so changing the WaitEvent func to send the application to permanent sleep until the next event happens is a pure win-win situation.

btw, i took the liberty to update the patch in bug 323 to latest SDL 1.2.15 release, so it should be even easier to port it to SDL2.
i also tested it with my apps and it works perfectly.

On 2021-01-11 22:26:38 +0000, Sam Lantinga wrote:

Unfortunately, there are other sources of events that do not come in through the X11 fd. Any real implementation would have to take all of these into account (joysticks, touchpads, application events sent via SDL_PushEvent(), etc.)

You're welcome to take the patch for SDL 1.2 and adapt it for your use case, but I'm not sure how all of the other event sources could be handled in a cross-platform manner.

Anyone have any ideas?

On 2021-01-13 15:04:41 +0000, Francesco Abbate wrote:

I am working on a desktop application using SDL2 and I had the same problem: high CPU usage when idle waiting for SDL_NextEvent() to return an event.

I have provided a solution I think in the following github branch:

https://github.com/franko/SDL/tree/blocking-wait-event

At your convenience you may inspect the changes introduced using the following URL:

SDL-mirror/SDL@master...franko:blocking-wait-event

It applies to 2.0.12 but I should be able to easily adapt it to 2.0.14, may be no adaptations are needed, just rebasing.

The Windows and xwindow implementation appear to work but for the moment I haven't done extensive testing. A Mac OS X implementation in given but it may not work or not even compile.

I have added an additional test "checkkeysthreads.c" to test the scenario when a thread is sending events using SDL_PushEvent. The test works on Windows and on Linux using X Window.

The solution I propose solves the problem by using a native blocking call to wait for the next event. At he same time I provide a mechanism to wakeup the main SDL thread when events are added from another thread using SDL_PushEvent.

I haven't give an implementation for other platforms but I have coded a condition to fall back on previous behavior if the required video device methods ar not defined.

Please review the proposition, all the feedback is welcome. The modification is not meant to be complete yet but I submit it to have some feedback before pursuing the work.

On 2021-01-13 20:08:02 +0000, wrote:

Created attachment 4662
franko's patch as of 13 jan 2021

i'm taking the liberty to attach franko's work as a patch so it doesn't lost/cluttered by pushes to SDL.

i was unable to check it out yet but will report back when i did.

On 2021-01-13 20:14:20 +0000, wrote:

Unfortunately, there are other sources of events that do not come in through the X11 fd. Any real implementation would have to take all of these into account (joysticks, touchpads, application events sent via SDL_PushEvent(), etc.)

yes, it's clear there's more than one fd involved here. afaik it should be possible to get fd's for any input devices, at least on linux.

as for SDL_PushEvent() it's really just an implementation detail. i don't know how it's currently done but it could be achieved via a pipe so the WaitEvent code can just add the pipe fd to the list used for poll() (or select() - but nowadays poll() should be prefered).

platforms that don't support devices as fds could be implemented as a polling loop that writes to a pipe too, but for starters they could just keep the old behaviour.

On 2021-01-13 22:39:40 +0000, Francesco Abbate wrote:

I have made more tests and I found that my patch as it is today is failing when running testjoystick. I need to refine the logic about the sending of wake-up event.

I need some more work to fix the issue and I will present a new patch.

On 2021-01-15 16:43:23 +0000, Francesco Abbate wrote:

Created attachment 4664
Patch for blocking wait call version 2

I have solved a few problems with the previous patch. Please find here the new one.

It is tested on Windows and on Linux using xwindow but not on OS X.

On 2021-01-17 15:09:39 +0000, Francesco Abbate wrote:

Created attachment 4676
Patch for blocking wait call version 3

I have improved the previous patch to be more accurate about the needs of sending the wake-up event.

Now the wake-up event is sent only if the SDL_PushEvent call comes from a different thread and if the main thread is actually blocking with a designated
window to send the event.

In this way only one wake-up event is sent to only one window only when needed. It is still possible that a wake-up event is sent when the blocking call to WaitNextEvent just terminated but the spurious wake-up will makes no harm as it will be ignored and not translate into an SDL_Event.

The new version of the patch was required to work well with the xwindow system. Without the corrections some errors happened on xwindow with multi-threaded applications.

On 2021-01-18 10:27:49 +0000, Francesco Abbate wrote:

Created attachment 4677
Additional patch to fix build without threads enabled

By testing more extensively the patch I noticed a problem if the SDL library was compiled without threads.

This new patch amend the previous one so that it works correctly even when the threads are disabled.

On 2021-01-21 17:24:49 +0000, wrote:

francesco, thanks for your effort! i looked over your latest patch revision and it seems to be really skillfully made.

there's one thing i noticed though:

+#define WM_SDL_WAKEUP (WM_USER + 201)

shouldn't this constant be defined outside of WM_USER namespace? it might otherwise collide with a user-set constant.

also may i ask that you attach cumulative patches in the future? thanks!

btw, it's great that you are able to develop and test your patch for windows and OSX too.

sam: would you mind taking a look at francesco's last patch version too? thanks in advance!

On 2021-01-21 18:28:09 +0000, Francesco Abbate wrote:

Thanks for taking the time to look at my patch, I appreciate.

It took me a lot of time to write the code and test it in different configurations on windows and on linux using xwindow.

shouldn't this constant be defined outside of WM_USER namespace? it might otherwise collide with a user-set constant.

I think you are right. At the time I wrote the patch I didn't bother about this problem and I left it to be fixed later. I think we should use the function RegisterWindowMessage:

https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-registerwindowmessagea

to avoid possible collisions.

also may i ask that you attach cumulative patches in the future? thanks!

okay, I will send only complete patch in the future.

I hope Sam will take the time to review the patch. In the meantime thank you for your encouraging words.

On 2021-01-23 18:54:25 +0000, Francesco Abbate wrote:

Created attachment 4694
Patch for blocking wait call version 4

Please find here a new version of the patch, it replaces the previous files.

The new version fix a logical error. In the previous version the blocking call to WaitNexTEvent was done only if at least one shown window was found. In reality we don't need that the window is shown because a window can accept events even if hidden or minimized.

The new version accept any window as long as the flag "is_destroying" is not set.

The most conspicuous problem with the previous version was that, when the window was minimized, SDL was falling back to polling events and the gain in CPU usage was lost.

In addition the new patch use RegisterWindowMessageA to register the wake-up window message number to avoid using user-reserved codes.

On 2021-01-25 13:48:18 +0000, RNMSS wrote:

I'm using v2.0.14 and I have the same issue.

Did the following tests.

Note:
some_event_handler is a trivial function which only checks the SDL_QUIT event.
The main window was put to background after created, not focused.
Mouse not moving over it.
Keyboard not pressed.
Running on macOS 10.15.7, MacBook Air Retina, 13-inch, 2020.

Case 1:

while (!window_closed) {
    SDL_Event event;

    if (SDL_WaitEvent(&event)) {
        some_event_handler(event);

        while (SDL_PollEvent(&event)) {
            some_event_handler(event);
        }
    }
}

CPU usage was 5% ~ 10%.

Case 2:

while (!window_closed) {
    SDL_Event event;

    if (SDL_WaitEvent(&event)) {
        some_event_handler(event);
    }
}

CPU usage was still 5% ~ 10%.

Case 3:

while (!window_closed) {
    SDL_Event event;
    if (SDL_WaitEvent(&event)) {
        some_event_handler(event);

        SDL_Delay(500); // <---------- added line
    }
}

CPU usage was still 5% ~ 10%.

Case 4:

while (!window_closed) {
    SDL_Event event;
    if (SDL_PollEvent(&event)) {
        some_event_handler(event);
        SDL_Delay(10);
    }
}

CPU usage dropped to 1%.

On 2021-01-25 14:18:46 +0000, Francesco Abbate wrote:

I'm using v2.0.14 and I have the same issue.

I may port my patch from 2.0.12 to 2.0.14 if you are interested in testing. The code for Mac OS X is there but it isn't tested. I made extensive tests but only on Windows and on Linux.

My patch reduce the CPU usage to zero when SDL_WaitEvent() is used.

On 2021-01-26 04:18:20 +0000, Sam Lantinga wrote:

Thanks for all your effort on this!

Your patch doesn't apply to the code in Mercurial, or the latest SDL snapshot:
http://www.libsdl.org/tmp/SDL-2.0.zip

Also, you need to initialize need_polling to SDL_FALSE

On 2021-01-26 17:32:38 +0000, Francesco Abbate wrote:

Created attachment 4707
Patch for blocking wait call version 5

Thank you for taking the time to look at my patch, I appreciate.

Please find here a new version that should apply cleanly to the master mercurial branch. The code to initialize the X11 video system changed a little bit so I made some adjustments.

I tested on Windows and on XWindow and everything seems fine.

I only had a problem compiling on Windows because you messed up the SDL_COMPOSE_ERROR macro in the renderer code for D3D11 and it was no longer compiling on Windows :-) Of course this was unrelated to my patch and I was able to compile by reverting the commit:

SDL-mirror/SDL@3671866

So this is unrelated but I report it here for your information. I was compiling using msys2 with mingw-w64 and gcc.

Also, you need to initialize need_polling to SDL_FALSE

Of course, I forget about that, I was fooled by the conditional #ifdefs. Now it is fixed in the new patch.

I hope you will be able to test now.

On 2021-02-04 17:04:46 +0000, wrote:

francesco, i just tried your patch, here's my feedback:

1. applies cleanly to 2.0.14 release :)
2. works splendidly with my own test app that calls SDL_WaitEvent - 0% cpu usage while inactive, yet anything seems to work correctly! great job, thanks!
3. interestingly, it doesn't have an effect on qemu (2.8.1.1) which i recompiled specifically for this purpose against SDL2. idle cpu usage with a guest that doesn't do anything is about 6%. the same qemu version, compiled against SDL 1.2.15 with the patch from bug 323, and the same guest OS, uses less than 1% while idle. so it seems that 1) it's not calling SDL_WaitEvent but SDL_PollEvent and 2) either the patch proposed in 323 had some additional improvements for SDL_PollEvent to enter a polling loop less often, the PollEvent code is otherwise much more effective in SDL1, or the code path checking for events in qemu is different when compiled for SDL1 vs SDL2.

On 2021-02-04 18:32:55 +0000, Francesco Abbate wrote:

2. works splendidly with my own test app that calls SDL_WaitEvent - 0% cpu
   usage while inactive, yet anything seems to work correctly! great job,
   thanks!

Thank you for testing the patch, it is great for me to have some feedback. I am glad that it works for you.

3. interestingly, it doesn't have an effect on qemu (2.8.1.1) which i
   recompiled specifically for this purpose against SDL2. idle cpu usage with a
   guest that doesn't do anything is about 6%. the same qemu version, compiled
   against SDL 1.2.15 with the patch from bug 323, and the same guest OS, uses
   less than 1% while idle. so it seems that 1) it's not calling SDL_WaitEvent
   but SDL_PollEvent and 2) either the patch proposed in 323 had some
   additional improvements for SDL_PollEvent to enter a polling loop less
   often, the PollEvent code is otherwise much more effective in SDL1, or the
   code path checking for events in qemu is different when compiled for SDL1 vs
   SDL2.

My patch only address the applications calling SDL_WaitEvent. I think that if application use SDL_PollEvent there is nothing we can do because the function quickly returns and it is only the application who dictate the CPU usage.

Yet there is an important think that I want to improve in addition to the current patch. I want to use the same approach of this patch for the SDL_WaitEventTimeout function. Normally the OSes allows to wait for events with a timeout so it can be done on most platforms. It was suggested on Discourse. The advantage of this new patch is that it will benefit also many games that run at a fixed framerate using SDL_WaitEventTimeout.

I didn't implement it for the moment because I would like to have the current patch reviewed and applied upstream before doing more development.

On 2021-02-04 20:08:08 +0000, Sam Lantinga wrote:

Go ahead and implement SDL_WaitEventTimeout(). I'm very interested in how this is progressing.

Thanks!

[rofl0r]

@franko : our issue was moved here, might make it easier to provide a PR for the work you did.

@slouken : i think @franko made it quite clear that he already invested a lot of work and would like at least a review before continuing with the SDL_WaitEventTimeout() code when there's a good chance it will ultimately be rejected.

[franko]

Thank you for you words @rofl0r.

Actually I will probably implement the SDL_WaitEventTimeout() code despite the fact that my patch was not reviewed and also that there is a significant chance to be rejected anyway.

I know it can be rejected anyway because it is mostly irrelevant for video games and video games are the main use case for SDL.

So, when I have time I may implement the SDL_WaitEventTimeout() code. Ultimately, if it is still rejected I will publish it on github as a patch for SDL2 for desktop applications and I will suggest to use it to the package maintainers of my projects.

[slouken]

I did review your patch, I'm just waiting for you to finish the SDL_WaitEventTimeout() implementation for final review. Has it been tested on macOS yet?

[franko]

May be it was not clear but just let me clarify: the current version of the
patch is finished and self-contained event without the new implementation for
SDL_WaitEventTimeout().
It just modifies the behavior of the SDL_WaitEvent() function.

Otherwise, as I said, unfortunately I cannot test on OS X myself and I didn't
get any help from no one.

I will try to implement the new SDL_WaitEventTimeout() variant I just didn't have enough spar time lately and I was hoping to have more feedback and support on the current version of the patch.

[Qix-]

Apologies if this is the wrong place - the migration made things kind of tricky to navigate (though welcome to Github nonetheless 🙂)

The current documentation states "SDL_WaitEventTimeout() also returns 0 if it timed out". This means that in order to determine if it timed out or actually errored, I'd have to do something nasty like SDL_ClearError() and then SDL_GetError()[0] == 0 later on to see if it timed out.

Could we possibly change it so that it returns -1 upon timing out? Otherwise the usefulness of this function is greatly reduced.

[franko]

The PR with the new implementation is there! I have given an implementation of the same approach by including now also a timeout.

[atfzl]

It would be great to fix this CPU usage issue on mac, I noticed other applications like DOSBox which use SDL are also affected by this.

This is the idle CPU usage of DOSBox on my macbook with 2.9 GHz 6-Core Intel Core i9 processor:

[franko]

I made a fork of the SDL repository with the fix to reduce CPU uage to zero on SDL_WaitEvent and WaitEventTimeout.

The repository is:

https://github.com/franko/sdl-4d

The tag https://github.com/franko/SDL-4d/releases/tag/release-2.0.14-s4d-2 corresponds to the SDL2 release 2.0.14 including the fix.

I made a PR but it was never accepted neither refused:

#4180

I closed the PR by accident because I deleted the branch but I may re-submit again the patch.

[rofl0r]

I closed the PR by accident because I deleted the branch but I may re-submit again the patch.

that could be helpful as most people don't look at closed issues/PRs

[franko]

I posted a new PR to replace this one, closed by error:

#4419

[rofl0r]

great, as #4419 was finally merged this can be closed 🎉