GUACAMOLE-2316: Allow the number of display worker threads per connection to be limited.

[msshapira]

Adds a per-connection limit on the number of encoder worker threads created for each guac_display, configurable via max_worker_threads in guacd.conf, -w on the command line, and MAX_WORKER_THREADS in the Docker image.

The limit is stored on guac_client (appended to the struct) and read by guac_display_alloc(), so it transports from guacd across the protocol-plugin boundary without changing any existing public signature. Default 0 preserves current behavior.

See GUACAMOLE-2316 for the motivation and the open question about whether the default should stay 0/unlimited or become a finite value.

[necouchman]

Overall I'm fine with the changes; however, I wonder if there's something a bit different we should do for the default maximum of threads? Maybe the default of scaling to the maximum number of processors isn't the best option? Is there some sort of default maximum that's actually more appropriate for performance - that is, over X number of threads, you don't see any additional benefit?

[msshapira]

Overall I'm fine with the changes; however, I wonder if there's something a bit different we should do for the default maximum of threads? Maybe the default of scaling to the maximum number of processors isn't the best option? Is there some sort of default maximum that's actually more appropriate for performance - that is, over X number of threads, you don't see any additional benefit?

@necouchman I agree that a hard limit may be a good idea. Opus suggested X=4, assuming more that that won't give much benefit anyway. Agree?

[necouchman]

Overall I'm fine with the changes; however, I wonder if there's something a bit different we should do for the default maximum of threads? Maybe the default of scaling to the maximum number of processors isn't the best option? Is there some sort of default maximum that's actually more appropriate for performance - that is, over X number of threads, you don't see any additional benefit?

@necouchman I agree that a hard limit may be a good idea. Opus suggested X=4, assuming more that that won't give much benefit anyway. Agree?

I'll defer to @mike-jumper on that one - he probably has a better idea of the cross-over point at which the number of threads will become more of a burden than it is a help.

[mike-jumper]

I think the default should remain as-is. There's plenty of parallelism to exploit in the render pipeline, even more so with WIP performance improvements to guac_display, and adding an arbitrary default ceiling would hobble performance on servers with the CPU to spare.

In my testing of the display, I've seen improvement in restricting the display to P-cores on platforms where the distinction exists, but a reduction in available workers only harms performance beyond that, especially since the FIFO ensures that the usage of the worker pool scales to the workload. Some workers may sit idle, but that's harmless vs. not having them available.

[otlg]

@mike-jumper @necouchman Hi,

We also need this change. In general, this change allows controlling the number of available threads per session, so it's the maintainer's responsibility if something goes wrong.

Is there anything else that should be done for approval and releasing a new patch based on it?

Thanks