You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gr...@apache.org on 2018/07/12 22:17:13 UTC
[2/2] mesos git commit: Optimized the generation of metrics snapshots.
Optimized the generation of metrics snapshots.
Profiling of metrics generation revealed a large amount of time spent
in map operations. This patch does three things to mitigate this:
* Stores the metrics as an ordered map so that we only pay the price
of sorting when the metric is first added.
* Makes use of vectors instead of maps for intermediate objects,
which eliminates the need for another intermediate object.
* Hints when inserting into the returned map, reducing the cost of
insertion into that ordered container.
Review: https://reviews.apache.org/r/67871
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/42becf22
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/42becf22
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/42becf22
Branch: refs/heads/master
Commit: 42becf22f07831ef8c3d4fbc8c1b80eb9d5b959a
Parents: 27ed915
Author: Greg Mann <gr...@gmail.com>
Authored: Mon Jul 9 22:59:12 2018 -0700
Committer: Greg Mann <gr...@gmail.com>
Committed: Thu Jul 12 13:29:38 2018 -0700
----------------------------------------------------------------------
.../include/process/metrics/metrics.hpp | 8 +--
3rdparty/libprocess/src/metrics/metrics.cpp | 71 +++++++++++---------
2 files changed, 45 insertions(+), 34 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/42becf22/3rdparty/libprocess/include/process/metrics/metrics.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/metrics/metrics.hpp b/3rdparty/libprocess/include/process/metrics/metrics.hpp
index f9b7202..3dc6bbf 100644
--- a/3rdparty/libprocess/include/process/metrics/metrics.hpp
+++ b/3rdparty/libprocess/include/process/metrics/metrics.hpp
@@ -25,7 +25,6 @@
#include <process/metrics/metric.hpp>
-#include <stout/hashmap.hpp>
#include <stout/nothing.hpp>
#include <stout/option.hpp>
@@ -71,11 +70,12 @@ private:
// capture with C++14.
Future<std::map<std::string, double>> __snapshot(
const Option<Duration>& timeout,
- hashmap<std::string, Future<double>>&& metrics,
- hashmap<std::string, Option<Statistics<double>>>&& statistics);
+ std::vector<std::string>&& keys,
+ std::vector<Future<double>>&& metrics,
+ std::vector<Option<Statistics<double>>>&& statistics);
// The Owned<Metric> is an explicit copy of the Metric passed to 'add'.
- hashmap<std::string, Owned<Metric>> metrics;
+ std::map<std::string, Owned<Metric>> metrics;
// Used to rate limit the snapshot endpoint.
Option<Owned<RateLimiter>> limiter;
http://git-wip-us.apache.org/repos/asf/mesos/blob/42becf22/3rdparty/libprocess/src/metrics/metrics.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/metrics/metrics.cpp b/3rdparty/libprocess/src/metrics/metrics.cpp
index 4883c9a..8220470 100644
--- a/3rdparty/libprocess/src/metrics/metrics.cpp
+++ b/3rdparty/libprocess/src/metrics/metrics.cpp
@@ -28,7 +28,6 @@
#include <stout/duration.hpp>
#include <stout/error.hpp>
#include <stout/foreach.hpp>
-#include <stout/hashmap.hpp>
#include <stout/numify.hpp>
#include <stout/option.hpp>
#include <stout/os.hpp>
@@ -121,7 +120,7 @@ string MetricsProcess::help()
Future<Nothing> MetricsProcess::add(Owned<Metric> metric)
{
- if (metrics.contains(metric->name())) {
+ if (metrics.count(metric->name()) > 0) {
return Failure("Metric '" + metric->name() + "' was already added");
}
@@ -132,7 +131,7 @@ Future<Nothing> MetricsProcess::add(Owned<Metric> metric)
Future<Nothing> MetricsProcess::remove(const string& name)
{
- if (!metrics.contains(name)) {
+ if (metrics.count(name) == 0) {
return Failure("Metric '" + name + "' not found");
}
@@ -145,29 +144,35 @@ Future<Nothing> MetricsProcess::remove(const string& name)
Future<map<string, double>> MetricsProcess::snapshot(
const Option<Duration>& timeout)
{
- hashmap<string, Future<double>> futures;
- hashmap<string, Option<Statistics<double>>> statistics;
-
- foreachkey (const string& name, metrics) {
- const Owned<Metric>& metric = metrics.at(name);
- futures[name] = metric->value();
- // TODO(dhamon): It would be nice to compute these asynchronously.
- statistics[name] = metric->statistics();
+ // To avoid creating a new vector when calling `await()` below, we use three
+ // ordered vectors, where the Nth key in `keys` is associated with the Nth
+ // items in each of `futures` and `statistics`.
+ vector<string> keys;
+ vector<Future<double>> futures;
+ vector<Option<Statistics<double>>> statistics;
+
+ keys.reserve(metrics.size());
+ futures.reserve(metrics.size());
+ statistics.reserve(metrics.size());
+
+ for (auto iter = metrics.begin(); iter != metrics.end(); ++iter) {
+ keys.emplace_back(iter->first);
+ futures.emplace_back(iter->second->value());
+ statistics.emplace_back(iter->second->statistics());
}
Future<Nothing> timedout =
after(timeout.getOrElse(Duration::max()));
- vector<Future<double>> values = futures.values();
-
// Return the response once it finishes or we time out.
return select<Nothing>({
timedout,
- await(std::move(values)).then([]{ return Nothing(); }) })
+ await(futures).then([]{ return Nothing(); }) })
.onAny([=]() mutable { timedout.discard(); }) // Don't accumulate timers.
.then(defer(self(),
&Self::__snapshot,
timeout,
+ std::move(keys),
std::move(futures),
std::move(statistics)));
}
@@ -209,34 +214,40 @@ Future<http::Response> MetricsProcess::_snapshot(
Future<map<string, double>> MetricsProcess::__snapshot(
const Option<Duration>& timeout,
- hashmap<string, Future<double>>&& metrics,
- hashmap<string, Option<Statistics<double>>>&& statistics)
+ vector<string>&& keys,
+ vector<Future<double>>&& metrics,
+ vector<Option<Statistics<double>>>&& statistics)
{
map<string, double> snapshot;
- foreachpair (const string& key, const Future<double>& value, metrics) {
+ for (size_t i = 0; i < metrics.size(); ++i) {
// TODO(dhamon): Maybe add the failure message for this metric to the
// response if value.isFailed().
+ const string& key = keys[i];
+ const Future<double>& value = metrics[i];
+
if (value.isPending()) {
CHECK_SOME(timeout);
VLOG(1) << "Exceeded timeout of " << timeout.get()
<< " when attempting to get metric '" << key << "'";
} else if (value.isReady()) {
- snapshot[key] = value.get();
+ snapshot.emplace_hint(snapshot.end(), key, value.get());
}
- Option<Statistics<double>> statistics_ = statistics.get(key).get();
-
- if (statistics_.isSome()) {
- snapshot[key + "/count"] = static_cast<double>(statistics_->count);
- snapshot[key + "/min"] = statistics_->min;
- snapshot[key + "/max"] = statistics_->max;
- snapshot[key + "/p50"] = statistics_->p50;
- snapshot[key + "/p90"] = statistics_->p90;
- snapshot[key + "/p95"] = statistics_->p95;
- snapshot[key + "/p99"] = statistics_->p99;
- snapshot[key + "/p999"] = statistics_->p999;
- snapshot[key + "/p9999"] = statistics_->p9999;
+ if (statistics[i].isSome()) {
+ Statistics<double>& statistics_ = statistics[i].get();
+ snapshot.emplace_hint(
+ snapshot.end(),
+ key + "/count",
+ static_cast<double>(statistics_.count));
+ snapshot.emplace_hint(snapshot.end(), key + "/max", statistics_.max);
+ snapshot.emplace_hint(snapshot.end(), key + "/min", statistics_.min);
+ snapshot.emplace_hint(snapshot.end(), key + "/p50", statistics_.p50);
+ snapshot.emplace_hint(snapshot.end(), key + "/p90", statistics_.p90);
+ snapshot.emplace_hint(snapshot.end(), key + "/p95", statistics_.p95);
+ snapshot.emplace_hint(snapshot.end(), key + "/p99", statistics_.p99);
+ snapshot.emplace_hint(snapshot.end(), key + "/p999", statistics_.p999);
+ snapshot.emplace_hint(snapshot.end(), key + "/p9999", statistics_.p9999);
}
}