From 67429fbada51a647977c419b2821bc72d78ace54 Mon Sep 17 00:00:00 2001 From: LoRd_MuldeR Date: Fri, 5 Dec 2014 22:39:31 +0100 Subject: [PATCH] Fixed a bug in AbstractTool class that could cause a severe slow-down on process creation, especially with a large number of threads: The same mutex was used in the constructor and the startProcess() function, which is unnecessary anyway. But even worse, there was a sleep() call in the startProcess() function that could *block* the mutex for a very long time! So if the "main" thread tried to create a new object while one of the "worker" threads was sleeping inside startProcess(), this blocked the whole "main" thread. D'oh! --- LameXP_VS2013.sln | 3 +++ doc/Changelog.html | 2 ++ src/Config.h | 4 ++-- src/Tool_Abstract.cpp | 50 +++++++++++++++++-------------------------- src/Tool_Abstract.h | 9 ++++---- 5 files changed, 32 insertions(+), 36 deletions(-) diff --git a/LameXP_VS2013.sln b/LameXP_VS2013.sln index ef0a17e8..4ebe063c 100644 --- a/LameXP_VS2013.sln +++ b/LameXP_VS2013.sln @@ -30,4 +30,7 @@ Global GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE EndGlobalSection + GlobalSection(Performance) = preSolution + HasPerformanceSessions = true + EndGlobalSection EndGlobal diff --git a/doc/Changelog.html b/doc/Changelog.html index 8f70b686..ecb6828e 100644 --- a/doc/Changelog.html +++ b/doc/Changelog.html @@ -27,6 +27,8 @@ a:visited { color: #0000EE; }
  • Updated Vorbis decoder to OggDec v1.10.1 (2014-06-25), using libVorbis v1.3.4
  • Updated GnuPG to v1.4.18 (2014-06-30), compiled with GCC 4.9.1
  • Fixed potential crash in Cue Sheet importer (occurred when *all* input files were missing) +
  • Fixed a severe performance bottleneck, especially with a large number of parallel instances +
  • The limit for the maximum number of parallel instances has been increased to 32
  • Experimental support for Windows 10 Technical Preview
    diff --git a/src/Config.h b/src/Config.h index eb5576d3..8dfa8619 100644 --- a/src/Config.h +++ b/src/Config.h @@ -34,8 +34,8 @@ #define VER_LAMEXP_MINOR_HI 1 #define VER_LAMEXP_MINOR_LO 1 #define VER_LAMEXP_TYPE Beta -#define VER_LAMEXP_PATCH 4 -#define VER_LAMEXP_BUILD 1615 +#define VER_LAMEXP_PATCH 5 +#define VER_LAMEXP_BUILD 1616 #define VER_LAMEXP_CONFG 1558 /////////////////////////////////////////////////////////////////////////////// diff --git a/src/Tool_Abstract.cpp b/src/Tool_Abstract.cpp index a1f7066b..7d7c96ca 100644 --- a/src/Tool_Abstract.cpp +++ b/src/Tool_Abstract.cpp @@ -39,34 +39,33 @@ #include /* - * Static vars + * Job Object */ -quint64 AbstractTool::s_lastLaunchTime = 0ui64; -QMutex AbstractTool::s_mutex_startProcess; -JobObject *AbstractTool::s_jobObject = NULL; -unsigned int AbstractTool::s_jobObjRefCount = 0U; +QScopedPointer AbstractTool::s_jobObject; +QMutex AbstractTool::s_jobObjMtx; + +/* + * Process Timer + */ +quint64 AbstractTool::s_startProcessTimer = 0ui64; +QMutex AbstractTool::s_startProcessMutex; /* * Const */ -static const unsigned int START_DELAY = 333; //in milliseconds -static const quint64 START_DELAY_NANO = START_DELAY * 1000 * 10; //in 100-nanosecond intervals +static const unsigned int START_DELAY = 50U; //in milliseconds +static const quint64 START_DELAY_NANO = quint64(START_DELAY) * 10000ui64; //in 100-nanosecond intervals /* * Constructor */ AbstractTool::AbstractTool(void) { - QMutexLocker lock(&s_mutex_startProcess); + QMutexLocker lock(&s_jobObjMtx); - if(s_jobObjRefCount < 1U) + if(s_jobObject.isNull()) { - s_jobObject = new JobObject(); - s_jobObjRefCount = 1U; - } - else - { - s_jobObjRefCount++; + s_jobObject.reset(new JobObject()); } m_firstLaunch = true; @@ -77,16 +76,6 @@ AbstractTool::AbstractTool(void) */ AbstractTool::~AbstractTool(void) { - QMutexLocker lock(&s_mutex_startProcess); - - if(s_jobObjRefCount >= 1U) - { - s_jobObjRefCount--; - if(s_jobObjRefCount < 1U) - { - MUTILS_DELETE(s_jobObject); - } - } } /* @@ -94,11 +83,13 @@ AbstractTool::~AbstractTool(void) */ bool AbstractTool::startProcess(QProcess &process, const QString &program, const QStringList &args) { - QMutexLocker lock(&s_mutex_startProcess); + QMutexLocker lock(&s_startProcessMutex); - if(MUtils::OS::current_file_time() <= s_lastLaunchTime) + while(MUtils::OS::current_file_time() <= s_startProcessTimer) { + lock.unlock(); MUtils::OS::sleep_ms(START_DELAY); + lock.relock(); } emit messageLogged(commandline2string(program, args) + "\n"); @@ -117,7 +108,6 @@ bool AbstractTool::startProcess(QProcess &process, const QString &program, const } MUtils::OS::change_process_priority(&process, -1); - lock.unlock(); if(m_firstLaunch) { @@ -125,7 +115,7 @@ bool AbstractTool::startProcess(QProcess &process, const QString &program, const m_firstLaunch = false; } - s_lastLaunchTime = MUtils::OS::current_file_time() + START_DELAY_NANO; + s_startProcessTimer = MUtils::OS::current_file_time() + START_DELAY_NANO; return true; } @@ -136,7 +126,7 @@ bool AbstractTool::startProcess(QProcess &process, const QString &program, const process.kill(); process.waitForFinished(-1); - s_lastLaunchTime = MUtils::OS::current_file_time() + START_DELAY_NANO; + s_startProcessTimer = MUtils::OS::current_file_time() + START_DELAY_NANO; return false; } diff --git a/src/Tool_Abstract.h b/src/Tool_Abstract.h index 9595c977..06725576 100644 --- a/src/Tool_Abstract.h +++ b/src/Tool_Abstract.h @@ -47,10 +47,11 @@ protected: static const int m_processTimeoutInterval = 600000; private: - static quint64 s_lastLaunchTime; - static QMutex s_mutex_startProcess; - static unsigned int s_jobObjRefCount; - static JobObject *s_jobObject; + static quint64 s_startProcessTimer; + static QMutex s_startProcessMutex; + + static QScopedPointer s_jobObject; + static QMutex s_jobObjMtx; bool m_firstLaunch; };