Bug 2029

Seems like a bug in the server; it clearly should account for the memory somehow.

XResQueryClientPixmapBytes(), server side, ends up calling
FindClientResourcesByType() with RT_PIXMAP which fire a callback which
accumalates used bytes via

(pix->devKind * pix->drawable.height)

for each pixmap. ( Is that even right - should it not take into account
pix->width and pix->bitsPerPixel ? )

Im thinking I could add another call to FindClientResourcesByType() but with
RT_WINDOW and calculate additional bytes via passed win->background.pixmap ?

Does that sound sane ?

Created attachment 1491 [details] [review]
Fix for Xres's QueryClientPixmapBytes to include window background pixels

Attached is a patch to kdrive's Xext/Xres.c which implements my suggestion.

It appears to work fine for me, with xrestop giving more real values.

Im still a little worried about the 'pix->devKind * pix->drawable.height'
byte calculation ( Im just doing the same thing for Win background pixmaps ),
though I *am* getting realistic values - approx ~1M per 640x480x24bpp win
backg\round pixmap.

If this is good, this should  be applied to Xorg Xext/Xres.c too.

Shouldn't this computation take into account the depth of the window?

If only 16 bits, you get 2 bytes/pixel; if 32, 4, and so on...

Yes thats what Im saying. 

Does something like;

pix->drawable.width * pix->drawable.height * (pix->drawable.bitsPerPixel>>3)

seem like a more realistic bytes calculation ?

Im confused to as why it would be pix->drawable.height * pix->devKind currently
and give what look like good values.
  

This patch will double-count any window background pixmaps which still have
resource table entries.

And, the usage of 'devKind' presumes a specific DDX architecture which will not
be accurate on xfree86 DDXen where the window background is in off-screen memory.

The computation involving height/width/bpp is probably closer to the real number.

Keithp and I have been chatting about this...

This isn't correct, and XRes currently does not report sensible sizes.

As Xres walks the resource database, it needs to take the size of objects it
discovers and divide by the refcount, so that the right fraction is attributed
to the client (note this will sum to one).

Created attachment 1498 [details] [review]
Second attempt at patch for fixing xres for window background pixmaps

This patch improves things via;

 - assuming pixmap memory usage via width * height * bpp>>3
 - Taking into account refcnt for a pixmap shared among multiple clients.

Im not sure what keith means when he says 'double-count any window background
pixmaps which still have resource table entries'. Is that the refcnt thing ?
If not could you explain a little more ?

I meant that if a window *and* an X resource pointed at a pixmap, you'd get
dunned for the pixmap twice -- once for the Pixmap ID and once for the background.

Dividing by the reference count should produce a more accurate answer for the
total client usage.  Note that this means you should also count pixmap memory
usage by GC tiles and stipples so that references from there are correctly
accounted for.  Oh, and you'll want to count composite pixmaps too, probably
accounting those against the top level window referencing them.

We should figure out if this is what we want the extension to do, or if we want
(instead) to provide some more comprehensive reporting mechanism to the client.

Also should probably include window border pixmaps ( though I cant even imagine
what kind of evil would use that ).

Keith, you'll have to give me a little clue on getting at composite pixmaps.
What are your thoughts on what the extension 'should do' ?  

On mr stones request, I've commited an improved version of this patch ( handling
pixmap borders and GC stipple / tile pixmaps ).