Write phony rules for harddeps
Write a rule for each target with at least two hard deps,
similar to what WritePublicInputsStampOrPhony() does for public_inputs:
build phony/foo/bar.harddeps: $
phony phony/foo/action phony/foo/dep.harddeps
It lists the target's direct hard deps. This mirrors
ResolvedTargetData::ComputeHardDeps(), so it depends on the same set
of hard deps, but its size is proportional to the number of direct deps.
A target's input deps then just refer to its own .harddeps rule.
Results for a Chromium build (mac, arm64, is_debug=false):
before after
size of all loaded .ninja files 1025 MB 427 MB
gn gen (hyperfine, 6 runs) 2.065s ± .048 1.947s ± .050
user / sys 12.0s / 4.3s 10.1s / 4.5s
Chromium no-op builds of base_unittests (target already built, hyperfine, M-series Mac):
before after
siso v1.6.0 (autoninja), -offline 735.5 ms ± 13.7 648.3 ms ± 22.8
user / sys 3.7s / 1.9s 2.3s / 2.2s
depot_tools ninja (1.12.1) 4.37s ± .02 2.25s ± .02
ninja v1.13.2 2.755s ± .009 1.532s ± .007
ninja with some local changes to tweak perf 535.8 ms ± 2.7 408.4 ms ± 8.1
(`siso -n` and `ninja -n` are within 2% of the no-op times.)
No intended behavior change.
Bug: chromium:40278737
Change-Id: I163e6bea334c2e626bf7c96cd8d942848a6766bf
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/26741
Reviewed-by: Takuto Ikuta <tikuta@google.com>
Reviewed-by: Nico Weber <thakis@google.com>
Commit-Queue: Nico Weber <thakis@chromium.org>
diff --git a/src/gn/ninja_copy_target_writer_unittest.cc b/src/gn/ninja_copy_target_writer_unittest.cc
index 6fc5e43..c81fa0b 100644
--- a/src/gn/ninja_copy_target_writer_unittest.cc
+++ b/src/gn/ninja_copy_target_writer_unittest.cc
@@ -208,10 +208,8 @@
writer.Run();
const char expected_linux[] =
- "build phony/foo/bar.inputdeps: phony || phony/foo/action1 "
- "phony/foo/action2\n"
- "build action1.copy: copy action1.out | phony/foo/bar.inputdeps\n"
- "build action2.copy: copy action2.out | phony/foo/bar.inputdeps\n"
+ "build action1.copy: copy action1.out || phony/foo/bar.harddeps\n"
+ "build action2.copy: copy action2.out || phony/foo/bar.harddeps\n"
"\n"
"build phony/foo/bar: phony action1.copy action2.copy\n";
std::string out_str = out.str();
diff --git a/src/gn/ninja_target_writer.cc b/src/gn/ninja_target_writer.cc
index d3d1eb1..7e1ba0e 100644
--- a/src/gn/ninja_target_writer.cc
+++ b/src/gn/ninja_target_writer.cc
@@ -193,6 +193,7 @@
}
WritePublicInputsStampOrPhony(target, resolved, rules);
+ WriteHardDepsStampOrPhony(target, resolved, rules);
if (needs_file_write) {
// Write the ninja file.
@@ -256,6 +257,90 @@
out << std::endl << std::endl;
}
+namespace {
+
+// BUNDLE_DATA hard deps are only input deps of CREATE_BUNDLE targets, which
+// list them explicitly. See WriteInputDepsStampOrPhonyAndGetDep().
+bool IsListedHardDep(const Target* dep) {
+ return dep->output_type() != Target::BUNDLE_DATA &&
+ dep->has_dependency_output();
+}
+
+} // namespace
+
+// static
+void NinjaTargetWriter::AppendHardDepsOutputs(
+ const Target* target,
+ const ResolvedTargetData& resolved,
+ std::vector<OutputFile>* outputs) {
+ // The iteration order of GetHardDeps() isn't deterministic, but this only
+ // depends on the set's contents.
+ const Target* single_hard_dep = nullptr;
+ for (const Target* hard_dep : resolved.GetHardDeps(target)) {
+ if (!IsListedHardDep(hard_dep))
+ continue;
+ if (single_hard_dep) {
+ outputs->push_back(
+ GetHardDepsOutputFile(target, target->settings()->build_settings()));
+ return;
+ }
+ single_hard_dep = hard_dep;
+ }
+ if (single_hard_dep)
+ outputs->push_back(single_hard_dep->dependency_output());
+}
+
+// static
+void NinjaTargetWriter::WriteHardDepsStampOrPhony(const Target* target,
+ ResolvedTargetData* resolved,
+ std::ostream& out) {
+ std::unique_ptr<ResolvedTargetData> resolved_owned;
+ if (!resolved) {
+ resolved_owned = std::make_unique<ResolvedTargetData>();
+ resolved = resolved_owned.get();
+ }
+
+ const BuildSettings* build_settings = target->settings()->build_settings();
+ OutputFile output = GetHardDepsOutputFile(target, build_settings);
+ std::vector<OutputFile> self;
+ AppendHardDepsOutputs(target, *resolved, &self);
+ if (self.empty() || self[0] != output)
+ return; // Fewer than two hard deps, nothing will refer to this rule.
+
+ // Keep in sync with ResolvedTargetData::ComputeHardDeps(). This produces
+ // the same set of targets, but refers to the hard deps rules of deps
+ // instead of flattening them.
+ std::vector<OutputFile> deps;
+ for (const Target* dep : resolved->GetTargetDeps(target).linked_deps()) {
+ if (target->hard_dep() || dep->hard_dep()) {
+ if (IsListedHardDep(dep))
+ deps.push_back(dep->dependency_output());
+ } else if (ResolvedTargetData::ForwardsHardDeps(dep)) {
+ AppendHardDepsOutputs(dep, *resolved, &deps);
+ }
+ }
+ std::sort(deps.begin(), deps.end());
+ deps.erase(std::unique(deps.begin(), deps.end()), deps.end());
+
+ PathOutput path_output(build_settings->build_dir(),
+ build_settings->root_path_utf8(), ESCAPE_NINJA);
+
+ // The deps are regular inputs instead of order-only deps so that actions,
+ // which have an implicit dependency on their hard deps, become dirty if
+ // one of their recursive hard deps changes. Everything else refers to this
+ // rule through an order-only dep.
+ out << "build ";
+ path_output.WriteFile(out, output);
+ if (build_settings->no_stamp_files()) {
+ out << ": " << BuiltinTool::kBuiltinToolPhony;
+ } else {
+ out << ": " << GetNinjaRulePrefixForToolchain(target->settings())
+ << GeneralTool::kGeneralToolStamp;
+ }
+ path_output.WriteFiles(out, deps);
+ out << std::endl;
+}
+
void NinjaTargetWriter::WriteEscapedSubstitution(const Substitution* type) {
EscapeOptions opts;
opts.mode = ESCAPE_NINJA;
@@ -541,13 +626,19 @@
// Hard dependencies that are direct or indirect dependencies.
// These are large (up to 100s), hence why we check other
const TargetSet& hard_deps = resolved().GetHardDeps(target_);
- for (const Target* target : hard_deps) {
+ std::vector<OutputFile> hard_deps_outputs;
+ if (target_->output_type() == Target::CREATE_BUNDLE) {
// BUNDLE_DATA should normally be treated as a data-only dependency
// (see Target::IsDataOnly()). Only the CREATE_BUNDLE target, that actually
// consumes this data, needs to have the BUNDLE_DATA as an input dependency.
- if (target->output_type() != Target::BUNDLE_DATA ||
- target_->output_type() == Target::CREATE_BUNDLE)
+ // So list all hard deps directly for CREATE_BUNDLE targets.
+ for (const Target* target : hard_deps)
input_deps_targets.push_back(target);
+ } else {
+ // For everything else, depend on the (at most one) file that in turn
+ // depends on all hard deps that aren't BUNDLE_DATA, to not write
+ // thousands of hard deps for every target.
+ AppendHardDepsOutputs(target_, resolved(), &hard_deps_outputs);
}
// Additional hard dependencies passed in. These are usually empty or small,
@@ -575,7 +666,7 @@
// Write the outputs.
if (input_deps_sources.empty() && input_deps_targets.empty() &&
- toolchain_deps_targets.empty())
+ hard_deps_outputs.empty() && toolchain_deps_targets.empty())
return InputDeps{}; // No input dependencies.
InputDeps deps;
@@ -621,6 +712,8 @@
deps.push_back(dep->dependency_output());
}
};
+ deps.order_only.insert(deps.order_only.end(), hard_deps_outputs.begin(),
+ hard_deps_outputs.end());
add_target_deps(deps.order_only, input_deps_targets);
add_target_deps(deps.implicit, toolchain_deps_targets);
diff --git a/src/gn/ninja_target_writer.h b/src/gn/ninja_target_writer.h
index d7b2883..043e834 100644
--- a/src/gn/ninja_target_writer.h
+++ b/src/gn/ninja_target_writer.h
@@ -61,6 +61,25 @@
ResolvedTargetData* resolved,
std::ostream& out);
+ // Targets need to depend on all their recursive hard deps. Listing all of
+ // them on each target makes the generated ninja files very large, since
+ // targets near the top of the dependency graph tend to have thousands of
+ // recursive hard deps. Instead, every target with at least two hard deps
+ // gets a stamp or phony rule for its hard deps which depends on the target's
+ // direct hard deps, and on the hard deps rules of the direct deps that hard
+ // deps are inherited from. BUNDLE_DATA targets are never included (see
+ // WriteInputDepsStampOrPhonyAndGetDep()).
+ static void WriteHardDepsStampOrPhony(const Target* target,
+ ResolvedTargetData* resolved,
+ std::ostream& out);
+
+ // Appends what a build statement needs to depend on to depend on all hard
+ // deps of |target|: Nothing if it has no hard deps, the hard dep itself if
+ // it has just one, else the rule written by WriteHardDepsStampOrPhony().
+ static void AppendHardDepsOutputs(const Target* target,
+ const ResolvedTargetData& resolved,
+ std::vector<OutputFile>* outputs);
+
virtual void Run() = 0;
protected:
diff --git a/src/gn/ninja_target_writer_unittest.cc b/src/gn/ninja_target_writer_unittest.cc
index 53b1a4d..fb00320 100644
--- a/src/gn/ninja_target_writer_unittest.cc
+++ b/src/gn/ninja_target_writer_unittest.cc
@@ -760,3 +760,138 @@
EXPECT_FALSE(out.contains("../../foo/g.in")) << out;
}
}
+
+TEST(NinjaTargetWriter, HardDeps) {
+ TestWithScope setup;
+ Err err;
+
+ auto make_action = [&](Target* target) {
+ target->set_output_type(Target::ACTION);
+ target->visibility().SetPublic();
+ target->SetToolchain(setup.toolchain());
+ target->action_values().set_script(SourceFile("//foo/script.py"));
+ std::string output = "//out/Debug/" + target->label().name() + ".out";
+ target->action_values().outputs() =
+ SubstitutionList::MakeForTest(output.c_str());
+ };
+ auto make_source_set = [&](Target* target) {
+ target->set_output_type(Target::SOURCE_SET);
+ target->visibility().SetPublic();
+ target->sources().push_back(SourceFile("//foo/source.cc"));
+ target->source_types_used().Set(SourceFile::SOURCE_CPP);
+ target->SetToolchain(setup.toolchain());
+ };
+ auto hard_deps_rule = [](const Target* target) {
+ std::ostringstream out;
+ NinjaTargetWriter::WriteHardDepsStampOrPhony(target, nullptr, out);
+ return out.str();
+ };
+ auto hard_deps_outputs = [](const Target* target) {
+ ResolvedTargetData resolved;
+ std::vector<OutputFile> outputs;
+ NinjaTargetWriter::AppendHardDepsOutputs(target, resolved, &outputs);
+ std::string result;
+ for (const OutputFile& output : outputs)
+ result += std::string(output.value()) + " ";
+ return result;
+ };
+
+ Target action1(setup.settings(), Label(SourceDir("//foo/"), "action1"));
+ make_action(&action1);
+ ASSERT_TRUE(action1.OnResolved(&err));
+ Target action2(setup.settings(), Label(SourceDir("//foo/"), "action2"));
+ make_action(&action2);
+ ASSERT_TRUE(action2.OnResolved(&err));
+ Target action3(setup.settings(), Label(SourceDir("//foo/"), "action3"));
+ make_action(&action3);
+ ASSERT_TRUE(action3.OnResolved(&err));
+ Target bundle_data(setup.settings(), Label(SourceDir("//foo/"), "data"));
+ bundle_data.set_output_type(Target::BUNDLE_DATA);
+ bundle_data.visibility().SetPublic();
+ bundle_data.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(bundle_data.OnResolved(&err));
+
+ // No hard deps: No rule, nothing to depend on.
+ EXPECT_EQ("", hard_deps_rule(&action1));
+ EXPECT_EQ("", hard_deps_outputs(&action1));
+
+ // A single hard dep: No rule, dependents depend on the hard dep itself.
+ // BUNDLE_DATA targets don't count.
+ Target single(setup.settings(), Label(SourceDir("//foo/"), "single"));
+ make_source_set(&single);
+ single.private_deps().push_back(LabelTargetPair(&action1));
+ single.private_deps().push_back(LabelTargetPair(&bundle_data));
+ ASSERT_TRUE(single.OnResolved(&err));
+ EXPECT_EQ("", hard_deps_rule(&single));
+ EXPECT_EQ("phony/foo/action1 ", hard_deps_outputs(&single));
+
+ // Several hard deps: The rule lists them.
+ Target mid(setup.settings(), Label(SourceDir("//foo/"), "mid"));
+ make_source_set(&mid);
+ mid.private_deps().push_back(LabelTargetPair(&action2));
+ mid.private_deps().push_back(LabelTargetPair(&action1));
+ mid.private_deps().push_back(LabelTargetPair(&bundle_data));
+ ASSERT_TRUE(mid.OnResolved(&err));
+ EXPECT_EQ(
+ "build phony/foo/mid.harddeps: phony phony/foo/action1 "
+ "phony/foo/action2\n",
+ hard_deps_rule(&mid));
+ EXPECT_EQ("phony/foo/mid.harddeps ", hard_deps_outputs(&mid));
+
+ // Hard deps of deps are referenced through the rule of the dep if there
+ // is one, and directly else. Duplicates are omitted.
+ Target top(setup.settings(), Label(SourceDir("//foo/"), "top"));
+ make_source_set(&top);
+ top.private_deps().push_back(LabelTargetPair(&mid));
+ top.private_deps().push_back(LabelTargetPair(&single));
+ top.private_deps().push_back(LabelTargetPair(&action3));
+ top.private_deps().push_back(LabelTargetPair(&action1));
+ ASSERT_TRUE(top.OnResolved(&err));
+ EXPECT_EQ(
+ "build phony/foo/top.harddeps: phony phony/foo/action1 "
+ "phony/foo/action3 phony/foo/mid.harddeps\n",
+ hard_deps_rule(&top));
+
+ // The input deps of a target just refer to its hard deps rule.
+ {
+ std::ostringstream stream;
+ TestingNinjaTargetWriter writer(&top, setup.toolchain(), stream);
+ auto deps = writer.WriteInputDepsStampOrPhonyAndGetDep({}, 10u);
+ EXPECT_EQ("", stream.str());
+ EXPECT_TRUE(deps.implicit.empty());
+ ASSERT_EQ(1u, deps.order_only.size());
+ EXPECT_EQ("phony/foo/top.harddeps", deps.order_only[0].value());
+ }
+
+ // Binary targets without public headers don't forward their hard deps.
+ Target no_public_headers(setup.settings(),
+ Label(SourceDir("//foo/"), "no_public_headers"));
+ make_source_set(&no_public_headers);
+ no_public_headers.set_all_headers_public(false);
+ no_public_headers.private_deps().push_back(LabelTargetPair(&action1));
+ no_public_headers.private_deps().push_back(LabelTargetPair(&action2));
+ ASSERT_TRUE(no_public_headers.OnResolved(&err));
+ EXPECT_EQ("phony/foo/no_public_headers.harddeps ",
+ hard_deps_outputs(&no_public_headers));
+ Target top2(setup.settings(), Label(SourceDir("//foo/"), "top2"));
+ make_source_set(&top2);
+ top2.private_deps().push_back(LabelTargetPair(&no_public_headers));
+ top2.private_deps().push_back(LabelTargetPair(&action3));
+ ASSERT_TRUE(top2.OnResolved(&err));
+ EXPECT_EQ("", hard_deps_rule(&top2));
+ EXPECT_EQ("phony/foo/action3 ", hard_deps_outputs(&top2));
+
+ // Targets that are hard deps themselves (here, an action) depend on all
+ // their deps, but don't inherit hard deps from them. Their input deps are
+ // implicit deps so that they re-run if a hard dep changes; that works
+ // through the hard deps rule since its deps aren't order-only.
+ Target action4(setup.settings(), Label(SourceDir("//foo/"), "action4"));
+ make_action(&action4);
+ action4.private_deps().push_back(LabelTargetPair(&mid));
+ action4.private_deps().push_back(LabelTargetPair(&action3));
+ ASSERT_TRUE(action4.OnResolved(&err));
+ EXPECT_EQ(
+ "build phony/foo/action4.harddeps: phony phony/foo/action3 "
+ "phony/foo/mid\n",
+ hard_deps_rule(&action4));
+}
diff --git a/src/gn/ninja_utils.cc b/src/gn/ninja_utils.cc
index bf30bfb..e8a046a 100644
--- a/src/gn/ninja_utils.cc
+++ b/src/gn/ninja_utils.cc
@@ -40,3 +40,14 @@
".public_inputs.stamp");
}
}
+
+OutputFile GetHardDepsOutputFile(const Target* target,
+ const BuildSettings* build_settings) {
+ if (build_settings->no_stamp_files()) {
+ return GetOutputFile(*target, BuildDirType::PHONY, target->label().name(),
+ ".harddeps");
+ } else {
+ return GetOutputFile(*target, BuildDirType::OBJ, target->label().name(),
+ ".harddeps.stamp");
+ }
+}
diff --git a/src/gn/ninja_utils.h b/src/gn/ninja_utils.h
index e629d91..1773038 100644
--- a/src/gn/ninja_utils.h
+++ b/src/gn/ninja_utils.h
@@ -24,6 +24,12 @@
// don't collide with rules from other toolchains.
std::string GetNinjaRulePrefixForToolchain(const Settings* settings);
+// Returns the name of the phony target (or stamp file) that depends on all
+// hard deps of |target|. Only written if |target| has at least two hard deps,
+// see NinjaTargetWriter::WriteHardDepsStampOrPhony().
+OutputFile GetHardDepsOutputFile(const Target* target,
+ const BuildSettings* build_settings);
+
// Returns the output file path for the target's public inputs stamp or phony
// target.
OutputFile GetPublicInputsOutputFile(const Target* target,
diff --git a/src/gn/resolved_target_data.cc b/src/gn/resolved_target_data.cc
index de936c5..b0ef9a6 100644
--- a/src/gn/resolved_target_data.cc
+++ b/src/gn/resolved_target_data.cc
@@ -58,6 +58,18 @@
info->has_framework_info = true;
}
+// static
+bool ResolvedTargetData::ForwardsHardDeps(const Target* dep) {
+ // If |dep| is binary target and |dep| has no public header,
+ // a target depending on it does not need to have |dep|'s hard_deps as its
+ // hard_deps to start compiles earlier. Unless the target compiles a
+ // Swift module (since they also generate a header that can be used
+ // by the current target).
+ return !(dep->IsBinary() && !dep->all_headers_public() &&
+ dep->public_headers().empty() && !dep->builds_swift_module());
+}
+
+// Keep in sync with NinjaTargetWriter::WriteHardDepsStampOrPhony().
void ResolvedTargetData::ComputeHardDeps(TargetInfo* info) const {
TargetSet all_hard_deps;
for (const Target* dep : info->deps.linked_deps()) {
@@ -66,15 +78,8 @@
all_hard_deps.insert(dep);
continue;
}
- // If |dep| is binary target and |dep| has no public header,
- // |this| target does not need to have |dep|'s hard_deps as its
- // hard_deps to start compiles earlier. Unless the target compiles a
- // Swift module (since they also generate a header that can be used
- // by the current target).
- if (dep->IsBinary() && !dep->all_headers_public() &&
- dep->public_headers().empty() && !dep->builds_swift_module()) {
+ if (!ForwardsHardDeps(dep))
continue;
- }
// Recursive hard dependencies of all dependencies.
const TargetInfo* dep_info = GetTargetHardDeps(dep);
diff --git a/src/gn/resolved_target_data.h b/src/gn/resolved_target_data.h
index 2c0f836..544872d 100644
--- a/src/gn/resolved_target_data.h
+++ b/src/gn/resolved_target_data.h
@@ -147,6 +147,11 @@
// Retrieves an ordered list of all order-only dependency outputs for this
// target.
+ // Returns true if a target depending on |dep| inherits the hard deps of
+ // |dep|. Used to compute GetHardDeps(), and by code that needs to mirror
+ // that computation.
+ static bool ForwardsHardDeps(const Target* dep);
+
const std::vector<OutputFile>& GetOrderOnlyDeps(const Target* target) const {
return GetTargetOrderOnlyDeps(target)->order_only_deps;
}
