[Client] Discard suspend-spanning gaps in LagMonitor

Windows counts sleep time in its monotonic clock, so a suspend would
fabricate one bogus stall per resume. Reset the clock on application
state changes and drop implausibly huge gaps; extract recordGap() for
testability.

Took 3 minutes
This commit is contained in:
Lukas Brübach 2026-08-23 00:19:18 +02:00
parent 45fe8b57a5
commit fa823e4b1c
2 changed files with 48 additions and 8 deletions

View file

@ -1,9 +1,13 @@
#include "lag_monitor.h" #include "lag_monitor.h"
#include <QCoreApplication>
#include <QEvent>
#include <QTimer> #include <QTimer>
LagMonitor::LagMonitor(QObject *parent) : QObject(parent) LagMonitor::LagMonitor(QObject *parent) : QObject(parent)
{ {
qApp->installEventFilter(this);
timer = new QTimer(this); timer = new QTimer(this);
timer->setInterval(TICK_INTERVAL_MS); timer->setInterval(TICK_INTERVAL_MS);
connect(timer, &QTimer::timeout, this, &LagMonitor::checkTick); connect(timer, &QTimer::timeout, this, &LagMonitor::checkTick);
@ -21,23 +25,39 @@ void LagMonitor::clearStalls()
stalls.clear(); stalls.clear();
} }
bool LagMonitor::eventFilter(QObject *obj, QEvent *event)
{
if (event->type() == QEvent::ApplicationStateChange) {
// The transition may span a suspend or an arbitrary unfocused period;
// discard the gap so it cannot be mistaken for a stall.
tickClock.restart();
}
return QObject::eventFilter(obj, event);
}
void LagMonitor::checkTick() void LagMonitor::checkTick()
{ {
const qint64 gap = tickClock.restart(); recordGap(tickClock.restart());
}
if (gap <= STALL_THRESHOLD_MS) { void LagMonitor::recordGap(qint64 gapMs)
{
if (gapMs <= STALL_THRESHOLD_MS) {
return; return;
} }
StallRecord record; if (gapMs > MAX_PLAUSIBLE_STALL_MS) {
record.timestampMsSinceEpoch = QDateTime::currentMSecsSinceEpoch(); // wall time, for log correlation qCDebug(LagMonitorLog, "Ignoring implausible %lld ms gap (likely suspend)", static_cast<long long>(gapMs));
record.durationMs = gap; return;
}
const StallRecord record{.timestampMsSinceEpoch = QDateTime::currentMSecsSinceEpoch(), .durationMs = gapMs};
stalls.append(record); stalls.append(record);
while (stalls.size() > MAX_RECORDED_STALLS) { while (stalls.size() > MAX_RECORDED_STALLS) {
stalls.removeFirst(); stalls.removeFirst();
} }
qCWarning(LagMonitorLog, "Event loop stalled for %lld ms (threshold: %d ms)", static_cast<long long>(gap), qCWarning(LagMonitorLog, "Event loop stalled for %lld ms (threshold: %d ms)", static_cast<long long>(gapMs),
STALL_THRESHOLD_MS); STALL_THRESHOLD_MS);
} }

View file

@ -14,6 +14,7 @@
inline Q_LOGGING_CATEGORY(LagMonitorLog, "lag_monitor"); inline Q_LOGGING_CATEGORY(LagMonitorLog, "lag_monitor");
class QEvent;
class QTimer; class QTimer;
/** /**
@ -24,6 +25,11 @@ class QTimer;
* event loop for roughly the overshooting duration. This is what separates * event loop for roughly the overshooting duration. This is what separates
* "my client froze" from "the network is lagging" in user reports. * "my client froze" from "the network is lagging" in user reports.
* *
* Gaps that span an application state change (suspend, minimize, focus
* loss) are discarded, and implausibly huge gaps are dropped, so operating
* system power events do not fabricate stalls. This handling is load-bearing
* on Windows, where the monotonic clock used by Qt counts sleep time.
*
* Healthy operation costs one timer wakeup per tick and two integer * Healthy operation costs one timer wakeup per tick and two integer
* comparisons. Allocations happen only when a stall is actually recorded. * comparisons. Allocations happen only when a stall is actually recorded.
*/ */
@ -35,13 +41,16 @@ public:
struct StallRecord struct StallRecord
{ {
qint64 timestampMsSinceEpoch = 0; ///< when the stalled period ended qint64 timestampMsSinceEpoch = 0; ///< when the stalled period ended
qint64 durationMs = 0; ///< approximate length of the freeze qint64 durationMs = 0; ///< approximate length of the freeze; measured tick to tick, so it can exceed the true stall by up to TICK_INTERVAL_MS
}; };
static constexpr int TICK_INTERVAL_MS = 500; static constexpr int TICK_INTERVAL_MS = 500;
static constexpr int STALL_THRESHOLD_MS = 2000; static constexpr int STALL_THRESHOLD_MS = 2000;
static constexpr int MAX_RECORDED_STALLS = 32; static constexpr int MAX_RECORDED_STALLS = 32;
/// Gaps beyond this are treated as suspend artifacts rather than stalls.
static constexpr qint64 MAX_PLAUSIBLE_STALL_MS = 600000;
explicit LagMonitor(QObject *parent = nullptr); explicit LagMonitor(QObject *parent = nullptr);
/** /**
@ -54,12 +63,23 @@ public:
void clearStalls(); void clearStalls();
/**
* @brief Feeds a measured tick-to-tick gap through the detection logic.
*
* Split out of checkTick so threshold, plausibility, and trim behavior
* stay unit-testable without real timing.
*/
void recordGap(qint64 gapMs);
protected:
bool eventFilter(QObject *obj, QEvent *event) override;
private slots: private slots:
void checkTick(); void checkTick();
private: private:
QTimer *timer; QTimer *timer;
QElapsedTimer tickClock; ///< monotonic clock, so wall clock steps and suspend do not fabricate stalls QElapsedTimer tickClock; ///< monotonic clock, so wall clock steps do not fabricate stalls
QList<StallRecord> stalls; QList<StallRecord> stalls;
}; };