From 3dc7a65416ce26486aaff68a70b2141b2029f4d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sat, 22 Aug 2026 23:41:41 +0200 Subject: [PATCH] Move params to struct, more informative debug Took 56 seconds Took 53 seconds Took 2 minutes Took 33 seconds --- .../remote_connection_controller.cpp | 1 - .../remote_connection_controller.h | 6 +---- .../client/abstract/abstract_client.cpp | 24 ++++++++---------- .../network/client/abstract/abstract_client.h | 25 ++++++++----------- .../network/client/abstract/latency_tracker.h | 3 +++ 5 files changed, 25 insertions(+), 34 deletions(-) diff --git a/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp b/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp index ee094dd28..890a621c8 100644 --- a/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp +++ b/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp @@ -45,7 +45,6 @@ void ConnectionController::wireClientSignals() connect(remoteClient, &RemoteClient::statusChanged, this, &ConnectionController::onStatusChanged); connect(remoteClient, &AbstractClient::pingStatsUpdated, this, &ConnectionController::pingStatsUpdated); - connect(remoteClient, &AbstractClient::pingSamplesUpdated, this, &ConnectionController::pingSamplesUpdated); connect(remoteClient, &RemoteClient::userInfoChanged, this, &ConnectionController::onUserInfoReceived, Qt::BlockingQueuedConnection); diff --git a/cockatrice/src/client/network/connection_controller/remote_connection_controller.h b/cockatrice/src/client/network/connection_controller/remote_connection_controller.h index a444f5d41..bae99a3e0 100644 --- a/cockatrice/src/client/network/connection_controller/remote_connection_controller.h +++ b/cockatrice/src/client/network/connection_controller/remote_connection_controller.h @@ -56,11 +56,7 @@ signals: // Forwarded from AbstractClient::pingStatsUpdated. See that signal for the // meaning of the parameters. - void pingStatsUpdated(int lastMs, int medianMs, int p95Ms, int maxMs, int sampleCount); - - // Forwarded from AbstractClient::pingSamplesUpdated. See that signal for - // the meaning of the parameter. - void pingSamplesUpdated(const QList &samplesMs); + void pingStatsUpdated(const LatencyTracker::Stats &stats, const QList &samplesMs); private slots: // Slots wired directly to RemoteClient signals diff --git a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp index c97a66a17..d6316deb3 100644 --- a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp +++ b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.cpp @@ -1,6 +1,7 @@ #include "abstract_client.h" #include +#include #include #include #include @@ -28,6 +29,7 @@ AbstractClient::AbstractClient(QObject *parent) qRegisterMetaType("Response"); qRegisterMetaType("Response::ResponseCode"); qRegisterMetaType("ClientStatus"); + qRegisterMetaType("LatencyTracker::Stats"); qRegisterMetaType("RoomEvent"); qRegisterMetaType("GameEventContainer"); qRegisterMetaType("Event_ServerIdentification"); @@ -169,10 +171,10 @@ void AbstractClient::queuePendingCommand(PendingCommand *pend) namespace { -constexpr int StatsEmitIntervalMs = 1000; +constexpr int STATS_EMIT_INTERVAL_MS = 1000; // Game actions are what players perceive as lag. Surface unusually slow ones // without requiring debug logging to be enabled. -constexpr qint64 SlowGameCommandWarnMs = 1500; +constexpr qint64 SLOW_GAME_COMMAND_WARN_MS = 1500; } // namespace void AbstractClient::recordLatency(PendingCommand &pend) @@ -189,29 +191,26 @@ void AbstractClient::recordLatency(PendingCommand &pend) << "command RTT:" << elapsed << "ms (cmd_id" << pend.getCommandContainer().cmd_id() << ")"; } - if (elapsed >= SlowGameCommandWarnMs && pend.getCommandContainer().game_command_size() > 0) { - qCWarning(AbstractClientLog) << "slow game command round trip:" << elapsed << "ms (cmd_id" - << pend.getCommandContainer().cmd_id() << ")"; + if (elapsed >= SLOW_GAME_COMMAND_WARN_MS && pend.getCommandContainer().game_command_size() > 0) { + qCWarning(AbstractClientLog).noquote() + << "slow game command round trip:" << elapsed << "ms | " << getSafeDebugString(pend.getCommandContainer()); } // Emit aggregated stats at most once per StatsEmitIntervalMs so that the // per-command hot path stays free of signal traffic. The keepalive ping // guarantees a fresh sample roughly every second while connected. - if (!statsEmitClockStarted || statsEmitClock.elapsed() >= StatsEmitIntervalMs) { + if (!statsEmitClockStarted || statsEmitClock.elapsed() >= STATS_EMIT_INTERVAL_MS) { statsEmitClock.start(); statsEmitClockStarted = true; const LatencyTracker::Stats stats = latencyTracker.stats(); - const QList recentSamples = latencyTracker.recentSamples(); QList samples; samples.reserve(stats.sampleCount); - for (qint64 sample : recentSamples) { + for (qint64 sample : latencyTracker.recentSamples()) { samples.append(static_cast(sample)); } - emit pingStatsUpdated(static_cast(stats.lastMs), static_cast(stats.medianMs), - static_cast(stats.p95Ms), static_cast(stats.maxMs), stats.sampleCount); - emit pingSamplesUpdated(samples); + emit pingStatsUpdated(stats, samples); } } @@ -219,8 +218,7 @@ void AbstractClient::clearLatencyStats() { latencyTracker.clear(); statsEmitClockStarted = false; - emit pingStatsUpdated(0, 0, 0, 0, 0); - emit pingSamplesUpdated({}); + emit pingStatsUpdated(LatencyTracker::Stats{}, {}); } PendingCommand *AbstractClient::prepareSessionCommand(const ::google::protobuf::Message &cmd) diff --git a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h index b189932e8..1ef9a31e4 100644 --- a/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h +++ b/libcockatrice_network/libcockatrice/network/client/abstract/abstract_client.h @@ -61,21 +61,16 @@ signals: void maxPingTime(int seconds, int maxSeconds); /** - * @brief Aggregated round-trip statistics, emitted at most once per second. + * @brief Aggregated round-trip statistics and a chronological snapshot of + * the rolling window, emitted at most once per second. * - * All values are in milliseconds. sampleCount is the number of samples - * currently in the rolling window. Emitted from the client thread. The - * connection to UI objects is automatically queued across threads. + * All values in the stats struct are in milliseconds; sampleCount is the + * number of samples currently in the rolling window. The samples list is + * ordered oldest first so graphs can redraw without polling the tracker + * across threads. Emitted from the client thread. The connection to UI + * objects is automatically queued across threads. */ - void pingStatsUpdated(int lastMs, int medianMs, int p95Ms, int maxMs, int sampleCount); - - /** - * @brief Chronological snapshot of the rolling window, oldest sample first. - * - * Emitted together with pingStatsUpdated under the same throttle, so - * graphs can redraw without polling the tracker across threads. - */ - void pingSamplesUpdated(const QList &samplesMs); + void pingStatsUpdated(const LatencyTracker::Stats &stats, const QList &samplesMs); // Room events void roomEventReceived(const RoomEvent &event); @@ -143,8 +138,8 @@ public: /** * @brief Drops all recorded round-trip samples and resets the stats - * emission throttle, emitting a zeroed pingStatsUpdated so that UI - * listeners can clear their display. + * emission throttle, emitting zeroed stats so that UI listeners can + * clear their display. * * Must be called from the client thread (as RemoteClient's disconnect * path does). The tracker is deliberately lock-free. diff --git a/libcockatrice_network/libcockatrice/network/client/abstract/latency_tracker.h b/libcockatrice_network/libcockatrice/network/client/abstract/latency_tracker.h index bf40ebfa0..75f18ac0c 100644 --- a/libcockatrice_network/libcockatrice/network/client/abstract/latency_tracker.h +++ b/libcockatrice_network/libcockatrice/network/client/abstract/latency_tracker.h @@ -7,6 +7,7 @@ #define LATENCY_TRACKER_H #include +#include #include /** @@ -45,4 +46,6 @@ private: int count = 0; ///< number of valid samples, capped at WindowSize }; +Q_DECLARE_METATYPE(LatencyTracker::Stats) + #endif