summaryrefslogtreecommitdiff
path: root/package
diff options
context:
space:
mode:
authorStephan Bergmann <sbergman@redhat.com>2022-06-02 15:14:56 +0200
committerStephan Bergmann <sbergman@redhat.com>2022-06-02 17:13:17 +0200
commitb4163877e722298b42f6e96831bd8ffef7785a20 (patch)
tree5f2f76cd4935c98577f2ef5b9f884e15b80e1a3b /package
parent86922db292f5708d2431959d15c5ceea70aee7fb (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 'package')
-rw-r--r--package/inc/ZipOutputStream.hxx3
-rw-r--r--package/source/zipapi/ZipOutputStream.cxx6
-rw-r--r--package/source/zippackage/ZipPackageStream.cxx3
3 files changed, 7 insertions, 5 deletions
diff --git a/package/inc/ZipOutputStream.hxx b/package/inc/ZipOutputStream.hxx
index aeb7ab1f86e8..f55ef59a8880 100644
--- a/package/inc/ZipOutputStream.hxx
+++ b/package/inc/ZipOutputStream.hxx
@@ -25,6 +25,7 @@
#include "ByteChucker.hxx"
#include <comphelper/threadpool.hxx>
+#include <cstddef>
#include <vector>
struct ZipEntry;
@@ -85,7 +86,7 @@ private:
public:
void reduceScheduledThreadTasksToGivenNumberOrLess(
- sal_Int32 nThreadTasks);
+ std::size_t nThreadTasks);
const std::shared_ptr<comphelper::ThreadTaskTag>& getThreadTaskTag() const { return mpThreadTaskTag; }
};
diff --git a/package/source/zipapi/ZipOutputStream.cxx b/package/source/zipapi/ZipOutputStream.cxx
index a4d67fcfd54a..33321627b6f9 100644
--- a/package/source/zipapi/ZipOutputStream.cxx
+++ b/package/source/zipapi/ZipOutputStream.cxx
@@ -143,13 +143,13 @@ void ZipOutputStream::consumeFinishedScheduledThreadTaskEntries()
m_aEntries = aNonFinishedEntries;
}
-void ZipOutputStream::reduceScheduledThreadTasksToGivenNumberOrLess(sal_Int32 nThreadTasks)
+void ZipOutputStream::reduceScheduledThreadTasksToGivenNumberOrLess(std::size_t nThreadTasks)
{
- while(static_cast< sal_Int32 >(m_aEntries.size()) > nThreadTasks)
+ while(m_aEntries.size() > nThreadTasks)
{
consumeFinishedScheduledThreadTaskEntries();
- if(static_cast< sal_Int32 >(m_aEntries.size()) > nThreadTasks)
+ if(m_aEntries.size() > nThreadTasks)
{
osl::Thread::wait(std::chrono::microseconds(100));
}
diff --git a/package/source/zippackage/ZipPackageStream.cxx b/package/source/zippackage/ZipPackageStream.cxx
index c4f94806b9b8..b63bf24eaf4f 100644
--- a/package/source/zippackage/ZipPackageStream.cxx
+++ b/package/source/zippackage/ZipPackageStream.cxx
@@ -57,6 +57,7 @@
#include <PackageConstants.hxx>
#include <algorithm>
+#include <cstddef>
using namespace com::sun::star::packages::zip::ZipConstants;
using namespace com::sun::star::packages::zip;
@@ -771,7 +772,7 @@ bool ZipPackageStream::saveChild(
// cores and allow 4-times the amount for having the queue well filled. The
// 2nd parameter is the time to wait between cleanups in 10th of a second.
// Both values may be added to the configuration settings if needed.
- static sal_Int32 nAllowedTasks(comphelper::ThreadPool::getPreferredConcurrency() * 4);
+ static std::size_t nAllowedTasks(comphelper::ThreadPool::getPreferredConcurrency() * 4); //TODO: overflow
rZipOut.reduceScheduledThreadTasksToGivenNumberOrLess(nAllowedTasks);
// Start a new thread task deflating this zip entry