Commit e8a3ce91 authored by Takuto Ikuta's avatar Takuto Ikuta Committed by Commit Bot

GN: do not make indirect dependency to direct dependency

If a target is hard dep, there is no need to have the action targets's
recursive deps as direct dependency, because such dependency is
transitive.

This is found by pcc in gn-dev
https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/BcrSfPQE84E

With this change, generated android's toolchain.ninja size is reduced
from 262MB to 31MB. And ninja's startup time reduced from 4.9~5.1s to
2.6s.
I used args.gn same with android_n5x_swarming_rel bot.

Also this patch reduced the time of `gn gen` from 9.5~10.3s to 6.8~7.2s
on my machine.

Change-Id: I0f0214d3abe74143516b263da839e98b3987fb64
Reviewed-on: https://chromium-review.googlesource.com/1041506Reviewed-by: default avatarBrett Wilson <brettw@chromium.org>
Reviewed-by: default avatarDirk Pranke <dpranke@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557365}
parent 28f42d2c
...@@ -409,3 +409,77 @@ TEST(NinjaActionTargetWriter, ForEachWithPool) { ...@@ -409,3 +409,77 @@ TEST(NinjaActionTargetWriter, ForEachWithPool) {
"build obj/foo/bar.stamp: stamp input1.out\n"; "build obj/foo/bar.stamp: stamp input1.out\n";
EXPECT_EQ(expected_linux, out.str()); EXPECT_EQ(expected_linux, out.str());
} }
TEST(NinjaActionTargetWriter, NoTransitiveHardDeps) {
Err err;
TestWithScope setup;
setup.build_settings()->set_python_path(
base::FilePath(FILE_PATH_LITERAL("/usr/bin/python")));
Target dep(setup.settings(), Label(SourceDir("//foo/"), "dep"));
dep.set_output_type(Target::ACTION);
dep.visibility().SetPublic();
dep.SetToolchain(setup.toolchain());
ASSERT_TRUE(dep.OnResolved(&err));
Target foo(setup.settings(), Label(SourceDir("//foo/"), "foo"));
foo.set_output_type(Target::ACTION);
foo.visibility().SetPublic();
foo.sources().push_back(SourceFile("//foo/input1.txt"));
foo.action_values().set_script(SourceFile("//foo/script.py"));
foo.private_deps().push_back(LabelTargetPair(&dep));
foo.SetToolchain(setup.toolchain());
foo.action_values().outputs() =
SubstitutionList::MakeForTest("//out/Debug/foo.out");
ASSERT_TRUE(foo.OnResolved(&err));
{
std::ostringstream out;
NinjaActionTargetWriter writer(&foo, out);
writer.Run();
const char expected_linux[] =
"rule __foo_foo___rule\n"
// These come from the args.
" command = /usr/bin/python ../../foo/script.py\n"
" description = ACTION //foo:foo()\n"
" restat = 1\n"
"\n"
"build foo.out: __foo_foo___rule | ../../foo/script.py"
" ../../foo/input1.txt obj/foo/dep.stamp\n"
"\n"
"build obj/foo/foo.stamp: stamp foo.out\n";
EXPECT_EQ(expected_linux, out.str());
}
Target bar(setup.settings(), Label(SourceDir("//bar/"), "bar"));
bar.set_output_type(Target::ACTION);
bar.sources().push_back(SourceFile("//bar/input1.txt"));
bar.action_values().set_script(SourceFile("//bar/script.py"));
bar.private_deps().push_back(LabelTargetPair(&foo));
bar.SetToolchain(setup.toolchain());
bar.action_values().outputs() =
SubstitutionList::MakeForTest("//out/Debug/bar.out");
ASSERT_TRUE(bar.OnResolved(&err)) << err.message();
{
std::ostringstream out;
NinjaActionTargetWriter writer(&bar, out);
writer.Run();
const char expected_linux[] =
"rule __bar_bar___rule\n"
// These come from the args.
" command = /usr/bin/python ../../bar/script.py\n"
" description = ACTION //bar:bar()\n"
" restat = 1\n"
"\n"
// Do not have obj/foo/dep.stamp as dependency.
"build bar.out: __bar_bar___rule | ../../bar/script.py"
" ../../bar/input1.txt obj/foo/foo.stamp\n"
"\n"
"build obj/bar/bar.stamp: stamp bar.out\n";
EXPECT_EQ(expected_linux, out.str());
}
}
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include <sstream> #include <sstream>
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "tools/gn/ninja_action_target_writer.h"
#include "tools/gn/ninja_target_writer.h" #include "tools/gn/ninja_target_writer.h"
#include "tools/gn/target.h" #include "tools/gn/target.h"
#include "tools/gn/test_with_scope.h" #include "tools/gn/test_with_scope.h"
...@@ -94,6 +95,23 @@ TEST(NinjaTargetWriter, WriteInputDepsStampAndGetDep) { ...@@ -94,6 +95,23 @@ TEST(NinjaTargetWriter, WriteInputDepsStampAndGetDep) {
EXPECT_EQ("obj/foo/base.stamp", dep[0].value()); EXPECT_EQ("obj/foo/base.stamp", dep[0].value());
} }
{
std::ostringstream stream;
NinjaActionTargetWriter writer(&action, stream);
writer.Run();
EXPECT_EQ(
"rule __foo_action___rule\n"
" command = ../../foo/script.py\n"
" description = ACTION //foo:action()\n"
" restat = 1\n"
"\n"
"build: __foo_action___rule | ../../foo/script.py"
" ../../foo/action_source.txt ./target\n"
"\n"
"build obj/foo/action.stamp: stamp\n",
stream.str());
}
// Input deps for action which should depend on the base since its a hard dep // Input deps for action which should depend on the base since its a hard dep
// that is a (indirect) dependency, as well as the the action source. // that is a (indirect) dependency, as well as the the action source.
{ {
...@@ -104,9 +122,10 @@ TEST(NinjaTargetWriter, WriteInputDepsStampAndGetDep) { ...@@ -104,9 +122,10 @@ TEST(NinjaTargetWriter, WriteInputDepsStampAndGetDep) {
ASSERT_EQ(1u, dep.size()); ASSERT_EQ(1u, dep.size());
EXPECT_EQ("obj/foo/action.inputdeps.stamp", dep[0].value()); EXPECT_EQ("obj/foo/action.inputdeps.stamp", dep[0].value());
EXPECT_EQ("build obj/foo/action.inputdeps.stamp: stamp ../../foo/script.py " EXPECT_EQ(
"../../foo/action_source.txt obj/foo/base.stamp\n", "build obj/foo/action.inputdeps.stamp: stamp ../../foo/script.py "
stream.str()); "../../foo/action_source.txt\n",
stream.str());
} }
} }
......
...@@ -372,7 +372,13 @@ bool Target::OnResolved(Err* err) { ...@@ -372,7 +372,13 @@ bool Target::OnResolved(Err* err) {
PullRecursiveBundleData(); PullRecursiveBundleData();
PullDependentTargetLibs(); PullDependentTargetLibs();
PullRecursiveHardDeps();
if (!hard_dep()) {
// If this target is not hard_dep type, we need to pull hard dependency from
// dependent target, because it may not transitive for compiling tasks.
PullRecursiveHardDeps();
}
if (!ResolvePrecompiledHeaders(err)) if (!ResolvePrecompiledHeaders(err))
return false; return false;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment