Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(349)

Unified Diff: remoting/client/plugin/chromoting_instance.cc

Issue 1181743005: Report video and network stats averaged over 1s, and create corresponding UMA metrics. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Add Chromoting UMA metrics to histograms.xml. Reduce # of buckets for bandwidth to 100. Created 5 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: remoting/client/plugin/chromoting_instance.cc
diff --git a/remoting/client/plugin/chromoting_instance.cc b/remoting/client/plugin/chromoting_instance.cc
index c32d31ded27bcd4d333f047d9af82c1536d5442a..24b30a9e91e59f0ca3d04e2e18bd795092bed831 100644
--- a/remoting/client/plugin/chromoting_instance.cc
+++ b/remoting/client/plugin/chromoting_instance.cc
@@ -32,6 +32,7 @@
#include "ppapi/cpp/dev/url_util_dev.h"
#include "ppapi/cpp/image_data.h"
#include "ppapi/cpp/input_event.h"
+#include "ppapi/cpp/private/uma_private.h"
#include "ppapi/cpp/rect.h"
#include "ppapi/cpp/var_array_buffer.h"
#include "ppapi/cpp/var_dictionary.h"
@@ -67,12 +68,25 @@ namespace {
// Default DPI to assume for old clients that use notifyClientResolution.
const int kDefaultDPI = 96;
-// Interval at which to sample performance statistics.
-const int kPerfStatsIntervalMs = 1000;
-
// URL scheme used by Chrome apps and extensions.
const char kChromeExtensionUrlScheme[] = "chrome-extension";
+// The boundary value for the FPS histogram: we don't expect video frame-rate to
+// be greater than 40fps. Leaving some room for future improvements, we'll set
+// the max frame rate to 60fps.
+// Histograms expect samples to be less than the boundary value, so set to 61.
+const int kMaxFrameRate = 61;
+
+// For bandwidth, based on expected real-world numbers, we'll use a histogram
+// ranging from 0 to 10MB/s, spread across 1000 buckets.
Alexei Svitkine (slow) 2015/07/13 21:30:06 Update comment re: # of buckets?
anandc 2015/07/14 19:44:22 Done.
+// Histograms are log-scaled by default. This results in fine-grained buckets at
+// lower values and wider-ranged buckets closer to the maximum.
+// Values above the maximum defined here end up in the right-most bucket.
+// See $/src/base/metrics/histogram.h for more details.
+const int kBandwidthHistogramMin = 1;
+const int kBandwidthHistogramMax = 10240000;
Sergey Ulanov 2015/07/13 21:55:28 nit: This should be either 10 MB = 10 * 1000 * 100
anandc 2015/07/14 19:44:23 Done.
+const int kBandwidthHistogramNumBuckets = 100;
+
#if defined(USE_OPENSSL)
// Size of the random seed blob used to initialize RNG in libjingle. Libjingle
// uses the seed only for OpenSSL builds. OpenSSL needs at least 32 bytes of
@@ -728,11 +742,12 @@ void ChromotingInstance::HandleConnect(const base::DictionaryValue& data) {
client_->Start(signal_strategy_.get(), authenticator.Pass(),
transport_factory.Pass(), host_jid, capabilities);
- // Start timer that periodically sends perf stats.
+ // Start timers that periodically send perf stats.
plugin_task_runner_->PostDelayedTask(
FROM_HERE, base::Bind(&ChromotingInstance::SendPerfStats,
weak_factory_.GetWeakPtr()),
- base::TimeDelta::FromMilliseconds(kPerfStatsIntervalMs));
+ base::TimeDelta::FromSeconds(
+ ChromotingStats::kStatsUpdateFrequencyInSeconds));
}
void ChromotingInstance::HandleDisconnect(const base::DictionaryValue& data) {
@@ -1052,8 +1067,12 @@ void ChromotingInstance::SendPerfStats() {
plugin_task_runner_->PostDelayedTask(
FROM_HERE, base::Bind(&ChromotingInstance::SendPerfStats,
weak_factory_.GetWeakPtr()),
- base::TimeDelta::FromMilliseconds(kPerfStatsIntervalMs));
+ base::TimeDelta::FromSeconds(
+ ChromotingStats::kStatsUpdateFrequencyInSeconds));
+ // Update performance stats and send them to the client for display to users.
+ // The rate metrics are averaged over 1s, and the latency metrics are averaged
+ // over the 10 most recent samples.
scoped_ptr<base::DictionaryValue> data(new base::DictionaryValue());
ChromotingStats* stats = video_renderer_->GetStats();
data->SetDouble("videoBandwidth", stats->video_bandwidth()->Rate());
@@ -1064,6 +1083,17 @@ void ChromotingInstance::SendPerfStats() {
data->SetDouble("renderLatency", stats->video_paint_ms()->Average());
data->SetDouble("roundtripLatency", stats->round_trip_ms()->Average());
PostLegacyJsonMessage("onPerfStats", data.Pass());
+
+ // Upload to UMA the video frame-rate, packet-rate and bandwidth stats,
+ // averaged over 1s.
+ pp::UMAPrivate uma(this);
+ uma.HistogramEnumeration("Chromoting.Video.FrameRate",
+ stats->video_frame_rate()->Rate(), kMaxFrameRate);
+ uma.HistogramEnumeration("Chromoting.Video.PacketRate",
+ stats->video_packet_rate()->Rate(), kMaxFrameRate);
+ uma.HistogramCustomCounts("Chromoting.Video.Bandwidth",
+ stats->video_bandwidth()->Rate(), kBandwidthHistogramMin,
+ kBandwidthHistogramMax, kBandwidthHistogramNumBuckets);
Alexei Svitkine (slow) 2015/07/13 21:30:06 Nit: Indent is wrong. Maybe run "git cl format"
anandc 2015/07/14 19:44:23 Thanks. Done. Ran 'git cl format' and it made a fa
}
// static

Powered by Google App Engine
This is Rietveld 408576698