Winit: Rapidly setting mouse cursor icon freezes system

[cheako]

I discovered that rapidly setting the mouse cursor icon prevents every window from drawing on X11 Ryzen 7 2700U/vega mobile.

This was a problem for imgui, though it wasn't hard to cache.

For example:

pub fn set_cursor_icon(&self, cursor: CursorIcon)
  Modifies the cursor icon of the window.

No mention of performance impact.

[murarth]

I suspect that this issue is not exclusive to set_cursor_icon. It's likely the result of winit sending a request to the X server and then waiting for a response. If a process does this many times, it will spend a lot of time waiting instead of performing more important work.

I'm not sure how to go about solving this. Perhaps, winit could save requested cursor information and only send it to the X server once per event loop iteration.

[goddessfreya]

If we switch to xlib-xcb, I believe winit would not have to wait for the response for responses it doesn't care about. Of course, the X protocol is still serial, but I think it would help alleviate this problem.

[cheako]

I think switching to xcb is a great idea, assuming it's not too much trouble and doesn't introduce other problems.

As for this report I don't think simply switching to xcb will solve the problem. As I tried to indicate EVERYTHING stops drawing, this includes other apps and Gnome3 WM. I know that's a system problem, but winit developers should be apprised to failure modes they might encounter.

[Osspial]

@cheako Could you post an example that reproduces this? I can imagine a couple different ways of rapidly setting the cursor icon, and it's unclear what exact method you're using to do it.

[cheako]

On every WindowEvent::RedrawRequested. It was the most practical when considering ImGui.

Edit: Sorry forgot to specify why RedrawRequested was being emitted all the time. That's because I'm calling window.request_redraw() on every EventsCleared. Here is an example that can be tested against, I think it's minimal given Vulkan and ImGui.

https://gitlab.com/cheako/simulide-rs/tree/f0358897ca76e7ba8a0d58f0e4e107f87be4dce6
https://gitlab.com/cheako/simulide-rs/blob/f0358897ca76e7ba8a0d58f0e4e107f87be4dce6/src/main.rs#L667 <--- This is where we only set to cursor conditionally.

[psychon]

I tried reproducing this via

diff --git a/examples/cursor.rs b/examples/cursor.rs
index d5bd34a..ae05260 100644
--- a/examples/cursor.rs
+++ b/examples/cursor.rs
@@ -1,8 +1,9 @@
 use winit::{
-    event::{ElementState, Event, KeyboardInput, WindowEvent},
+    event::{ElementState, Event, KeyboardInput, WindowEvent, StartCause},
     event_loop::{ControlFlow, EventLoop},
     window::{CursorIcon, WindowBuilder},
 };
