You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2021/01/06 08:08:06 UTC
[GitHub] [guacamole-server] andrewsali opened a new pull request #322: Force fast WebP compression
andrewsali opened a new pull request #322:
URL: https://github.com/apache/guacamole-server/pull/322
This PR is based on re-evaluating WebP lossless and is meant for testing and opening a discussion. The background is the question and response (by @mike-jumper and @necouchman ) here: http://mail-archives.apache.org/mod_mbox/guacamole-dev/202008.mbox/browser
Based on our testing, using a very-fast WebP lossless compression significantly improves the responsiveness (e.g. scrolling, moving windows around) of Guacamole compared to the default settings. This was tested using libwebp > 0.6.1.
In our configuration there is little latency between the Remote Desktop server and the Guacamole gateway, so most of the latency was due to the compression time for PNG. With using fast WebP this is much improved.
It might be possible that our results differ from the original evaluation of WebP due to network bandwith improving faster than compute speed over the last few years.
Caveat: It might also be the case that if there is much latency between the Remote Desktop server and the Gateway, then this change doesn't have a beneficial effect.
This PR is of course just a suggestion, feel free to discard, however it might be worthwhile to test in different scenarios as per our testing the UX is much improved.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] necouchman commented on pull request #322: Force fast WebP compression
Posted by GitBox <gi...@apache.org>.
necouchman commented on pull request #322:
URL: https://github.com/apache/guacamole-server/pull/322#issuecomment-758750276
@andrewsali:
> Given that 4 Megabits / sec bandwidth is nowadays considered fairly slow (usually it's at least an order of magnitude faster), in most cases the faster compression should result in better UX.
My only comment about the bandwidth would be that, while 4 Mb/s is on the low end of individual internet connections, if you're aggregating a bunch of those together, you have the potential to hit limits. For example, if 200 users have active sessions, that has at least the potential to be 800 Mb/s, which, even for larger offices, may be at or beyond the limit of those connections. And, besides that, Guacamole may not be the only thing that those 200 users are doing - they probably have other websites opened, and, if these theoretical users are anything like the real ones I encounter day-to-day, they also have Pandora or Spotify or YouTube going, so all of the sudden 800Mb/s isn't quite as much as it used to be, and now I start to figure out how to chop that 4 Mb/s connection down some.
Aside from that, it's also useful to consider the other side of the connection - the servers running Tomcat and/or guacd where those connections terminate. Not just for raw bandwidth sake, but also for cost - for example, if I'm running guacd in a public cloud provider, I'm paying for both the CPU time and the bandwidth, and it may be more cost-effective to purchase one over the other.
All that is not to say that I'm opposed to the changes, just that I agree with Mike that I'm not sure hard-coding some of those parameters is a good idea - there should either be some logic and detection of connection performance that impacts them, or they should be configurable, or, perhaps, both.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] andrewsali commented on pull request #322: Force fast WebP compression
Posted by GitBox <gi...@apache.org>.
andrewsali commented on pull request #322:
URL: https://github.com/apache/guacamole-server/pull/322#issuecomment-755181167
Sorry, this PR is not meant to be a definitive code-base change at all - it is just a suggestion for testing and if others can reproduce the improved speed, then formalize the aspects requested in the code-review.
For encoding speed / bandwidth, what we tested was to take a screenshot PNG (Full HD screen) and see how much time it is to convert to webp using the cwebp utility.
Using m=0, q=30, converting 50 times
```
real 0m2.549s
user 0m2.151s
sys 0m0.404s
```
Output file size: 220K
Using m=2, q=30, converting 50 times:
```
real 0m13.037s
user 0m12.575s
sys 0m0.466s
```
Output file size: 112K
The original PNG file size was also 220K.
Using m=0, q=90, converting 50 times:
```
real 0m3.201s
user 0m2.945s
sys 0m0.261s
```
Output file size: 186K.
Using m=2, q=90, converting 50 times:
```
real 0m15.867s
user 0m15.531s
sys 0m0.333s
```
Output file size: 103K.
So compared to the m=2 setting, the bandwidth requirement seems about double, but encode speeds are about x5.
Given the improvements in bandwidth, it might be worthwhile to go for faster compression. In our subjective experience this has helped substantially. But it would be good for some tests in other deployments to see if it is really a good way to go.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] andrewsali commented on pull request #322: Force fast WebP compression
Posted by GitBox <gi...@apache.org>.
andrewsali commented on pull request #322:
URL: https://github.com/apache/guacamole-server/pull/322#issuecomment-758597440
Just for some back-of-the-envelope calculations:
With WebP lossless q=0, the average compression time is 0.05s, whereas with q=2 it is around 0.25s, however increases bandwidth by around 100KB. So if the bandwidth is larger than 100KB / (0.25 - 0.05)s ~ 500KB/s (4 Megabits / sec), then faster compression should be beneficial.
Given that 4 Megabits / sec bandwidth is nowadays considered fairly slow (usually it's at least an order of magnitude faster), in most cases the faster compression should result in better UX.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] mike-jumper commented on pull request #322: Force fast WebP compression
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on pull request #322:
URL: https://github.com/apache/guacamole-server/pull/322#issuecomment-755163844
Interesting. It definitely sounds like this is worth reevaluating, though I am skeptical that we should force lossless across the board vs. force WebP across the board where possible.
How did bandwidth consumption compare with PNG vs. forced WebP?
How did performance compare when allowing _both_ lossless and lossy WebP, with lossless being used only where PNG would have been used?
What did you observe in terms of performance with `config.method` set to `0` vs. the previous value of `2`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #322: Force fast WebP compression
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #322:
URL: https://github.com/apache/guacamole-server/pull/322#discussion_r552435569
##########
File path: src/libguac/encode-webp.c
##########
@@ -192,10 +192,10 @@ int guac_webp_write(guac_socket* socket, guac_stream* stream,
return -1;
/* Add additional tuning */
- config.lossless = lossless;
- config.quality = quality;
- config.thread_level = 1; /* Multi threaded */
- config.method = 2; /* Compression method (0=fast/larger, 6=slow/smaller) */
+ config.lossless = 1; /* force lossless */
Review comment:
The `guac_webp_write()` function should definitely not ignore its `lossless` parameter entirely, as that's essentially crippling a library function. If lossless WebP should be used across the board wherever WebP would be used, it should be the surface implementation making that decision, not the internals of the generic WebP writer.
##########
File path: src/libguac/encode-webp.c
##########
@@ -192,10 +192,10 @@ int guac_webp_write(guac_socket* socket, guac_stream* stream,
return -1;
/* Add additional tuning */
- config.lossless = lossless;
- config.quality = quality;
- config.thread_level = 1; /* Multi threaded */
- config.method = 2; /* Compression method (0=fast/larger, 6=slow/smaller) */
+ config.lossless = 1; /* force lossless */
+ config.quality = 30.f; /* prefer fast encoding over size */
Review comment:
The `quality` parameter should not be ignored, either. If `30.f` is the desired value here, shouldn't that be what's passed in?
##########
File path: src/libguac/encode-webp.c
##########
@@ -192,10 +192,10 @@ int guac_webp_write(guac_socket* socket, guac_stream* stream,
return -1;
/* Add additional tuning */
- config.lossless = lossless;
- config.quality = quality;
- config.thread_level = 1; /* Multi threaded */
- config.method = 2; /* Compression method (0=fast/larger, 6=slow/smaller) */
+ config.lossless = 1; /* force lossless */
+ config.quality = 30.f; /* prefer fast encoding over size */
+ config.thread_level = 0; /* Disable Multi thread */
Review comment:
Why?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org