-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71520/
-----------------------------------------------------------
Review request for mesos, Benno Evers and Benjamin Mahler.
Bugs: MESOS-9948
https://issues.apache.org/jira/browse/MESOS-9948
Repository: mesos
Description
-------
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
for `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 read 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 since otherwise we might spend a lot of time creating
unnecessary temporary values.
This patch fixes some easily identifiable case found by manually
grooming 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 lie of an inlined form trading some additional
lookups for temporary avoidance,
Option<X> x = map.contains(k) ? Some(map.at(k)) : Option<X>::none();
Diffs
-----
3rdparty/libprocess/src/metrics/metrics.cpp
623d44adbe838f995ddbe89ee26f5bcc9c600be5
Diff: https://reviews.apache.org/r/71520/diff/1/
Testing
-------
`make check`
Thanks,
Benjamin Bannier
