diff options
author | Stephan Bergmann <sbergman@redhat.com> | 2022-06-02 15:14:56 +0200 |
---|---|---|
committer | Stephan Bergmann <sbergman@redhat.com> | 2022-06-02 17:13:17 +0200 |
commit | b4163877e722298b42f6e96831bd8ffef7785a20 (patch) | |
tree | 5f2f76cd4935c98577f2ef5b9f884e15b80e1a3b /comphelper/qa/unit | |
parent | 86922db292f5708d2431959d15c5ceea70aee7fb (diff) |
Use more appropriate return type for ThreadPool::getPreferredConcurrency
All call sites already effectively asked for an unsigned return type, including:
* The ThreadPool ctor took an nWorkers argument of type sal_Int32, but
internally stores that as std::size_t mnMaxWorkers.
* ZipOutputStream::reduceScheduledThreadTasksToGivenNumberOrLess apparently
benefits from an unsigned nThreadTasks parameter, getting rid of various casts
in its implementation that were necessary to silence signed vs. unsigned
comparison warnings.
The only drawback is that
comphelper::ThreadPool::getPreferredConcurrency() * 4
in package/source/zippackage/ZipPackageStream.cxx would now silently wrap around
instead of causing UB on overflow (which could be detected with appropriate
tools). Ideally, it would use some o3tl::saturating_mul if we had that, so add
a TODO comment for now.
While std::thread::hardware_concurrency returns unsigned, it looked more natural
to go with std::size_t here, as some call sites already used that (see above),
so the implementation of ThreadPool::getPreferredConcurrency is a natural place
to hide clamping std::thread::hardware_concurrency() to std::size_t (in the
unlikely case that std::size_t is of smaller rank than unsigned).
This required addition of o3tl::clamp_to_unsigned in o3tl/safeint.hxx.
Change-Id: I0a04a8b32e63ebfeb39f924c4b38520455a6fb38
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/135309
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Diffstat (limited to 'comphelper/qa/unit')
-rw-r--r-- | comphelper/qa/unit/threadpooltest.cxx | 5 |
1 files changed, 3 insertions, 2 deletions
diff --git a/comphelper/qa/unit/threadpooltest.cxx b/comphelper/qa/unit/threadpooltest.cxx index 14da59988ef4..13eaf210a18b 100644 --- a/comphelper/qa/unit/threadpooltest.cxx +++ b/comphelper/qa/unit/threadpooltest.cxx @@ -17,6 +17,7 @@ #include <stdlib.h> #include <atomic> +#include <cstddef> #include <thread> #include <mutex> @@ -42,7 +43,7 @@ void ThreadPoolTest::testPreferredConcurrency() { // Check default. auto nThreads = comphelper::ThreadPool::getPreferredConcurrency(); - sal_Int32 nExpected = 4; // UTs are capped to 4. + std::size_t nExpected = 4; // UTs are capped to 4. CPPUNIT_ASSERT_MESSAGE("Expected no more than 4 threads", nExpected >= nThreads); #ifndef _WIN32 @@ -51,7 +52,7 @@ void ThreadPoolTest::testPreferredConcurrency() setenv("MAX_CONCURRENCY", std::to_string(nThreads).c_str(), true); nThreads = comphelper::ThreadPool::getPreferredConcurrency(); CPPUNIT_ASSERT_MESSAGE("Expected no more than hardware threads", - nThreads <= static_cast<sal_Int32>(std::thread::hardware_concurrency())); + nThreads <= std::thread::hardware_concurrency()); // Revert and check. Again, nothing should change. unsetenv("MAX_CONCURRENCY"); |