New issue
Advanced search Search tips

Issue 607776 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows , Mac
Pri: 3
Type: Bug



Sign in to add a comment

gyp-win-tool and gyp-mac-tool are regenerated excessively, triggering overbuild

Project Member Reported by brucedaw...@chromium.org, Apr 29 2016

Issue description

Whenever .ninja files are regenerated, due to running "gn gen" or "build\gyp_chromium", or modifying a single leaf-node BUILD.gn file, the gyp-win-tool or gyp-mac-tool file is created (if building on Windows or Mac).

This is a problem because many build steps have gyp-win-tool as a dependency so when its file date changes many build steps run. This is particularly noticeable when using "gn" because the .ninja generation step is so fast.

The fix is to not touch the file if it hasn't changed. I don't believe this will have any ill consequences, but it should improve build times on developer machines and build machines.

Example - note that 16 targets need rebuilding if the project files are regenerated:
>ninja -C out\Debug_gn_component d8
ninja: Entering directory `out\Debug_gn_component'
ninja: no work to do.

>gn gen out\Debug_gn_component & ninja -C out\Debug_gn_component d8
Done. Wrote 3716 targets from 859 files in 2829ms
ninja: Entering directory `out\Debug_gn_component'
[0 processes, 16/16 @ 5.0/s : 3.202s ] LINK d8.exe

If gyp-win-tool is not unnecessarily generated then the second build step does nothing.

 
Ah, good find! I was wondering what caused that, but not enough to actually, y'know, check. :)

(gyp doesn't specify gyp-win-tool as an input to build rules, so that's why it doesn't happen there.)
I was wondering why it didn't happen on gyp builds. Should we remove gyp-win-tool from the dependencies on gn builds, or just apply the fix I created?

BTW, I just tested a debug gn component build and when I touch gyp-win-tool it triggers 4898 build steps. Those build steps took 14.5 minutes to execute on a Z840. Your mileage may vary.

Without touching gyp-win-tool there are two build steps that run, but that's a separate bug.
It doesn't change often, but there's few things e.g. https://code.google.com/p/chromium/codesearch#chromium/src/tools/gyp/pylib/gyp/win_tool.py&l=244 which could modify the command line, so it's probably correct to keep the dependency. So your fix sgtm.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 29 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e8cfc6cac3577dc1bd061a59afedb05a5199df3e

commit e8cfc6cac3577dc1bd061a59afedb05a5199df3e
Author: brucedawson <brucedawson@chromium.org>
Date: Fri Apr 29 18:44:33 2016

Only write gyp-win-tool when needed

The gyp-win-tool is written every time .ninja files are regenerated.
This can cause significant overbuild. This change simply avoids writing
it (and touching its time-stamp) if nothing has changed.

A similar fix will be needed for Mac.

BUG= 607776 

Review-Url: https://codereview.chromium.org/1932133002
Cr-Commit-Position: refs/heads/master@{#390707}

[modify] https://crrev.com/e8cfc6cac3577dc1bd061a59afedb05a5199df3e/build/toolchain/win/setup_toolchain.py

Project Member

Comment 5 by bugdroid1@chromium.org, May 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0305180fdbb1b8413f1d67b2f8d4af1cb70259d7

commit 0305180fdbb1b8413f1d67b2f8d4af1cb70259d7
Author: brucedawson <brucedawson@chromium.org>
Date: Mon May 02 17:34:41 2016

Only write gyp-mac-tool when needed

The gyp-mac-tool is written every time .ninja files are regenerated.
This can cause significant overbuild. This change simply avoids writing
it (and touching its time-stamp) if nothing has changed.

The equivalent fix was done for Windows in crrev.com/1932133002

BUG= 607776 

Review-Url: https://codereview.chromium.org/1928303003
Cr-Commit-Position: refs/heads/master@{#390983}

[modify] https://crrev.com/0305180fdbb1b8413f1d67b2f8d4af1cb70259d7/build/toolchain/mac/setup_toolchain.py

Status: Fixed (was: Assigned)

Sign in to add a comment