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

Huge mouse-click delays #1107

Closed
madduck opened this issue Sep 21, 2016 · 11 comments
Closed

Huge mouse-click delays #1107

madduck opened this issue Sep 21, 2016 · 11 comments
Assignees
Milestone

Comments

@madduck
Copy link
Contributor

madduck commented Sep 21, 2016

Output of awesome --version:
awesome v3.5.2-2001-gb4d6bfa (The Fox)
• Compiled against Lua 5.1.5 (running with Lua 5.1)
• D-Bus support:

How to reproduce the issue:

Start it, use the mouse a bit, observe.

Actual result:

Between click of a mouse button and reaction in X client, there's an increasing delay up to several seconds.

Expected result:

Instantaneous reaction.

Here's why I think this is a problem with awesome:

In short, because if I replace awesome with fvwm2, the problems go away. If I replace fvwm with awesome again, the problem comes back.

The issue appears with evdev, as well as libinput, with the internal trackpad, as well as an external mouse. xev and evemu-record both see the event immediately. All clients experience the delay, no matter what framework, including pure xterm. Curiously, I don't know why xev (which has its own window) doesn't experience the delay, but I suppose it hooks into the driver or hardware much higher up. Neither X server nor any drivers were updated before the problem appeared, but the issues did start in the wake up replacing awesom 3.4.15 with current Git master, and they exist too with stock rc.lua.

I'll try to record a video as soon as I am back in a work environment.

@psychon
Copy link
Member

psychon commented Sep 21, 2016

Could you try running awesome with --no-argb? The theory is that not the "actual input" is slow, but updating the screen is slow. The reason for this theory is that xev doesn't have delays, but it really should have...
Also, could you check your ~/.xsession-errors (or wherever awesome's stderr goes to; ls -l /proc/$(pidof awesome)/fd/2 might help you (or not)) for messages like the following:

2016-09-21 14:31:16 W: awesome: a_glib_poll:373: Last main loop iteration took 0.159048 seconds! Increasing limit for this warning to that value.

Such messages (with times of "several seconds") would really point towards awesome.
If so, what does your rc.lua look like? Are you doing anything slow in there?

@madduck
Copy link
Contributor Author

madduck commented Sep 21, 2016

Hey,

I am already running with no-argb. And unfortunately, there is no message of the type you're looking for anywhere… :/

The rc.lua is from the Git repo, i.e. I can reproduce this from stock.

Here is the video (250Mb!): http://scratch.madduck.net/.tmp__20160921_133528.mp4

@juw
Copy link
Contributor

juw commented Sep 21, 2016

Hi,

I also see such delays. It seems like the problem was introduce somewhere between 39aace5 and 75ed165, since I do not see it with 39aace5. Sadly I do not have the time to further investigate it at the moment, so I just downgraded to 39aace5 for the time being.

Cheers.

@madduck
Copy link
Contributor Author

madduck commented Sep 21, 2016

Thanks for the pointer to 39aace5. I went ahead and bisected and found baaff93 to be the offending commit.

And in fact, reverting it on top of the current master HEAD fixes the delays. @psychon @juw

@Elv13
Copy link
Member

Elv13 commented Sep 21, 2016

Thanks for bisecting this. @psychon We could just revert the commit and move on? I am unconvinced this feature is improving performance by much, does it?

@psychon
Copy link
Member

psychon commented Sep 22, 2016

I'd like to know why it is slow. How many clients do you have open for this to become slow?
Back when I wrote that code, I measured it and all of awesome_refresh() was finished in "no time at all" except for luaA_emit_refresh().

Iterating over all clients should be fast. My guess would then be that client_ignore_enterleave_events(); is slow. Could someone test the following patch?

diff --git a/globalconf.h b/globalconf.h
index 877be22..124d157 100644
--- a/globalconf.h
+++ b/globalconf.h
@@ -180,6 +180,7 @@ typedef struct
     uint32_t preferred_icon_size;
     /** Cached wallpaper information */
     cairo_surface_t *wallpaper;
+    bool need_client_geometry_refresh;
 } awesome_t;

 extern awesome_t globalconf;
