Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mmap improvements: zero-copy, avoid wrap-around, memcopy, mmap.ACCESS_COPY #67

Closed
totaam opened this issue Jan 6, 2012 · 17 comments
Closed

Comments

@totaam
Copy link
Collaborator

totaam commented Jan 6, 2012

Issue migrated from trac ticket # 67

component: server | priority: minor | resolution: fixed

2012-01-06 09:56:51: totaam created the issue


mmap is fast but can be made faster:

  • rather than extracting the pixel data from the mmap area, use it in place by creating a pixel buffer pointing to the mmap location
  • avoid wrapping around the end of the mmap area (split buffer) when we have enough free area: this avoids having to re-construct the buffer from 2 non-contiguous chunks and allows us to write it more quickly too
  • set the mmap area as mmap.ACCESS_COPY so the OS does not try to keep it in sync - this may require us to flush it, may not work/be worthwhile
  • use memcopy to copy the pixbuf pixels into the mmap area?
  • probably not do-able: create a pixbuf (or even the pixmap?) backed by an mmap area so we don't need to call get_pixels() at all
@totaam
Copy link
Collaborator Author

totaam commented Jan 6, 2012

2012-01-06 09:59:38: totaam uploaded file xpra-mmap-zerocopy.patch (2.7 KiB)

patch for measuring the improvement using zerocopy

@totaam
Copy link
Collaborator Author

totaam commented Jan 6, 2012

2012-01-06 10:16:35: totaam changed status from new to accepted

@totaam
Copy link
Collaborator Author

totaam commented Jan 6, 2012

2012-01-06 10:16:35: totaam changed owner from antoine to totaam

@totaam
Copy link
Collaborator Author

totaam commented Jan 6, 2012

2012-01-06 10:16:35: totaam commented


Part 1: zero-copy
The patch above allows us to measure the zero-copy case against the standard code (one must add and False to make it go through the non zero-copy case for non-split areas).
Some numbers with a 1024x1024 glxgears on a fast system (ignoring small screen updates which skew the number due to the high overhead of method calls compared to the actual time spent preparing the data - which means the benefit is less, almost negligible, when used on small pixel buffers):

  • in all cases, the actual draw_rgb_image takes from 1ms to 10ms (average around 5ms)
  • with "directly from mmap", creating the pixel data reference takes about 0.01ms
  • with the regular copy case, creating the pixel data buffer takes about 1ms
    So the benefit is in the 10% to 50% range, average around 20%.

Merged in r400

@totaam
Copy link
Collaborator Author

totaam commented Jan 6, 2012

2012-01-06 10:57:58: totaam changed priority from major to minor

@totaam
Copy link
Collaborator Author

totaam commented Jan 6, 2012

2012-01-06 10:57:58: totaam changed component from android to server

@totaam
Copy link
Collaborator Author

totaam commented Jan 6, 2012

2012-01-06 10:57:58: totaam

@totaam
Copy link
Collaborator Author

totaam commented Jan 6, 2012

2012-01-06 10:57:58: totaam commented


Part 2: avoid wrapping around
Done in r401, which also adds diagrams to show how the mmap transfers work.

@totaam
Copy link
Collaborator Author

totaam commented Feb 20, 2012

2012-02-20 19:32:53: totaam

@totaam
Copy link
Collaborator Author

totaam commented May 4, 2012

2012-05-04 10:57:34: antoine

@totaam
Copy link
Collaborator Author

totaam commented May 4, 2012

2012-05-04 10:57:34: antoine commented


This is good enough for now, the only features left to consider are:

  • set the mmap area as mmap.ACCESS_COPY so the OS does not try to keep it in sync - this may require us to flush it, may not work/be worthwhile - does not seem to work!
  • use memcopy to copy the pixbuf pixels into the mmap area
  • probably not do-able: create a pixbuf (or even the pixmap / cairo draw) backed by an mmap area so we don't need to call get_pixels() at all?

@totaam
Copy link
Collaborator Author

totaam commented Jul 16, 2013

2013-07-16 07:02:16: antoine changed status from accepted to new

@totaam
Copy link
Collaborator Author

totaam commented Jul 16, 2013

2013-07-16 07:02:16: antoine commented


We now use XShm (see #345).

@totaam
Copy link
Collaborator Author

totaam commented Jul 16, 2013

2013-07-16 07:02:54: antoine changed status from new to closed

@totaam
Copy link
Collaborator Author

totaam commented Jul 16, 2013

2013-07-16 07:02:54: antoine changed resolution from ** to fixed

@totaam totaam closed this as completed Jul 16, 2013
@totaam
Copy link
Collaborator Author

totaam commented May 19, 2014

2014-05-19 13:36:15: totaam

@totaam
Copy link
Collaborator Author

totaam commented May 19, 2014

2014-05-19 13:36:15: totaam commented


(setting correct milestone the work was completed in)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant