From 616091b38d38fb101ab4ad5c54c9dcd902945819 Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Tue, 14 May 2024 12:37:47 +0200 Subject: [PATCH 1/8] refactor(util): Modernize & shrink ScopedTimer internals Instead of emulating an optional by use of a pointer and placement-`new` construction, use a `std::optional` directly. Also remove `m_cancel` as the empty optional is sufficient for indicating a cancelled timer. Removing these members shrunk the memory footprint from 72 to 56 bytes, reducing the memory overhead of ScopedTimer compared to the underyling Timer to 8 bytes (Timer currently being 48 bytes large). While these improvements are certainly nice, the purpose of this PR is the improved underlying code. However, there is still lots that could be improved in the future. --- src/util/timer.h | 58 +++++++++++++++++++++--------------------------- 1 file changed, 25 insertions(+), 33 deletions(-) diff --git a/src/util/timer.h b/src/util/timer.h index 8be1adf939f..37f27d5bef2 100644 --- a/src/util/timer.h +++ b/src/util/timer.h @@ -1,6 +1,8 @@ #pragma once #include +#include +#include #include "control/controlproxy.h" #include "util/cmdlineargs.h" @@ -52,61 +54,51 @@ class SuspendableTimer : public Timer { class ScopedTimer { public: - ScopedTimer(const char* key, int i, - Stat::ComputeFlags compute = kDefaultComputeFlags) - : m_pTimer(NULL), - m_cancel(false) { + ScopedTimer(const char* key, int i, Stat::ComputeFlags compute = kDefaultComputeFlags) + : m_timer(std::nullopt) { if (CmdlineArgs::Instance().getDeveloper()) { initialize(QString(key), QString::number(i), compute); } } - ScopedTimer(const char* key, const char *arg = NULL, - Stat::ComputeFlags compute = kDefaultComputeFlags) - : m_pTimer(NULL), - m_cancel(false) { + ScopedTimer(const char* key, + const char* arg = nullptr, + Stat::ComputeFlags compute = kDefaultComputeFlags) + : m_timer(std::nullopt) { if (CmdlineArgs::Instance().getDeveloper()) { - initialize(QString(key), arg ? QString(arg) : QString(), compute); + initialize(QString(key), arg != nullptr ? QString(arg) : QString(), compute); } } - ScopedTimer(const char* key, const QString& arg, - Stat::ComputeFlags compute = kDefaultComputeFlags) - : m_pTimer(NULL), - m_cancel(false) { + ScopedTimer(const char* key, + const QString& arg, + Stat::ComputeFlags compute = kDefaultComputeFlags) + : m_timer(std::nullopt) { if (CmdlineArgs::Instance().getDeveloper()) { initialize(QString(key), arg, compute); } } - virtual ~ScopedTimer() { - if (m_pTimer) { - if (!m_cancel) { - m_pTimer->elapsed(true); - } - m_pTimer->~Timer(); + ~ScopedTimer() { + if (m_timer.has_value()) { + m_timer->elapsed(true); } } - inline void initialize(const QString& key, const QString& arg, - Stat::ComputeFlags compute = kDefaultComputeFlags) { - QString strKey; - if (arg.isEmpty()) { - strKey = key; - } else { - strKey = key.arg(arg); - } - m_pTimer = new(m_timerMem) Timer(strKey, compute); - m_pTimer->start(); + void initialize(const QString& key, + const QString& arg, + Stat::ComputeFlags compute = kDefaultComputeFlags) { + QString strKey = arg.isEmpty() ? key : key.arg(arg); + m_timer = std::make_optional(strKey, compute); + m_timer->start(); } void cancel() { - m_cancel = true; + m_timer.reset(); } private: - Timer* m_pTimer; - char m_timerMem[sizeof(Timer)]; - bool m_cancel; + // nullopt also counts as cancelled + std::optional m_timer; }; // A timer that provides a similar API to QTimer but uses render events from the From b5aa4009ac9021c06b96375676dbfc424e11c0ed Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Tue, 14 May 2024 14:45:53 +0200 Subject: [PATCH 2/8] feat: ScopedTimer ctor accept `QString::arg-compatible` formatstring --- src/util/timer.h | 49 ++++++++++++-------------- src/waveform/waveformwidgetfactory.cpp | 4 +-- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/src/util/timer.h b/src/util/timer.h index 37f27d5bef2..5598cdf9331 100644 --- a/src/util/timer.h +++ b/src/util/timer.h @@ -3,6 +3,7 @@ #include #include #include +#include #include "control/controlproxy.h" #include "util/cmdlineargs.h" @@ -54,29 +55,33 @@ class SuspendableTimer : public Timer { class ScopedTimer { public: - ScopedTimer(const char* key, int i, Stat::ComputeFlags compute = kDefaultComputeFlags) + // Allows the timer to contain a format string which is only assembled + // when we're not in `--developer` mode. + /// @param compute Flags to use for the Stat::ComputeFlags (can be omitted) + /// @param key The format string for the key + /// @param args The arguments to pass to the format string + template + ScopedTimer(Stat::ComputeFlags compute, const QString& key, Ts&&... args) : m_timer(std::nullopt) { if (CmdlineArgs::Instance().getDeveloper()) { - initialize(QString(key), QString::number(i), compute); + QString assembledKey = key; + if constexpr (sizeof...(args) > 0) { + // only try to call QString::arg when we've been given parameters + assembledKey = key.arg(std::forward(args)...); + } + m_timer = std::make_optional(assembledKey, compute); + m_timer->start(); } } - - ScopedTimer(const char* key, - const char* arg = nullptr, - Stat::ComputeFlags compute = kDefaultComputeFlags) - : m_timer(std::nullopt) { - if (CmdlineArgs::Instance().getDeveloper()) { - initialize(QString(key), arg != nullptr ? QString(arg) : QString(), compute); - } + template + ScopedTimer(const QString& key, Ts&&... args) + : ScopedTimer(kDefaultComputeFlags, key, std::forward(args)...) { } - - ScopedTimer(const char* key, - const QString& arg, - Stat::ComputeFlags compute = kDefaultComputeFlags) - : m_timer(std::nullopt) { - if (CmdlineArgs::Instance().getDeveloper()) { - initialize(QString(key), arg, compute); - } + // for lazy users that don't wrap the key in a QStringLiteral(...) + template + ScopedTimer(const char* key, Ts&&... args) + : ScopedTimer(CmdlineArgs::Instance().getDeveloper() ? QString(key) : QString(), + std::forward(args)...) { } ~ScopedTimer() { @@ -85,14 +90,6 @@ class ScopedTimer { } } - void initialize(const QString& key, - const QString& arg, - Stat::ComputeFlags compute = kDefaultComputeFlags) { - QString strKey = arg.isEmpty() ? key : key.arg(arg); - m_timer = std::make_optional(strKey, compute); - m_timer->start(); - } - void cancel() { m_timer.reset(); } diff --git a/src/waveform/waveformwidgetfactory.cpp b/src/waveform/waveformwidgetfactory.cpp index c3b9e2ddbbe..ee3295ff8f7 100644 --- a/src/waveform/waveformwidgetfactory.cpp +++ b/src/waveform/waveformwidgetfactory.cpp @@ -688,8 +688,8 @@ void WaveformWidgetFactory::notifyZoomChange(WWaveformViewer* viewer) { } void WaveformWidgetFactory::renderSelf() { - ScopedTimer t("WaveformWidgetFactory::render() %1waveforms", - static_cast(m_waveformWidgetHolders.size())); + ScopedTimer t("WaveformWidgetFactory::render() %1 waveforms", + QString::number(m_waveformWidgetHolders.size())); if (!m_skipRender) { if (m_type) { // no regular updates for an empty waveform From e4263ae6a5ebb47b1365490b658afa8aa211e939 Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Wed, 15 May 2024 14:28:53 +0200 Subject: [PATCH 3/8] feat: ScopedTimer accept numeric arguments as formatstring arguments --- src/util/qstringformat.h | 42 ++++++++++++++++++++++++++ src/util/timer.h | 36 +++++++++++----------- src/waveform/waveformwidgetfactory.cpp | 2 +- 3 files changed, 60 insertions(+), 20 deletions(-) create mode 100644 src/util/qstringformat.h diff --git a/src/util/qstringformat.h b/src/util/qstringformat.h new file mode 100644 index 00000000000..ed39ba4d451 --- /dev/null +++ b/src/util/qstringformat.h @@ -0,0 +1,42 @@ +#pragma once + +#include +#include +#include + +namespace { + +// taken from Qt +template +static constexpr bool is_convertible_to_view_or_qstring_v = + std::is_convertible_v || + std::is_convertible_v || + std::is_convertible_v; + +// check if we can call QString::number(T) with T +template +static constexpr bool is_number_compatible_v = + std::is_invocable_v()))(T), T>; + +// always false, used for static_assert and workaround for compilers without +// https://cplusplus.github.io/CWG/issues/2518.html +template +static constexpr bool always_false_v = false; +} // namespace + +// Try to convert T to a type that would be accepted by QString::args(Args&&...) +template +auto convertToQStringConvertible(T&& arg) { + if constexpr (is_convertible_to_view_or_qstring_v) { + // no need to do anything, just return verbatim + return std::forward(arg); + } else if constexpr (is_number_compatible_v) { + return QString::number(std::forward(arg)); + } else { + static_assert(always_false_v, "Unsupported type for QString::arg"); + // unreachable, but returning a QString results in a better error message + // because the log won't be spammed with all the QString::arg overloads + // it couldn't match with `void`. + return QString(); + } +} diff --git a/src/util/timer.h b/src/util/timer.h index 5598cdf9331..3eba46837c2 100644 --- a/src/util/timer.h +++ b/src/util/timer.h @@ -10,6 +10,7 @@ #include "util/duration.h" #include "util/parented_ptr.h" #include "util/performancetimer.h" +#include "util/qstringformat.h" #include "util/stat.h" const Stat::ComputeFlags kDefaultComputeFlags = Stat::COUNT | Stat::SUM | Stat::AVERAGE | @@ -60,28 +61,25 @@ class ScopedTimer { /// @param compute Flags to use for the Stat::ComputeFlags (can be omitted) /// @param key The format string for the key /// @param args The arguments to pass to the format string - template - ScopedTimer(Stat::ComputeFlags compute, const QString& key, Ts&&... args) + template + ScopedTimer(Stat::ComputeFlags compute, T&& key, Ts&&... args) : m_timer(std::nullopt) { - if (CmdlineArgs::Instance().getDeveloper()) { - QString assembledKey = key; - if constexpr (sizeof...(args) > 0) { - // only try to call QString::arg when we've been given parameters - assembledKey = key.arg(std::forward(args)...); - } - m_timer = std::make_optional(assembledKey, compute); - m_timer->start(); + if (!CmdlineArgs::Instance().getDeveloper()) { + return; // leave timer in cancelled state } + // key is explicitly `auto` to allow passing non-QString types such as `const char*` + // its conversion is delayed until after we've checked if we're in developer mode + auto assembledKey = QString(std::forward(key)); + if constexpr (sizeof...(args) > 0) { + // only try to call QString::arg when we've been given parameters + assembledKey = assembledKey.arg(convertToQStringConvertible(std::forward(args))...); + } + m_timer = std::make_optional(assembledKey, compute); + m_timer->start(); } - template - ScopedTimer(const QString& key, Ts&&... args) - : ScopedTimer(kDefaultComputeFlags, key, std::forward(args)...) { - } - // for lazy users that don't wrap the key in a QStringLiteral(...) - template - ScopedTimer(const char* key, Ts&&... args) - : ScopedTimer(CmdlineArgs::Instance().getDeveloper() ? QString(key) : QString(), - std::forward(args)...) { + template + ScopedTimer(T&& key, Ts&&... args) + : ScopedTimer(kDefaultComputeFlags, std::forward(key), std::forward(args)...) { } ~ScopedTimer() { diff --git a/src/waveform/waveformwidgetfactory.cpp b/src/waveform/waveformwidgetfactory.cpp index ee3295ff8f7..5c71a28e73f 100644 --- a/src/waveform/waveformwidgetfactory.cpp +++ b/src/waveform/waveformwidgetfactory.cpp @@ -689,7 +689,7 @@ void WaveformWidgetFactory::notifyZoomChange(WWaveformViewer* viewer) { void WaveformWidgetFactory::renderSelf() { ScopedTimer t("WaveformWidgetFactory::render() %1 waveforms", - QString::number(m_waveformWidgetHolders.size())); + m_waveformWidgetHolders.size()); if (!m_skipRender) { if (m_type) { // no regular updates for an empty waveform From 2ea4c4fbc08fc8309a5806fe28de2051388bf5fa Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Fri, 17 May 2024 12:59:25 +0200 Subject: [PATCH 4/8] convert ScopedTimer constructions to use QStringLiteral to improve performance --- src/analyzer/analyzerebur128.cpp | 2 +- src/analyzer/analyzergain.cpp | 2 +- src/coreservices.cpp | 4 ++-- src/engine/channelmixer.cpp | 4 ++-- src/engine/enginebuffer.cpp | 2 +- src/engine/enginemixer.cpp | 2 +- src/library/dao/trackdao.cpp | 2 +- src/library/scanner/importfilestask.cpp | 2 +- src/library/scanner/libraryscanner.cpp | 10 +++++----- src/library/scanner/recursivescandirectorytask.cpp | 2 +- src/mixxxmainwindow.cpp | 4 ++-- src/skin/legacy/legacyskinparser.cpp | 2 +- src/skin/skinloader.cpp | 2 +- src/sources/soundsourcemodplug.cpp | 2 +- src/vinylcontrol/vinylcontrolprocessor.cpp | 2 +- src/vinylcontrol/vinylcontrolxwax.cpp | 2 +- src/waveform/renderers/waveformrendererendoftrack.cpp | 2 +- src/widget/woverview.cpp | 2 +- src/widget/woverviewhsv.cpp | 2 +- src/widget/woverviewlmh.cpp | 2 +- src/widget/woverviewrgb.cpp | 2 +- src/widget/wvumeterbase.cpp | 2 +- src/widget/wvumeterlegacy.cpp | 2 +- 23 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/analyzer/analyzerebur128.cpp b/src/analyzer/analyzerebur128.cpp index b46b5e20de0..5c5510f8416 100644 --- a/src/analyzer/analyzerebur128.cpp +++ b/src/analyzer/analyzerebur128.cpp @@ -49,7 +49,7 @@ bool AnalyzerEbur128::processSamples(const CSAMPLE* pIn, SINT count) { VERIFY_OR_DEBUG_ASSERT(m_pState) { return false; } - ScopedTimer t("AnalyzerEbur128::processSamples()"); + ScopedTimer t(QStringLiteral("AnalyzerEbur128::processSamples()")); size_t frames = count / mixxx::kAnalysisChannels; int e = ebur128_add_frames_float(m_pState, pIn, frames); VERIFY_OR_DEBUG_ASSERT(e == EBUR128_SUCCESS) { diff --git a/src/analyzer/analyzergain.cpp b/src/analyzer/analyzergain.cpp index d9823875de7..16315baa35d 100644 --- a/src/analyzer/analyzergain.cpp +++ b/src/analyzer/analyzergain.cpp @@ -37,7 +37,7 @@ void AnalyzerGain::cleanup() { } bool AnalyzerGain::processSamples(const CSAMPLE* pIn, SINT count) { - ScopedTimer t("AnalyzerGain::process()"); + ScopedTimer t(QStringLiteral("AnalyzerGain::process()")); SINT numFrames = count / mixxx::kAnalysisChannels; if (numFrames > static_cast(m_pLeftTempBuffer.size())) { diff --git a/src/coreservices.cpp b/src/coreservices.cpp index e8387058db9..e4a830f3a26 100644 --- a/src/coreservices.cpp +++ b/src/coreservices.cpp @@ -111,7 +111,7 @@ CoreServices::CoreServices(const CmdlineArgs& args, QApplication* pApp) m_isInitialized(false) { m_runtime_timer.start(); mixxx::Time::start(); - ScopedTimer t("CoreServices::CoreServices"); + ScopedTimer t(QStringLiteral("CoreServices::CoreServices")); // All this here is running without without start up screen // Defer long initializations to CoreServices::initialize() which is // called after the GUI is initialized @@ -211,7 +211,7 @@ void CoreServices::initialize(QApplication* pApp) { return; } - ScopedTimer t("CoreServices::initialize"); + ScopedTimer t(QStringLiteral("CoreServices::initialize")); VERIFY_OR_DEBUG_ASSERT(SoundSourceProxy::registerProviders()) { qCritical() << "Failed to register any SoundSource providers"; diff --git a/src/engine/channelmixer.cpp b/src/engine/channelmixer.cpp index fa077d14512..9a5a1468350 100644 --- a/src/engine/channelmixer.cpp +++ b/src/engine/channelmixer.cpp @@ -23,7 +23,7 @@ void ChannelMixer::applyEffectsAndMixChannels(const EngineMixer::GainCalculator& // D) Mixes the temporary buffer into pOutput // The original channel input buffers are not modified. SampleUtil::clear(pOutput, iBufferSize); - ScopedTimer t("EngineMixer::applyEffectsAndMixChannels"); + ScopedTimer t(QStringLiteral("EngineMixer::applyEffectsAndMixChannels")); for (auto* pChannelInfo : activeChannels) { EngineMixer::GainCache& gainCache = (*channelGainCache)[pChannelInfo->m_index]; CSAMPLE_GAIN oldGain = gainCache.m_gain; @@ -68,7 +68,7 @@ void ChannelMixer::applyEffectsInPlaceAndMixChannels( // A) Applies the calculated gain to the channel buffer, modifying the original input buffer // B) Applies effects to the buffer, modifying the original input buffer // 4. Mix the channel buffers together to make pOutput, overwriting the pOutput buffer from the last engine callback - ScopedTimer t("EngineMixer::applyEffectsInPlaceAndMixChannels"); + ScopedTimer t(QStringLiteral("EngineMixer::applyEffectsInPlaceAndMixChannels")); SampleUtil::clear(pOutput, iBufferSize); for (auto* pChannelInfo : activeChannels) { EngineMixer::GainCache& gainCache = (*channelGainCache)[pChannelInfo->m_index]; diff --git a/src/engine/enginebuffer.cpp b/src/engine/enginebuffer.cpp index c7dce1c6056..7e2447d8dee 100644 --- a/src/engine/enginebuffer.cpp +++ b/src/engine/enginebuffer.cpp @@ -838,7 +838,7 @@ void EngineBuffer::slotKeylockEngineChanged(double dIndex) { void EngineBuffer::processTrackLocked( CSAMPLE* pOutput, const int iBufferSize, mixxx::audio::SampleRate sampleRate) { - ScopedTimer t("EngineBuffer::process_pauselock"); + ScopedTimer t(QStringLiteral("EngineBuffer::process_pauselock")); m_trackSampleRateOld = mixxx::audio::SampleRate::fromDouble(m_pTrackSampleRate->get()); m_trackEndPositionOld = getTrackEndPosition(); diff --git a/src/engine/enginemixer.cpp b/src/engine/enginemixer.cpp index c7eda1a7d97..1587f071cd0 100644 --- a/src/engine/enginemixer.cpp +++ b/src/engine/enginemixer.cpp @@ -291,7 +291,7 @@ void EngineMixer::processChannels(int iBufferSize) { m_activeTalkoverChannels.clear(); m_activeChannels.clear(); - // ScopedTimer timer("EngineMixer::processChannels"); + // ScopedTimer timer(QStringLiteral("EngineMixer::processChannels")); EngineChannel* pLeaderChannel = m_pEngineSync->getLeaderChannel(); // Reserve the first place for the main channel which // should be processed first diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index 32b3206d623..500e793c3fe 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -1368,7 +1368,7 @@ TrackPointer TrackDAO::getTrackById(TrackId trackId) const { // be executed with a lock on the GlobalTrackCache. The GlobalTrackCache // will be locked again after the query has been executed (see below) // and potential race conditions will be resolved. - ScopedTimer t("TrackDAO::getTrackById"); + ScopedTimer t(QStringLiteral("TrackDAO::getTrackById")); QSqlRecord queryRecord; { diff --git a/src/library/scanner/importfilestask.cpp b/src/library/scanner/importfilestask.cpp index 6f4b6fede3c..0d8311b2db1 100644 --- a/src/library/scanner/importfilestask.cpp +++ b/src/library/scanner/importfilestask.cpp @@ -21,7 +21,7 @@ ImportFilesTask::ImportFilesTask(LibraryScanner* pScanner, } void ImportFilesTask::run() { - ScopedTimer timer("ImportFilesTask::run"); + ScopedTimer timer(QStringLiteral("ImportFilesTask::run")); for (const QFileInfo& fileInfo: m_filesToImport) { // If a flag was raised telling us to cancel the library scan then stop. if (m_scannerGlobal->shouldCancel()) { diff --git a/src/library/scanner/libraryscanner.cpp b/src/library/scanner/libraryscanner.cpp index 266a79d65f3..fe60f2be499 100644 --- a/src/library/scanner/libraryscanner.cpp +++ b/src/library/scanner/libraryscanner.cpp @@ -489,7 +489,7 @@ void LibraryScanner::cancel() { void LibraryScanner::queueTask(ScannerTask* pTask) { //kLogger.debug() << "queueTask" << pTask; - ScopedTimer timer("LibraryScanner::queueTask"); + ScopedTimer timer(QStringLiteral("LibraryScanner::queueTask")); if (m_scannerGlobal.isNull() || m_scannerGlobal->shouldCancel()) { return; } @@ -531,7 +531,7 @@ void LibraryScanner::queueTask(ScannerTask* pTask) { void LibraryScanner::slotDirectoryHashedAndScanned(const QString& directoryPath, bool newDirectory, mixxx::cache_key_t hash) { - ScopedTimer timer("LibraryScanner::slotDirectoryHashedAndScanned"); + ScopedTimer timer(QStringLiteral("LibraryScanner::slotDirectoryHashedAndScanned")); //kLogger.debug() << "sloDirectoryHashedAndScanned" << directoryPath // << newDirectory << hash; @@ -550,7 +550,7 @@ void LibraryScanner::slotDirectoryHashedAndScanned(const QString& directoryPath, } void LibraryScanner::slotDirectoryUnchanged(const QString& directoryPath) { - ScopedTimer timer("LibraryScanner::slotDirectoryUnchanged"); + ScopedTimer timer(QStringLiteral("LibraryScanner::slotDirectoryUnchanged")); //kLogger.debug() << "slotDirectoryUnchanged" << directoryPath; if (m_scannerGlobal) { m_scannerGlobal->addVerifiedDirectory(directoryPath); @@ -560,7 +560,7 @@ void LibraryScanner::slotDirectoryUnchanged(const QString& directoryPath) { void LibraryScanner::slotTrackExists(const QString& trackPath) { //kLogger.debug() << "slotTrackExists" << trackPath; - ScopedTimer timer("LibraryScanner::slotTrackExists"); + ScopedTimer timer(QStringLiteral("LibraryScanner::slotTrackExists")); if (m_scannerGlobal) { m_scannerGlobal->addVerifiedTrack(trackPath); } @@ -568,7 +568,7 @@ void LibraryScanner::slotTrackExists(const QString& trackPath) { void LibraryScanner::slotAddNewTrack(const QString& trackPath) { //kLogger.debug() << "slotAddNewTrack" << trackPath; - ScopedTimer timer("LibraryScanner::addNewTrack"); + ScopedTimer timer(QStringLiteral("LibraryScanner::addNewTrack")); // For statistics tracking and to detect moved tracks TrackPointer pTrack = m_trackDao.addTracksAddFile( trackPath, diff --git a/src/library/scanner/recursivescandirectorytask.cpp b/src/library/scanner/recursivescandirectorytask.cpp index 0d64d9b4a68..5a2a4e97592 100644 --- a/src/library/scanner/recursivescandirectorytask.cpp +++ b/src/library/scanner/recursivescandirectorytask.cpp @@ -20,7 +20,7 @@ RecursiveScanDirectoryTask::RecursiveScanDirectoryTask( } void RecursiveScanDirectoryTask::run() { - ScopedTimer timer("RecursiveScanDirectoryTask::run"); + ScopedTimer timer(QStringLiteral("RecursiveScanDirectoryTask::run")); if (m_scannerGlobal->shouldCancel()) { setSuccess(false); return; diff --git a/src/mixxxmainwindow.cpp b/src/mixxxmainwindow.cpp index 980b04b6d47..6ab5e28208f 100644 --- a/src/mixxxmainwindow.cpp +++ b/src/mixxxmainwindow.cpp @@ -673,7 +673,7 @@ void MixxxMainWindow::slotUpdateWindowTitle(TrackPointer pTrack) { } void MixxxMainWindow::createMenuBar() { - ScopedTimer t("MixxxMainWindow::createMenuBar"); + ScopedTimer t(QStringLiteral("MixxxMainWindow::createMenuBar")); DEBUG_ASSERT(m_pCoreServices->getKeyboardConfig()); m_pMenuBar = make_parented( this, m_pCoreServices->getSettings(), m_pCoreServices->getKeyboardConfig().get()); @@ -687,7 +687,7 @@ void MixxxMainWindow::connectMenuBar() { // This function might be invoked multiple times on startup // so all connections must be unique! - ScopedTimer t("MixxxMainWindow::connectMenuBar"); + ScopedTimer t(QStringLiteral("MixxxMainWindow::connectMenuBar")); connect(this, &MixxxMainWindow::skinLoaded, m_pMenuBar, diff --git a/src/skin/legacy/legacyskinparser.cpp b/src/skin/legacy/legacyskinparser.cpp index c87b21aca78..e2a1d0052c4 100644 --- a/src/skin/legacy/legacyskinparser.cpp +++ b/src/skin/legacy/legacyskinparser.cpp @@ -324,7 +324,7 @@ Qt::MouseButton LegacySkinParser::parseButtonState(const QDomNode& node, } QWidget* LegacySkinParser::parseSkin(const QString& skinPath, QWidget* pParent) { - ScopedTimer timer("SkinLoader::parseSkin"); + ScopedTimer timer(QStringLiteral("SkinLoader::parseSkin")); qDebug() << "LegacySkinParser loading skin:" << skinPath; m_pContext = std::make_unique(m_pConfig, skinPath + "/skin.xml"); diff --git a/src/skin/skinloader.cpp b/src/skin/skinloader.cpp index 280f4b922a9..a91a2fda62b 100644 --- a/src/skin/skinloader.cpp +++ b/src/skin/skinloader.cpp @@ -131,7 +131,7 @@ QString SkinLoader::getDefaultSkinName() const { QWidget* SkinLoader::loadConfiguredSkin(QWidget* pParent, QSet* pSkinCreatedControls, mixxx::CoreServices* pCoreServices) { - ScopedTimer timer("SkinLoader::loadConfiguredSkin"); + ScopedTimer timer(QStringLiteral("SkinLoader::loadConfiguredSkin")); SkinPointer pSkin = getConfiguredSkin(); // If we don't have a skin then fail. This makes sense here, because the diff --git a/src/sources/soundsourcemodplug.cpp b/src/sources/soundsourcemodplug.cpp index daab349513b..7fcb0f9289c 100644 --- a/src/sources/soundsourcemodplug.cpp +++ b/src/sources/soundsourcemodplug.cpp @@ -117,7 +117,7 @@ SoundSourceModPlug::importTrackMetadataAndCoverImage( SoundSource::OpenResult SoundSourceModPlug::tryOpen( OpenMode /*mode*/, const OpenParams& /*config*/) { - ScopedTimer t("SoundSourceModPlug::open()"); + ScopedTimer t(QStringLiteral("SoundSourceModPlug::open()")); // read module file to byte array const QString fileName(getLocalFileName()); diff --git a/src/vinylcontrol/vinylcontrolprocessor.cpp b/src/vinylcontrol/vinylcontrolprocessor.cpp index 649d6800144..0c8ae04b6ba 100644 --- a/src/vinylcontrol/vinylcontrolprocessor.cpp +++ b/src/vinylcontrol/vinylcontrolprocessor.cpp @@ -204,7 +204,7 @@ bool VinylControlProcessor::deckConfigured(int index) const { void VinylControlProcessor::receiveBuffer(const AudioInput& input, const CSAMPLE* pBuffer, unsigned int nFrames) { - ScopedTimer t("VinylControlProcessor::receiveBuffer"); + ScopedTimer t(QStringLiteral("VinylControlProcessor::receiveBuffer")); if (input.getType() != AudioPathType::VinylControl) { qDebug() << "WARNING: AudioInput type is not VINYLCONTROL. Ignoring incoming buffer."; return; diff --git a/src/vinylcontrol/vinylcontrolxwax.cpp b/src/vinylcontrol/vinylcontrolxwax.cpp index 9de8c5d4c33..971724c9751 100644 --- a/src/vinylcontrol/vinylcontrolxwax.cpp +++ b/src/vinylcontrol/vinylcontrolxwax.cpp @@ -200,7 +200,7 @@ bool VinylControlXwax::writeQualityReport(VinylSignalQualityReport* pReport) { void VinylControlXwax::analyzeSamples(CSAMPLE* pSamples, size_t nFrames) { - ScopedTimer t("VinylControlXwax::analyzeSamples"); + ScopedTimer t(QStringLiteral("VinylControlXwax::analyzeSamples")); auto gain = static_cast(m_pVinylControlInputGain->get()); // We only support amplifying with the VC pre-amp. diff --git a/src/waveform/renderers/waveformrendererendoftrack.cpp b/src/waveform/renderers/waveformrendererendoftrack.cpp index b491c87324b..330d0101fcc 100644 --- a/src/waveform/renderers/waveformrendererendoftrack.cpp +++ b/src/waveform/renderers/waveformrendererendoftrack.cpp @@ -57,7 +57,7 @@ void WaveformRendererEndOfTrack::draw(QPainter* painter, return; } - //ScopedTimer t("WaveformRendererEndOfTrack::draw"); + // ScopedTimer t(QStringLiteral("WaveformRendererEndOfTrack::draw")); const int elapsed = m_timer.elapsed().toIntegerMillis() % kBlinkingPeriodMillis; diff --git a/src/widget/woverview.cpp b/src/widget/woverview.cpp index 6fa681aea3e..2b18073f32e 100644 --- a/src/widget/woverview.cpp +++ b/src/widget/woverview.cpp @@ -577,7 +577,7 @@ void WOverview::leaveEvent(QEvent* pEvent) { void WOverview::paintEvent(QPaintEvent* pEvent) { Q_UNUSED(pEvent); - ScopedTimer t("WOverview::paintEvent"); + ScopedTimer t(QStringLiteral("WOverview::paintEvent")); QPainter painter(this); painter.fillRect(rect(), m_backgroundColor); diff --git a/src/widget/woverviewhsv.cpp b/src/widget/woverviewhsv.cpp index f7873db3f37..44b5f5ff5c3 100644 --- a/src/widget/woverviewhsv.cpp +++ b/src/widget/woverviewhsv.cpp @@ -18,7 +18,7 @@ WOverviewHSV::WOverviewHSV( } bool WOverviewHSV::drawNextPixmapPart() { - ScopedTimer t("WOverviewHSV::drawNextPixmapPart"); + ScopedTimer t(QStringLiteral("WOverviewHSV::drawNextPixmapPart")); //qDebug() << "WOverview::drawNextPixmapPart()"; diff --git a/src/widget/woverviewlmh.cpp b/src/widget/woverviewlmh.cpp index df43cd9469b..30f2f4aad01 100644 --- a/src/widget/woverviewlmh.cpp +++ b/src/widget/woverviewlmh.cpp @@ -18,7 +18,7 @@ WOverviewLMH::WOverviewLMH( } bool WOverviewLMH::drawNextPixmapPart() { - ScopedTimer t("WOverviewLMH::drawNextPixmapPart"); + ScopedTimer t(QStringLiteral("WOverviewLMH::drawNextPixmapPart")); //qDebug() << "WOverview::drawNextPixmapPart()"; diff --git a/src/widget/woverviewrgb.cpp b/src/widget/woverviewrgb.cpp index acd956aa3bc..e9e03145377 100644 --- a/src/widget/woverviewrgb.cpp +++ b/src/widget/woverviewrgb.cpp @@ -17,7 +17,7 @@ WOverviewRGB::WOverviewRGB( } bool WOverviewRGB::drawNextPixmapPart() { - ScopedTimer t("WOverviewRGB::drawNextPixmapPart"); + ScopedTimer t(QStringLiteral("WOverviewRGB::drawNextPixmapPart")); //qDebug() << "WOverview::drawNextPixmapPart()"; diff --git a/src/widget/wvumeterbase.cpp b/src/widget/wvumeterbase.cpp index 2de27c012ba..dca9eb122a0 100644 --- a/src/widget/wvumeterbase.cpp +++ b/src/widget/wvumeterbase.cpp @@ -181,7 +181,7 @@ void WVuMeterBase::render(VSyncThread* vSyncThread) { return; } - ScopedTimer t("WVuMeterBase::render"); + ScopedTimer t(QStringLiteral("WVuMeterBase::render")); updateState(vSyncThread->sinceLastSwap()); diff --git a/src/widget/wvumeterlegacy.cpp b/src/widget/wvumeterlegacy.cpp index 951d4cfd951..97a065b0ae9 100644 --- a/src/widget/wvumeterlegacy.cpp +++ b/src/widget/wvumeterlegacy.cpp @@ -165,7 +165,7 @@ void WVuMeterLegacy::showEvent(QShowEvent* e) { } void WVuMeterLegacy::paintEvent(QPaintEvent* /*unused*/) { - ScopedTimer t("WVuMeterLegacy::paintEvent"); + ScopedTimer t(QStringLiteral("WVuMeterLegacy::paintEvent")); QPainter p(this); From e25bc2ca998e52016cc0df68bc8142c4ea4878ce Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Fri, 17 May 2024 13:02:17 +0200 Subject: [PATCH 5/8] feat(scopedtimer): forbid inefficient construction from non-utf16 string --- src/util/timer.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/util/timer.h b/src/util/timer.h index 3eba46837c2..1a016d30d2e 100644 --- a/src/util/timer.h +++ b/src/util/timer.h @@ -3,6 +3,7 @@ #include #include #include +#include #include #include "control/controlproxy.h" @@ -64,6 +65,9 @@ class ScopedTimer { template ScopedTimer(Stat::ComputeFlags compute, T&& key, Ts&&... args) : m_timer(std::nullopt) { + static_assert(char_type_size() == sizeof(QString::value_type), + "string type likely not UTF-16, please pass QStringLiteral by " + "means of u\"key text\"_s"); if (!CmdlineArgs::Instance().getDeveloper()) { return; // leave timer in cancelled state } @@ -92,6 +96,19 @@ class ScopedTimer { m_timer.reset(); } private: + // utility function + template + constexpr static std::size_t char_type_size() { + if constexpr (std::is_array_v) { + return sizeof(std::remove_all_extents_t); + } else if constexpr (std::is_pointer_v) { + // assume this is some point to char array then. + return sizeof(std::remove_pointer_t); + } else { + // assume this is some QString or std::string type + return sizeof(typename T::value_type); + } + } // nullopt also counts as cancelled std::optional m_timer; }; From 1bf5bce428c4831685f517cc6a003e7234f41c7a Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Fri, 17 May 2024 13:05:38 +0200 Subject: [PATCH 6/8] fixup! convert ScopedTimer constructions to use QStringLiteral to improve performance --- src/soundio/sounddevicenetwork.cpp | 2 +- src/soundio/sounddeviceportaudio.cpp | 6 +++--- src/waveform/waveformwidgetfactory.cpp | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/soundio/sounddevicenetwork.cpp b/src/soundio/sounddevicenetwork.cpp index 4ae8bd1c965..80d65c769b4 100644 --- a/src/soundio/sounddevicenetwork.cpp +++ b/src/soundio/sounddevicenetwork.cpp @@ -486,7 +486,7 @@ void SoundDeviceNetwork::callbackProcessClkRef() { m_pSoundManager->readProcess(framesPerBuffer); { - ScopedTimer t("SoundDevicePortAudio::callbackProcess prepare %1", + ScopedTimer t(QStringLiteral("SoundDevicePortAudio::callbackProcess prepare %1"), m_deviceId.name); m_pSoundManager->onDeviceOutputCallback(framesPerBuffer); } diff --git a/src/soundio/sounddeviceportaudio.cpp b/src/soundio/sounddeviceportaudio.cpp index 4a8f5e19d59..88b20481c5c 100644 --- a/src/soundio/sounddeviceportaudio.cpp +++ b/src/soundio/sounddeviceportaudio.cpp @@ -976,7 +976,7 @@ int SoundDevicePortAudio::callbackProcessClkRef( // Send audio from the soundcard's input off to the SoundManager... if (in) { - ScopedTimer t("SoundDevicePortAudio::callbackProcess input %1", + ScopedTimer t(QStringLiteral("SoundDevicePortAudio::callbackProcess input %1"), m_deviceId.debugName()); composeInputBuffer(in, framesPerBuffer, 0, m_inputParams.channelCount); m_pSoundManager->pushInputBuffers(m_audioInputs, framesPerBuffer); @@ -985,13 +985,13 @@ int SoundDevicePortAudio::callbackProcessClkRef( m_pSoundManager->readProcess(framesPerBuffer); { - ScopedTimer t("SoundDevicePortAudio::callbackProcess prepare %1", + ScopedTimer t(QStringLiteral("SoundDevicePortAudio::callbackProcess prepare %1"), m_deviceId.debugName()); m_pSoundManager->onDeviceOutputCallback(framesPerBuffer); } if (out) { - ScopedTimer t("SoundDevicePortAudio::callbackProcess output %1", + ScopedTimer t(QStringLiteral("SoundDevicePortAudio::callbackProcess output %1"), m_deviceId.debugName()); if (m_outputParams.channelCount <= 0) { diff --git a/src/waveform/waveformwidgetfactory.cpp b/src/waveform/waveformwidgetfactory.cpp index 5c71a28e73f..6d1012b3d11 100644 --- a/src/waveform/waveformwidgetfactory.cpp +++ b/src/waveform/waveformwidgetfactory.cpp @@ -688,7 +688,7 @@ void WaveformWidgetFactory::notifyZoomChange(WWaveformViewer* viewer) { } void WaveformWidgetFactory::renderSelf() { - ScopedTimer t("WaveformWidgetFactory::render() %1 waveforms", + ScopedTimer t(QStringLiteral("WaveformWidgetFactory::render() %1 waveforms"), m_waveformWidgetHolders.size()); if (!m_skipRender) { @@ -762,7 +762,7 @@ void WaveformWidgetFactory::render() { } void WaveformWidgetFactory::swapSelf() { - ScopedTimer t("WaveformWidgetFactory::swap() %1waveforms", + ScopedTimer t(QStringLiteral("WaveformWidgetFactory::swap() %1waveforms"), static_cast(m_waveformWidgetHolders.size())); // Do this in an extra slot to be sure to hit the desired interval From f4db737213e1d682f8404929eab9f1123c72adbf Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Fri, 17 May 2024 13:24:35 +0200 Subject: [PATCH 7/8] refactor: deny constructing ScopedTimer from char* for performance --- src/util/timer.h | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/util/timer.h b/src/util/timer.h index 1a016d30d2e..3d7a85078d5 100644 --- a/src/util/timer.h +++ b/src/util/timer.h @@ -65,7 +65,7 @@ class ScopedTimer { template ScopedTimer(Stat::ComputeFlags compute, T&& key, Ts&&... args) : m_timer(std::nullopt) { - static_assert(char_type_size() == sizeof(QString::value_type), + static_assert(!std::is_same_v>, char*>, "string type likely not UTF-16, please pass QStringLiteral by " "means of u\"key text\"_s"); if (!CmdlineArgs::Instance().getDeveloper()) { @@ -96,19 +96,6 @@ class ScopedTimer { m_timer.reset(); } private: - // utility function - template - constexpr static std::size_t char_type_size() { - if constexpr (std::is_array_v) { - return sizeof(std::remove_all_extents_t); - } else if constexpr (std::is_pointer_v) { - // assume this is some point to char array then. - return sizeof(std::remove_pointer_t); - } else { - // assume this is some QString or std::string type - return sizeof(typename T::value_type); - } - } // nullopt also counts as cancelled std::optional m_timer; }; From c76bdca54f6a59089d6ccf26e4ae362acc6ffcbb Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Sat, 18 May 2024 22:17:27 +0200 Subject: [PATCH 8/8] fixup! refactor: deny constructing ScopedTimer from char* for performance --- src/util/timer.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/timer.h b/src/util/timer.h index 3d7a85078d5..5c24627970d 100644 --- a/src/util/timer.h +++ b/src/util/timer.h @@ -65,7 +65,7 @@ class ScopedTimer { template ScopedTimer(Stat::ComputeFlags compute, T&& key, Ts&&... args) : m_timer(std::nullopt) { - static_assert(!std::is_same_v>, char*>, + static_assert(!std::is_same_v, char const*>, "string type likely not UTF-16, please pass QStringLiteral by " "means of u\"key text\"_s"); if (!CmdlineArgs::Instance().getDeveloper()) { @@ -74,6 +74,8 @@ class ScopedTimer { // key is explicitly `auto` to allow passing non-QString types such as `const char*` // its conversion is delayed until after we've checked if we're in developer mode auto assembledKey = QString(std::forward(key)); + // ensure the string was indeed a literal an not unnecessarily heap-allocated + DEBUG_ASSERT(assembledKey.capacity() == 0); if constexpr (sizeof...(args) > 0) { // only try to call QString::arg when we've been given parameters assembledKey = assembledKey.arg(convertToQStringConvertible(std::forward(args))...);