rust-windowing / winit

Window handling library in pure Rust
https://docs.rs/winit/
Apache License 2.0
4.7k stars 887 forks source link

High CPU load when moving window between monitors #706

Closed NickeZ closed 5 years ago

NickeZ commented 5 years ago

Initially reported here: https://github.com/jwilm/alacritty/issues/1790

Which operating system does the issue occur on? Ubuntu 18.04.1 Linux (kernel version 4.15)

If on linux, are you using X11 or Wayland?

Using the window example I can cause excessive cpu load. Should the example application window.rs be an empty see-through window?

Is there anything I can do to debug it when it happens?

high_load

francesca64 commented 5 years ago

Thanks for reporting!

Should the example application window.rs be an empty see-through window?

If nothing is rendered to it, yes. Though, in my experience, rather than being see-through it usually merely has the same appearance of whatever was behind it when it was opened.

Is there anything I can do to debug it when it happens?

Does anything look odd about the frequency/latency of the events being printed out? My best guess is that something's going wrong when we try to listen for new events. If not, I'll look into more rigorous ways to investigate this.

chrisduerr commented 5 years ago

I've just checked this to make sure Alacritty's not doing anything wrong and I've come to the conclusion that this must be something with winit.

Note that the run_forever closure isn't actually executed continuously, which could trigger a problem (like an infinite loop without any content). In fact in my testing it was completely silent after moving the example to a different screen, however the CPU was still running hot.

NickeZ commented 5 years ago

I updated the topic because the reproducible way to get to this state is by moving the window between monitors. I totally forgot that I moved the example window to a second screen...

Does anything look odd about the frequency/latency of the events being printed out? My best guess is that something's going wrong when we try to listen for new events. If not, I'll look into more rigorous ways to investigate this.

It doesn't print anything special when CPU usage goes up. Just the regular keyboard/mouse events.

NickeZ commented 5 years ago

I ran it with strace. I get continuous, very high frequency, output of these:

poll([{fd=3, events=POLLIN}], 1, -1)    = 1 ([{fd=3, revents=POLLIN}])
recvmsg(3, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="\226\0\313\235\2\0\300\2\2\0\300\2\0\0\0\0B\v\2\0\274\3\221\4\0\0\0\0\0\0\0\0"..., iov_len=4096}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 64
recvmsg(3, {msg_namelen=0}, 0)          = -1 EAGAIN (Resource temporarily unavailable)
poll([{fd=3, events=POLLIN|POLLOUT}], 1, -1) = 1 ([{fd=3, revents=POLLOUT}])
writev(3, [{iov_base="\f(\5\0\2\0\300\2\f\0\0\0>\2\0\0\275\2\0\0\f\0\5\0\2\0\300\2\f\0\0\0"..., iov_len=40}, {iov_base=NULL, iov_len=0}, {iov_base="", iov_len=0}], 3) = 40
poll([{fd=3, events=POLLIN}], 1, -1)    = 1 ([{fd=3, revents=POLLIN}])
recvmsg(3, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="\226\0\315\235\2\0\300\2\2\0\300\2\0\0\0\0B\v\2\0\274\3\221\4\0\0\0\0\0\0\0\0"..., iov_len=4096}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 64
recvmsg(3, {msg_namelen=0}, 0)          = -1 EAGAIN (Resource temporarily unavailable)
poll([{fd=3, events=POLLIN|POLLOUT}], 1, -1) = 1 ([{fd=3, revents=POLLOUT}])
writev(3, [{iov_base="\f(\5\0\2\0\300\2\f\0\0\0>\2\0\0\275\2\0\0\f\0\5\0\2\0\300\2\f\0\0\0"..., iov_len=40}, {iov_base=NULL, iov_len=0}, {iov_base="", iov_len=0}], 3) = 40
poll([{fd=3, events=POLLIN}], 1, -1)    = 1 ([{fd=3, revents=POLLIN}])
recvmsg(3, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="\226\0\317\235\2\0\300\2\2\0\300\2\0\0\0\0B\v\2\0\274\3\221\4\0\0\0\0\0\0\0\0"..., iov_len=4096}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 64
recvmsg(3, {msg_namelen=0}, 0)          = -1 EAGAIN (Resource temporarily unavailable)
^C--- SIGINT {si_signo=SIGINT, si_code=SI_KERNEL} ---
Jasu commented 5 years ago

I bisected this down to this:

1b74822c - (refs/bisect/bad) DPI for everyone (#548) (6 months ago) <Francesca Frangipane> U

The commit added code to keep the DPI-adjusted window size when moving to another monitor. The code attempts to resize the window until the window size matches the desired size (i.e. it will keep firing an endless stream of Resize commands). The resize will invariably fail on i3, since the window size is constrained by the layout.

Also, for some reason it attempted to make the window impossibly huge (6400x3263) when moving from a standard DPI 1600x1200 monitor to a 4K-monitor. I'm not sure if this would fail on non-tiling WMs, or if it would just make the window extend past the monitor edges.

This is the code behind the issue: https://github.com/tomaka/winit/blob/master/src/platform/linux/x11/mod.rs#L501

Commenting out the lines from 501 to 518 fixed the issue for me. Since I'm not sure what the hack is trying to achieve, I don't know how to fix this - perhaps someone familiar with the purpose of the hack could take a look? Or perhaps simply limiting the number of retries could do the trick?

NickeZ commented 5 years ago

It seems @vberger wrote that code for PR https://github.com/tomaka/winit/pull/668

elinorbgr commented 5 years ago

Note that I only modified the pre-existing hack, by changing the order in which it processes events. I'll admit it was kind of a "I don't know what I'm doing" moment though, given my x11 knowledge is pretty low. The code was added in #548, as bisected by @Jasu.

The main purpose of this code is related to DPI handling. Winit has chosen to follow an approach where the logical size of a window remains constant across DPI changes, and thus its physical size adapts. This code implements this behavior. I am however not qualified to evaluate if it does it correctly, or if it could be done in a better way.

Jasu commented 5 years ago

It looks like Xfwm stores the size of the window before move, and restores it in some cases (e.g. when dragging a maximized window), and that's what the hack is for. I tried to looking at ICCCM hints etc., but couldn't find anything that could be used to figure out if the window size is being constrained by a layout or monitor size or something.

I'm pretty much out of ideas, except for checking the window manager name from _NET_WM_NAME and only applying the hack on Xfwm.

francesca64 commented 5 years ago

@Jasu thanks for getting to the bottom of this! It's a huge help.

except for checking the window manager name from _NET_WM_NAME and only applying the hack on Xfwm

I'd be totally fine with doing that; we've already had to resort to it in other places: https://github.com/tomaka/winit/blob/cb0a085968252dc4f0178a40fb5ba88d8530bb31/src/platform/linux/x11/window.rs#L911

Would you like to make a PR for this?

Jasu commented 5 years ago

@francesca64 No problem, I'll try to make a PR today.

Jasu commented 5 years ago

There's a PR now, #737

francesca64 commented 5 years ago

Fixed via #737