Set the minimum number of threads of the Multithreaded executor to 2

[AlexisTM]

Signed-off-by: Alexis Paques paa1ti@bosch.com

Closes #2029

This ensures the number of threads of a Multi-threaded executor is at least 2 unless defined explicitly as 1 (why not use the SingleThreadedExecutor?)
Indeed, if we are calling a service from a callback and call the get function of the future directly, we effectively block until the service request resolves but it can't as the reply will not be processed, causing a deadlock. This is either solved by having a Multi-threaded executor and multiple callback groups for non time critical software (which has to wait for the result anyway).

The default number of threads is std::thread::hardware_concurrency which can be 1 for a VM or a small target.
This means those targets (or VM) will not be able to receive the message of an otherwise correct node, while working on everybody else's machine.

The second motivation is to solve the misconception a Multithreaded executor always has multiple thread, by making it have always multiple treads by default.

[AlexisTM]

The use of std::thread::hardware_concurrency also bothers me. Indeed, the normal user will use the default Multi-threaded executor which would spin 128 threads on a Threadripper machine for each executor.

Instead, I would prefer having the application developer define the strict minimum amount of threads (with 2 by default) for reproducible results. Plus, for computation intensive software (reentrant data callbacks to start multiple long processing, maybe for CV), a user option to set the number of threads higher would be great (in that case as the number of cores).

I understand this is a non-issue for power-users which will manage themselves the single executor for the app or use composition, but it will avoid a whole range of issues. (aka what I just had, someone having a deadlock for a software which works for everybody else).

[alsora]

I would add a warning / error /something if a user tries to set the number of threads of this executor to 1.

Besides this, the change looks good to me.

[AlexisTM]

The check for if (number_of_threads_ == 0) has been replaced for the check if (number_of_threads_ == 1) to output a warning.

The case of 0 is not possible anymore as if std::thread::hardware_concurrency() returns 0, it will use 2 threads.

[alsora]

Minor: number_of_threads is a size_t, not a boolean, so prefer doing an explicit check with number_of_threads > 0

[clalancette]

This seems reasonable to me (though I'm going to update the title; we aren't setting the default to 2, we are setting the minimum to 2). I'll run CI on it next.

[clalancette]

CI:

• Linux
• Linux-aarch64
• Windows

[clalancette]

CI is happy here, so going ahead with this. Thanks for the contribution.

[fujitatomoya]

following up the review, lgtm