Subject: FVWM CVS: fvwm dane

From: CVS <[email-masked]>
Date: Fri, 23 Apr 1999 16:54:05 -0500 (CDT)

CVSROOT:	/home/cvs/fvwm
Module name:	fvwm
Changes by:	dane	99/04/23 16:54:05

Modified files:
	.              : ChangeLog 
	libs           : Bindings.c 

Log message:
dont add double bindings to binding table

--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dominik Vogt <[email-masked]>
Date: Sun, 25 Apr 1999 21:38:20 +0200

> dont add double bindings to binding table

Dan, can you tell me exactly what you've done? I don't recognize
it iin the code. Does it help with the slow iconification?

Bye

Dominik ^_^

--
Dominik Vogt, [email-masked]
Reply-To: [email-masked]
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dan Espen <[email-masked]>
Date: Mon, 26 Apr 1999 13:24:31 -0400

> it iin the code. Does it help with the slow iconification?

All the  code is  in libs/Bindings.c.   Look for  this comment and the
following code:

    /* If this is a mouse binding we'll fall through the for loop (maxmods is
     * zero) and the if condition is always true (type is zero). Since min ==
     * max == button there is no loop at all is case of a mouse binding. */

The change I  made checks the  entry about to be  added to the binding
table to see if  it is identical  to  the binding just added.   If the
entries are identical, the duplicate entry is not added.

For me, this cut the  number of entries in  the binding table from 400
to 200.  This cut my iconify time from 7 seconds to 3 seconds.
Still way too slow.

The change you made didn't have a measurable impact.

After all the recent changes, I counted the number of grabs being
done.  For each iconify event, I count 200 key grabs.  About 2/3 of
them are in the logic with the comment "dead key grab".  I think
the "IgnoreModifiers" command my have something to do with this,
so this evening I want to run another test with no modifiers being
ignored.

By the way, the man page says this command is slow on a network.
I don't see how networks come into this, my X Server is on my
desktop.

--
Dan Espen
444 Hoes Lane  Room RRC 1C-214           E-mail: [email-masked]
Piscataway, NJ 08854                     Phone: (732) 699-5570
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dan Espen <[email-masked]>
Date: Mon, 26 Apr 1999 13:32:50 -0400

> All the  code is  in libs/Bindings.c.   Look for  this comment and the
... Sorry, wrong comment.  Might as well do the comment and the code:

        /* If the modifier (m) doesn't change the keys value,
           (for example, num-lock on a letter),
           don't add the key twice, check against the one just added...
        */
        if (*pblist != 0 &&
            (*pblist)->Button_Key == i &&
            (*pblist)->type == type &&
            (*pblist)->Context == contexts &&
            (*pblist)->Modifier == modifiers &&
            ((type == KEY_BINDING && strcmp((*pblist)->key_name,key_name) == 0)
             ||
             (type == MOUSE_BINDING && (*pblist)->key_name == NULL)) &&
            (*pblist)->Action == action &&
            (*pblist)->Action2 == action2) {
          continue;
        }

---

From: Dominik Vogt <[email-masked]>
Date: Tue, 27 Apr 1999 11:51:18 +0200

>      * max == button there is no loop at all is case of a mouse binding. */

[after reading second reply]

Yes, I have seen the code but couldn't see what the new part is.

> The change you made didn't have a measurable impact.

Indeed.

> done.  For each iconify event, I count 200 key grabs.

That's it. 200 grabs can take a considerable amount of time. We
have to figure out why this enormous amount of grabs is done.

> About 2/3 of

The network may be involved because the grabs contact the server.
Running the X server on the same machine as your desktop doesn't
necessarily mean the network is not involved. There are definitely
X servers (e.g. the X server supplied with AIX 4.2) stupid enough
to use the network if you set your 'DISPLAY' variable to
'<hostname>:0.0'. To prevent network access this should be ':0.0'.
(This forced me to do some complicated shell setup to get DISPLAY
set right in the past). Coult this be part of the problem?

Bye

Dominik ^_^