+use std::time::{Duration, Instant};

 fn main() {
     let event_loop = EventLoop::new();
@@ -12,7 +13,16 @@ fn main() {

     let mut cursor_idx = 0;

+    let timer_length = Duration::new(0, 1);
+
     event_loop.run(move |event, _, control_flow| match event {
+        Event::NewEvents(StartCause::Init) => {
+            *control_flow = ControlFlow::WaitUntil(Instant::now() + timer_length)
+        }
+        Event::NewEvents(StartCause::ResumeTimeReached { .. }) => {
+            *control_flow = ControlFlow::WaitUntil(Instant::now() + timer_length);
+            window.set_cursor_icon(CURSORS[cursor_idx]);
+        }
         Event::WindowEvent {
             event:
                 WindowEvent::KeyboardInput {

The result is Xorg taking about 80% CPU and cursor 65%.

Taking a look at this with xtrace/x11trace shows lots of repeats of the following:

3.515 000:<:4bf5f: 16: Request(53): CreatePixmap depth=0x20 pid=0x024194fa drawable=0x0000014d width=24 height=24
3.515 000:<:4bf60: 16: Request(55): CreateGC cid=0x024194fb drawable=0x024194fa values={}
3.515 000:<:4bf61:2328: Request(72): PutImage format=ZPixmap(0x02) drawable=0x024194fa gc=0x024194fb width=24 height=24 dst-x=0 dst-y=0 left-pad=0x00 depth=0x20
3.515 000:<:4bf62:  8: Request(60): FreeGC gc=0x024194fb
3.515 000:<:4bf63: 20: RENDER-Request(139,4): CreatePicture pid=0x024194fc drawable=0x024194fa format=0x00000026 values={}
3.515 000:<:4bf64:  8: Request(54): FreePixmap drawable=0x024194fa
3.515 000:<:4bf65: 16: RENDER-Request(139,27): CreateCursor cid=0x024194fd src=0x024194fc x=4 y=4
3.515 000:<:4bf66:  8: RENDER-Request(139,7): FreePicture picture=0x024194fc
3.515 000:<:4bf67: 20: XFIXES-Request(138,23): SetCursorName cursor=0x024194fd name='left_ptr'
3.515 000:<:4bf68: 20: XFIXES-Request(138,23): SetCursorName cursor=0x024194fd name='left_ptr'
3.515 000:<:4bf69: 16: Request(2): ChangeWindowAttributes window=0x02400001 value-list={cursor=0x024194fd}
3.515 000:<:4bf6a:  8: Request(95): FreeCursor cursor=0x024194fd

So, I guess cursors could be cached. The GCs that are used could be cached. The duplicate SetCursorName could be omitted. But none of this will make this super fast. How about caching the current cursor and turning re-setting the current cursor into a no-op?

It's likely the result of winit sending a request to the X server and then waiting for a response

Nope, none of these requests generate a reply. All of this code is asynchronous, even with Xlib.

---

Taking a look at this with strace shows that the left_ptr cursor is loaded repeatedly from disk. This could be cached as well. The repeating chunk here is:

epoll_wait(4, [], 32, 0)                = 0
openat(AT_FDCWD, "/home/psychon/.icons/default/cursors/left_ptr", O_RDONLY) = -1 ENOENT (Datei oder Verzeichnis nicht gefunden)
openat(AT_FDCWD, "/home/psychon/.icons/default/index.theme", O_RDONLY) = -1 ENOENT (Datei oder Verzeichnis nicht gefunden)
openat(AT_FDCWD, "/usr/share/icons/default/cursors/left_ptr", O_RDONLY) = -1 ENOENT (Datei oder Verzeichnis nicht gefunden)
openat(AT_FDCWD, "/usr/share/icons/default/index.theme", O_RDONLY) = 7
fstat(7, {st_mode=S_IFREG|0644, st_size=30, ...}) = 0
read(7, "[Icon Theme]\nInherits=Adwaita\n", 4096) = 30
close(7)                                = 0
openat(AT_FDCWD, "/usr/share/pixmaps/default/cursors/left_ptr", O_RDONLY) = -1 ENOENT (Datei oder Verzeichnis nicht gefunden)
openat(AT_FDCWD, "/home/psychon/.icons/Adwaita/cursors/left_ptr", O_RDONLY) = -1 ENOENT (Datei oder Verzeichnis nicht gefunden)
openat(AT_FDCWD, "/home/psychon/.icons/Adwaita/index.theme", O_RDONLY) = -1 ENOENT (Datei oder Verzeichnis nicht gefunden)
openat(AT_FDCWD, "/usr/share/icons/Adwaita/cursors/left_ptr", O_RDONLY) = 7
fstat(7, {st_mode=S_IFREG|0644, st_size=69120, ...}) = 0
read(7, "Xcur\20\0\0\0\0\0\1\0\5\0\0\0\2\0\375\377\30\0\0\0L\0\0\0\2\0\375\377"..., 4096) = 4096
lseek(7, 0, SEEK_SET)                   = 0
read(7, "Xcur\20\0\0\0\0\0\1\0\5\0\0\0\2\0\375\377\30\0\0\0L\0\0\0\2\0\375\377"..., 4096) = 4096
close(7)                                = 0
poll([{fd=3, events=POLLIN|POLLOUT}], 1, -1) = 1 ([{fd=3, revents=POLLOUT}])
writev(3, [{iov_base="5 \4\0vI@\2M\1\0\0\30\0\30\0007\0\4\0wI@\2vI@\2\0\0\0\0"..., iov_len=2484}, {iov_base=NULL, iov_len=0}, {iov_base="", iov_len=0}], 3) = 2484
recvmsg(3, {msg_namelen=0}, 0)          = -1 EAGAIN (Die Ressource ist zur Zeit nicht verfügbar)
recvmsg(3, {msg_namelen=0}, 0)          = -1 EAGAIN (Die Ressource ist zur Zeit nicht verfügbar)
recvmsg(3, {msg_namelen=0}, 0)          = -1 EAGAIN (Die Ressource ist zur Zeit nicht verfügbar)
recvmsg(3, {msg_namelen=0}, 0)          = -1 EAGAIN (Die Ressource ist zur Zeit nicht verfügbar)
recvmsg(3, {msg_namelen=0}, 0)          = -1 EAGAIN (Die Ressource ist zur Zeit nicht verfügbar)
recvmsg(3, {msg_namelen=0}, 0)          = -1 EAGAIN (Die Ressource ist zur Zeit nicht verfügbar)
epoll_wait(4, [], 32, 0)                = 0

[murarth]

So, I guess cursors could be cached. The GCs that are used could be cached. The duplicate SetCursorName could be omitted. But none of this will make this super fast. How about caching the current cursor and turning re-setting the current cursor into a no-op?

Making setting a CursorIcon that is the same as the current icon into a no-op is sufficient to solve the performance issue in the above example. However, I believe it would be fairly easy and straightforward to also cache and reuse Xlib cursor values. This would improve performance for applications that frequently change cursor icons, although perhaps by a negligible amount.

@Osspial @ZeGentzy, what do you think? I can submit a PR with the simple fix or one with added cursor caching.