[mesos] 02/02: Fixed inefficient `hashmap` access patterns.

[bbannier]

This is an automated email from the ASF dual-hosted git repository.

bbannier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit b7d4e7aff3e34c724945f47b9b967e078f9f46a1
Author: Benjamin Bannier <bbann...@apache.org>
AuthorDate: Thu Sep 19 15:23:56 2019 +0200

    Fixed inefficient `hashmap` access patterns.
    
    This patch fixes some inefficient access patterns around `hashmap::get`.
    Since this function returns an `Option` it can be used as a shorthand
    for a `contains` check and subsequent creation of a value (`Option`
    always contains a value). It was never not intended and is inefficient
    as `contains` itself (e.g., via `hashmap::get::isSome`), and for cases
    where only access to parts of the value in the `hashmap` is required
    (e.g., to access a member of an optional value). In both these cases we
    neither want nor need to create a temporary, and should instead either
    just use `contains`, or access the value with `hashmap::at` after a
    `contains` check; otherwise we might spend a lot of time creating
    unnecessary temporary values.
    
    This patch fixes some easily identifiable cases found from skimming the
    result of the following clang-query command:
    
        match cxxMemberCallExpr(
          on(hasType(cxxRecordDecl(hasName("hashmap")))),
          unless(
            hasParent(cxxBindTemporaryExpr(
              hasParent(materializeTemporaryExpr(
                hasParent(exprWithCleanups(
                  hasParent(varDecl())))))))),
          callee(cxxMethodDecl(hasName("get"))))
    
    This most probably misses a lot of cases. Given how easy it is to misuse
    `hashmap::get` we should consider whether it makes sense to get rid of
    it completely in lieu of an inlined form trading some additional lookups
    for temporary avoidance,
    
        Option<X> x = map.contains(k) ? Some(map.at(k)) : Option<X>::none();
    
    Review: https://reviews.apache.org/r/71520
---
 3rdparty/libprocess/src/metrics/metrics.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/3rdparty/libprocess/src/metrics/metrics.cpp 
b/3rdparty/libprocess/src/metrics/metrics.cpp
index 623d44a..66e78b3 100644
--- a/3rdparty/libprocess/src/metrics/metrics.cpp
+++ b/3rdparty/libprocess/src/metrics/metrics.cpp
@@ -196,7 +196,7 @@ Future<http::Response> MetricsProcess::_snapshot(
   Option<Duration> timeout;
 
   if (request.url.query.contains("timeout")) {
-    string parameter = request.url.query.get("timeout").get();
+    string parameter = request.url.query.at("timeout");
 
     Try<Duration> duration = Duration::parse(parameter);