-- 
Dominik Vogt, Hewlett-Packard GmbH, Dept. BVS
Herrenberger Str.130, 71034 Boeblingen, Germany
phone: 07031/14-4596, fax: 07031/14-3883, [email-masked]
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dan Espen <[email-masked]>
Date: Tue, 27 Apr 1999 08:38:36 -0400

> set right in the past). Coult this be part of the problem?

My home  machine  definitely has $DISPLAY  set to  ":0".   It was slow
before  I set  "IgnoreModifiers".  Now  its  OK.  Perhaps the man page
should say, this option  can be slow on very  slow machines, or with a
lot of key or mouse bindings, or when the X Server is on network.

---

From: Dominik Vogt <[email-masked]>
Date: Tue, 27 Apr 1999 14:51:12 +0200

> lot of key or mouse bindings, or when the X Server is on network.

But the default shouldn't be slower than before. Only if you add
additional modifiers to be ignored it should have any impact.

Bye

Dominik ^_^

---

From: Dan Espen <[email-masked]>
Date: Tue, 27 Apr 1999 09:05:50 -0400

>From the man page, the default is:

IgnoreModifiers L

thats slower than before.

IgnoreModifiers

is the same speed as before,  so I think that should be the default.

I tried

IgnoreModifiers L25

it wasn't any slower than just "L".

If you are saying  "L"  should be the  same  speed as before, I'm  not
finding that to be true.

---

From: Dominik Vogt <[email-masked]>
Date: Tue, 27 Apr 1999 15:36:44 +0200

> thats slower than before.

Caps-lock was ignored as a modifier before I touched the code.
I can vaguely remember that there was some bug that missed a few
bindings, but not to an extent that should matter here.

> is the same speed as before,  so I think that should be the default.

That should be *faster* since caps-lock works like any other
modifier then.

> it wasn't any slower than just "L".

Duh, I can't explain that one. Should be a lot slower.

> If you are saying  "L"  should be the  same  speed as before, I'm  not

Yes, exactly.

Bye

Dominik ^_^

---

From: Dominik Vogt <[email-masked]>
Date: Tue, 27 Apr 1999 15:50:48 +0200

> > If you are saying  "L"  should be the  same  speed as before, I'm  not

Uh, oh, I can see what the problem is. In GrabWindowKey (Binding.c) there
is a loop doing the grabs:

  register unsigned int max = dead_modifiers | binding->Modifier;
                                             ^^^^^^^^^^^^^^^^^^^^
  /* handle all bindings for the dead modifiers */
  for (mods = 1; mods <= max; mods++)
    {
      ...
    }

Because of the '| binding->Modifier' it multiplies all bindings.
I'm working on a quick patch.

Bye

Dominik ^_^

---

From: Dominik Vogt <[email-masked]>
Date: Tue, 27 Apr 1999 15:59:59 +0200

>   register unsigned int max = dead_modifiers | binding->Modifier;

