Refactor/modernize shrink scopedtimer #13236

Swiftb0y · 2024-05-14T13:48:58Z

first commit just modernizes it a bit, and doesn't touch the API, the second commit goes a step further and leverages QString::arg(Args&&...) to provide arbitrary formastring support. let me know what you think.

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.

daschuer · 2024-05-14T18:26:51Z

src/waveform/waveformwidgetfactory.cpp

-    ScopedTimer t("WaveformWidgetFactory::render() %1waveforms",
-            static_cast<int>(m_waveformWidgetHolders.size()));
+    ScopedTimer t("WaveformWidgetFactory::render() %1 waveforms",
+            QString::number(m_waveformWidgetHolders.size()));


The idea was that all ScopeTimer statements have almost zero CPU overhead, when developer mode is disabled.
QString does allocate memory even if the scoped timer is disabled so this is not the best idea.

We may look for a solution that is equal fast in normal mode and faster in developer mode. This can be archived with the constexpr QLatin1String constructor. I have proposed here a Qt < 6.4 solution:

mixxx/src/track/taglib/trackmetadata_file.cpp

Lines 48 to 54 in 751a104

#if QT_VERSION >= QT_VERSION_CHECK(6, 4, 0)

using namespace Qt::Literals::StringLiterals;

#else

constexpr inline QLatin1String operator""_L1(const char* str, size_t size) noexcept {

return QLatin1String{str, static_cast<int>(size)};

}

#endif

The idea was that all ScopeTimer statements have almost zero CPU overhead, when developer mode is disabled.

Yes. I got that from the design and I tried to achieve the same here. This is the one case where the newer API does not exactly match the old one, but I figured this was a small-enough prize to pay in this case. ~~There is also small-string-optimization, which should lessen the cost of the construction considerably.~~ Nevermind, apparently QString only features CoW, no SSO.

We may look for a solution that is equal fast in normal mode and faster in developer mode. This can be archived with the constexpr QLatin1String constructor.

I'm afraid I can't quite follow you here. How does that help in the QString::number case?

Just don't use QString::number. It is a heap allocating function. Does passing an int not work?

No because the QString::arg(Args&&...) version only accepts objects implicitly convertible to int...

I can try creating a wrapper that does also accept numeric types and calls QString::number on them, though I don't know how complex the metaprogramming for that would be.

