use XShm to grab window pixels rather than the slower XGetImage

[totaam]

Issue migrated from trac ticket # 345

component: server | priority: minor | resolution: fixed

2013-05-26 14:54:21: totaam created the issue

---

Pointers:

• shm spec
• XSHM Tutorial

The patch attached allows us to reduce the time it takes to grab pixels for a 1024x1024 window from about 12ms (was ~17ms with GDK Pixbuf instead of XGetImage - before ~r3368) down to just ~3ms.

Things still to be addressed:

• cannot be used for non-fullscreen updates because we cannot request a specific area, the whole XImage gets updated when we call XShmGetImage. The cost isn't very high, but we would still need a way of extracting the area we are interested in. (which does not fit well with how things are done right now as the ImageWrapper is also used to carry the requested area location and size)
• handle resizing: we need to destroy and re-create everything. We could re-use the same XImage when the new window size is smaller, but again we would need to be able to extract the area we are interested in.
• mmap and rgb encodings: maybe those will clash since they can create a PIL image referencing the pixels in-place as part of the rgb_reformat step. Either we force this to use a new buffer (use Image.fromstring instead of Image.frombuffer), or we have to be absolutely certain that they will not modify the data (or maybe they can?), and we have to make sure the XImage does not get garbage collected until we have finished using the pixels.
• we don't use damage events to invalidate the buffer, so we request one every time. no big deal since we only do full screen updates for now, but once we handle smaller areas, we can skip requesting a new copy until we see a damage event.

[totaam]

2013-05-26 14:54:55: totaam uploaded file xshm-v2.patch (16.0 KiB)

XShm basic implementation

[totaam]

2013-05-26 17:09:12: totaam uploaded file xshm-v3.patch (14.3 KiB)

updated patch on top of r3512

[totaam]

2013-05-26 17:58:19: totaam changed status from new to assigned

[totaam]

2013-05-26 17:58:19: totaam changed owner from antoine to totaam

[totaam]

2013-05-26 17:58:19: totaam commented

---

And maybe this should be worked around (Segfault on server ctrl-C):

Program received signal SIGSEGV, Segmentation fault.
(gdb) bt
#0  0x00007f29f1bb9b93 in __pyx_pf_4xpra_3x11_7gtk_x11_12gdk_bindings_13XImageWrapper_34__dealloc__ (__pyx_v_self=0x1bbe590) \
  at xpra/x11/gtk_x11/gdk_bindings.c:7906
#1  __pyx_pw_4xpra_3x11_7gtk_x11_12gdk_bindings_13XImageWrapper_35__dealloc__ (__pyx_v_self=<xpra.x11.gtk_x11.gdk_bindings.XImageWrapper \
  at remote 0x1bbe590>)
    at xpra/x11/gtk_x11/gdk_bindings.c:7862
#2  __pyx_tp_dealloc_4xpra_3x11_7gtk_x11_12gdk_bindings_XImageWrapper (o=<xpra.x11.gtk_x11.gdk_bindings.XImageWrapper at remote 0x1bbe590>) \
  at xpra/x11/gtk_x11/gdk_bindings.c:16124
#3  0x00000035b1e818e3 in meth_dealloc (m=0x7f29e403fe18) \
  at /usr/src/debug/Python-2.7.3/Objects/methodobject.c:134
#4  0x00007f29f1bb9ddf in __pyx_pf_4xpra_3x11_7gtk_x11_12gdk_bindings_13XImageWrapper_34__dealloc__ (__pyx_v_self=0x1bbe590) \
  at xpra/x11/gtk_x11/gdk_bindings.c:7975
#5  __pyx_pw_4xpra_3x11_7gtk_x11_12gdk_bindings_13XImageWrapper_35__dealloc__ (__pyx_v_self=<xpra.x11.gtk_x11.gdk_bindings.XImageWrapper at remote 0x1bbe590>)
    at xpra/x11/gtk_x11/gdk_bindings.c:7862
#6  __pyx_tp_dealloc_4xpra_3x11_7gtk_x11_12gdk_bindings_XImageWrapper (o=<xpra.x11.gtk_x11.gdk_bindings.XImageWrapper at remote 0x1bbe590>) \
  at xpra/x11/gtk_x11/gdk_bindings.c:16124
#7  0x00007f29f1bb91dc in __pyx_pf_4xpra_3x11_7gtk_x11_12gdk_bindings_11XShmWrapper_6cleanup (__pyx_v_self=0x1bc15f0) \
  at xpra/x11/gtk_x11/gdk_bindings.c:5595
#8  __pyx_pw_4xpra_3x11_7gtk_x11_12gdk_bindings_11XShmWrapper_7cleanup (__pyx_v_self=<xpra.x11.gtk_x11.gdk_bindings.XShmWrapper \
  at remote 0x1bc15f0>, unused=<optimized out>)
    at xpra/x11/gtk_x11/gdk_bindings.c:5502