Okay, here we go. It's a patch for key bindings only for now. Dan,
can you check this? I can't test at work. Just replace the code
piece in Bindings.c (GrabWindowKey) with this one (you'll know where).

  {
    register unsigned int mods;
    register unsigned int max = dead_modifiers; 
    register unsigned int living_modifiers = ~dead_modifiers;
    
    /* handle all bindings for the dead modifiers */
    for (mods = 1; mods <= max; mods++)
      {
        /* Since mods starts with 1 we don't need to test if mods
         * contains a dead modifier. Otherwise both, dead and living
         * modifiers would be zero ==> mods == 0 */
        if (mods & living_modifiers)
          continue;
        if (fGrab)
          XGrabKey(dpy, binding->Button_Key, mods|binding->Modifier, w,
                   True, GrabModeAsync, GrabModeAsync);
        else
          XUngrabKey(dpy, binding->Button_Key, mods|binding->Modifier,
                     w);
      }
  }

Bye

Dominik ^_^

---

From: Dominik Vogt <[email-masked]>
Date: Tue, 27 Apr 1999 16:22:58 +0200

>           XUngrabKey(dpy, binding->Button_Key, mods|binding->Modifier,

Here's a patch that should fix key and button bindings (untested).

Bye

Dominik ^_^

-- 
Dominik Vogt, Hewlett-Packard GmbH, Dept. BVS
Herrenberger Str.130, 71034 Boeblingen, Germany
phone: 07031/14-4596, fax: 07031/14-3883, [email-masked]
*** Bindings.c.orig	Tue Apr 27 15:55:35 1999
--- Bindings.c	Tue Apr 27 16:25:29 1999
***************
*** 323,334 ****
  	 dead_modifiers != 0)
  	{
  	  register unsigned int mods;
! 	  register unsigned int max = dead_modifiers | binding->Modifier;
  
  	  /* handle all bindings for the dead modifiers */
  	  for (mods = 1; mods <= max; mods++)
  	    {
! 	      if ((mods & ~dead_modifiers) != binding->Modifier)
  		continue;
  	      if (fGrab)
  		XGrabKey(dpy, binding->Button_Key, mods|binding->Modifier, w,
--- 323,338 ----
  	 dead_modifiers != 0)
  	{
  	  register unsigned int mods;
! 	  register unsigned int max = dead_modifiers;
! 	  register unsigned int living_modifiers = ~dead_modifiers;
  
  	  /* handle all bindings for the dead modifiers */
  	  for (mods = 1; mods <= max; mods++)
  	    {
! 	      /* Since mods starts with 1 we don't need to test if mods
! 	       * contains a dead modifier. Otherwise both, dead and living
! 	       * modifiers would be zero ==> mods == 0 */
! 	      if (mods & living_modifiers)
  		continue;
  	      if (fGrab)
  		XGrabKey(dpy, binding->Button_Key, mods|binding->Modifier, w,
***************
*** 383,394 ****
  	     dead_modifiers != 0)
  	    {
  	      register unsigned int mods;
! 	      register unsigned int max = dead_modifiers | binding->Modifier;
  
  	      /* handle all bindings for the dead modifiers */
  	      for (mods = 1; mods <= max; mods++)
  		{
! 		  if ((mods & ~dead_modifiers) != binding->Modifier)
  		    continue;
  		  if (fGrab)
  		    XGrabButton(dpy, button, mods|binding->Modifier, w, True,
--- 387,402 ----
  	     dead_modifiers != 0)
  	    {
  	      register unsigned int mods;
! 	      register unsigned int max = dead_modifiers;
! 	      register unsigned int living_modifiers = ~dead_modifiers;
  
  	      /* handle all bindings for the dead modifiers */
  	      for (mods = 1; mods <= max; mods++)
  		{
! 		  /* Since mods starts with 1 we don't need to test if mods
! 		   * contains a dead modifier. Otherwise both, dead and living
! 		   * modifiers would be zero ==> mods == 0 */
! 		  if (mods & living_modifiers)
  		    continue;
  		  if (fGrab)
  		    XGrabButton(dpy, button, mods|binding->Modifier, w, True,

---

From: Greg Badros <[email-masked]>
Date: 27 Apr 1999 15:33:32 -0700

I've noticed recently that lots of the messages here are responses to
commits.  Unfortunately, it's hard to tell what the subject matter
really is, so it'd be nice if people would get in the habit of changing
the subject line (e.g., as I did here) to reflect the conversation's
topics.  That way, it's easier for people who may have helpful
suggestions but not a lot of time to pick and choose which messages are
most worth their time looking at.

Thanks,
Greg

---

From: Dan Espen <[email-masked]>
Date: Tue, 27 Apr 1999 20:25:00 -0400

> Here's a patch that should fix key and button bindings (untested).

The patch seemed to do the trick.

---

From: Jason L Tibbitts III <[email-masked]>
Date: 28 Apr 1999 11:43:51 -0500

>>>>> "GB" == Greg Badros <[email-masked]> writes:

GB> I've noticed recently that lots of the messages here are responses to
GB> commits.  Unfortunately, it's hard to tell what the subject matter
GB> really is [...]

I have hacked the script a bit to include the first line of the log message
in the Subject: header.  Hopefully this will help things a bit.  (Hopefully
it will actually work; the whole thing is a rather nasty hack.)

 - J<