LocalCAS: Prefer hard link over copy for owned files

5 files changed, 48 insertions, 34 deletions

diff --git a/src/buildtool/execution_api/local/file_storage.hpp b/src/buildtool/execution_api/local/file_storage.hpp
index df02d6a5..01e8085d 100644
--- a/src/buildtool/execution_api/local/file_storage.hpp
+++ b/src/buildtool/execution_api/local/file_storage.hpp
@@ -26,10 +26,11 @@ class FileStorage {
 
     /// \brief Add file to storage.
     /// \returns true if file exists afterward.
-    [[nodiscard]] auto AddFromFile(
-        std::string const& id,
-        std::filesystem::path const& source_path) const noexcept -> bool {
-        return AtomicAdd(id, source_path);
+    [[nodiscard]] auto AddFromFile(std::string const& id,
+                                   std::filesystem::path const& source_path,
+                                   bool is_owner = false) const noexcept
+        -> bool {
+        return AtomicAdd(id, source_path, is_owner);
     }
 
     /// \brief Add bytes to storage.
@@ -37,7 +38,7 @@ class FileStorage {
     [[nodiscard]] auto AddFromBytes(std::string const& id,
                                     std::string const& bytes) const noexcept
         -> bool {
-        return AtomicAdd(id, bytes);
+        return AtomicAdd(id, bytes, /*is_owner=*/true);
     }
 
     [[nodiscard]] auto GetPath(std::string const& name) const noexcept
@@ -57,14 +58,15 @@ class FileStorage {
     /// \returns true if file exists afterward.
     template <class T>
     [[nodiscard]] auto AtomicAdd(std::string const& id,
-                                 T const& data) const noexcept -> bool {
+                                 T const& data,
+                                 bool is_owner) const noexcept -> bool {
         auto file_path = storage_root_ / id;
         if (kMode == StoreMode::LastWins or
             not FileSystemManager::Exists(file_path)) {
             auto unique_path = CreateUniquePath(file_path);
             if (unique_path and
                 FileSystemManager::CreateDirectory(file_path.parent_path()) and
-                CreateFileFromData(*unique_path, data) and
+                CreateFileFromData(*unique_path, data, is_owner) and
                 StageFile(*unique_path, file_path)) {
                 Logger::Log(
                     LogLevel::Trace, "created entry {}.", file_path.string());
@@ -77,17 +79,23 @@ class FileStorage {
     /// \brief Create file from file path.
     [[nodiscard]] static auto CreateFileFromData(
         std::filesystem::path const& file_path,
-        std::filesystem::path const& other_path) noexcept -> bool {
+        std::filesystem::path const& other_path,
+        bool is_owner) noexcept -> bool {
+        // For files owned by us (e.g., generated files from the execution
+        // directory), prefer faster creation of hard links instead of a copy.
         // Copy executables without opening any writeable file descriptors in
         // this process to avoid those from being inherited by child processes.
-        return FileSystemManager::CopyFileAs<kType>(
-            other_path, file_path, fd_less_);
+        return (is_owner and FileSystemManager::CreateFileHardlinkAs<kType>(
+                                 other_path, file_path)) or
+               FileSystemManager::CopyFileAs<kType>(
+                   other_path, file_path, fd_less_);
     }
 
     /// \brief Create file from bytes.
     [[nodiscard]] static auto CreateFileFromData(
         std::filesystem::path const& file_path,
-        std::string const& bytes) noexcept -> bool {
+        std::string const& bytes,
+        bool /*unused*/) noexcept -> bool {
         // Write executables without opening any writeable file descriptors in
         // this process to avoid those from being inherited by child processes.
         return FileSystemManager::WriteFileAs<kType>(
diff --git a/src/buildtool/execution_api/local/local_action.cpp b/src/buildtool/execution_api/local/local_action.cpp
index 37c3c085..91ab0ba5 100644
--- a/src/buildtool/execution_api/local/local_action.cpp
+++ b/src/buildtool/execution_api/local/local_action.cpp
@@ -110,11 +110,11 @@ auto LocalAction::Run(bazel_re::Digest const& action_id) const noexcept
         Output result{};
         result.action.set_exit_code(command_output->return_value);
         if (gsl::owner<bazel_re::Digest*> digest_ptr =
-                DigestFromFile(command_output->stdout_file)) {
+                DigestFromOwnedFile(command_output->stdout_file)) {
             result.action.set_allocated_stdout_digest(digest_ptr);
         }
         if (gsl::owner<bazel_re::Digest*> digest_ptr =
-                DigestFromFile(command_output->stderr_file)) {
+                DigestFromOwnedFile(command_output->stderr_file)) {
             result.action.set_allocated_stderr_digest(digest_ptr);
         }
 
@@ -205,7 +205,8 @@ auto LocalAction::CollectOutputFile(std::filesystem::path const& exec_path,
         return std::nullopt;
     }
     bool is_executable = IsExecutableObject(*type);
-    auto digest = storage_->StoreBlob(file_path, is_executable);
+    auto digest =
+        storage_->StoreBlob</*kOwner=*/true>(file_path, is_executable);
     if (digest) {
         auto out_file = bazel_re::OutputFile{};
         out_file.set_path(local_path);
@@ -229,7 +230,7 @@ auto LocalAction::CollectOutputDir(std::filesystem::path const& exec_path,
     auto digest = BazelMsgFactory::CreateDirectoryDigestFromLocalTree(
         dir_path,
         [this](auto path, auto is_exec) {
-            return storage_->StoreBlob(path, 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);
@@ -286,9 +287,9 @@ auto LocalAction::CollectAndStoreOutputs(
     return true;
 }
 
-auto LocalAction::DigestFromFile(std::filesystem::path const& file_path)
+auto LocalAction::DigestFromOwnedFile(std::filesystem::path const& file_path)
     const noexcept -> gsl::owner<bazel_re::Digest*> {
-    if (auto digest = storage_->StoreBlob(file_path)) {
+    if (auto digest = storage_->StoreBlob</*kOwner=*/true>(file_path)) {
         return new bazel_re::Digest{std::move(*digest)};
     }
     return nullptr;
diff --git a/src/buildtool/execution_api/local/local_action.hpp b/src/buildtool/execution_api/local/local_action.hpp
index 3bf49fd2..43569ea3 100644
--- a/src/buildtool/execution_api/local/local_action.hpp
+++ b/src/buildtool/execution_api/local/local_action.hpp
@@ -115,8 +115,9 @@ class LocalAction final : public IExecutionAction {
         std::filesystem::path const& exec_path) const noexcept -> bool;
 
     /// \brief Store file from path in file CAS and return pointer to digest.
-    [[nodiscard]] auto DigestFromFile(std::filesystem::path const& file_path)
-        const noexcept -> gsl::owner<bazel_re::Digest*>;
+    [[nodiscard]] auto DigestFromOwnedFile(
+        std::filesystem::path const& file_path) const noexcept
+        -> gsl::owner<bazel_re::Digest*>;
 };
 
 #endif  // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_LOCAL_LOCAL_ACTION_HPP
diff --git a/src/buildtool/execution_api/local/local_cas.hpp b/src/buildtool/execution_api/local/local_cas.hpp
index 4a28e796..470c38b9 100644
--- a/src/buildtool/execution_api/local/local_cas.hpp
+++ b/src/buildtool/execution_api/local/local_cas.hpp
@@ -28,12 +28,13 @@ class LocalCAS {
 
     [[nodiscard]] auto StoreBlobFromBytes(std::string const& bytes)
         const noexcept -> std::optional<bazel_re::Digest> {
-        return StoreBlob(bytes);
+        return StoreBlob(bytes, /*is_owner=*/true);
     }
 
-    [[nodiscard]] auto StoreBlobFromFile(std::filesystem::path const& file_path)
-        const noexcept -> std::optional<bazel_re::Digest> {
-        return StoreBlob(file_path);
+    [[nodiscard]] auto StoreBlobFromFile(std::filesystem::path const& file_path,
+                                         bool is_owner = false) const noexcept
+        -> std::optional<bazel_re::Digest> {
+        return StoreBlob(file_path, is_owner);
     }
 
     [[nodiscard]] auto BlobPath(bazel_re::Digest const& digest) const noexcept
@@ -71,25 +72,25 @@ class LocalCAS {
 
     /// \brief Store blob from bytes to storage.
     [[nodiscard]] auto StoreBlobData(std::string const& blob_id,
-                                     std::string const& bytes) const noexcept
-        -> bool {
+                                     std::string const& bytes,
+                                     bool /*unused*/) const noexcept -> bool {
         return file_store_.AddFromBytes(blob_id, bytes);
     }
 
     /// \brief Store blob from file path to storage.
-    [[nodiscard]] auto StoreBlobData(
-        std::string const& blob_id,
-        std::filesystem::path const& file_path) const noexcept -> bool {
-        return file_store_.AddFromFile(blob_id, file_path);
+    [[nodiscard]] auto StoreBlobData(std::string const& blob_id,
+                                     std::filesystem::path const& file_path,
+                                     bool is_owner) const noexcept -> bool {
+        return file_store_.AddFromFile(blob_id, file_path, is_owner);
     }
 
     /// \brief Store blob from unspecified data to storage.
     template <class T>
-    [[nodiscard]] auto StoreBlob(T const& data) const noexcept
+    [[nodiscard]] auto StoreBlob(T const& data, bool is_owner) const noexcept
         -> std::optional<bazel_re::Digest> {
         auto digest = CreateDigest(data);
         if (digest) {
-            if (StoreBlobData(digest->hash(), data)) {
+            if (StoreBlobData(digest->hash(), data, is_owner)) {
                 return digest;
             }
             logger_.Emit(
diff --git a/src/buildtool/execution_api/local/local_storage.hpp b/src/buildtool/execution_api/local/local_storage.hpp
index 0b90cf63..a071a5d5 100644
--- a/src/buildtool/execution_api/local/local_storage.hpp
+++ b/src/buildtool/execution_api/local/local_storage.hpp
@@ -25,19 +25,22 @@ class LocalStorage {
           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)
         const noexcept -> std::optional<bazel_re::Digest> {
-        return StoreBlob(file_path, FileSystemManager::IsExecutable(file_path));
+        return StoreBlob<kOwner>(file_path,
+                                 FileSystemManager::IsExecutable(file_path));
     }
 
     /// \brief Store blob from file path with x-bit.
+    template <bool kOwner = false>
     [[nodiscard]] auto StoreBlob(std::filesystem::path const& file_path,
                                  bool is_executable) const noexcept
         -> std::optional<bazel_re::Digest> {
         if (is_executable) {
-            return cas_exec_.StoreBlobFromFile(file_path);
+            return cas_exec_.StoreBlobFromFile(file_path, kOwner);
         }
-        return cas_file_.StoreBlobFromFile(file_path);
+        return cas_file_.StoreBlobFromFile(file_path, kOwner);
     }
 
     /// \brief Store blob from bytes with x-bit (default: non-executable).