Implemented in the latest commit, wdyt?
The assembly looks pretty decent in the argument-less case. In the lazy case (no compile time QString construction, it obviously looks a little worse, but I think thats fine. I didn't compare it with the current state.

daschuer

Thank you for this nice improvement.

Swiftb0y · 2024-05-15T15:21:04Z

build all green. I think its easier to review if I squash all the fixups first. Or do you already have a review pending? @daschuer

daschuer · 2024-05-15T22:35:18Z

Do we have a use case where we need to pass a QString as key or args? We should not allow this IMHO. The current implementation and the related comments will be tempting preferably use a QString.
I think the fastest solution is to use a constexpr QLatin1String

template<typename... Ts>
ScopedTimer(const QLatin1String& key, Ts&&... args) {
    static_assert(std::is_constexpr_v<decltype(key)>);

Alternatinve that does not require ""_L1 but also avoids searching \0 at runtime:

template<std::size_t N, typename... Ts>
ScopedTimer(const char key[N], Ts&&... args) {
    QString k(key, N-1)

(Both untested)

ronso0 · 2024-05-15T23:11:04Z

src/util/qstringformat.h

+    } else if constexpr (is_number_compatible_v<T>) {
+        return QString::number(std::forward<T>(arg));
+    } else {
+        static_assert(always_false_v<T>, "Unsupported type for QString::arg");


unfortunately this throws an error when picked for main (#12548), dunno if that is to be expected, let alone how to fix it (except using static_assert(false, ...) )
https://github.com/mixxxdj/mixxx/actions/runs/9103424016/job/25025246943?pr=12548

/src/util/stringformat.h: In function ‘auto convertToQStringConvertible(T&&)’: Error: /src/util/stringformat.h:36:23: error: reference to ‘always_false_v’ is ambiguous 36 | static_assert(always_false_v<T>, "Unsupported type for QString::arg"); | ^~~~~~~~~~~~~~ /src/util/stringformat.h:24:23: note: candidates are: ‘template<class T> constexpr const bool {anonymous}::always_false_v<T>’ 24 | static constexpr bool always_false_v = false; | ^~~~~~~~~~~~~~

I think its getting confused because always_false_v is also already defined in other headers (likely from #12781). So you should be able to fix it by shuffling some code around. Potentially just putting a single definition in a single header and then using that across both files could be enough

Ah, okay, in main this seems to clash with always_false_v declared in midimessage.h (where it is inline constexpr bool)

mixxx/src/controllers/midi/midimessage.h

Line 183 in 8f64790

inline constexpr bool always_false_v = false;

(comment race condition : )

Potentially just putting a single definition in a single header and then using that across both files could be enough

What place would you pick?

Well it would need to be accessible in both places and should not assume many dependencies, so I'd just put it in a new src/util/ header for now.

Swiftb0y · 2024-05-16T08:17:10Z

Do we have a use case where we need to pass a QString as key or args? We should not allow this IMHO. The current implementation and the related comments will be tempting preferably use a QString.

Well, currently both "QLatin1String" and const char[N] passing works, but because we end up passing the QString key to the Timer, it sooner or later needs to become a proper QString anyways (or at least an owning string type).

daschuer · 2024-05-16T08:23:16Z

We need to make sure that the QString creation is always delayed until the developer flag is checked to not malloc memory or issue any other locking system call during the normal run.

Swiftb0y · 2024-05-16T08:33:30Z

Yes, I'm aware. The only way that could happen is if the caller explicitly constructs a the string the wrong way for example by QString("some literal"). Using a literal directly works correctly (and delays construction as intended). I don't want to blacklist passing a QString because that would prevent passing a QStringLiteral.

daschuer · 2024-05-16T09:12:28Z

Ah, QStringLiteral. This way we get around the UTF-16 conversion.
You may check if it is a string literal by assert that the capacity() == 0 but size() > 0

Maybe we create our own macro that tuns existing calls into a QStringLiteral call? This way we can add a private function that accepts "const QString&" only.

`always_false_v<T>` already exists in midimessage.h, move to new /util/always_false_v.h

Swiftb0y · 2024-05-16T10:28:34Z

You may check if it is a string literal by assert that the capacity() == 0 but size() > 0

Why would I need to check that? I'd need to do that at compile time for it to be useful.

Maybe we create our own macro that tuns existing calls into a QStringLiteral call?

Something like this?

#define BENCH(key, ...) \
ScopedTimer(QStringLiteral(key), __VA_ARGS__)

That would allow usage like:

ScopedTimer t{BENCH("key")};

Swiftb0y · 2024-05-16T10:51:10Z

tbh, I'd prefer to avoid the macro and instead construct the literal directly:

// assumes previous `using namespace Qt::StringLiterals`
ScopedTimer t(u"key"_s);

I think the ergonomics are decent enough. The stringliteral is also constructed at compile time once you enable -O1. Demo

daschuer · 2024-05-16T11:31:48Z

With the proposed ScopedTimer t(u"key"_s); call, it is IMHO way to easy to use it like ScopedTimer t("key"); which allocates memory at runtime.

You may check if it is a string literal by assert that the capacity() == 0 but size() > 0

Why would I need to check that? I'd need to do that at compile time for it to be useful.

To avoid that users are starting to compose the QString outside the function. This would be DEBUG_ASSERT at runtime.
Or is there a way to check it at compile time?

One option would be to hide the ScopedTimer(const QString& key) somehow.
That can be done with a macro.

Like

#define ScopedTimer(key, ...) \
ScopedTimerPrivate(QStringLiteral(key), __VA_ARGS__)

Which has the benefit the we need to change nothing in the calling code.

tbh, I'd prefer to avoid the macro and instead construct the literal directly:

I don't see a way around it, because QStringLiteral is also a macro.

ronso0 · 2024-05-16T12:08:09Z

Infamous "Close with comment"?

Swiftb0y · 2024-05-16T12:19:11Z

With the proposed ScopedTimer t(u"key"_s); call, it is IMHO way to easy to use it like ScopedTimer t("key"); which allocates memory at runtime.

Yes, just as it did previously. but in the latest version, the const char* version only allocates after the devmode check. Also if that fallback is too graceful, we can just static_assert that the result is still a QString. Then the only way to shoot yourself in the foot here is if you explicitly do the QString cast on a literal ( ScopedTimer t(QString("key")), which at this point is an anti-pattern by itself independent of ScopedTimer (clazy's qstring-unneeded-heap-allocations eliminates this and should already be enabled).

I don't see a way around it, because QStringLiteral is also a macro.

but operator""_s isn't. Also any macro would still have to call a public constructor which people could also just call directly.

This would be DEBUG_ASSERT at runtime.
Or is there a way to check it at compile time?

Not that I would know. Sure we can assert at runtime, but should that be after or before the devmode check. Before it would reduce in overhead, after it would have a good chance to be missed without compiling with -DDEBUG_ASSERTIONS_FATAL.

daschuer · 2024-05-16T16:54:36Z

My idea was to put it after the developer mode check. If they introduce the timer, I am sure they will also use it.

Does the ScopedTimer t("key"); version count until \0?
Do we want to prevent that because of the UTF-16 conversion?
A macro version as discussed above will always use the fastest way and we can disallow all other ways.

Swiftb0y · 2024-05-16T17:24:23Z

Does the ScopedTimer t("key"); version count until \0?

yes. but only after the check. It probably wouldn't with QT_RESTRICTED_CAST_FROM_ASCII defined. But that might require a bigger change. Also strlen is cheap compared to the required allocation and especially from fromUtf8 conversion. But as I said, we can simply disable construction from const char* if you think thats an issue.

A macro version as discussed above will always use the fastest way and we can disallow all other ways.

We can achieve the same if not more without a macro. Can you restate your exact requirements? I'm sure I can find a macro-less solution.

daschuer · 2024-05-16T21:58:24Z

OK here is the list:

The current solution prevents allocating when not in developer mode by 100%. I think a new solution should do the same to be not considered as regression
Optional: Fix implicit use of strlen() at runtime when it loops through the whole string
Optional: Fix UTF-16 conversion at runtime

Swiftb0y · 2024-05-17T10:14:11Z

The current solution prevents allocating when not in developer mode by 100%. I think a new solution should do the same to be not considered as regression

This is already the case.

Fix implicit use of strlen() at runtime when it loops through the whole string.

I assume with "fix" you mean "avoid"? I don't think thats really feasible. The only API that could do that would either be QString::fromUtf8/16 but that would require special case handling which is unnecessarily clunky to deal with.

Optional: Fix UTF-16 conversion at runtime

Not entirely sure how to do that reliably, but I can add a heuristic static_assert that tries to verify that the char type is 16-bit wide. Wdyt?

Swiftb0y · 2024-05-18T20:18:03Z

I can unfortunately not confirm that it fails after:

-    ScopedTimer t(QStringLiteral("AnalyzerEbur128::processSamples()"));
+    ScopedTimer t("AnalyzerEbur128::processSamples()");

I cannot confirm it is working in the current state.

whoops, yeah, forgot const char* is not char const*. fixed in fixup

Swiftb0y · 2024-05-18T20:20:22Z

Oh yes, I missed the fact that if constexpr is optimized away.

Yeah, fortunately not only optimized away but guaranteed to not be called because otherwise trying to call QString::arg without any parameters would be a compile-error (as already pointed out by the comment in the code)

Swiftb0y · 2024-05-18T20:21:20Z

once this lgty I'll rebase out the unnecessary intermediate commits and squash the fixups

daschuer · 2024-05-18T21:58:41Z

Now I get this error message. Unfortunately it is like a wall of text not pointing directly to the point of the error.

In file included from src/analyzer/analyzerebur128.cpp:9:
/home/sperry/workspace/mixxx2/src/util/timer.h: In instantiation of ‘ScopedTimer::ScopedTimer(Stat::ComputeFlags, T&&, Ts&& ...) [with T = const char (&)[34]; Ts = {}; Stat::ComputeFlags = int]’:
src/util/timer.h:88:96:   required from ‘ScopedTimer::ScopedTimer(T&&, Ts&& ...) [with T = const char (&)[34]; Ts = {}]’
src/analyzer/analyzerebur128.cpp:54:55:   required from here
src/util/timer.h:68:23: error: static assertion failed: string type likely not UTF-16, please pass QStringLiteral by means of u"key text"_s
   68 |         static_assert(!std::is_same_v<std::decay_t<T>, char const*>,
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[7/38] Building CXX object CMakeFiles/...lib.dir/src/library/dao/trackdao.cpp.o
ninja: build stopped: subcommand failed.

Do you really think this text is easier to understand than just the one demonstrated above: "use of deleted function" with the faulty line of code shown?

daschuer · 2024-05-18T21:43:30Z

src/util/timer.h

+    // 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


Suggested change

/// @param key The format string for the key

/// @param key The format string as QStringLiteral to identify the timer

daschuer · 2024-05-18T22:01:13Z

src/util/timer.h

+        // 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<T>(key));


This is obsolete now, we don't want something else that a QString generated form a QStringLiteral.

not really because we need a local copy we can assign the result of key.arg(...) to.

Performance wise, we can rely on CoW here IMO.

Moreover, since we're taking an universal reference T&& the type will be QString&& anyways, resulting simply in a move which should be even cheaper.

daschuer · 2024-05-18T22:17:56Z

When one uses the ScopedTimer t3(u"AnalyzerEbur128::processSamples()"); version introduced in main

mixxx/src/analyzer/analyzerebur128.cpp

Line 52 in 8f64790

ScopedTimer t(u"AnalyzerEbur128::processSamples()");

the error message becomes awful:

In file included from src/analyzer/analyzerebur128.cpp:9:
src/util/timer.h: In instantiation of ‘ScopedTimer::ScopedTimer(Stat::ComputeFlags, T&&, Ts&& ...) [with T = const char16_t (&)[34]; Ts = {}; Stat::ComputeFlags = int]’:
src/util/timer.h:88:96:   required from ‘ScopedTimer::ScopedTimer(T&&, Ts&& ...) [with T = const char16_t (&)[34]; Ts = {}]’
src/analyzer/analyzerebur128.cpp:55:56:   required from here
src/util/timer.h:76:29: error: conversion from ‘const char16_t [34]’ to ‘QChar’ is ambiguous
   76 |         auto assembledKey = QString(std::forward<T>(key));
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/x86_64-linux-gnu/qt5/QtCore/qstring.h:48,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/qhashfunctions.h:44,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/qlist.h:47,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/QList:1,
                 from src/track/track_decl.h:5,
                 from src/analyzer/analyzertrack.h:5,
                 from src/analyzer/analyzer.h:3,
                 from src/analyzer/analyzerebur128.h:5,
                 from src/analyzer/analyzerebur128.cpp:1:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qchar.h:90:22: note: candidate: ‘constexpr QChar::QChar(char16_t)’ <near match>
   90 |     Q_DECL_CONSTEXPR QChar(char16_t ch) Q_DECL_NOTHROW : ucs(ushort(ch)) {} // implicit
      |                      ^~~~~
/usr/include/x86_64-linux-gnu/qt5/QtCore/qchar.h:90:22: note:   conversion of argument 1 would be ill-formed:
In file included from src/analyzer/analyzerebur128.cpp:9:
src/util/timer.h:76:29: error: invalid conversion from ‘const char16_t*’ to ‘char16_t’ [-fpermissive]
   76 |         auto assembledKey = QString(std::forward<T>(key));
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                             |
      |                             const char16_t*
In file included from /usr/include/x86_64-linux-gnu/qt5/QtCore/qstring.h:48,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/qhashfunctions.h:44,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/qlist.h:47,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/QList:1,
                 from src/track/track_decl.h:5,
                 from src/analyzer/analyzertrack.h:5,
                 from src/analyzer/analyzer.h:3,
                 from src/analyzer/analyzerebur128.h:5,
                 from src/analyzer/analyzerebur128.cpp:1:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qchar.h:86:22: note: candidate: ‘constexpr QChar::QChar(int)’ <near match>
   86 |     Q_DECL_CONSTEXPR QChar(int rc) Q_DECL_NOTHROW : ucs(ushort(rc & 0xffff)) {}
      |                      ^~~~~
/usr/include/x86_64-linux-gnu/qt5/QtCore/qchar.h:86:22: note:   conversion of argument 1 would be ill-formed:
In file included from src/analyzer/analyzerebur128.cpp:9:
src/util/timer.h:76:29: error: invalid conversion from ‘const char16_t*’ to ‘int’ [-fpermissive]
   76 |         auto assembledKey = QString(std::forward<T>(key));
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                             |
      |                             const char16_t*
In file included from /usr/include/x86_64-linux-gnu/qt5/QtCore/qstring.h:48,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/qhashfunctions.h:44,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/qlist.h:47,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/QList:1,
                 from src/track/track_decl.h:5,
                 from src/analyzer/analyzertrack.h:5,
                 from src/analyzer/analyzer.h:3,
                 from src/analyzer/analyzerebur128.h:5,
                 from src/analyzer/analyzerebur128.cpp:1:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qchar.h:85:22: note: candidate: ‘constexpr QChar::QChar(uint)’ <near match>
   85 |     Q_DECL_CONSTEXPR QChar(uint rc) Q_DECL_NOTHROW : ucs(ushort(rc & 0xffff)) {}
      |                      ^~~~~
/usr/include/x86_64-linux-gnu/qt5/QtCore/qchar.h:85:22: note:   conversion of argument 1 would be ill-formed:
In file included from src/analyzer/analyzerebur128.cpp:9:
src/util/timer.h:76:29: error: invalid conversion from ‘const char16_t*’ to ‘uint’ {aka ‘unsigned int’} [-fpermissive]
   76 |         auto assembledKey = QString(std::forward<T>(key));
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                             |
      |                             const char16_t*
In file included from /usr/include/x86_64-linux-gnu/qt5/QtCore/qstring.h:48,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/qhashfunctions.h:44,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/qlist.h:47,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/QList:1,
                 from src/track/track_decl.h:5,
                 from src/analyzer/analyzertrack.h:5,
                 from src/analyzer/analyzer.h:3,
                 from src/analyzer/analyzerebur128.h:5,
                 from src/analyzer/analyzerebur128.cpp:1:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qchar.h:84:22: note: candidate: ‘constexpr QChar::QChar(short int)’ <near match>
   84 |     Q_DECL_CONSTEXPR QChar(short rc) Q_DECL_NOTHROW : ucs(ushort(rc)) {} // implicit
      |                      ^~~~~
/usr/include/x86_64-linux-gnu/qt5/QtCore/qchar.h:84:22: note:   conversion of argument 1 would be ill-formed:
In file included from src/analyzer/analyzerebur128.cpp:9:
src/util/timer.h:76:29: error: invalid conversion from ‘const char16_t*’ to ‘short int’ [-fpermissive]
   76 |         auto assembledKey = QString(std::forward<T>(key));
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                             |
      |                             const char16_t*
In file included from /usr/include/x86_64-linux-gnu/qt5/QtCore/qstring.h:48,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/qhashfunctions.h:44,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/qlist.h:47,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/QList:1,
                 from src/track/track_decl.h:5,
                 from src/analyzer/analyzertrack.h:5,
                 from src/analyzer/analyzer.h:3,
                 from src/analyzer/analyzerebur128.h:5,
                 from src/analyzer/analyzerebur128.cpp:1:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qchar.h:82:22: note: candidate: ‘constexpr QChar::QChar(ushort)’ <near match>
   82 |     Q_DECL_CONSTEXPR QChar(ushort rc) Q_DECL_NOTHROW : ucs(rc) {} // implicit
      |                      ^~~~~
/usr/include/x86_64-linux-gnu/qt5/QtCore/qchar.h:82:22: note:   conversion of argument 1 would be ill-formed:
In file included from src/analyzer/analyzerebur128.cpp:9:
src/util/timer.h:76:29: error: invalid conversion from ‘const char16_t*’ to ‘ushort’ {aka ‘short unsigned int’} [-fpermissive]
   76 |         auto assembledKey = QString(std::forward<T>(key));
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                             |
      |                             const char16_t*
In file included from /usr/include/x86_64-linux-gnu/qt5/QtCore/qhashfunctions.h:44,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/qlist.h:47,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/QList:1,
                 from src/track/track_decl.h:5,
                 from src/analyzer/analyzertrack.h:5,
                 from src/analyzer/analyzer.h:3,
                 from src/analyzer/analyzerebur128.h:5,
                 from src/analyzer/analyzerebur128.cpp:1:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qstring.h:225:19: note:   initializing argument 1 of ‘QString::QString(QChar)’
  225 |     QString(QChar c);
      |             ~~~~~~^
[7/34] Building CXX object CMakeFiles/...src/skin/legacy/legacyskinparser.cpp.o
ninja: build stopped: subcommand failed.

Swiftb0y · 2024-05-19T10:40:50Z

Do you really think this text is easier to understand than just the one demonstrated above: "use of deleted function" with the faulty line of code shown?

I'd say so yes because the solution with the deleted overload just says "use of deleted function" with no clear guidance on how to solve the issue. In the static_assert text we can provide an explanation and guidance on how to fix it. Yeah its a little more to read because the error is not on the first line for some reason, but IMO its still easy enough to spot where the error is when you look for the "error: [...]" line.

When one uses the ScopedTimer t3(u"AnalyzerEbur128::processSamples()"); version introduced in main

Ah, I forgot about that PR. the changes in main are very much incompatible with the changes made here anyways. I'll rebase on main then.

daschuer · 2024-05-19T11:09:13Z

I would prefer the deleted function with an explaining source code comment, but I will not insists.

daschuer · 2024-05-19T11:20:16Z

src/util/timer.h

+                "string type likely not UTF-16, please pass QStringLiteral by "
+                "means of u\"key text\"_s");


Suggested change

"string type likely not UTF-16, please pass QStringLiteral by "

"means of u\"key text\"_s");

"Passing char[] is not supported. Wrap it via QStringLiteral() "

"to avoid runtime UTF-16 conversion.");

…pedtimer-main Refactor/shrink modernize scopedtimer (Alternative to #13236)

Swiftb0y · 2024-05-31T13:10:03Z

Superseeded by #13258

github-actions bot added the waveform label May 14, 2024

daschuer reviewed May 14, 2024

View reviewed changes

daschuer requested changes May 14, 2024

View reviewed changes

Swiftb0y force-pushed the refactor/modernize-shrink-scopedtimer branch from c94babc to 1f27406 Compare May 14, 2024 20:25

ronso0 mentioned this pull request May 15, 2024

add multi-track property editor / batch tag editor #12548

Merged

16 tasks

Swiftb0y added 2 commits May 15, 2024 20:47

feat: ScopedTimer ctor accept QString::arg-compatible formatstring

b5aa400

feat: ScopedTimer accept numeric arguments as formatstring arguments

e4263ae

Swiftb0y force-pushed the refactor/modernize-shrink-scopedtimer branch from ca7c9e1 to e4263ae Compare May 15, 2024 18:47

ronso0 added a commit to ronso0/mixxx that referenced this pull request May 15, 2024

pick util/stringformat.h from mixxxdj#13236

5565888

ronso0 added a commit to ronso0/mixxx that referenced this pull request May 15, 2024

pick util/stringformat.h from mixxxdj#13236

f4d5b07

ronso0 reviewed May 15, 2024

View reviewed changes

ronso0 added a commit to ronso0/mixxx that referenced this pull request May 16, 2024

pick util/stringformat.h from mixxxdj#13236

944d379

`always_false_v<T>` already exists in midimessage.h, move to new /util/always_false_v.h

daschuer closed this May 16, 2024

ronso0 reopened this May 16, 2024

github-actions bot added scanner waveform vinylcontrol soundsource analyzer soundio labels May 18, 2024

daschuer requested changes May 18, 2024

View reviewed changes

daschuer removed library engine ui scanner waveform vinylcontrol soundsource analyzer soundio labels May 19, 2024

daschuer requested changes May 19, 2024

View reviewed changes

This was referenced May 22, 2024

Refactor/shrink modernize scopedtimer (Alternative to #13236) #13258

Merged

Multithreaded rubberband #13143

Open

daschuer added a commit that referenced this pull request May 30, 2024

Merge pull request #13258 from Swiftb0y/refactor/shrink-modernize-sco…

c0407e5

…pedtimer-main Refactor/shrink modernize scopedtimer (Alternative to #13236)

Swiftb0y closed this May 31, 2024

Swiftb0y deleted the refactor/modernize-shrink-scopedtimer branch May 31, 2024 13:10

	#if QT_VERSION >= QT_VERSION_CHECK(6, 4, 0)
	using namespace Qt::Literals::StringLiterals;
	#else
	constexpr inline QLatin1String operator""_L1(const char* str, size_t size) noexcept {
	return QLatin1String{str, static_cast<int>(size)};
	}
	#endif

	/// @param key The format string for the key
	/// @param key The format string as QStringLiteral to identify the timer

		"string type likely not UTF-16, please pass QStringLiteral by "
		"means of u\"key text\"_s");

Refactor/modernize shrink scopedtimer #13236

Refactor/modernize shrink scopedtimer #13236

Conversation

Swiftb0y commented May 14, 2024

Choose a reason for hiding this comment

Swiftb0y May 14, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daschuer left a comment

Choose a reason for hiding this comment

Swiftb0y commented May 15, 2024

daschuer commented May 15, 2024

Choose a reason for hiding this comment

Swiftb0y May 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Swiftb0y commented May 16, 2024

daschuer commented May 16, 2024

Swiftb0y commented May 16, 2024

daschuer commented May 16, 2024

Swiftb0y commented May 16, 2024 • edited

Swiftb0y commented May 16, 2024

daschuer commented May 16, 2024

ronso0 commented May 16, 2024

Swiftb0y commented May 16, 2024

daschuer commented May 16, 2024

Swiftb0y commented May 16, 2024

daschuer commented May 16, 2024

Swiftb0y commented May 17, 2024

Swiftb0y commented May 18, 2024 • edited

Swiftb0y commented May 18, 2024 • edited

Swiftb0y commented May 18, 2024

daschuer commented May 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daschuer commented May 18, 2024

Swiftb0y commented May 19, 2024

daschuer commented May 19, 2024

Choose a reason for hiding this comment

Swiftb0y commented May 31, 2024

Swiftb0y May 14, 2024 •

edited

Swiftb0y May 16, 2024 •

edited

Swiftb0y commented May 16, 2024 •

edited

Swiftb0y commented May 18, 2024 •

edited

Swiftb0y commented May 18, 2024 •

edited