New issue
Advanced search Search tips

Issue 647525 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

gn builds are never clean with symbol_level = 0

Project Member Reported by brucedaw...@chromium.org, Sep 16 2016

Issue description

When building with symbol_level = 0 no .pdb files are created which means that there are three build steps for chrome that are always dirty. After a successful build this is what happens:

>ninja -d explain -v -C out\gn_debug_analyze chrome
ninja: Entering directory `out\gn_debug_analyze'
ninja explain: output chrome.exe.pdb doesn't exist
ninja explain: chrome.exe is dirty
ninja explain: chrome.exe.pdb is dirty
ninja explain: obj/chrome/reorder_imports.stamp is dirty
ninja explain: obj/chrome/chrome.stamp is dirty
[0 processes, 1/3 @ 0.0/s : 12372.760s ] c:/src/depot_tools/python276_bin/python.exe ../../build/win/reorder-imports.py -i initialexe -o ../gn_debug_analyze -a x86
[0 processes, 2/3 @ 0.0/s : 12372.864s ] c:/src/depot_tools/python276_bin/python.exe ../../build/toolchain/win/tool_wrapper.py stamp obj/chrome/reorder_imports.stamp
[0 processes, 3/3 @ 0.0/s : 12372.972s ] c:/src/depot_tools/python276_bin/python.exe ../../build/toolchain/win/tool_wrapper.py stamp obj/chrome/chrome.stamp

The full set of gn args that I used is shown below, but I'm sure that use_vs_code_analysis is not needed, and probably the others are irrelevant also.

disable_precompiled_headers = true
is_component_build = true
symbol_level = 0
target_cpu = "x86"
use_vs_code_analysis = true

 
Another variant of this bug is that when remove_webcore_debug_symbols is true then some PDBs will be skipped and those DLLs will always be considered 'dirty'.
I encountered this issue while trying to make my first contribution to Chromium. My machine is not the greatest, so I added remove_webcore_debug_symbols=true to my args.gn to speed up compiling, as suggested on https://www.chromium.org/developers/gn-build-configuration. Unfortunately since this breaks incremental linking, compiling after changing code instead takes tens of times longer, than it would without that option. I think this should at least be added to the documentation, to prevent hours (or days) of wasted time for those unaware of this bug.
I experimented with fixing the remove_webcore_debug_symbols issue by reinterpreting that to mean minimal_symbols instead of no_symbols. That helps in some ways (it prevents blink_platform.dll, blink_core.dll, blink_modules.dll, and blink_web.dll from always being dirty) but it increases their incremental link time (on my machine) from a couple of seconds to 13+ seconds. I'd never realized that symbol_level = 1 had that significant a cost (not huge in the big picture, but not ignorable either).

The good news is that the original bug, for chrome.exe.pdb specifically, is easy to fix. I'm uploading a CL for that now.

I'm not sure whether my mitigation of changing the meaning of remove_webcore_debug_symbols is wise or not. Windows only maybe?

I found the BUILD.gn file that tells ninja to expect .pdb files to be created. It's this block:
      if (symbol_level != 0) {
        outputs += [ pdbname ]
        runtime_outputs += [ pdbname ]
      }
from src\build\toolchain\win\BUILD.gn.

However I'm not sure of a tidy way to remove pdbname from the outputs/runtime_outputs for just the affected targets - move that logic into the symbol configs ("minimal_symbols", etc.) in build\config\compiler\BUILD.gn?

Owner: brucedaw...@chromium.org
Status: Started (was: Untriaged)
I have fixes for both issues out for review.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 26 2017

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

commit 4d7f37267b37b01230f7cd91626d508eab281c9e
Author: brucedawson <brucedawson@chromium.org>
Date: Mon Jun 26 22:27:02 2017

Fix gn builds never being clean at symbol_level = 0

Builds with symbol_level = 0 don't generate PDB files, and our gn files
mostly know about this. One exception was chrome.exe.pdb which was
unconditionally specified as an output. This fixes that to make that
output conditional on symbol_level != 0.

BUG= 647525 

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

[modify] https://crrev.com/4d7f37267b37b01230f7cd91626d508eab281c9e/chrome/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 28 2017

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

commit b919d95a7f8be9ca291d6bbff942cfb42d08d5a1
Author: brucedawson <brucedawson@chromium.org>
Date: Wed Jun 28 08:19:58 2017

Fix remove_webcore_debug_symbols to avoid constant building

The original gn implementation of remove_webcore_debug_symbols (in
crrev.com/1716523003) didn't generate PDBs for some of the webkit DLLs
in component builds, but that caused problems because ninja still
expects them to be outputs. This caused the build to never be clean (on
Windows) when this flag is used.

This change gets these DLLs to use minimal_symbols (on Windows builds
where symbol_level != 0) so that a PDB is always generated when
expected.

Doing test builds on my Windows machine of blink_core.dll I see the
following full-link times and chrome.dll.pdb sizes when I do a debug
component build with symbol_level = 2 when remove_webcore_debug_symbols
is set to the following configs:

        symbols: 71 s, 348 MB
minimal_symbols: 49 s,  82 MB
     no_symbols: 49 s, missing - builds never clean

This suggests that minimal_symbols is a reasonable choice (no increase
in build times) and a good fix for the not-clean builds.

This change does not alter the behavior on other platforms because
they don't have the not-clean issue.

In addition to avoiding constant building this also means that there
will be enough symbols for full call stacks.

BUG= 647525 

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

[modify] https://crrev.com/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1/third_party/WebKit/Source/config.gni
[modify] https://crrev.com/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1/third_party/WebKit/Source/core/core.gni
[modify] https://crrev.com/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1/third_party/WebKit/Source/modules/BUILD.gn
[modify] https://crrev.com/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1/third_party/WebKit/Source/modules/modules.gni
[modify] https://crrev.com/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1/third_party/WebKit/Source/platform/wtf/BUILD.gn
[modify] https://crrev.com/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1/third_party/WebKit/Source/web/BUILD.gn

Status: Fixed (was: Started)
I believe these issues are all fixed now. No documentation changes are needed because remove_webcore_debug_symbols=true should now work correctly.
Status: Assigned (was: Fixed)
Still broken, due to the newly added blink_controller. Fix is in progress.
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 6 2017

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

commit 961bda534caf2a184153b1458aae16be169bc951
Author: brucedawson <brucedawson@chromium.org>
Date: Thu Jul 06 18:13:38 2017

Fix remove_webcore_debug_symbols again to avoid constant building

The initial fix to remove_webcore_debug_symbols to make builds clean
when using it (crrev.com/2948363004) happened in parallel with a
change to add blink_controller (crrev.com/c/547379), so blink_controller
didn't use the new pattern, and is therefore always dirty when
remove_webcore_debug_symbols is true. This change finishes the fix to
make remove_webcore_debug_symbols work properly.

R=tkent@chromium.org
BUG= 647525 

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

[modify] https://crrev.com/961bda534caf2a184153b1458aae16be169bc951/third_party/WebKit/Source/controller/BUILD.gn

Status: Fixed (was: Assigned)
Fixed again. I would expect that it will probably stay fixed now since any future changes to remove_webcore_debug_symbols will copy the new pattern.

Sign in to add a comment