xpra icon
Bug tracker and wiki

Opened 7 weeks ago

Last modified 2 weeks ago

#1426 new task

scrolling improvements

Reported by: Antoine Martin Owned by: alas
Priority: major Milestone: 2.0
Component: encodings Version: trunk
Keywords: scroll Cc:

Description (last modified by Antoine Martin)

follow up from #1232.

Issues:

  • it can take a lot of CPU to figure out the scroll encoding (figures given here are made worse by the instrumentation and profiling tools enabled):
    scroll encoding total time: 89ms
    

So high that we might has well not bother!

  • we should add the ability to enable it even when there are no video encodings available (ie: for html5 client)
  • we make a copy of the xshm backing for each subimage we send to the non-scroll compression - these could be handled as memoryviews

Attachments (4)

scroll-timing.patch (2.2 KB) - added by Antoine Martin 7 weeks ago.
add logging for scroll timing within encode_scrolling function
scroll-cythonized.patch (27.9 KB) - added by Antoine Martin 7 weeks ago.
this cythonized version of the code is super fast, but also not visually correct (yet)
scroll-cythonized-v3.patch (34.5 KB) - added by Antoine Martin 7 weeks ago.
almost done - almost renders correctly
scroll-cythonized-v4.patch (34.6 KB) - added by Antoine Martin 7 weeks ago.
avoid back and forth: stay in cython

Download all attachments as: .zip

Change History (11)

Changed 7 weeks ago by Antoine Martin

Attachment: scroll-timing.patch added

add logging for scroll timing within encode_scrolling function

comment:1 Changed 7 weeks ago by Antoine Martin

Using the patch above, we can see that there are 2 main areas that take too long to complete:

2017-02-02 02:01:12,113 updated scroll data, previously set: True
2017-02-02 02:01:12,116 best scroll guess took 4ms, matches 76% of 1485 lines: -80
2017-02-02 02:01:12,118 encode_scrolling(XShmImageWrapper(BGRX: 0, 0, 1923, 1485), {..}, [], [], {'scroll': True}) window-dimensions=(1923, 1485)
2017-02-02 02:01:12,175  will send scroll packets for yscroll=[-80, -79, -81, -78, -82, -77, -83, -76, -84, -75, -85, -74, -86, -73, -87, -72, -88, -71, 0, -70, -89, -69, -90, -68, -67, -91, -66, -92, -65, -64, -93, -63, -62, -94, -61, -95, -60, -96, -97, -59, -97, -59, -98, -58, -98, -58, 11, -57, 11, -57], elapsed=56ms
2017-02-02 02:01:12,180  scroll groups for distance=-80 : [(71, 429), (599, 167), (865, 540)]
2017-02-02 02:01:12,183  scroll groups for distance=-79 : [(1405, 1)]
2017-02-02 02:01:12,186  scroll groups for distance=-78 : [(1406, 1)]
2017-02-02 02:01:12,189  scroll groups for distance=-77 : [(1407, 1)]
2017-02-02 02:01:12,192  scroll groups for distance=-76 : [(1408, 1)]
2017-02-02 02:01:12,195  scroll groups for distance=-75 : [(1409, 1)]
2017-02-02 02:01:12,198  scroll groups for distance=-74 : [(1410, 1)]
2017-02-02 02:01:12,201  scroll groups for distance=-73 : [(1411, 1)]
2017-02-02 02:01:12,204  scroll groups for distance=-72 : [(1412, 1)]
2017-02-02 02:01:12,208  scroll groups for distance=-71 : [(1413, 1)]
2017-02-02 02:01:12,211  87 lines have not changed
2017-02-02 02:01:12,215  scroll groups for distance=-70 : [(1414, 1)]
2017-02-02 02:01:12,218  scroll groups for distance=-69 : [(1415, 1)]
2017-02-02 02:01:12,221  scroll groups for distance=-68 : [(1416, 1)]
2017-02-02 02:01:12,224  scroll groups for distance=-67 : [(1417, 1)]
2017-02-02 02:01:12,226  scroll groups for distance=-66 : [(1418, 1)]
2017-02-02 02:01:12,229  scroll groups for distance=-65 : [(1419, 1)]
2017-02-02 02:01:12,232  scroll groups for distance=-64 : [(1420, 1)]
2017-02-02 02:01:12,234  scroll groups for distance=-63 : [(1421, 1)]
2017-02-02 02:01:12,237  scroll groups for distance=-62 : [(1422, 1)]
2017-02-02 02:01:12,240  scroll groups for distance=-61 : [(1423, 1)]
2017-02-02 02:01:12,243  scroll groups for distance=-60 : [(1424, 1)]
2017-02-02 02:01:12,248  scroll groups for distance=11 : [(784, 9), (1450, 2)]
2017-02-02 02:01:12,251  distance matching took 73ms
2017-02-02 02:01:12,253  non scroll: 6 packets: [(500, 99), (766, 18), (793, 25), (820, 45), (1439, 11), (1452, 33)]
2017-02-02 02:01:12,257 compress: 137.4ms for 1923x1485 pixels at    0,0    for wid=1     using scroll as  25 rectangles  (11154KB)           , sequence    90, client_options={'scroll': True, 'flush': 6}
2017-02-02 02:01:12,260  splitting took 6ms
2017-02-02 02:01:12,262 sub_image 190377 pixels: 0.1ms
2017-02-02 02:01:12,268 compress:   6.0ms for 1923x99   pixels at    0,500  for wid=1     using   jpeg with ratio   5.4%  (  743KB to    40KB), sequence    91, client_options={'scroll': True, 'flush': 5}
2017-02-02 02:01:12,271 sub_image 34614 pixels: 0.1ms
2017-02-02 02:01:12,276 compress:  14.1ms for 1923x18   pixels at    0,766  for wid=1     using   jpeg with ratio   6.1%  (  135KB to     8KB), sequence    92, client_options={'scroll': True, 'flush': 4}
2017-02-02 02:01:12,279 sub_image 48075 pixels: 0.1ms
2017-02-02 02:01:12,285 compress:  22.5ms for 1923x25   pixels at    0,793  for wid=1     using   jpeg with ratio   1.3%  (  187KB to     2KB), sequence    93, client_options={'scroll': True, 'flush': 3}
2017-02-02 02:01:12,288 sub_image 86535 pixels: 0.1ms
2017-02-02 02:01:12,296 compress:  33.8ms for 1923x45   pixels at    0,820  for wid=1     using   jpeg with ratio   5.0%  (  338KB to    17KB), sequence    94, client_options={'scroll': True, 'flush': 2}
2017-02-02 02:01:12,299 sub_image 21153 pixels: 0.0ms
2017-02-02 02:01:12,304 compress:  41.5ms for 1923x11   pixels at    0,1439 for wid=1     using   jpeg with ratio   1.8%  (   82KB to     1KB), sequence    95, client_options={'scroll': True, 'flush': 1}
2017-02-02 02:01:12,306 sub_image 63459 pixels: 0.1ms
2017-02-02 02:01:12,313 compress:  50.4ms for 1923x33   pixels at    0,1452 for wid=1     using   jpeg with ratio   4.1%  (  247KB to    10KB), sequence    96, client_options={'scroll': True}
2017-02-02 02:01:12,315 non-scroll encoding using jpeg (quality=42, speed=90) took 52ms for 6 rectangles
2017-02-02 02:01:12,318 scroll encoding total time: 199ms
  • elapsed=56ms is the time spent figuring out which scroll values to send
  • distance matching took 73ms
  • the rest is the region encoding (already improved in r14952 by using jpeg instead of png - quality / speed settings permitting)

