[PATCH] Avoid huge slowdown when many resources

[Roberto Ragusa]

Hi,
(please CC me, I'm not subscribed)

here is a patch to better handle a big number of resources.
In my case I had been experiencing a huge slowdown of the system
after a few days of uptime, which I tracked to an application (psi)
continuously pushing pixmaps to plasma-desktop to achieve a pulsating
icon in the system tray.

Maybe something is wrong with psi, with the toolkit, with plasma-desktop,
but the final result was that Xorg was not able to cope with
that number of resources. The entire machine starts crawling; and this
is a powerful machine (quad i7, 16GiB).

There are a few bug reports about the slowness of GetXIDRange.
This one is exactly my case:
  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=677470
It was promptly closed because the reporter was using nvidia binary drivers.
I'm on Intel chipset, instead.

Look at this xrestop result (plasma-desktop is the interesting one):

xrestop - Display: localhost:0
          Monitoring 55 clients. XErrors: 0
          Pixmaps:  216340K total, Other:    5910K total, All:  222251K total

res-base Wins  GCs Fnts Pxms Misc   Pxm mem  Other   Total   PID Identifier
6c00000    71   51    1  905 1320    70962K     34K  70997K  2300 <<redacted>>
3600000    18   18    1   56   91    35631K      3K  35635K 25355 <<redacted>>
1c00000   310   40    1  130  984    27093K     32K  27125K 19902 <<redacted>>
2000000    87   86    1  115 244337    12414K   5731K  18145K 20349 plasma-desktop
5e00000    61   44    1  318  637    16431K     18K  16449K 11086 <<redacted>>
5c00000    39   53    1  260  527    13804K     15K  13819K  7176 <<redacted>>
7800000    43   48    1  119  221     9866K      8K   9874K 10198 <<redacted>>
6e00000    14   27    1   11   23     4541K      2K   4543K 17304 <<redacted>>
6200000    23   29    1   10   40     3907K      3K   3910K 23546 <<redacted>>
4400000    41    5    1   19   63     3587K      3K   3590K  1730 <<redacted>>
3e00000    47    6    1   35   74     3112K      3K   3116K 25842 <<redacted>>
1400000     6    5    0   19   22     2474K    792B   2474K  1981 <<redacted>>

The unpatched system becomes unusable at 180,000 misc resources.
The patched system is perfectly running at 244,337, at the moment.

For my curiosity, I would like to understand if this is a real leak, or
it is expected that numbers can go so high, and some kind of cleaning
is just not triggering because there is a lot memory available and these
are small objects.

Recipe for reproduction: run KDE, run psi, have psi trying to connect
for a few hours while flashing the icon in the systemtray, see
plasma-dekstop raise in xrestop results. (killing/restarting plasma-desktop
is a work-around to release the resources)

Here is the patch.
With MAXHASHSIZE=11 there are at most 2^11=2048 buckets, which means
more than 100 elements per bucket at 244k elements.
By increasing MAXHASHSIZE to 16 we have 2^16=65536 buckets, so about 4
elements per bucket at 244k elements.
(Note that the preexisting hash scale up logic triggers
when elements > 4*buckets)

By the way, could GetXIDRange be sped up in other ways?
I admit I'm not entirely sure why it does what it does
(this is the first time I've seen xorg-server source code).

diff --git a/dix/resource.c b/dix/resource.c
index eb9f049..bddb10a 100644
--- a/dix/resource.c
+++ b/dix/resource.c
@@ -159,7 +159,7 @@ static void RebuildTable(

 #define INITBUCKETS 64
 #define INITHASHSIZE 6
-#define MAXHASHSIZE 11
+#define MAXHASHSIZE 16

 typedef struct _Resource {
     struct _Resource   *next;
@@ -347,6 +347,16 @@ Hash(int client, XID id)
            return ((int)(0x3FF & (id ^ (id>>10))));
        case 11:
            return ((int)(0x7FF & (id ^ (id>>11))));
+       case 12:
+           return ((int)(0xFFF & (id ^ (id>>12))));
+       case 13:
+           return ((int)(0x1FFF & (id ^ (id>>13))));
+       case 14:
+           return ((int)(0x3FFF & (id ^ (id>>14))));
+       case 15:
+           return ((int)(0x7FFF & (id ^ (id>>15))));
+       case 16:
+           return ((int)(0xFFFF & (id ^ (id>>16))));
     }
     return -1;
 }

I can verify that hashsize correctly goes up to 16 with this
additional patch.

diff --git a/dix/resource.c b/dix/resource.c
index eb9f049..bddb10a 100644
--- a/dix/resource.c
+++ b/dix/resource.c
@@ -542,6 +552,7 @@ RebuildTable(int client)
        *tptr = rptr;
     }
     clientTable[client].hashsize++;
+    LogMessage(X_INFO, "new table size:%d\n",clientTable[client].hashsize);
     for (j = clientTable[client].buckets,
         rptr = clientTable[client].resources;
         --j >= 0;

Best regards.

-- 
   Roberto Ragusa    mail at robertoragusa.it