#9  0x00000035b1edcfd6 in call_function (oparg=<optimized out>, pp_stack=0x7fff9376d5c8) at /usr/src/debug/Python-2.7.3/Python/ceval.c:4082
#10 PyEval_EvalFrameEx (f=<optimized out>, throwflag=throwflag@entry=0) at /usr/src/debug/Python-2.7.3/Python/ceval.c:2740
#11 0x00000035b1edcef1 in fast_function (nk=<optimized out>, na=1, n=<optimized out>, pp_stack=0x7fff9376d7c8, func=<function at remote 0x17006e0>)
    at /usr/src/debug/Python-2.7.3/Python/ceval.c:4184
#12 call_function (oparg=<optimized out>, pp_stack=0x7fff9376d7c8) at /usr/src/debug/Python-2.7.3/Python/ceval.c:4119
#13 PyEval_EvalFrameEx (f=f@entry=
    Frame 0x1895610, for file /usr/lib64/python2.7/site-packages/xpra/x11/gtk_x11/window.py, line 351, in do_unmanaged (self= \
      <WindowModel(_input_field=1, _gproperties={'size-hints': <WMSizeHints(max_aspect_ratio=None, min_aspect_ratio=None, \
        base_size=(19, 4), min_aspect=None, max_aspect=None, min_size=(25, 17), win_gravity=1, resize_inc=(6, 13), max_size=None) at remote 0x17eeed0>, \
      'client-machine': u'desktop', 'pid': 15439, 'owner': None, 'requested-size': (499, 316), \
      'title': u'antoine@desktop:~/projects/Xpra/trunk/src', 'group-leader': None, 'icon-title': u'antoine@desktop:~/projects/Xpra/trunk/src', \
      'icon-pixmap': None, 'user-friendly-size': (80, 24), 'state': frozenset([]), 'transient-for': None, \
      'class-instance': (u'xterm', u'XTerm'), 'iconic': False, 'strut': None, 'window-type': [<gtk.gdk.GdkAtom at remote 0x1716af0>], \
      'requested-position': (0, 0), 'actual-size': (499, 316), 'protocols': ['WM_DELETE_WINDOW'], 'icon': None, \
      'client-window': <gtk.gdk.Window at remote 0x17f70a0>, 'modal': False}, client_wind...(truncated), \
     throwflag=throwflag@entry=0) at /usr/src/debug/Python-2.7.3/Python/ceval.c:2740

[totaam]

2013-05-27 11:49:27: totaam commented

---

r3530 adds initial support (work in progress)

[totaam]

2013-06-07 15:20:27: totaam commented

---

r3596 fixes segfaults caused by XDestroyImage on an image we still use! (oops)

I still get segfault crashes on exit via control-C, similar to #352 but the solution posted there isn't enough in this case (looks like we must ensure we cleanup the shared memory in the right order)

Handling smaller regions should be doable without too much trouble, we just need to return a new XShmImageWrapper for each region, all pointing to the underlying XShmWrapper - the problem is keeping track of all those objects for managing the garbage collection (lists are easy in python, not so in C/Cython)

Then we still need to handle the resizing issues...

[totaam]

2013-06-08 06:27:44: totaam commented

---

working XShm in r3598 (see commit message for details)

[totaam]

2013-06-10 14:24:41: totaam commented

---

Still todo:

• avoid XShm setup for initial frames (similar logic to the get_best_encoding code)
• verify mmap mode, delta mode
• ensure we never modify pixels in place with PIL from_buffer

[totaam]

2013-06-17 13:12:48: totaam changed status from assigned to closed

[totaam]

2013-06-17 13:12:48: totaam changed resolution from ** to fixed

[totaam]

2013-06-17 13:12:48: totaam commented

---

XShm is now enabled by default as of r3613

[totaam]

2013-06-22 04:25:17: antoine changed status from closed to reopened

[totaam]

2013-06-22 04:25:17: antoine changed resolution from fixed to **

[totaam]

2013-06-22 04:25:17: antoine commented

---

One more problem: we need to deal with failures and figure out if they are transient, for this window only or if we can forget about using XShm altogether on the server by looking at errno when we get an error from shmget:

• EACCES The user does not have permission to access the shared memory segment, and does not have the CAP_IPC_OWNER capability.
  Probably not worth trying again, we lack permissions.
• EEXIST IPC_CREAT | IPC_EXCL was specified and the segment exists.
  Not applicable.
• EINVAL A new segment was to be created and size < SHMMIN or size > SHMMAX, or no new segment was to be created, a segment with given key existed, but size is greater than the size of that segment.
  In this case, we don't want to try again for this window unless its size is reduced (not worth trying to parse /proc/sys/kernel/shmmax)
• ENFILE The system limit on the total number of open files has been reached.
  Could be worth trying again later, but we would only make things worse again..
• ENOENT No segment exists for the given key, and IPC_CREAT was not specified.
  Not applicable.
• ENOMEM No memory could be allocated for segment overhead.
  Should try again later if segments are freed?
• ENOSPC All possible shared memory IDs have been taken (SHMMNI), or allocating a segment of the requested size would cause the system to exceed the system-wide limit on shared memory (SHMALL).
  Should try again later if segments are freed?

[totaam]

2013-06-22 12:20:51: antoine changed status from reopened to closed

[totaam]

2013-06-22 12:20:51: antoine changed resolution from ** to fixed

[totaam]

2013-06-22 12:20:51: antoine commented

---

Closing again: dealing with failures is now done in r3694