[Chromoting] Retrieve process resource usage (ProcessStats) and its tests

remoting/base/process_stats.cc

Index: remoting/base/process_stats.cc
diff --git a/remoting/base/process_stats.cc b/remoting/base/process_stats.cc
new file mode 100644
index 0000000000000000000000000000000000000000..4cb580b1c51f9328548454181f608e56bc345be3
--- /dev/null
+++ b/remoting/base/process_stats.cc
@@ -0,0 +1,52 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <utility>
+
+#include "base/location.h"
+#include "base/logging.h"
+#include "remoting/base/process_stats.h"

[Sergey Ulanov, 2017/04/03 23:50:21]

This should be the first include in this file.

[Hzj_jie, 2017/04/05 20:52:16]

Done.

+#include "remoting/base/process_stats_util.h"
+
+namespace remoting {
+
+ProcessStats::ProcessStats(ProcessStatsStub* host_stats_stub,
+                           base::TimeDelta interval)
+    : host_stats_stub_(host_stats_stub), interval_(interval) {
+  DCHECK(host_stats_stub_);
+  DCHECK(!interval_.is_zero());
+}
+
+ProcessStats::~ProcessStats() = default;
+
+void ProcessStats::AddProcessStatsAgent(
+    std::unique_ptr<ProcessStatsAgent> agent) {
+  agents_.push_back(std::move(agent));
+}
+
+void ProcessStats::Start() {
+  timer_.Start(FROM_HERE, interval_, this, &ProcessStats::ReportUsage);
+}
+
+void ProcessStats::Stop() {
+  timer_.Stop();
+}
+
+void ProcessStats::ReportUsage() {
+  if (agents_.empty()) {
+    return;
+  }
+
+  std::vector<protocol::ProcessResourceUsage> usages;
+  for (const auto& agent : agents_) {
+    protocol::ProcessResourceUsage usage = agent->GetResourceUsage();

[Sergey Ulanov, 2017/04/03 23:50:21]

Can this operation block? Would it be better to run it on a background thread?

[Hzj_jie, 2017/04/05 20:52:16]

Emm, good point. On Windows and Mac OS, this operation does not contains IO
operations. But on Linux, it needs to access /proc/<pid/task file. But at
https://cs.chromium.org/chromium/src/base/process/process_metrics_linux.cc?rc...,
the comment states accessing /proc files does not require disk operations.

So we should be fine here. I would suggest we keep it as-is. Once we hit the IO
violence, a background thread wrapper would help.

[Sergey Ulanov, 2017/04/06 05:55:50]

Even if it doesn't contain any IO operation it may still take significant time
to finish this operation, depending on how it's implemented. This is
particularly problematic for WebRTC as it's sensetive to delays (e.g. delay of
2ms is already too much and may affect BW estimate). It's unlikely to be a
problem when we getting stats for the current process. Maybe add a comment to
mention that this may be a problem.

[Hzj_jie, 2017/04/06 19:41:05]

Definitely. The comment is added into ProcessStatsAgent::GetResourceUsage().

+    if (!IsEmptyProcessResourceUsage(usage)) {
+      usages.push_back(std::move(usage));
+    }
+  }
+
+  host_stats_stub_->OnProcessStats(AggregateProcessResourceUsage(usages));
+}
+
+}  // namespace remoting