Replace ContentBlobContainer<T> with std::unordered_set

8 files changed, 35 insertions, 33 deletions

diff --git a/src/buildtool/execution_api/bazel_msg/bazel_blob_container.hpp b/src/buildtool/execution_api/bazel_msg/bazel_blob_container.hpp
index 039ddf9c..9ac738d6 100644
--- a/src/buildtool/execution_api/bazel_msg/bazel_blob_container.hpp
+++ b/src/buildtool/execution_api/bazel_msg/bazel_blob_container.hpp
@@ -15,10 +15,12 @@
 #ifndef INCLUDED_SRC_BUILDTOOL_EXECUTION_API_BAZEL_MSG_BAZEL_BLOB_CONTAINER_HPP
 #define INCLUDED_SRC_BUILDTOOL_EXECUTION_API_BAZEL_MSG_BAZEL_BLOB_CONTAINER_HPP
 
+#include <unordered_set>
+
 #include "src/buildtool/common/bazel_types.hpp"
 #include "src/buildtool/execution_api/common/content_blob_container.hpp"
 
 using BazelBlob = ContentBlob<bazel_re::Digest>;
-using BazelBlobContainer = ContentBlobContainer<bazel_re::Digest>;
+using BazelBlobContainer = std::unordered_set<BazelBlob>;
 
 #endif  // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_BAZEL_MSG_BAZEL_BLOB_CONTAINER_HPP
diff --git a/src/buildtool/execution_api/common/artifact_blob_container.hpp b/src/buildtool/execution_api/common/artifact_blob_container.hpp
index 32f3fc38..76b0692e 100644
--- a/src/buildtool/execution_api/common/artifact_blob_container.hpp
+++ b/src/buildtool/execution_api/common/artifact_blob_container.hpp
@@ -15,10 +15,12 @@
 #ifndef INCLUDED_SRC_BUILDTOOL_EXECUTION_API_COMMON_ARTIFACT_BLOB_CONTAINER_HPP
 #define INCLUDED_SRC_BUILDTOOL_EXECUTION_API_COMMON_ARTIFACT_BLOB_CONTAINER_HPP
 
+#include <unordered_set>
+
 #include "src/buildtool/common/artifact_digest.hpp"
 #include "src/buildtool/execution_api/common/content_blob_container.hpp"
 
 using ArtifactBlob = ContentBlob<ArtifactDigest>;
-using ArtifactBlobContainer = ContentBlobContainer<ArtifactDigest>;
+using ArtifactBlobContainer = std::unordered_set<ArtifactBlob>;
 
 #endif  // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_COMMON_ARTIFACT_BLOB_CONTAINER_HPP
diff --git a/src/buildtool/execution_api/common/common_api.hpp b/src/buildtool/execution_api/common/common_api.hpp
index f58e4893..5060c086 100644
--- a/src/buildtool/execution_api/common/common_api.hpp
+++ b/src/buildtool/execution_api/common/common_api.hpp
@@ -15,6 +15,7 @@
 #ifndef INCLUDED_SRC_BUILDTOOL_EXECUTION_API_COMMON_COMMON_API_HPP
 #define INCLUDED_SRC_BUILDTOOL_EXECUTION_API_COMMON_COMMON_API_HPP
 
+#include <cstddef>
 #include <cstdio>
 #include <exception>
 #include <functional>
