Avoid unnecessary work in accessing container entries

- in sequence containers, use operator[] instead of .at() when
accessing indices guaranteed to be in bound;
- in associative containers, prefer .find() and reusing the
returned const iterator to using .contains() and .at(); while
there, make any so obtained iterators const if they are read-only.

7 files changed, 40 insertions, 34 deletions

diff --git a/src/buildtool/build_engine/expression/evaluator.cpp b/src/buildtool/build_engine/expression/evaluator.cpp
index 3283c2b1..0aa553e8 100644
--- a/src/buildtool/build_engine/expression/evaluator.cpp
+++ b/src/buildtool/build_engine/expression/evaluator.cpp
@@ -840,14 +840,11 @@ auto LookupExpr(SubExprEvaluator&& eval,
         throw Evaluator::EvaluationError{fmt::format(
             "Map expected to be mapping but found {}.", d->ToString())};
     }
-    auto lookup = Expression::kNone;
-    if (d->Map().contains(k->String())) {
-        lookup = d->Map().at(k->String());
+    auto const& lookup = d->Map().Find(k->String());
+    if (lookup and (*lookup)->IsNotNull()) {
+        return **lookup;
     }
-    if (lookup->IsNone()) {
-        lookup = eval(expr->Get("default", Expression::none_t()), env);
-    }
-    return lookup;
+    return eval(expr->Get("default", Expression::none_t()), env);
 }
 
 auto ArrayAccessExpr(SubExprEvaluator&& eval,
@@ -936,7 +933,7 @@ auto ToSubdirExpr(SubExprEvaluator&& eval,
     else {
         for (auto const& el : d->Map()) {
             auto new_key = ToNormalPath(subdir / el.first).string();
-            if (auto it = result.find(new_key);
+            if (auto const it = result.find(new_key);
                 it != result.end() and
                 (not((it->second == el.second) and el.second->IsCacheable()))) {
                 auto msg_expr = expr->Map().Find("msg");
@@ -979,7 +976,7 @@ auto FromSubdirExpr(SubExprEvaluator&& eval,
             std::filesystem::path(el.first).lexically_relative(subdir));
         if (PathIsNonUpwards(new_path)) {
             auto new_key = new_path.string();
-            if (auto it = result.find(new_key);
+            if (auto const it = result.find(new_key);
                 it != result.end() &&
                 (!((it->second == el.second) && el.second->IsCacheable()))) {
                 throw Evaluator::EvaluationError{
diff --git a/src/buildtool/common/cli.hpp b/src/buildtool/common/cli.hpp
index ff4db0bb..096044e0 100644
--- a/src/buildtool/common/cli.hpp
+++ b/src/buildtool/common/cli.hpp
@@ -26,6 +26,7 @@
 #include <string>
 #include <thread>
 #include <type_traits>
+#include <unordered_map>
 #include <vector>
 
 #include "CLI/CLI.hpp"
@@ -705,8 +706,9 @@ static inline auto SetupToAddArguments(
     app->add_option_function<std::string>(
         "--resolve-special",
         [clargs](auto const& raw_value) {
-            if (kResolveSpecialMap.contains(raw_value)) {
-                clargs->resolve_special = kResolveSpecialMap.at(raw_value);
+            if (auto const it = kResolveSpecialMap.find(raw_value);
+                it != kResolveSpecialMap.end()) {
+                clargs->resolve_special = it->second;
             }
             else {
                 Logger::Log(LogLevel::Warning,
diff --git a/src/buildtool/execution_api/common/bytestream_utils.cpp b/src/buildtool/execution_api/common/bytestream_utils.cpp
index 93acede8..fa91cd98 100644
--- a/src/buildtool/execution_api/common/bytestream_utils.cpp
+++ b/src/buildtool/execution_api/common/bytestream_utils.cpp
@@ -34,15 +34,15 @@ namespace {
         std::size_t shift = 0;
         for (std::size_t length = 0; shift + length < request.size();
              ++length) {
-            if (request.at(shift + length) == '/') {
-                parts.emplace_back(&request.at(shift), length);
+            if (request[shift + length] == '/') {
+                parts.emplace_back(&request[shift], length);
                 shift += length + 1;
                 length = 0;
             }
         }
 
         if (shift < request.size()) {
-            parts.emplace_back(&request.at(shift), request.size() - shift);
+            parts.emplace_back(&request[shift], request.size() - shift);
         }
     } catch (...) {
         return {};
@@ -80,15 +80,15 @@ auto ByteStreamUtils::ReadRequest::FromString(
 
     auto const parts = ::SplitRequest(request);
     if (parts.size() != kReadRequestPartsCount or
-        parts.at(kBlobsIndex).compare(ByteStreamUtils::kBlobs) != 0) {
+        parts[kBlobsIndex].compare(ByteStreamUtils::kBlobs) != 0) {
         return std::nullopt;
     }
 
     ReadRequest result;
-    result.instance_name_ = std::string(parts.at(kInstanceNameIndex));
-    result.hash_ = std::string(parts.at(kHashIndex));
+    result.instance_name_ = std::string(parts[kInstanceNameIndex]);
+    result.hash_ = std::string(parts[kHashIndex]);
     try {
-        result.size_ = std::stoi(std::string(parts.at(kSizeIndex)));
+        result.size_ = std::stoi(std::string(parts[kSizeIndex]));
     } catch (...) {
         return std::nullopt;
     }
@@ -126,17 +126,17 @@ auto ByteStreamUtils::WriteRequest::FromString(
 
     auto const parts = ::SplitRequest(request);
     if (parts.size() != kWriteRequestPartsCount or
-        parts.at(kUploadsIndex).compare(ByteStreamUtils::kUploads) != 0 or
-        parts.at(kBlobsIndex).compare(ByteStreamUtils::kBlobs) != 0) {
+        parts[kUploadsIndex].compare(ByteStreamUtils::kUploads) != 0 or
+        parts[kBlobsIndex].compare(ByteStreamUtils::kBlobs) != 0) {
         return std::nullopt;
     }
 
     WriteRequest result;
-    result.instance_name_ = std::string(parts.at(kInstanceNameIndex));
-    result.uuid_ = std::string(parts.at(kUUIDIndex));
-    result.hash_ = std::string(parts.at(kHashIndex));
+    result.instance_name_ = std::string(parts[kInstanceNameIndex]);
+    result.uuid_ = std::string(parts[kUUIDIndex]);
+    result.hash_ = std::string(parts[kHashIndex]);
     try {
-        result.size_ = std::stoul(std::string(parts.at(kSizeIndex)));
+        result.size_ = std::stoul(std::string(parts[kSizeIndex]));
     } catch (...) {
         return std::nullopt;
     }
diff --git a/src/buildtool/execution_api/local/local_action.cpp b/src/buildtool/execution_api/local/local_action.cpp
index d202233f..174caf08 100644
--- a/src/buildtool/execution_api/local/local_action.cpp
+++ b/src/buildtool/execution_api/local/local_action.cpp
@@ -337,7 +337,7 @@ auto LocalAction::StageInputs(
         return false;
     }
     for (std::size_t i{}; i < result->paths.size(); ++i) {
-        if (not StageInput(result->paths.at(i), result->infos.at(i), copies)) {
+        if (not StageInput(result->paths[i], result->infos[i], copies)) {
             return false;
         }
     }
diff --git a/src/buildtool/execution_engine/executor/executor.hpp b/src/buildtool/execution_engine/executor/executor.hpp
index f009783d..5f5c2e92 100644
--- a/src/buildtool/execution_engine/executor/executor.hpp
+++ b/src/buildtool/execution_engine/executor/executor.hpp
@@ -632,8 +632,8 @@ class ExecutorImpl {
         std::vector<DependencyGraph::NamedArtifactNodePtr> const& artifacts)
         -> std::optional<ArtifactDigest> {
         if (artifacts.size() == 1 and
-            (artifacts.at(0).path == "." or artifacts.at(0).path.empty())) {
-            auto const& info = artifacts.at(0).node->Content().Info();
+            (artifacts[0].path == "." or artifacts[0].path.empty())) {
+            auto const& info = artifacts[0].node->Content().Info();
             if (info and IsTreeObject(info->type)) {
                 // Artifact list contains single tree with path "." or "". Reuse
                 // the existing tree artifact by returning its digest.
@@ -831,7 +831,7 @@ class ExecutorImpl {
             msg << nlohmann::json(action->Env()).dump();
         }
         auto const& origin_map = progress->OriginMap();
-        auto origins = origin_map.find(action->Content().Id());
+        auto const origins = origin_map.find(action->Content().Id());
         if (origins != origin_map.end() and not origins->second.empty()) {
             msg << "\nrequested by";
             for (auto const& origin : origins->second) {
@@ -887,7 +887,7 @@ class ExecutorImpl {
              remote_context->exec_config->dispatch) {
             bool match = true;
             for (auto const& [k, v] : pred) {
-                auto v_it = properties.find(k);
+                auto const v_it = properties.find(k);
                 if (not(v_it != properties.end() and v_it->second == v)) {
                     match = false;
                 }
diff --git a/src/buildtool/execution_engine/tree_operations/tree_operations_utils.cpp b/src/buildtool/execution_engine/tree_operations/tree_operations_utils.cpp
index ecf08e8e..fb7d3bf0 100644
--- a/src/buildtool/execution_engine/tree_operations/tree_operations_utils.cpp
+++ b/src/buildtool/execution_engine/tree_operations/tree_operations_utils.cpp
@@ -221,9 +221,16 @@ auto TreeOperationsUtils::SerializeGitTree(
         }
         auto it = git_entries.find(*git_hash);
         if (it == git_entries.end()) {
-            git_entries.insert({*git_hash, std::vector<GitRepo::TreeEntry>{}});
+            if (auto res = git_entries.insert(
+                    {*git_hash, std::vector<GitRepo::TreeEntry>{}});
+                res.second) {
+                it = std::move(res).first;
+            }
+            else {
+                return std::nullopt;
+            }
         }
-        git_entries.at(*git_hash).emplace_back(name, entry.info.type);
+        it->second.emplace_back(name, entry.info.type);
     }
 
     // Serialize git entries.
diff --git a/src/buildtool/main/build_utils.cpp b/src/buildtool/main/build_utils.cpp
index 684d2dd1..17b26d80 100644
--- a/src/buildtool/main/build_utils.cpp
+++ b/src/buildtool/main/build_utils.cpp
@@ -92,7 +92,8 @@ auto CreateTargetCacheWriterMap(
             // get the TaretCacheKey corresponding to this Id
             TargetCacheKey tc_key{key};
             // check if entry actually needs storing
-            if (not cache_targets.contains(tc_key)) {
+            auto const tc_key_it = cache_targets.find(tc_key);
+            if (tc_key_it == cache_targets.end()) {
                 if (tc.Read(tc_key)) {
                     // entry already in target-cache, so nothing to be done
                     (*setter)(nullptr);
@@ -104,9 +105,8 @@ auto CreateTargetCacheWriterMap(
                           true);
                 return;
             }
-            auto const& target = cache_targets.at(tc_key);
             auto entry = TargetCacheEntry::FromTarget(
-                apis->remote->GetHashType(), target, extra_infos);
+                apis->remote->GetHashType(), tc_key_it->second, extra_infos);
             if (not entry) {
                 (*logger)(
                     fmt::format("Failed creating target cache entry for key {}",