The whole thing should be cythonized.
For the v1.0.x branch, maybe we can just limit the size of the search space (<1K vertical?)

comment:2 Changed 7 weeks ago by Antoine Martin

Description: modified (diff)
Status: newassigned

Changed 7 weeks ago by Antoine Martin

Attachment: scroll-cythonized.patch added

this cythonized version of the code is super fast, but also not visually correct (yet)

Changed 7 weeks ago by Antoine Martin

Attachment: scroll-cythonized-v3.patch added

almost done - almost renders correctly

comment:3 Changed 7 weeks ago by Antoine Martin

r14965 avoids the unnecessary memory copy with the X11 image wrapper class.
The patch above is almost complete (just needs an off-by-one fixed)

Other improvements planned:

  • CRC Image function should be added to the cython class so we can allocate the checksum list directly in C code (no need to go back and forth between integers and cython numbers)
  • we could probably split each line in chunks of ~512bytes (128 pixels) so that we could send partial scroll packets: only the right edge (less than 128 pixels) would be sent as a picture in most cases. We can do this search if we have found a scroll distance already (fast - worth the extra hash cost)

comment:4 Changed 7 weeks ago by Antoine Martin

After much time spent debugging, the new scroll code is just fine, the real problem comes from the new jpeg encoder instead: #1423.

So r14972 commits the new scroll code, and turns off jpeg for non-scroll region updates.

Still TODO:

  • avoid using python arrays for checksums altogether
  • enable scrolling without video modes

Useful debugging steps for future reference:

  • run the client with:
    XPRA_PAINT_FLUSH=0 \
    XPRA_OPENGL_SAVE_BUFFERS=png \
       xpra attach -d draw --no-mmap
    

This will make the paint code really slow (lack of delayed flush) and save each FBO to disk so we can see the screen updates one by one, including the scroll ones.

  • run the server with:
    XPRA_SAVE_TO_FILE=png \
        xpra start -d scroll
    

So we can see the original region we sent for non-scroll areas.

Changed 7 weeks ago by Antoine Martin

Attachment: scroll-cythonized-v4.patch added

avoid back and forth: stay in cython

comment:5 Changed 7 weeks ago by Antoine Martin

Cythonization complete in r14978.
We no longer use python-xxhash and ship xxhash with a cython wrapper instead. I've kept "python-xxhash" packaging in the tree for now so we can continue to build updates from trunk for v1.0.x.

Remaining tasks:

  • enable scrolling without video encodings (for html5 client)
  • split checksums so we can do partial line scrolling (we're likely to still invalidate the whole line for simplicity)

comment:6 Changed 7 weeks ago by Antoine Martin

Owner: changed from Antoine Martin to alas
Status: assignednew

r14984 enables scrolling without requiring video support - also some minor fixes and improvements in r14994, r15002. It can be tested with the python client using:

xpra attach --encodings=png,jpeg,rgb --no-mmap -d draw

Or with the HTML5 client, but that one seems to have paint issues with the "scroll" encoding... which will need to be fixed and backported, see #1432.

The non-scroll paints will also be less optimal than before, because we end up in the video codepath - which will often encode a larger region.. This can happen if the page has lots of animation on it.

Partial line scrolling has been moved to #1429, so this is ready for testing.


@afarr: mostly a FYI at this point, scroll should perform better than before but that's quite difficult to quantify without running the profiling tools.
The more important improvement is for the HTML5 client, but this still needs fixing...

Last edited 7 weeks ago by Antoine Martin (previous) (diff)

comment:7 Changed 2 weeks ago by Antoine Martin

Fixed some visual corruption, see ticket:1432#comment:4, the html5 client still needs fixing...

Edit: now fixed, see ticket:1432#comment:5

Last edited 2 weeks ago by Antoine Martin (previous) (diff)
Note: See TracTickets for help on using tickets.