@@ -122,10 +123,11 @@ template <typename TValue, typename TIterator>
 /// \returns Returns true on success, false otherwise (failures or exceptions).
 template <typename TDigest>
 auto UpdateContainerAndUpload(
-    gsl::not_null<ContentBlobContainer<TDigest>*> const& container,
+    gsl::not_null<std::unordered_set<ContentBlob<TDigest>>*> const& container,
     ContentBlob<TDigest>&& blob,
     bool exception_is_fatal,
-    std::function<bool(ContentBlobContainer<TDigest>&&)> const& uploader,
+    std::function<bool(std::unordered_set<ContentBlob<TDigest>>&&)> const&
+        uploader,
     Logger const* logger = nullptr) noexcept -> bool {
     // Optimize upload of blobs with respect to the maximum transfer limit, such
     // that we never store unnecessarily more data in the container than we need
@@ -133,24 +135,32 @@ auto UpdateContainerAndUpload(
     try {
         if (blob.data->size() > kMaxBatchTransferSize) {
             // large blobs use individual stream upload
-            if (not uploader(ContentBlobContainer<TDigest>{{blob}})) {
+            if (not uploader(std::unordered_set<ContentBlob<TDigest>>{
+                    {std::move(blob)}})) {
                 return false;
             }
         }
         else {
-            if (container->ContentSize() + blob.data->size() >
-                kMaxBatchTransferSize) {
-                // swap away from original container to allow move during upload
-                ContentBlobContainer<TDigest> tmp_container{};
-                std::swap(*container, tmp_container);
-                // if we would surpass the transfer limit, upload the current
-                // container and clear it before adding more blobs
-                if (not uploader(std::move(tmp_container))) {
-                    return false;
+            if (not container->contains(blob)) {
+                std::size_t content_size = 0;
+                for (auto const& blob : *container) {
+                    content_size += blob.data->size();
                 }
+
+                if (content_size + blob.data->size() > kMaxBatchTransferSize) {
+                    // swap away from original container to allow move during
+                    // upload
+                    std::unordered_set<ContentBlob<TDigest>> tmp_container{};
+                    std::swap(*container, tmp_container);
+                    // if we would surpass the transfer limit, upload the
+                    // current container and clear it before adding more blobs
+                    if (not uploader(std::move(tmp_container))) {
+                        return false;
+                    }
+                }
+                // add current blob to container
+                container->emplace(std::move(blob));
             }
-            // add current blob to container
-            container->Emplace(std::move(blob));
         }
     } catch (std::exception const& ex) {
         if (exception_is_fatal) {
diff --git a/src/buildtool/execution_api/local/TARGETS b/src/buildtool/execution_api/local/TARGETS
index b9d839a9..a8fd90f0 100644
--- a/src/buildtool/execution_api/local/TARGETS
+++ b/src/buildtool/execution_api/local/TARGETS
@@ -44,7 +44,6 @@
     , ["src/utils/cpp", "expected"]
     , ["src/utils/cpp", "path"]
     , ["src/utils/cpp", "tmp_dir"]
-    , ["src/utils/cpp", "transformed_range"]
     ]
   , "stage": ["src", "buildtool", "execution_api", "local"]
   , "private-deps":
diff --git a/src/buildtool/execution_api/local/local_api.hpp b/src/buildtool/execution_api/local/local_api.hpp
index a9d6b660..07bef457 100644
--- a/src/buildtool/execution_api/local/local_api.hpp
+++ b/src/buildtool/execution_api/local/local_api.hpp
@@ -60,7 +60,6 @@
 #include "src/buildtool/storage/config.hpp"
 #include "src/buildtool/storage/storage.hpp"
 #include "src/utils/cpp/expected.hpp"
-#include "src/utils/cpp/transformed_range.hpp"
 
 /// \brief API for local execution.
 class LocalApi final : public IExecutionApi {
@@ -250,10 +249,9 @@ class LocalApi final : public IExecutionApi {
     [[nodiscard]] auto Upload(ArtifactBlobContainer&& blobs,
                               bool /*skip_find_missing*/) const noexcept
         -> bool final {
-        auto const range = blobs.Blobs();
         return std::all_of(
-            range.begin(),
-            range.end(),
+            blobs.begin(),
+            blobs.end(),
             [&cas = local_context_.storage->CAS()](ArtifactBlob const& blob) {
                 auto const cas_digest =
                     blob.digest.IsTree()
diff --git a/src/buildtool/execution_api/remote/TARGETS b/src/buildtool/execution_api/remote/TARGETS
index 1470d62a..1a7a15af 100644
--- a/src/buildtool/execution_api/remote/TARGETS
+++ b/src/buildtool/execution_api/remote/TARGETS
@@ -63,7 +63,6 @@
     , ["src/utils/cpp", "back_map"]
     , ["src/utils/cpp", "gsl"]
     , ["src/utils/cpp", "path"]
-    , ["src/utils/cpp", "transformed_range"]
     ]
   }
 , "bazel":
@@ -99,7 +98,6 @@
     , ["src/buildtool/multithreading", "task_system"]
     , ["src/utils/cpp", "back_map"]
     , ["src/utils/cpp", "expected"]
-    , ["src/utils/cpp", "transformed_range"]
     ]
   }
 , "config":
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
index 1c8fd96d..9c3de203 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
@@ -49,7 +49,6 @@
 #include "src/buildtool/multithreading/task_system.hpp"
 #include "src/utils/cpp/back_map.hpp"
 #include "src/utils/cpp/expected.hpp"
-#include "src/utils/cpp/transformed_range.hpp"
 
 namespace {
 
@@ -144,8 +143,8 @@ namespace {
     -> std::optional<std::unordered_set<BazelBlob>> {
     std::unordered_set<BazelBlob> blobs;
     try {
-        blobs.reserve(container.Size());
-        for (const auto& blob : container.Blobs()) {
+        blobs.reserve(container.size());
+        for (const auto& blob : container) {
             blobs.emplace(ArtifactDigestFactory::ToBazel(blob.digest),
                           blob.data,
                           blob.is_exec);
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp
index 2a06985d..3aaecb88 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp
@@ -20,7 +20,6 @@
 #include <filesystem>
 #include <functional>
 #include <iterator>
-#include <unordered_set>
 #include <vector>
 
 #include "fmt/core.h"
@@ -41,7 +40,6 @@
 #include "src/buildtool/logging/logger.hpp"
 #include "src/utils/cpp/gsl.hpp"
 #include "src/utils/cpp/path.hpp"
-#include "src/utils/cpp/transformed_range.hpp"
 
 namespace {
 
@@ -278,11 +276,7 @@ auto BazelResponse::UploadTreeMessageDirectories(
     bazel_re::Tree const& tree) const -> expected<ArtifactDigest, std::string> {
     auto const upload_callback =
         [&network = *network_](BazelBlobContainer&& blobs) -> bool {
-        std::unordered_set<BazelBlob> bazel_blobs;
-        for (auto const& blob : blobs.Blobs()) {
-            bazel_blobs.emplace(blob);
-        }
-        return network.UploadBlobs(std::move(bazel_blobs));
+        return network.UploadBlobs(std::move(blobs));
     };
     auto const hash_function = network_->GetHashFunction();
     BazelBlobContainer dir_blobs{};