GN: Don't write stamp files for one rule.

GN: Don't write stamp files for one rule.

When handling input deps, GN would previously always write a stamp file, even
if that stamp file just stamped one input file. This is a wasteful extra rule.

The previous code was also a bit fragile because it first computed if there
was any output via a 6-line conditional, and then wrote that output in a way
that the conditions must match the previous computation.

This new structure is a little more clear since we accumulate the inputs
and then check the size, so nothing must be kept in sync.

Savings in Chrome Linux build is a reduction in 388 build rules and 376K of
.ninja files.

BUG=552701
Committed: https://crrev.com/4445f6179019a0a62cd4a7854723dba7316c5ac0
Cr-Commit-Position: refs/heads/master@{#373375}

[brettw, 2016-02-03 21:31:39]

Description was changed from

==========
GN: Don't write stamp files for one rule.

When handling input deps, GN would previously always write a stamp file, even
if that stamp file just stamped one input file. This is a wasteful extra rule.

The previous code was also a bit fragile because it first computed if there
was any output via a 6-line conditional, and then wrote that output in a way
that the conditions must match the previous computation.

This new structure is a little more clear since we accumulate the inputs
and then check the size, so nothing must be kept in sync.

Savings in Chrome Linux build is a reduction in 388 build rules and 376K of
.ninja files.
==========

to

==========
GN: Don't write stamp files for one rule.

When handling input deps, GN would previously always write a stamp file, even
if that stamp file just stamped one input file. This is a wasteful extra rule.

The previous code was also a bit fragile because it first computed if there
was any output via a 6-line conditional, and then wrote that output in a way
that the conditions must match the previous computation.

This new structure is a little more clear since we accumulate the inputs
and then check the size, so nothing must be kept in sync.

Savings in Chrome Linux build is a reduction in 388 build rules and 376K of
.ninja files.

BUG=552701
==========

[brettw, 2016-02-03 21:31:39]

brettw@chromium.org changed reviewers:
+ scottmg@chromium.org

[brettw, 2016-02-03 21:32:07]

The CQ bit was checked by brettw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-02-03 21:33:37]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1663813003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1663813003/1

[commit-bot: I haz the power, 2016-02-03 22:11:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-03 22:11:21]

Dry run: This issue passed the CQ dry run.

[scottmg, 2016-02-03 22:41:18]

lgtm

https://codereview.chromium.org/1663813003/diff/1/tools/gn/ninja_target_write...
File tools/gn/ninja_target_writer.cc (right):

https://codereview.chromium.org/1663813003/diff/1/tools/gn/ninja_target_write...
tools/gn/ninja_target_writer.cc:214: if (input_deps_sources.size() == 1 &&
input_deps_targets.size() == 0)
Did you check if it's worth pushing multiple through for 2 or 3, etc?

[brettw, 2016-02-03 22:50:44]

https://codereview.chromium.org/1663813003/diff/1/tools/gn/ninja_target_write...
File tools/gn/ninja_target_writer.cc (right):

https://codereview.chromium.org/1663813003/diff/1/tools/gn/ninja_target_write...
tools/gn/ninja_target_writer.cc:214: if (input_deps_sources.size() == 1 &&
input_deps_targets.size() == 0)
On 2016/02/03 22:41:18, scottmg wrote:
> Did you check if it's worth pushing multiple through for 2 or 3, etc?

No, but doing this for 2 will definitely increase ninja file size with how
things are structured (the stamp files and input files tend to be similar
length, so the target would need to have <= 2 sources in it to make this a win).

There's a question about how to trade this off vs. graph complexity inside Ninja
and extra stamp operations at build-time, but I don't even know how to measure
that and doubt file size is the only thing that matters much.

[brettw, 2016-02-03 22:50:49]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2016-02-03 22:51:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1663813003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1663813003/1

[commit-bot: I haz the power, 2016-02-03 22:57:20]

Description was changed from

==========
GN: Don't write stamp files for one rule.

When handling input deps, GN would previously always write a stamp file, even
if that stamp file just stamped one input file. This is a wasteful extra rule.

The previous code was also a bit fragile because it first computed if there
was any output via a 6-line conditional, and then wrote that output in a way
that the conditions must match the previous computation.

This new structure is a little more clear since we accumulate the inputs
and then check the size, so nothing must be kept in sync.

Savings in Chrome Linux build is a reduction in 388 build rules and 376K of
.ninja files.

BUG=552701
==========

to

==========
GN: Don't write stamp files for one rule.

When handling input deps, GN would previously always write a stamp file, even
if that stamp file just stamped one input file. This is a wasteful extra rule.

The previous code was also a bit fragile because it first computed if there
was any output via a 6-line conditional, and then wrote that output in a way
that the conditions must match the previous computation.

This new structure is a little more clear since we accumulate the inputs
and then check the size, so nothing must be kept in sync.

Savings in Chrome Linux build is a reduction in 388 build rules and 376K of
.ninja files.

BUG=552701
==========

[commit-bot: I haz the power, 2016-02-03 22:57:21]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-02-03 22:58:48]

Description was changed from

==========
GN: Don't write stamp files for one rule.

When handling input deps, GN would previously always write a stamp file, even
if that stamp file just stamped one input file. This is a wasteful extra rule.

The previous code was also a bit fragile because it first computed if there
was any output via a 6-line conditional, and then wrote that output in a way
that the conditions must match the previous computation.

This new structure is a little more clear since we accumulate the inputs
and then check the size, so nothing must be kept in sync.

Savings in Chrome Linux build is a reduction in 388 build rules and 376K of
.ninja files.

BUG=552701
==========

to

==========
GN: Don't write stamp files for one rule.

When handling input deps, GN would previously always write a stamp file, even
if that stamp file just stamped one input file. This is a wasteful extra rule.

The previous code was also a bit fragile because it first computed if there
was any output via a 6-line conditional, and then wrote that output in a way
that the conditions must match the previous computation.

This new structure is a little more clear since we accumulate the inputs
and then check the size, so nothing must be kept in sync.

Savings in Chrome Linux build is a reduction in 388 build rules and 376K of
.ninja files.

BUG=552701

Committed: https://crrev.com/4445f6179019a0a62cd4a7854723dba7316c5ac0
Cr-Commit-Position: refs/heads/master@{#373375}
==========

[commit-bot: I haz the power, 2016-02-03 22:58:49]

Patchset 1 (id:??) landed as
https://crrev.com/4445f6179019a0a62cd4a7854723dba7316c5ac0
Cr-Commit-Position: refs/heads/master@{#373375}

[Nico, 2016-02-03 23:49:14]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2016-02-03 23:49:15]

nice!