From 67431e4168dd272d4c422a8bec01580091544fda Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Thu, 6 Apr 2023 14:48:15 -0300 Subject: [PATCH] QMutex/Unix: remove the pthread_mutex_t-based content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For our purposes, the sem_t code is better, so we've preferred it since commit c7ab816af110ea08809e1aabd8bf1e08967c6d53 (5.7). Olivier wrote in that commit's message, "It makes tst_QMutex::contendedQMutex with no msleep 8 times faster". That's true because QMutex didn't lock the underlying pthread_mutex_t in QMutex::lock(), as we used the inlined atomic code for the uncontended case. It is probably possible to merge the qmutex_mac.cpp and qmutex_unix.cpp code now (both are based on semaphores), but I won't do that for two reasons: 1) At best, the PThread functions are going to be thin-wrappers around the code we already have, like FreeBSD's are around usem (see [1]) 2) Darwin has a private API that resembles futexes so we may want to go that way eventually (see [2]) [1] https://github.com/freebsd/freebsd-src/blob/main/lib/libc/gen/sem_new.c [2] https://codereview.qt-project.org/335849 Change-Id: Idd5e1bb52be047d7b4fffffd175369b13ba47bed Reviewed-by: Mårten Nordheim Reviewed-by: Marc Mutz --- src/corelib/thread/qmutex_p.h | 17 +------ src/corelib/thread/qmutex_unix.cpp | 54 ---------------------- src/corelib/thread/qwaitcondition_unix.cpp | 4 +- 3 files changed, 3 insertions(+), 72 deletions(-) diff --git a/src/corelib/thread/qmutex_p.h b/src/corelib/thread/qmutex_p.h index 13a9f10b019..e02686ad687 100644 --- a/src/corelib/thread/qmutex_p.h +++ b/src/corelib/thread/qmutex_p.h @@ -28,10 +28,7 @@ #if defined(Q_OS_DARWIN) # include #elif defined(Q_OS_UNIX) -# if _POSIX_VERSION-0 >= 200112L || _XOPEN_VERSION-0 >= 600 # include -# define QT_UNIX_SEMAPHORE -# endif #endif struct timespec; @@ -87,23 +84,11 @@ public: //platform specific stuff #if defined(Q_OS_DARWIN) semaphore_t mach_semaphore; -#elif defined(QT_UNIX_SEMAPHORE) - sem_t semaphore; #elif defined(Q_OS_UNIX) - bool wakeup; - pthread_mutex_t mutex; - pthread_cond_t cond; + sem_t semaphore; #endif }; - -#ifdef Q_OS_UNIX -// helper functions for qmutex_unix.cpp and qwaitcondition_unix.cpp -// they are in qwaitcondition_unix.cpp actually -void qt_initialize_pthread_cond(pthread_cond_t *cond, const char *where); -void qt_abstime_for_timeout(struct timespec *ts, QDeadlineTimer deadline); -#endif - QT_END_NAMESPACE #endif // QMUTEX_P_H diff --git a/src/corelib/thread/qmutex_unix.cpp b/src/corelib/thread/qmutex_unix.cpp index f665f192a6c..0cbf4625274 100644 --- a/src/corelib/thread/qmutex_unix.cpp +++ b/src/corelib/thread/qmutex_unix.cpp @@ -25,8 +25,6 @@ static void report_error(int code, const char *where, const char *what) qErrnoWarning(code, "%s: %s failure", where, what); } -#ifdef QT_UNIX_SEMAPHORE - QMutexPrivate::QMutexPrivate() { report_error(sem_init(&semaphore, 0, 0), "QMutex", "sem_init"); @@ -68,56 +66,4 @@ void QMutexPrivate::wakeUp() noexcept report_error(sem_post(&semaphore), "QMutex::unlock", "sem_post"); } -#else // QT_UNIX_SEMAPHORE - -QMutexPrivate::QMutexPrivate() - : wakeup(false) -{ - report_error(pthread_mutex_init(&mutex, NULL), "QMutex", "mutex init"); - qt_initialize_pthread_cond(&cond, "QMutex"); -} - -QMutexPrivate::~QMutexPrivate() -{ - report_error(pthread_cond_destroy(&cond), "QMutex", "cv destroy"); - report_error(pthread_mutex_destroy(&mutex), "QMutex", "mutex destroy"); -} - -bool QMutexPrivate::wait(int timeout) -{ - report_error(pthread_mutex_lock(&mutex), "QMutex::lock", "mutex lock"); - int errorCode = 0; - while (!wakeup) { - if (timeout < 0) { - errorCode = pthread_cond_wait(&cond, &mutex); - } else { - timespec ti; - qt_abstime_for_timeout(&ti, QDeadlineTimer(timeout)); - errorCode = pthread_cond_timedwait(&cond, &mutex, &ti); - } - if (errorCode) { - if (errorCode == ETIMEDOUT) { - if (wakeup) - errorCode = 0; - break; - } - report_error(errorCode, "QMutex::lock()", "cv wait"); - } - } - bool ret = wakeup; - wakeup = false; - report_error(pthread_mutex_unlock(&mutex), "QMutex::lock", "mutex unlock"); - return ret; -} - -void QMutexPrivate::wakeUp() noexcept -{ - report_error(pthread_mutex_lock(&mutex), "QMutex::unlock", "mutex lock"); - wakeup = true; - report_error(pthread_cond_signal(&cond), "QMutex::unlock", "cv signal"); - report_error(pthread_mutex_unlock(&mutex), "QMutex::unlock", "mutex unlock"); -} - -#endif - QT_END_NAMESPACE diff --git a/src/corelib/thread/qwaitcondition_unix.cpp b/src/corelib/thread/qwaitcondition_unix.cpp index ec69a04f452..48d167e0a92 100644 --- a/src/corelib/thread/qwaitcondition_unix.cpp +++ b/src/corelib/thread/qwaitcondition_unix.cpp @@ -52,7 +52,7 @@ static void report_error(int code, const char *where, const char *what) qErrnoWarning(code, "%s: %s failure", where, what); } -void qt_initialize_pthread_cond(pthread_cond_t *cond, const char *where) +static void qt_initialize_pthread_cond(pthread_cond_t *cond, const char *where) { pthread_condattr_t *attrp = nullptr; @@ -69,7 +69,7 @@ void qt_initialize_pthread_cond(pthread_cond_t *cond, const char *where) report_error(pthread_cond_init(cond, attrp), where, "cv init"); } -void qt_abstime_for_timeout(timespec *ts, QDeadlineTimer deadline) +static void qt_abstime_for_timeout(timespec *ts, QDeadlineTimer deadline) { using namespace std::chrono; using Clock =