From d88c7734c386013b48170836236739f58f0affc0 Mon Sep 17 00:00:00 2001 From: Brentley Jones Date: Wed, 2 Jun 2021 15:24:01 -0500 Subject: [PATCH 1/2] Reuse swiftmodule for incremental builds By doing this, for some parts of the build graph, the swiftmodule doesn't have to be recreated. Here is an example with Swift 5.5: Before: ```console remark: Incremental compilation: Enabling incremental cross-module building remark: Incremental compilation: May skip current input: {compile: A.swift.o <= A.swift} remark: Incremental compilation: Skipping input: {compile: A.swift.o <= A.swift} remark: Incremental compilation: Not skipping job: Merging module A; Missing output 'bazel-out/darwin-fastbuild/bin/A.swiftmodule' remark: Starting Merging module A remark: Finished Merging module A remark: Skipped Compiling A.swift ``` After: ```console remark: Incremental compilation: Enabling incremental cross-module building remark: Incremental compilation: May skip current input: {compile: A.swift.o <= A.swift} remark: Incremental compilation: Skipping input: {compile: A.swift.o <= A.swift} remark: Incremental compilation: Skipping job: Merging module A; oldest output is current 'bazel-out/darwin-fastbuild/bin/A.swiftmodule' remark: Skipped Compiling A.swift ``` Shout out to Cody Vandermyn (@codeman9) for his initial investigation and PoC on this. --- tools/common/file_system.cc | 9 +++++ tools/common/file_system.h | 6 +++ tools/worker/output_file_map.cc | 10 +++++ tools/worker/output_file_map.h | 9 +++++ tools/worker/work_processor.cc | 71 +++++++++++++++++++++++++++------ 5 files changed, 92 insertions(+), 13 deletions(-) diff --git a/tools/common/file_system.cc b/tools/common/file_system.cc index f04f55dab..fff23cbb5 100644 --- a/tools/common/file_system.cc +++ b/tools/common/file_system.cc @@ -22,6 +22,7 @@ #ifdef __APPLE__ #include +#include #else #include #include @@ -37,6 +38,14 @@ std::string GetCurrentDirectory() { return cwd; } +bool FileExists(const std::string &path) { + return access(path.c_str(), 0) == 0; +} + +bool RemoveFile(const std::string &path) { + return removefile(path.c_str(), nullptr, 0); +} + bool CopyFile(const std::string &src, const std::string &dest) { #ifdef __APPLE__ // The `copyfile` function with `COPYFILE_ALL` mode preserves permissions and diff --git a/tools/common/file_system.h b/tools/common/file_system.h index 2d233809a..644917de3 100644 --- a/tools/common/file_system.h +++ b/tools/common/file_system.h @@ -20,6 +20,12 @@ // Gets the path to the current working directory. std::string GetCurrentDirectory(); +// Returns true if something exists at path. +bool FileExists(const std::string &path); + +// Removes the file at path. Returns true if successful. +bool RemoveFile(const std::string &path); + // Copies the file at src to dest. Returns true if successful. bool CopyFile(const std::string &src, const std::string &dest); diff --git a/tools/worker/output_file_map.cc b/tools/worker/output_file_map.cc index 57bb94807..126d366d2 100644 --- a/tools/worker/output_file_map.cc +++ b/tools/worker/output_file_map.cc @@ -57,6 +57,7 @@ void OutputFileMap::WriteToPath(const std::string &path) { void OutputFileMap::UpdateForIncremental(const std::string &path) { nlohmann::json new_output_file_map; std::map incremental_outputs; + std::map incremental_inputs; // The empty string key is used to represent outputs that are for the whole // module, rather than for a particular source file. @@ -111,6 +112,15 @@ void OutputFileMap::UpdateForIncremental(const std::string &path) { new_output_file_map[src] = src_map; } + auto swiftmodule_path = ReplaceExtension(path, ".swiftmodule", /*all_extensions=*/true); + auto copied_swiftmodule_path = MakeIncrementalOutputPath(swiftmodule_path); + incremental_inputs[swiftmodule_path] = copied_swiftmodule_path; + + auto swiftdoc_path = ReplaceExtension(path, ".swiftdoc", /*all_extensions=*/true); + auto copied_swiftdoc_path = MakeIncrementalOutputPath(swiftdoc_path); + incremental_inputs[swiftdoc_path] = copied_swiftdoc_path; + json_ = new_output_file_map; incremental_outputs_ = incremental_outputs; + incremental_inputs_ = incremental_inputs; } diff --git a/tools/worker/output_file_map.h b/tools/worker/output_file_map.h index b52e7f601..4e17f16fe 100644 --- a/tools/worker/output_file_map.h +++ b/tools/worker/output_file_map.h @@ -39,6 +39,14 @@ class OutputFileMap { return incremental_outputs_; } + // A map containing expected output files that will be generated in the + // non-incremental storage area, but need to be copied back at the start of + // the next compile. The key is the original object path; the corresponding + // value is its location in the incremental storage area. + const std::map incremental_inputs() const { + return incremental_inputs_; + } + // Reads the output file map from the JSON file at the given path, and updates // it to support incremental builds. void ReadFromPath(const std::string &path); @@ -53,6 +61,7 @@ class OutputFileMap { nlohmann::json json_; std::map incremental_outputs_; + std::map incremental_inputs_; }; #endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_OUTPUT_FILE_MAP_H_ diff --git a/tools/worker/work_processor.cc b/tools/worker/work_processor.cc index 8e824ade0..ba0ca7a0a 100644 --- a/tools/worker/work_processor.cc +++ b/tools/worker/work_processor.cc @@ -12,8 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "tools/worker/work_processor.h" - #include #include @@ -28,6 +26,7 @@ #include "tools/common/temp_file.h" #include "tools/worker/output_file_map.h" #include "tools/worker/swift_runner.h" +#include "tools/worker/work_processor.h" namespace { @@ -38,6 +37,15 @@ static bool ArgumentEnablesWMO(const std::string &arg) { arg == "-force-single-frontend-invocation"; } +static void FinalizeWorkRequest(const blaze::worker::WorkRequest &request, + blaze::worker::WorkResponse *response, + int exit_code, + const std::ostringstream &output) { + response->set_exit_code(exit_code); + response->set_output(output.str()); + response->set_request_id(request.request_id()); +} + }; // end namespace WorkProcessor::WorkProcessor(const std::vector &args) { @@ -111,6 +119,8 @@ void WorkProcessor::ProcessWorkRequest( processed_args.push_back("@" + params_file->GetPath()); params_file_stream.close(); + std::ostringstream stderr_stream; + if (!is_wmo) { for (const auto &expected_object_pair : output_file_map.incremental_outputs()) { @@ -119,15 +129,30 @@ void WorkProcessor::ProcessWorkRequest( // incremental storage area. auto dir_path = Dirname(expected_object_pair.second); if (!MakeDirs(dir_path, S_IRWXU)) { - std::cerr << "Could not create directory " << dir_path << " (errno " - << errno << ")\n"; + stderr_stream << "swift_worker: Could not create directory " << dir_path + << " (errno " << errno << ")\n"; + FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); + } + } + + // Copy some input files from the incremental storage area to the locations + // where Bazel will generate them. + for (const auto &expected_object_pair : + output_file_map.incremental_inputs()) { + if (FileExists(expected_object_pair.second)) { + if (!CopyFile(expected_object_pair.second, + expected_object_pair.first)) { + stderr_stream << "swift_worker: Could not copy " + << expected_object_pair.second << " to " + << expected_object_pair.first << " (errno " << errno + << ")\n"; + FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); + } } } } - std::ostringstream stderr_stream; SwiftRunner swift_runner(processed_args, /*force_response_file=*/true); - int exit_code = swift_runner.Run(&stderr_stream, /*stdout_to_stderr=*/true); if (!is_wmo) { @@ -136,14 +161,34 @@ void WorkProcessor::ProcessWorkRequest( for (const auto &expected_object_pair : output_file_map.incremental_outputs()) { if (!CopyFile(expected_object_pair.second, expected_object_pair.first)) { - std::cerr << "Could not copy " << expected_object_pair.second << " to " - << expected_object_pair.first << " (errno " << errno << ")\n"; - exit_code = EXIT_FAILURE; + stderr_stream << "swift_worker: Could not copy " + << expected_object_pair.second << " to " + << expected_object_pair.first << " (errno " << errno + << ")\n"; + FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); } } - } - response->set_exit_code(exit_code); - response->set_output(stderr_stream.str()); - response->set_request_id(request.request_id()); + // Copy the replaced input files back to the incremental storage for the + // next run. + for (const auto &expected_object_pair : + output_file_map.incremental_inputs()) { + if (FileExists(expected_object_pair.first)) { + if (FileExists(expected_object_pair.second)) { + // CopyFile fails if the file already exists + RemoveFile(expected_object_pair.second); + } + if (!CopyFile(expected_object_pair.first, + expected_object_pair.second)) { + stderr_stream << "swift_worker: Could not copy " + << expected_object_pair.first << " to " + << expected_object_pair.second << " (errno " << errno + << ")\n"; + FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); + } + } + } + + FinalizeWorkRequest(request, response, exit_code, stderr_stream); + } } From 98d20a5c1ca74431afc5fd7b71f0f58b23f64e7f Mon Sep 17 00:00:00 2001 From: Brentley Jones Date: Fri, 4 Jun 2021 13:12:02 -0500 Subject: [PATCH 2/2] Remove file on non-macOS --- tools/common/file_system.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/common/file_system.cc b/tools/common/file_system.cc index fff23cbb5..15b2c274f 100644 --- a/tools/common/file_system.cc +++ b/tools/common/file_system.cc @@ -43,7 +43,13 @@ bool FileExists(const std::string &path) { } bool RemoveFile(const std::string &path) { +#ifdef __APPLE__ return removefile(path.c_str(), nullptr, 0); +#elif __unix__ + return remove(path.c_str()); +#else +#error Only macOS and Unix are supported at this time. +#endif } bool CopyFile(const std::string &src, const std::string &dest) {