diff options
author | Oliver Reiche <oliver.reiche@huawei.com> | 2022-10-06 12:18:25 +0200 |
---|---|---|
committer | Oliver Reiche <oliver.reiche@huawei.com> | 2022-10-07 13:30:06 +0200 |
commit | 51f8b186803292f111011d04d5291deb374dc34c (patch) | |
tree | 818606bb46be19cc56618f1d56eb5e3f309f4fae /src | |
parent | 64b8da270611ebb997c3801f09bd06878aee026a (diff) | |
download | justbuild-51f8b186803292f111011d04d5291deb374dc34c.tar.gz |
LocalTreeMap: Prevent tree objects from being stored
... to align with the original idea of caching a flat list
of blob objects, without the need to recursively traverse
any trees. Consequently, we cannot create any map entry in
places where we do not have all sub-tree entries at hand
(e.g., LocalAPI, BazelAPI, BazelResponse).
Diffstat (limited to 'src')
14 files changed, 172 insertions, 226 deletions
diff --git a/src/buildtool/execution_api/common/local_tree_map.hpp b/src/buildtool/execution_api/common/local_tree_map.hpp index 2758c339..915c7a71 100644 --- a/src/buildtool/execution_api/common/local_tree_map.hpp +++ b/src/buildtool/execution_api/common/local_tree_map.hpp @@ -40,19 +40,23 @@ class LocalTreeMap { }; public: - /// \brief Maps blob locations to object infos. + /// \brief Maps blob paths to object infos. + /// Stores a flat list of blobs, without any trees. Subtrees are represented + /// by joining the tree segments for the blob's path. class LocalTree { friend class LocalTreeMap; public: /// \brief Add a new path and info pair to the tree. /// Path must not be absolute, empty, or contain dot-segments. + /// Object MUST NOT be a tree object. /// \param path The location to add the object info. /// \param info The object info to add. /// \returns true if successfully inserted or info existed before. [[nodiscard]] auto AddInfo(std::filesystem::path const& path, Artifact::ObjectInfo const& info) noexcept -> bool { + gsl_Expects(not IsTreeObject(info.type)); auto norm_path = path.lexically_normal(); if (norm_path.is_absolute() or norm_path.empty() or *norm_path.begin() == "..") { diff --git a/src/buildtool/execution_api/local/local_action.cpp b/src/buildtool/execution_api/local/local_action.cpp index 0c15e788..af0f2476 100644 --- a/src/buildtool/execution_api/local/local_action.cpp +++ b/src/buildtool/execution_api/local/local_action.cpp @@ -248,20 +248,9 @@ auto LocalAction::CollectOutputDir(std::filesystem::path const& exec_path, [this](auto path, auto is_exec) { return storage_->StoreBlob</*kOwner=*/true>(path, is_exec); }, - [this](auto bytes, auto dir) -> std::optional<bazel_re::Digest> { - auto digest = storage_->StoreBlob(bytes); - if (digest and not tree_map_->HasTree(*digest)) { - auto tree = tree_map_->CreateTree(); - if (not BazelMsgFactory::ReadObjectInfosFromDirectory( - dir, - [&tree](auto path, auto info) { - return tree.AddInfo(path, info); - }) or - not tree_map_->AddTree(*digest, std::move(tree))) { - return std::nullopt; - } - } - return digest; + [this](auto bytes, + auto /*dir*/) -> std::optional<bazel_re::Digest> { + return storage_->StoreBlob(bytes); }); } else { @@ -271,27 +260,8 @@ auto LocalAction::CollectOutputDir(std::filesystem::path const& exec_path, return storage_->StoreBlob</*kOwner=*/true>(path, is_exec); }, [this](auto bytes, - auto entries) -> std::optional<bazel_re::Digest> { - auto digest = storage_->StoreTree(bytes); - if (digest and not tree_map_->HasTree(*digest)) { - auto tree = tree_map_->CreateTree(); - for (auto const& [raw_id, es] : entries) { - auto id = ToHexString(raw_id); - for (auto const& entry : es) { - auto info = Artifact::ObjectInfo{ - ArtifactDigest{ - id, 0, entry.type == ObjectType::Tree}, - entry.type}; - if (not tree.AddInfo(entry.name, info)) { - return std::nullopt; - } - } - } - if (not tree_map_->AddTree(*digest, std::move(tree))) { - return std::nullopt; - } - } - return digest; + auto /*entries*/) -> std::optional<bazel_re::Digest> { + return storage_->StoreTree(bytes); }); } if (digest) { diff --git a/src/buildtool/execution_api/local/local_action.hpp b/src/buildtool/execution_api/local/local_action.hpp index 43569ea3..315897e9 100644 --- a/src/buildtool/execution_api/local/local_action.hpp +++ b/src/buildtool/execution_api/local/local_action.hpp @@ -37,7 +37,6 @@ class LocalAction final : public IExecutionAction { private: Logger logger_{"LocalExecution"}; std::shared_ptr<LocalStorage> storage_; - std::shared_ptr<LocalTreeMap> tree_map_; ArtifactDigest root_digest_{}; std::vector<std::string> cmdline_{}; std::vector<std::string> output_files_{}; @@ -48,7 +47,6 @@ class LocalAction final : public IExecutionAction { CacheFlag cache_flag_{CacheFlag::CacheOutput}; LocalAction(std::shared_ptr<LocalStorage> storage, - std::shared_ptr<LocalTreeMap> tree_map, ArtifactDigest root_digest, std::vector<std::string> command, std::vector<std::string> output_files, @@ -56,7 +54,6 @@ class LocalAction final : public IExecutionAction { std::map<std::string, std::string> env_vars, std::map<std::string, std::string> const& properties) noexcept : storage_{std::move(storage)}, - tree_map_{std::move(tree_map)}, root_digest_{std::move(root_digest)}, cmdline_{std::move(command)}, output_files_{std::move(output_files)}, diff --git a/src/buildtool/execution_api/local/local_api.hpp b/src/buildtool/execution_api/local/local_api.hpp index 38b98601..13bcb7e3 100644 --- a/src/buildtool/execution_api/local/local_api.hpp +++ b/src/buildtool/execution_api/local/local_api.hpp @@ -10,7 +10,6 @@ #include "src/buildtool/compatibility/native_support.hpp" #include "src/buildtool/execution_api/bazel_msg/bazel_blob.hpp" #include "src/buildtool/execution_api/common/execution_api.hpp" -#include "src/buildtool/execution_api/common/local_tree_map.hpp" #include "src/buildtool/execution_api/local/local_action.hpp" #include "src/buildtool/execution_api/local/local_storage.hpp" @@ -26,7 +25,6 @@ class LocalApi final : public IExecutionApi { std::map<std::string, std::string> const& properties) noexcept -> IExecutionAction::Ptr final { return IExecutionAction::Ptr{new LocalAction{storage_, - tree_map_, root_digest, command, output_files, @@ -202,11 +200,9 @@ class LocalApi final : public IExecutionApi { std::vector<DependencyGraph::NamedArtifactNodePtr> const& artifacts) noexcept -> std::optional<ArtifactDigest> final { BlobContainer blobs{}; - auto tree = tree_map_->CreateTree(); auto digest = BazelMsgFactory::CreateDirectoryDigestFromTree( artifacts, - [&blobs](BazelBlob&& blob) { blobs.Emplace(std::move(blob)); }, - [&tree](auto path, auto info) { return tree.AddInfo(path, info); }); + [&blobs](BazelBlob&& blob) { blobs.Emplace(std::move(blob)); }); if (not digest) { Logger::Log(LogLevel::Debug, "failed to create digest for tree."); return std::nullopt; @@ -217,10 +213,7 @@ class LocalApi final : public IExecutionApi { return std::nullopt; } - if (tree_map_->AddTree(*digest, std::move(tree))) { - return ArtifactDigest{*digest}; - } - return std::nullopt; + return ArtifactDigest{*digest}; } [[nodiscard]] auto IsAvailable(ArtifactDigest const& digest) const noexcept @@ -247,9 +240,7 @@ class LocalApi final : public IExecutionApi { } private: - std::shared_ptr<LocalTreeMap> tree_map_{std::make_shared<LocalTreeMap>()}; - std::shared_ptr<LocalStorage> storage_{ - std::make_shared<LocalStorage>(tree_map_)}; + std::shared_ptr<LocalStorage> storage_{std::make_shared<LocalStorage>()}; }; #endif // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_LOCAL_LOCAL_API_HPP diff --git a/src/buildtool/execution_api/local/local_storage.cpp b/src/buildtool/execution_api/local/local_storage.cpp index e6d9571c..483f664f 100644 --- a/src/buildtool/execution_api/local/local_storage.cpp +++ b/src/buildtool/execution_api/local/local_storage.cpp @@ -114,69 +114,77 @@ auto LocalStorage::ReadObjectInfosRecursively( std::filesystem::path const& parent, bazel_re::Digest const& digest) const noexcept -> bool { // read from in-memory tree map - if (tree_map_) { - auto const* tree = tree_map_->GetTree(digest); - if (tree != nullptr) { - return std::all_of( - tree->begin(), - tree->end(), - // NOLINTNEXTLINE(misc-no-recursion) - [this, &store_info, &parent](auto const& entry) { - try { - auto const& [path, info] = entry; - return IsTreeObject(info->type) - ? ReadObjectInfosRecursively(store_info, - parent / path, - info->digest) - : store_info(parent / path, *info); - } catch (...) { // satisfy clang-tidy, store_info() could - return false; - } - }); - } - Logger::Log( - LogLevel::Debug, "tree {} not found in tree map", digest.hash()); + auto const* tree = tree_map_.GetTree(digest); + if (tree != nullptr) { + return std::all_of( + tree->begin(), + tree->end(), + [&store_info, &parent](auto const& entry) { + try { + // LocalTree (from tree_map_) is flat, no recursion needed + auto const& [path, info] = entry; + return store_info(parent / path, *info); + } catch (...) { // satisfy clang-tidy, store_info() could + return false; + } + }); } + Logger::Log( + LogLevel::Debug, "tree {} not found in tree map", digest.hash()); // fallback read from CAS and cache it in in-memory tree map if (Compatibility::IsCompatible()) { if (auto dir = ReadDirectory(this, digest)) { - auto tree = tree_map_ ? std::make_optional(tree_map_->CreateTree()) - : std::nullopt; + auto tree = tree_map_.CreateTree(); return BazelMsgFactory::ReadObjectInfosFromDirectory( *dir, [this, &store_info, &parent, &tree](auto path, auto info) { - return (not tree or tree->AddInfo(path, info)) and - (IsTreeObject(info.type) - ? ReadObjectInfosRecursively( - store_info, - parent / path, - info.digest) - : store_info(parent / path, info)); + if (IsTreeObject(info.type)) { + // LocalTree (from tree_map_) is flat, so + // recursively traverse subtrees and add blobs. + auto tree_store_info = + [&store_info, &tree, &parent](auto path, + auto info) { + auto tree_path = + path.lexically_relative(parent); + return tree.AddInfo(tree_path, info) and + store_info(path, info); + }; + return ReadObjectInfosRecursively( + tree_store_info, parent / path, info.digest); + } + return tree.AddInfo(path, info) and + store_info(parent / path, info); }) and - (not tree_map_ or - tree_map_->AddTree(digest, std::move(*tree))); + tree_map_.AddTree(digest, std::move(tree)); } } else { if (auto entries = ReadGitTree(this, digest)) { - auto tree = tree_map_ ? std::make_optional(tree_map_->CreateTree()) - : std::nullopt; + auto tree = tree_map_.CreateTree(); return BazelMsgFactory::ReadObjectInfosFromGitTree( *entries, [this, &store_info, &parent, &tree](auto path, auto info) { - return (not tree or tree->AddInfo(path, info)) and - (IsTreeObject(info.type) - ? ReadObjectInfosRecursively( - store_info, - parent / path, - info.digest) - : store_info(parent / path, info)); + if (IsTreeObject(info.type)) { + // LocalTree (from tree_map_) is flat, so + // recursively traverse subtrees and add blobs. + auto tree_store_info = + [&store_info, &tree, &parent](auto path, + auto info) { + auto tree_path = + path.lexically_relative(parent); + return tree.AddInfo(tree_path, info) and + store_info(path, info); + }; + return ReadObjectInfosRecursively( + tree_store_info, parent / path, info.digest); + } + return tree.AddInfo(path, info) and + store_info(parent / path, info); }) and - (not tree_map_ or - tree_map_->AddTree(digest, std::move(*tree))); + tree_map_.AddTree(digest, std::move(tree)); } } return false; diff --git a/src/buildtool/execution_api/local/local_storage.hpp b/src/buildtool/execution_api/local/local_storage.hpp index 4ffe2881..f9594218 100644 --- a/src/buildtool/execution_api/local/local_storage.hpp +++ b/src/buildtool/execution_api/local/local_storage.hpp @@ -12,10 +12,6 @@ class LocalStorage { public: - explicit LocalStorage( - std::shared_ptr<LocalTreeMap> tree_map = nullptr) noexcept - : tree_map_{std::move(tree_map)} {} - /// \brief Store blob from file path with x-bit determined from file system. template <bool kOwner = false> [[nodiscard]] auto StoreBlob(std::filesystem::path const& file_path) @@ -89,7 +85,7 @@ class LocalStorage { LocalCAS<ObjectType::Executable> cas_exec_{}; LocalCAS<ObjectType::Tree> cas_tree_{}; LocalAC ac_{&cas_file_}; - std::shared_ptr<LocalTreeMap> tree_map_; + mutable LocalTreeMap tree_map_; /// \brief Try to sync blob between file CAS and executable CAS. /// \param digest Blob digest. diff --git a/src/buildtool/execution_api/remote/bazel/bazel_action.cpp b/src/buildtool/execution_api/remote/bazel/bazel_action.cpp index 34fc5380..c2c44280 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_action.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_action.cpp @@ -6,7 +6,6 @@ BazelAction::BazelAction( std::shared_ptr<BazelNetwork> network, - std::shared_ptr<LocalTreeMap> tree_map, bazel_re::Digest root_digest, std::vector<std::string> command, std::vector<std::string> output_files, @@ -14,7 +13,6 @@ BazelAction::BazelAction( std::map<std::string, std::string> const& env_vars, std::map<std::string, std::string> const& properties) noexcept : network_{std::move(network)}, - tree_map_{std::move(tree_map)}, root_digest_{std::move(root_digest)}, cmdline_{std::move(command)}, output_files_{std::move(output_files)}, @@ -47,7 +45,7 @@ auto BazelAction::Execute(Logger const* logger) noexcept network_->GetCachedActionResult(action, output_files_)) { if (result->exit_code() == 0) { return IExecutionResponse::Ptr{new BazelResponse{ - action.hash(), network_, tree_map_, {*result, true}}}; + action.hash(), network_, {*result, true}}}; } } } @@ -59,14 +57,11 @@ auto BazelAction::Execute(Logger const* logger) noexcept auto action_id = CreateBundlesForAction(nullptr, root_digest_, false).hash(); output->cached_result = true; - return IExecutionResponse::Ptr{ - new BazelResponse{std::move(action_id), - network_, - tree_map_, - std::move(*output)}}; + return IExecutionResponse::Ptr{new BazelResponse{ + std::move(action_id), network_, std::move(*output)}}; } - return IExecutionResponse::Ptr{new BazelResponse{ - action.hash(), network_, tree_map_, std::move(*output)}}; + return IExecutionResponse::Ptr{ + new BazelResponse{action.hash(), network_, std::move(*output)}}; } } diff --git a/src/buildtool/execution_api/remote/bazel/bazel_action.hpp b/src/buildtool/execution_api/remote/bazel/bazel_action.hpp index 7eb9a9e0..84922a86 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_action.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_action.hpp @@ -26,7 +26,6 @@ class BazelAction final : public IExecutionAction { private: std::shared_ptr<BazelNetwork> network_; - std::shared_ptr<LocalTreeMap> tree_map_; bazel_re::Digest const root_digest_; std::vector<std::string> const cmdline_; std::vector<std::string> output_files_; @@ -37,7 +36,6 @@ class BazelAction final : public IExecutionAction { std::chrono::milliseconds timeout_{kDefaultTimeout}; BazelAction(std::shared_ptr<BazelNetwork> network, - std::shared_ptr<LocalTreeMap> tree_map, bazel_re::Digest root_digest, std::vector<std::string> command, std::vector<std::string> output_files, diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp index a8846325..d1de9a68 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp @@ -24,9 +24,8 @@ BazelApi::BazelApi(std::string const& instance_name, std::string const& host, Port port, ExecutionConfiguration const& exec_config) noexcept { - tree_map_ = std::make_shared<LocalTreeMap>(); - network_ = std::make_shared<BazelNetwork>( - instance_name, host, port, exec_config, tree_map_); + network_ = + std::make_shared<BazelNetwork>(instance_name, host, port, exec_config); } // implement move constructor in cpp, where all members are complete types @@ -44,7 +43,6 @@ auto BazelApi::CreateAction( std::map<std::string, std::string> const& properties) noexcept -> IExecutionAction::Ptr { return std::unique_ptr<BazelAction>{new BazelAction{network_, - tree_map_, root_digest, command, output_files, @@ -227,11 +225,9 @@ auto BazelApi::CreateAction( std::vector<DependencyGraph::NamedArtifactNodePtr> const& artifacts) noexcept -> std::optional<ArtifactDigest> { BlobContainer blobs{}; - auto tree = tree_map_->CreateTree(); auto digest = BazelMsgFactory::CreateDirectoryDigestFromTree( artifacts, - [&blobs](BazelBlob&& blob) { blobs.Emplace(std::move(blob)); }, - [&tree](auto path, auto info) { return tree.AddInfo(path, info); }); + [&blobs](BazelBlob&& blob) { blobs.Emplace(std::move(blob)); }); if (not digest) { Logger::Log(LogLevel::Debug, "failed to create digest for tree."); return std::nullopt; @@ -248,10 +244,8 @@ auto BazelApi::CreateAction( Logger::Log(LogLevel::Debug, "failed to upload blobs for tree."); return std::nullopt; } - if (tree_map_->AddTree(*digest, std::move(tree))) { - return ArtifactDigest{*digest}; - } - return std::nullopt; + + return ArtifactDigest{*digest}; } [[nodiscard]] auto BazelApi::IsAvailable( diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.hpp b/src/buildtool/execution_api/remote/bazel/bazel_api.hpp index 3fccf226..27310f07 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_api.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_api.hpp @@ -10,7 +10,6 @@ #include "src/buildtool/common/artifact_digest.hpp" #include "src/buildtool/execution_api/bazel_msg/bazel_common.hpp" #include "src/buildtool/execution_api/common/execution_api.hpp" -#include "src/buildtool/execution_api/common/local_tree_map.hpp" #include "src/buildtool/execution_api/remote/config.hpp" // forward declaration for actual implementations @@ -68,7 +67,6 @@ class BazelApi final : public IExecutionApi { private: std::shared_ptr<BazelNetwork> network_; - std::shared_ptr<LocalTreeMap> tree_map_; }; #endif // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_REMOTE_BAZEL_BAZEL_API_HPP diff --git a/src/buildtool/execution_api/remote/bazel/bazel_network.cpp b/src/buildtool/execution_api/remote/bazel/bazel_network.cpp index ff151824..fa09b16c 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_network.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_network.cpp @@ -80,14 +80,12 @@ namespace { BazelNetwork::BazelNetwork(std::string instance_name, std::string const& host, Port port, - ExecutionConfiguration const& exec_config, - std::shared_ptr<LocalTreeMap> tree_map) noexcept + ExecutionConfiguration const& exec_config) noexcept : instance_name_{std::move(instance_name)}, exec_config_{exec_config}, cas_{std::make_unique<BazelCasClient>(host, port)}, ac_{std::make_unique<BazelAcClient>(host, port)}, - exec_{std::make_unique<BazelExecutionClient>(host, port)}, - tree_map_{std::move(tree_map)} {} + exec_{std::make_unique<BazelExecutionClient>(host, port)} {} auto BazelNetwork::IsAvailable(bazel_re::Digest const& digest) const noexcept -> bool { @@ -245,8 +243,7 @@ auto BazelNetwork::ReadTreeInfos(bazel_re::Digest const& tree_digest, -> std::optional<std::pair<std::vector<std::filesystem::path>, std::vector<Artifact::ObjectInfo>>> { std::optional<DirectoryMap> dir_map{std::nullopt}; - if (Compatibility::IsCompatible() and - (request_remote_tree or not tree_map_)) { + if (Compatibility::IsCompatible() and request_remote_tree) { // Query full tree from remote CAS. Note that this is currently not // supported by Buildbarn revision c3c06bbe2a. auto dirs = @@ -289,102 +286,117 @@ auto BazelNetwork::ReadObjectInfosRecursively( std::filesystem::path const& parent, bazel_re::Digest const& digest) const noexcept -> bool { // read from in-memory tree map - if (tree_map_) { - auto const* tree = tree_map_->GetTree(digest); - if (tree != nullptr) { - return std::all_of( - tree->begin(), - tree->end(), - // NOLINTNEXTLINE(misc-no-recursion) - [this, &dir_map, &store_info, &parent](auto const& entry) { + auto const* tree = tree_map_.GetTree(digest); + if (tree != nullptr) { + return std::all_of( + tree->begin(), + tree->end(), + [&store_info, &parent](auto const& entry) { + try { + // LocalTree (from tree_map_) is flat, no recursion needed auto const& [path, info] = entry; - try { - return IsTreeObject(info->type) - ? ReadObjectInfosRecursively(dir_map, - store_info, - parent / path, - info->digest) - : store_info(parent / path, *info); - } catch ( - ...) { // satisfy clang-tidy, store_info() could throw - return false; - } - }); - } - Logger::Log( - LogLevel::Debug, "tree {} not found in tree map", digest.hash()); + return store_info(parent / path, *info); + } catch (...) { // satisfy clang-tidy, store_info() could throw + return false; + } + }); } + Logger::Log( + LogLevel::Debug, "tree {} not found in tree map", digest.hash()); if (Compatibility::IsCompatible()) { // read from in-memory Directory map and cache it in in-memory tree map if (dir_map) { if (dir_map->contains(digest)) { - auto tree = tree_map_ - ? std::make_optional(tree_map_->CreateTree()) - : std::nullopt; + auto tree = tree_map_.CreateTree(); return BazelMsgFactory::ReadObjectInfosFromDirectory( dir_map->at(digest), [this, &dir_map, &store_info, &parent, &tree]( auto path, auto info) { - return (not tree or - tree->AddInfo(path, info)) and - (IsTreeObject(info.type) - ? (not tree or - tree->AddInfo(path, info)) and - ReadObjectInfosRecursively( - dir_map, - store_info, - parent / path, - info.digest) - : store_info(parent / path, info)); + if (IsTreeObject(info.type)) { + // LocalTree (from tree_map_) is flat, so + // recursively traverse subtrees and add + // blobs. + auto tree_store_info = [&store_info, + &tree, + &parent](auto path, + auto info) { + auto tree_path = + path.lexically_relative(parent); + return tree.AddInfo(tree_path, info) and + store_info(path, info); + }; + return ReadObjectInfosRecursively( + dir_map, + tree_store_info, + parent / path, + info.digest); + } + return tree.AddInfo(path, info) and + store_info(parent / path, info); }) and - (not tree_map_ or - tree_map_->AddTree(digest, std::move(*tree))); + tree_map_.AddTree(digest, std::move(tree)); } - Logger::Log( - LogLevel::Debug, "tree {} not found in dir map", digest.hash()); } // fallback read from CAS and cache it in in-memory tree map if (auto dir = ReadDirectory(this, digest)) { - auto tree = tree_map_ ? std::make_optional(tree_map_->CreateTree()) - : std::nullopt; + auto tree = tree_map_.CreateTree(); return BazelMsgFactory::ReadObjectInfosFromDirectory( *dir, [this, &dir_map, &store_info, &parent, &tree]( auto path, auto info) { - return (not tree or tree->AddInfo(path, info)) and - (IsTreeObject(info.type) - ? ReadObjectInfosRecursively( - dir_map, - store_info, - parent / path, - info.digest) - : store_info(parent / path, info)); + if (IsTreeObject(info.type)) { + // LocalTree (from tree_map_) is flat, so + // recursively traverse subtrees and add blobs. + auto tree_store_info = + [&store_info, &tree, &parent](auto path, + auto info) { + auto tree_path = + path.lexically_relative(parent); + return tree.AddInfo(tree_path, info) and + store_info(path, info); + }; + return ReadObjectInfosRecursively( + dir_map, + tree_store_info, + parent / path, + info.digest); + } + return tree.AddInfo(path, info) and + store_info(parent / path, info); }) and - (not tree_map_ or - tree_map_->AddTree(digest, std::move(*tree))); + tree_map_.AddTree(digest, std::move(tree)); } } else { if (auto entries = ReadGitTree(this, digest)) { - auto tree = tree_map_ ? std::make_optional(tree_map_->CreateTree()) - : std::nullopt; + auto tree = tree_map_.CreateTree(); return BazelMsgFactory::ReadObjectInfosFromGitTree( *entries, [this, &dir_map, &store_info, &parent, &tree]( auto path, auto info) { - return (not tree or tree->AddInfo(path, info)) and - (IsTreeObject(info.type) - ? ReadObjectInfosRecursively( - dir_map, - store_info, - parent / path, - info.digest) - : store_info(parent / path, info)); + if (IsTreeObject(info.type)) { + // LocalTree (from tree_map_) is flat, so + // recursively traverse subtrees and add blobs. + auto tree_store_info = + [&store_info, &tree, &parent](auto path, + auto info) { + auto tree_path = + path.lexically_relative(parent); + return tree.AddInfo(tree_path, info) and + store_info(path, info); + }; + return ReadObjectInfosRecursively( + dir_map, + tree_store_info, + parent / path, + info.digest); + } + return tree.AddInfo(path, info) and + store_info(parent / path, info); }) and - (not tree_map_ or - tree_map_->AddTree(digest, std::move(*tree))); + tree_map_.AddTree(digest, std::move(tree)); } } return false; diff --git a/src/buildtool/execution_api/remote/bazel/bazel_network.hpp b/src/buildtool/execution_api/remote/bazel/bazel_network.hpp index 13a5bcf7..6e9d9c85 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_network.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_network.hpp @@ -45,8 +45,7 @@ class BazelNetwork { BazelNetwork(std::string instance_name, std::string const& host, Port port, - ExecutionConfiguration const& exec_config, - std::shared_ptr<LocalTreeMap> tree_map = nullptr) noexcept; + ExecutionConfiguration const& exec_config) noexcept; /// \brief Check if digest exists in CAS /// \param[in] digest The digest to look up @@ -105,7 +104,7 @@ class BazelNetwork { std::unique_ptr<BazelCasClient> cas_{}; std::unique_ptr<BazelAcClient> ac_{}; std::unique_ptr<BazelExecutionClient> exec_{}; - std::shared_ptr<LocalTreeMap> tree_map_{}; + mutable LocalTreeMap tree_map_{}; template <class T_Iter> [[nodiscard]] auto DoUploadBlobs(T_Iter const& first, diff --git a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp index d05517a1..6ae2457e 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp @@ -5,6 +5,17 @@ #include "src/buildtool/execution_api/remote/bazel/bazel_cas_client.hpp" #include "src/buildtool/logging/logger.hpp" +namespace { + +auto ProcessDirectoryMessage(bazel_re::Directory const& dir) noexcept + -> std::optional<BazelBlob> { + auto data = dir.SerializeAsString(); + auto digest = ArtifactDigest::Create(data); + return BazelBlob{std::move(digest), std::move(data)}; +} + +} // namespace + auto BazelResponse::ReadStringBlob(bazel_re::Digest const& id) noexcept -> std::string { auto blobs = network_->ReadBlobs({id}).Next(); @@ -120,24 +131,3 @@ auto BazelResponse::UploadTreeMessageDirectories( } return ArtifactDigest{root_digest}; } - -auto BazelResponse::ProcessDirectoryMessage( - bazel_re::Directory const& dir) const noexcept -> std::optional<BazelBlob> { - auto data = dir.SerializeAsString(); - auto digest = ArtifactDigest::Create(data); - - if (tree_map_ and not tree_map_->HasTree(digest)) { - // cache in local tree map - auto tree = tree_map_->CreateTree(); - if (not BazelMsgFactory::ReadObjectInfosFromDirectory( - dir, - [&tree](auto path, auto info) { - return tree.AddInfo(path, info); - }) or - not tree_map_->AddTree(digest, std::move(tree))) { - return std::nullopt; - } - } - - return BazelBlob{std::move(digest), std::move(data)}; -} diff --git a/src/buildtool/execution_api/remote/bazel/bazel_response.hpp b/src/buildtool/execution_api/remote/bazel/bazel_response.hpp index 778efa0a..f304210f 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_response.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_response.hpp @@ -47,16 +47,13 @@ class BazelResponse final : public IExecutionResponse { private: std::string action_id_{}; std::shared_ptr<BazelNetwork> const network_{}; - std::shared_ptr<LocalTreeMap> const tree_map_{}; BazelExecutionClient::ExecutionOutput output_{}; BazelResponse(std::string action_id, std::shared_ptr<BazelNetwork> network, - std::shared_ptr<LocalTreeMap> tree_map, BazelExecutionClient::ExecutionOutput output) : action_id_{std::move(action_id)}, network_{std::move(network)}, - tree_map_{std::move(tree_map)}, output_{std::move(output)} {} [[nodiscard]] auto ReadStringBlob(bazel_re::Digest const& id) noexcept @@ -69,9 +66,6 @@ class BazelResponse final : public IExecutionResponse { [[nodiscard]] auto UploadTreeMessageDirectories( bazel_re::Tree const& tree) const -> std::optional<ArtifactDigest>; - - [[nodiscard]] auto ProcessDirectoryMessage(bazel_re::Directory const& dir) - const noexcept -> std::optional<BazelBlob>; }; #endif // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_REMOTE_BAZEL_BAZEL_RESPONSE_HPP |