diff --git a/objects/client.c b/objects/client.c
index e561aae..91a431f 100644
--- a/objects/client.c
+++ b/objects/client.c
@@ -1138,6 +1138,9 @@ client_border_refresh(void)
 static void
 client_geometry_refresh(void)
 {
+    if (!globalconf.need_client_geometry_refresh)
+        return;
+    globalconf.need_client_geometry_refresh = false;
     client_ignore_enterleave_events();
     foreach(_c, globalconf.clients)
     {
@@ -1555,6 +1558,8 @@ client_resize_do(client_t *c, area_t geometry)
 {
     lua_State *L = globalconf_get_lua_State();

+    globalconf.need_client_geometry_refresh = true;
+
     screen_t *new_screen = c->screen;
     if(!screen_area_in_screen(new_screen, geometry))
         new_screen = screen_getbycoord(geometry.x, geometry.y);

Also, to check if the slow part really is client_ignore_enterleave_events, could someone also test the following patch? (Not ontop of the previous one or something like that; just completely on its own)

diff --git a/objects/client.c b/objects/client.c
index e561aae..85426f7 100644
--- a/objects/client.c
+++ b/objects/client.c
@@ -1003,33 +1003,11 @@ client_ban(client_t *c)
 void
 client_ignore_enterleave_events(void)
 {
-    foreach(c, globalconf.clients)
-    {
-        xcb_change_window_attributes(globalconf.connection,
-                                     (*c)->window,
-                                     XCB_CW_EVENT_MASK,
-                                     (const uint32_t []) { CLIENT_SELECT_INPUT_EVENT_MASK & ~(XCB_EVENT_MASK_ENTER_WINDOW | XCB_EVENT_MASK_LEAVE_WINDOW) });
-        xcb_change_window_attributes(globalconf.connection,
-                                     (*c)->frame_window,
-                                     XCB_CW_EVENT_MASK,
-                                     (const uint32_t []) { FRAME_SELECT_INPUT_EVENT_MASK & ~(XCB_EVENT_MASK_ENTER_WINDOW | XCB_EVENT_MASK_LEAVE_WINDOW) });
-    }
 }

 void
 client_restore_enterleave_events(void)
 {
-    foreach(c, globalconf.clients)
-    {
-        xcb_change_window_attributes(globalconf.connection,
-                                     (*c)->window,
-                                     XCB_CW_EVENT_MASK,
-                                     (const uint32_t []) { CLIENT_SELECT_INPUT_EVENT_MASK });
-        xcb_change_window_attributes(globalconf.connection,
-                                     (*c)->frame_window,
-                                     XCB_CW_EVENT_MASK,
-                                     (const uint32_t []) { FRAME_SELECT_INPUT_EVENT_MASK });
-    }
 }

 /** Record that a client got focus.

Neither of these patches are meant for merging. This is just for figuring out what the problem is.

@psychon psychon added this to the v3.6 milestone Sep 22, 2016
@madduck
Copy link
Contributor Author

madduck commented Sep 22, 2016

Hey @psychon, thanks for your continued work on this! I am very happy I can try to help by testing your patches.

Both patches (tried individually) make the problem go away when applied on top of the current master HEAD.

As per your initial question: I can have dozens of clients open without the problem, but if any of Firefox, Thunderbird or Chromium are open (even on an invisible tag), then the delays come back.

@psychon psychon self-assigned this Sep 22, 2016
psychon added a commit to psychon/awesome that referenced this issue Sep 23, 2016
This commit makes the function only call client_ignore_enterleave_events() when
it actually has to. Since we expect that most of the time, no client's geometry
is changed, this means that most of the time this function is not called.

Fixes: awesomeWM#1107
Signed-off-by: Uli Schlachter <psychon@znc.in>
@psychon
Copy link
Member

psychon commented Sep 23, 2016

Thanks for testing. I got another patch. I'd be really surprised if it didn't work, but better be safe than sorry. ;-)

diff --git a/objects/client.c b/objects/client.c
index e561aae..bdb3726 100644
--- a/objects/client.c
+++ b/objects/client.c
@@ -1138,7 +1138,7 @@ client_border_refresh(void)
 static void
 client_geometry_refresh(void)
 {
-    client_ignore_enterleave_events();
+    bool ignored_enterleave = false;
     foreach(_c, globalconf.clients)
     {
         client_t *c = *_c;
@@ -1164,6 +1164,11 @@ client_geometry_refresh(void)
                 && AREA_EQUAL(real_geometry, c->x11_client_geometry))
             continue;

+        if (!ignored_enterleave) {
+            client_ignore_enterleave_events();
+            ignored_enterleave = true;
+        }
+
         xcb_configure_window(globalconf.connection, c->frame_window,
                 XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
                 (uint32_t[]) { geometry.x, geometry.y, geometry.width, geometry.height });
@@ -1177,7 +1182,8 @@ client_geometry_refresh(void)
         /* ICCCM 4.2.3 says something else, but Java always needs this... */
         client_send_configure(c);
     }
-    client_restore_enterleave_events();
+    if (ignored_enterleave)
+        client_restore_enterleave_events();
 }

 void

Thanks a lot for testing these patches!

@madduck
Copy link
Contributor Author

madduck commented Sep 23, 2016

Yeah, that patch also seems to work fine, i.e. there are no mouse click delays after applying it on top of 204e2ff @psychon

psychon added a commit to psychon/awesome that referenced this issue Sep 25, 2016
There are some situations where we do things that can make the mouse pointer
enter another window. We do not want to react to these "self inflicted" mouse
enter and leave events, because they aren't "real" (= generated by the user).

Before this commit, this is done by going through all windows and toggling the
"please send us enter and leave events"-bit on them. This becomes slower when
many windows are visible and floods the server with requests.

This commit changes this to a constant-time logic. Each event contains the
sequence number of the last request that the X11 server handled. Thus, we just
remember the right sequence numbers and ignore any events that comes in whose
sequence number falls into the ignored range.

In detail, we keep a list of "begin" and "end" sequence numbers and ignore any
enter and leave events that fall in this range. If we get any event with a
sequence number higher than "end", we remove this pair from the list, since it
is no longer needed.

To generate these pairs, we use a GrabServer request in
client_ignore_enterleave_events(). This gives us a sequence number and makes
sure that nothing else besides us can cause events. The server is ours! In
client_restore_enterleave_events(), we first do a NoOperation request to
generate the sequence number for the end of the pair and then do UngrabServer.
Any event that is generated after UngrabServer will have at least the sequence
number of the UngrabServer request and thus no longer fails between begin and
end.

Fixes: awesomeWM#1107
Signed-off-by: Uli Schlachter <psychon@znc.in>
@psychon
Copy link
Member

psychon commented Sep 25, 2016

If you still got time and want to help me a bit, I have something else to test:
Use an old awesome version (one which still has this bug) and test if the following patch also fixes this problem: 886a32f
I just submitted that as #1117 and I really think it should help, but you never know...

@madduck
Copy link
Contributor Author

madduck commented Sep 25, 2016

I replied at #1117

petoju pushed a commit to petoju/awesome that referenced this issue Sep 27, 2016
This commit makes the function only call client_ignore_enterleave_events() when
it actually has to. Since we expect that most of the time, no client's geometry
is changed, this means that most of the time this function is not called.

Fixes: awesomeWM#1107
Signed-off-by: Uli Schlachter <psychon@znc.in>
psychon added a commit to psychon/awesome that referenced this issue Sep 30, 2016
There are some situations where we do things that can make the mouse pointer
enter another window. We do not want to react to these "self inflicted" mouse
enter and leave events, because they aren't "real" (= generated by the user).

Before this commit, this is done by going through all windows and toggling the
"please send us enter and leave events"-bit on them. This becomes slower when
many windows are visible and floods the server with requests.

This commit changes this to a constant-time logic. Each event contains the
sequence number of the last request that the X11 server handled. Thus, we just
remember the right sequence numbers and ignore any events that comes in whose
sequence number falls into the ignored range.

In detail, we keep a list of "begin" and "end" sequence numbers and ignore any
enter and leave events that fall in this range. If we get any event with a
sequence number higher than "end", we remove this pair from the list, since it
is no longer needed.

To generate these pairs, we use a GrabServer request in
client_ignore_enterleave_events(). This gives us a sequence number and makes
sure that nothing else besides us can cause events. The server is ours! In
client_restore_enterleave_events(), we first do a NoOperation request to
generate the sequence number for the end of the pair and then do UngrabServer.
Any event that is generated after UngrabServer will have at least the sequence
number of the UngrabServer request and thus no longer fails between begin and
end.

Fixes: awesomeWM#1107
Signed-off-by: Uli Schlachter <psychon@znc.in>
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

4 participants