Build non-linkable deps async with Ninja's validaitons
This CL introduces a build setting flag `async_non_linkable_deps` that
write non-linkable deps as Ninja's `validations` instead of order-only
deps.
I've tested locally that it can build Chromium without errors.
Having the feature flag allows me to try the feature in Chromium's CQ
with various build configs on multiple platforms.
Bug: 413507213
Change-Id: Ic527feebfc64323c5136c83f83264eb577a9560f
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/18681
Reviewed-by: Takuto Ikuta <tikuta@google.com>
Commit-Queue: Takuto Ikuta <tikuta@google.com>
diff --git a/src/gn/build_settings.h b/src/gn/build_settings.h
index e21851a..6cf05e0 100644
--- a/src/gn/build_settings.h
+++ b/src/gn/build_settings.h
@@ -77,6 +77,16 @@
no_stamp_files_ = no_stamp_files;
}
+ // The 'async_non_linkable_deps' boolean flag can be set to generate Ninja
+ // files that build non-linkable deps asynchronously using Ninja validations
+ // instead of order-only dependencies.
+ // This speeds up build by increasing the parallerism of the build graph.
+ // This requires Ninja 1.11 for the `validations` feature.
+ bool async_non_linkable_deps() const { return async_non_linkable_deps_; }
+ void set_async_non_linkable_deps(bool async_non_linkable_deps) {
+ async_non_linkable_deps_ = async_non_linkable_deps;
+ }
+
const SourceFile& build_config_file() const { return build_config_file_; }
void set_build_config_file(const SourceFile& f) { build_config_file_ = f; }
@@ -153,6 +163,7 @@
// See 40045b9 for the reason behind using 1.7.2 as the default version.
Version ninja_required_version_{1, 7, 2};
bool no_stamp_files_ = true;
+ bool async_non_linkable_deps_ = false;
SourceFile build_config_file_;
SourceFile arg_file_template_path_;
diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc
index b3a4f2c..6e27c68 100644
--- a/src/gn/ninja_c_binary_target_writer.cc
+++ b/src/gn/ninja_c_binary_target_writer.cc
@@ -679,6 +679,8 @@
}
// Append data dependencies as order-only dependencies.
+ // If `async_non_linkable_deps` flag is set, it uses
+ // validations instead.
//
// This will include data dependencies and input dependencies (like when
// this target depends on an action). Having the data dependencies in this
@@ -691,7 +693,11 @@
// on the sources, there is already an implicit order-only dependency.
// However, it's extra work to separate these out and there's no disadvantage
// to listing them again.
- WriteOrderOnlyDependencies(classified_deps.non_linkable_deps);
+ if (settings_->build_settings()->async_non_linkable_deps()) {
+ WriteValidations(classified_deps.non_linkable_deps);
+ } else {
+ WriteOrderOnlyDependencies(classified_deps.non_linkable_deps);
+ }
// End of the link "build" line.
out_ << std::endl;
@@ -760,15 +766,32 @@
void NinjaCBinaryTargetWriter::WriteOrderOnlyDependencies(
const UniqueVector<const Target*>& non_linkable_deps) {
- if (!non_linkable_deps.empty()) {
- out_ << " ||";
+ if (non_linkable_deps.empty())
+ return;
- // Non-linkable targets.
- for (auto* non_linkable_dep : non_linkable_deps) {
- if (non_linkable_dep->has_dependency_output()) {
- out_ << " ";
- path_output_.WriteFile(out_, non_linkable_dep->dependency_output());
- }
+ out_ << " ||";
+
+ // Non-linkable targets.
+ for (auto* non_linkable_dep : non_linkable_deps) {
+ if (non_linkable_dep->has_dependency_output()) {
+ out_ << " ";
+ path_output_.WriteFile(out_, non_linkable_dep->dependency_output());
+ }
+ }
+}
+
+void NinjaCBinaryTargetWriter::WriteValidations(
+ const UniqueVector<const Target*>& non_linkable_deps) {
+ if (non_linkable_deps.empty())
+ return;
+
+ out_ << " |@";
+
+ // Non-linkable targets.
+ for (auto* non_linkable_dep : non_linkable_deps) {
+ if (non_linkable_dep->has_dependency_output()) {
+ out_ << " ";
+ path_output_.WriteFile(out_, non_linkable_dep->dependency_output());
}
}
}
diff --git a/src/gn/ninja_c_binary_target_writer.h b/src/gn/ninja_c_binary_target_writer.h
index f60790f..b03a85e 100644
--- a/src/gn/ninja_c_binary_target_writer.h
+++ b/src/gn/ninja_c_binary_target_writer.h
@@ -103,6 +103,14 @@
void WriteOrderOnlyDependencies(
const UniqueVector<const Target*>& non_linkable_deps);
+ // Writes the validations for the link or stamp line. This is
+ // the "|@" and everything following it on the ninja line.
+ //
+ // The validations are the non-linkable deps passed in as an argument, plus
+ // the data file dependencies in the target.
+ void WriteValidations(
+ const UniqueVector<const Target*>& non_linkable_deps);
+
// Checks for duplicates in the given list of output files. If any duplicates
// are found, throws an error and return false.
bool CheckForDuplicateObjectFiles(const std::vector<OutputFile>& files) const;
diff --git a/src/gn/ninja_c_binary_target_writer_unittest.cc b/src/gn/ninja_c_binary_target_writer_unittest.cc
index f336ac7..ce6d72b 100644
--- a/src/gn/ninja_c_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_c_binary_target_writer_unittest.cc
@@ -2846,3 +2846,51 @@
std::string out_str = out.str();
EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
}
+
+TEST_F(NinjaCBinaryTargetWriterTest, AsyncNonLinkableDeps) {
+ Err err;
+ TestWithScope setup;
+ BuildSettings* settings = setup.build_settings();
+ settings->set_async_non_linkable_deps(true);
+
+ Target datadep(setup.settings(), Label(SourceDir("//foo/"), "datadep"));
+ datadep.set_output_type(Target::ACTION);
+ datadep.visibility().SetPublic();
+ datadep.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(datadep.OnResolved(&err));
+
+ Target target(setup.settings(), Label(SourceDir("//foo/"), "bar"));
+ target.sources().push_back(SourceFile("//foo/source.cc"));
+ target.set_output_type(Target::EXECUTABLE);
+ target.visibility().SetPublic();
+ target.SetToolchain(setup.toolchain());
+ target.data_deps().push_back(LabelTargetPair(&datadep));
+ ASSERT_TRUE(target.OnResolved(&err));
+
+ std::ostringstream out;
+ NinjaBinaryTargetWriter writer(&target, out);
+ writer.Run();
+
+ const char expected[] =
+ "defines =\n"
+ "include_dirs =\n"
+ "root_out_dir = .\n"
+ "target_gen_dir = gen/foo\n"
+ "target_out_dir = obj/foo\n"
+ "target_output_name = bar\n"
+ "\n"
+ "build obj/foo/bar.source.o: cxx ../../foo/source.cc\n"
+ " source_file_part = source.cc\n"
+ " source_name_part = source\n"
+ "\n"
+ "build ./bar: link obj/foo/bar.source.o |@ phony/foo/datadep\n"
+ " ldflags =\n"
+ " libs =\n"
+ " frameworks =\n"
+ " swiftmodules =\n"
+ " output_extension =\n"
+ " output_dir =\n";
+
+ std::string out_str = out.str();
+ EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
+}
diff --git a/src/gn/setup.cc b/src/gn/setup.cc
index 0232089..4f8d231 100644
--- a/src/gn/setup.cc
+++ b/src/gn/setup.cc
@@ -1186,5 +1186,15 @@
export_compile_commands_.push_back(std::move(pat));
}
+ // Async non-linkable deps.
+ const Value* async_non_linkable_deps =
+ dotfile_scope_.GetValue("async_non_linkable_deps", true);
+ if (async_non_linkable_deps) {
+ if (!async_non_linkable_deps->VerifyTypeIs(Value::BOOLEAN, err)) {
+ return false;
+ }
+ build_settings_.set_async_non_linkable_deps(async_non_linkable_deps->boolean_value());
+ }
+
return true;
}
