Skip to content

Commit

Permalink
Implement global index store cache
Browse files Browse the repository at this point in the history
swift and clang write index data based on the status of what resides in
`index-store-path`. When using a transient per-swift-library index, it
writes O( M imports * N libs) indexer data and slows down compilation
significantly: to the tune of 300% slow down and 6GB+ index data in my
testing. Bazel likes to use per `swift_library` data in order to remote
cache them, which needs extra consideration to work with the design of
swift and clang and preserve the performance

This PR does 2 things to make index while building use the original
model of the index-store so we don't have to patch clang and swift for
this yet. When the flag `-Xwrapped-swift-enable-global-index-store` is
set:

1. it writes to a global index cache, `bazel-out/global_index_store`
2. it copies indexstore data (records and units) to where Bazel
   expected them for caching: e.g. the original location with
   `swift.index_while_building`

Note that while this uses a `index-import` binary, it is very different
than how index-import currently works. In the variant this expects, it
is program that can _copy_ index data for specific compiler outputs.
This has the effect fo remote caching the subset that was used in
compilation for this library.

I added a detailed description of this feature in the RFC to add it
there MobileNativeFoundation/index-import#53.  In practice,
transitive imports will be indexed by deps and if they aren't cached
then the fallback is Xcode wil index transitive files if we set those as
deps on the unit files
  • Loading branch information
jerrymarino committed Feb 23, 2021
1 parent 9e743aa commit c697ca5
Showing 1 changed file with 83 additions and 4 deletions.
87 changes: 83 additions & 4 deletions tools/worker/work_processor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@
#include <nlohmann/json.hpp>
#include <sstream>
#include <string>
#include <vector>

#include "tools/common/file_system.h"
#include "tools/common/path_utils.h"
#include "tools/common/process.h"
#include "tools/common/string_utils.h"
#include "tools/common/temp_file.h"
#include "tools/worker/output_file_map.h"
Expand Down Expand Up @@ -64,6 +66,10 @@ void WorkProcessor::ProcessWorkRequest(
bool is_wmo = false;

std::string prev_arg;

std::string bazel_index_store_path;
bool enable_global_index_store = false;

for (auto arg : request.arguments()) {
auto original_arg = arg;
// Peel off the `-output-file-map` argument, so we can rewrite it if
Expand All @@ -77,16 +83,42 @@ void WorkProcessor::ProcessWorkRequest(
is_wmo = true;
}

/// Peel off index-store-path, we will conditionally pop on a new one
if (arg == "-index-store-path") {
arg.clear();
} else if (prev_arg == "-index-store-path") {
bazel_index_store_path = arg;
arg.clear();
}
if (arg == "-Xwrapped-swift-enable-global-index-store") {
arg.clear();
enable_global_index_store = true;
}

if (!arg.empty()) {
params_file_stream << arg << '\n';
}

prev_arg = original_arg;
}

// Material within this index store is namespaced to arcitecture, OS
// version, and material // and there isn't a per architecture index.
auto exec_root = GetCurrentDirectory();
auto global_index_store = exec_root + "/bazel-out/global_index_store";
if (enable_global_index_store) {
processed_args.push_back("-index-store-path");
processed_args.push_back(global_index_store);
} else if (!bazel_index_store_path.empty()) {
// If there is a global index store
processed_args.push_back("-index-store-path");
processed_args.push_back(bazel_index_store_path);
}

if (!output_file_map_path.empty()) {
output_file_map.ReadFromPath(output_file_map_path);

if (!is_wmo) {
output_file_map.ReadFromPath(output_file_map_path);

// Rewrite the output file map to use the incremental storage area and
// pass the compiler the path to the rewritten file.
Expand All @@ -112,6 +144,7 @@ void WorkProcessor::ProcessWorkRequest(
processed_args.push_back("@" + params_file->GetPath());
params_file_stream.close();


if (!is_wmo) {
for (const auto &expected_object_pair :
output_file_map.incremental_outputs()) {
Expand All @@ -129,7 +162,53 @@ void WorkProcessor::ProcessWorkRequest(
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);
int swift_exit_code = swift_runner.Run(&stderr_stream, /*stdout_to_stderr=*/true);

// Import global indexes if compilation succeeds
if (swift_exit_code != EXIT_FAILURE && !bazel_index_store_path.empty()) {
std::map<std::string, std::string>::iterator it;

std::vector<std::string> ii_args;

// Perhaps add this as an external dep of rules_swift
// needs the feature -import-output-file added here
// https://github.com/lyft/index-import/pull/53
auto index_import_path = "/usr/local/bin/index-import";
ii_args.push_back(index_import_path);

// Use the output path map to deterine what compilation ouputs to import
auto outputs = output_file_map.incremental_outputs();
for (it = outputs.begin(); it != outputs.end(); it++) {
auto output_path = it->second;
auto file_type = output_path.substr(output_path.find_last_of(".") + 1);
if (file_type == "o") {
ii_args.push_back("-import-output-file");
ii_args.push_back(output_path);
}
}

// Currently, it doesn't remap anything. The global index is an implementation
// detail used to copy records and units into `bazel-out`. Consider adding a
// way in index-import to not remap
ii_args.push_back("-remap");
ii_args.push_back("NOREMAPJUSTCOPY=0");

// Copy back from the global index store to bazel's index store
ii_args.push_back(global_index_store);

ii_args.push_back(exec_root + "/" + bazel_index_store_path);

const std::vector<std::string>& ii_args_ = ii_args;
// Dupes the output into the action for failure cases
int ii_exit_code = RunSubProcess(ii_args_, &stderr_stream, /*stdout_to_stderr=*/true);

if (ii_exit_code != 0) {
std::cerr << "index-import failed: " << ii_exit_code << "\n";
response->set_exit_code(ii_exit_code);
response->set_output(stderr_stream.str());
return;
}
}

if (!is_wmo) {
// Copy the output files from the incremental storage area back to the
Expand All @@ -139,11 +218,11 @@ void WorkProcessor::ProcessWorkRequest(
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;
swift_exit_code = EXIT_FAILURE;
}
}
}

response->set_exit_code(exit_code);
response->set_exit_code(swift_exit_code);
response->set_output(stderr_stream.str());
}

0 comments on commit c697ca5

Please sign in to comment.