[RFC xserver 1/1] xwayland: reduce over-damage

[Pekka Paalanen]

On Mon, 22 Jan 2018 13:14:10 -0500
Adam Jackson <ajax at nwnk.net> wrote:

> On Mon, 2018-01-22 at 14:51 +0000, Daniel Stone wrote:
> >  Hi Pekka,
> > 
> > On 20 December 2017 at 11:18, Pekka Paalanen <ppaalanen at gmail.com> wrote:  
> > > If an X11 app draws a little here, some there, and a tiny bit in the
> > > opposite corner, using RegionExtents for the damage to be sent to the
> > > Wayland compositor will cause massive over-damaging.
> > > 
> > > However, we cannot blindly send an arbitrary number of damage
> > > rectangles, because there is a risk of overflowing the Wayland
> > > connection. If that happens, it triggers an abort in libwayland-client.
> > > 
> > > Try to be more accurate with the damage by sending up to 500 rectangles
> > > per window, and fall back to extents otherwise. The number is completely
> > > arbitrary.  
> > 
> > I might have said this on IRC, but 500 sails close enough to our
> > request limit for comfort. I can't find the mail where I did the
> > maths, but ISTR for damage requests, it's around the 6000-7000 mark.
> > Pulling 256 as an equally arbitrary number out of the air (arguably
> > even that number of TexSubImage requests represents a DoS), this is:
> > Reviewed-by: Daniel Stone <daniels at collabora.com>  
> 
> Changed the magic number to 256 and merged, thanks:
> 
> remote: E: failed to find patch for rev f72587ecc7e1dedfb20a999a0e600b83c06a1b29.
> remote: I: 0 patch(es) updated to state Accepted.
> To ssh://git.freedesktop.org/git/xorg/xserver
>    a5e9bcad7..f72587ecc  master -> master

Thank you both!

Daniel, a damage request is 8 + 4 * 4 = 24 B. 500 requests is 12000 B,
256 requests is 6144 B. The libwayland internal send buffer is 4096 B,
so for both values we are relying on the kernel socket buffer already.
Furthermore, the limit is per window and number of windows is
effectively unlimited. Xwayland loops over windows without a
possibility to poll for writable.


On Mon, 22 Jan 2018 14:56:12 +0100
Olivier Fourdan <fourdan at gmail.com> wrote:

> Hi Pekka,
> 
> On 22 January 2018 at 14:34, Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> wrote:

> > should I invest the effort on a better algorithm than in this patch or
> > would this be acceptable for landing?
> >  
> 
> Not sure which better algorithm you might have in mind here, but the
> approach taken in your patch seems simple enough, do we have a rough idea
> of the average number of rectangles we usually deal with here?

The two other approaches in the cover letter:
https://lists.x.org/archives/xorg-devel/2017-December/055474.html
Of course, the optimization approach could use various different
methods. There is also an implementation detail: pixman_region*_t uses
a banded format which increases the number of rectangles compared to a
free format, both using non-overlapping rectangles. The Wayland
protocol does not require the banded format.

I have no statistics about the rectangles on any general use case.

In summary, the Wayland connection overflowing issue would need a
specific solution. Sending optimized damage rectangles may help making
the overflow less likely, but it will never eliminate the possibility.
This patch is about a performance improvement, which is mostly
orthogonal to avoiding the overflow.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.x.org/archives/xorg-devel/attachments/20180123/ff65b4fe/attachment.sig>