Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(87)

Issue 1258273004: Annotate large GN targets for precompiled headers (Closed)

Created:
5 years, 4 months ago by brettw
Modified:
5 years, 4 months ago
Reviewers:
Dirk Pranke
CC:
chromium-reviews, sadrul, yusukes+watch_chromium.org, zea+watch_chromium.org, tzik, binji+watch_chromium.org, bradnelson+warch_chromium.org, browser-components-watch_chromium.org, teravest+watch_chromium.org, tim+watch_chromium.org, extensions-reviews_chromium.org, cbentzel+watch_chromium.org, grt+watch_chromium.org, chromoting-reviews_chromium.org, jam, pvalenzuela+watch_chromium.org, darin-cc_chromium.org, rouslan+autofillwatch_chromium.org, devtools-reviews_chromium.org, chromium-apps-reviews_chromium.org, erikwright+watch_chromium.org, ihf+watch_chromium.org, mlamouri+watch-content_chromium.org, mlamouri+watch-notifications_chromium.org, rickyz+watch_chromium.org, feature-media-reviews_chromium.org, kalyank, piman+watch_chromium.org, mkwst+moarreviews-shell_chromium.org, peter+watch_chromium.org, jochen+watch_chromium.org, maniscalco+watch_chromium.org, wfh+watch_chromium.org, maxbogue+watch_chromium.org, yurys, plaree+watch_chromium.org, tfarina, estade+watch_chromium.org, cc-bugs_chromium.org, pfeldman
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Annotate large GN targets for precompiled headers Adds the precompiled header config to most large-ish targets in the build, but keeps the config a no-op (so no precompiled headers will be used but this can bw switched with a one-line change). Removes Windows files from the precompiled header. This does not seem to affect the build speed much because most Chrome files don't depend on Windows any more. And windows.h injects typedefs and defines that conflict with some third party libraries and prevent using precompiled headers for those targets or any target that includes them. I counted ~50 files or bigger as large. The 50 file threshold is based on some previous approximate measurements (since the precompile step is an extra per-target compile, it can actually make small targets compile slower). For borderline cases, I added the precompiled header flag if I thought it was likely to have more files added, and didn't add it if I thought the target was likely to be static. This is a reland of https://codereview.chromium.org/1250273002/ with the config disabled for easier re-landing and iterating CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel TBR=dpranke Committed: https://crrev.com/bc8b2a244ab981b264a86f5f2a64464089e05e32 Cr-Commit-Position: refs/heads/master@{#340728}

Patch Set 1 : Original patch from reverted issue #

Patch Set 2 : disable #

Unified diffs Side-by-side diffs Delta from patch set Stats (+188 lines, -106 lines) Patch
M ash/BUILD.gn View 3 chunks +4 lines, -1 line 0 comments Download
M base/BUILD.gn View 1 chunk +4 lines, -1 line 0 comments Download
M base/test/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
M build/config/BUILD.gn View 1 1 chunk +2 lines, -1 line 0 comments Download
M build/config/compiler/BUILD.gn View 1 chunk +6 lines, -0 lines 0 comments Download
M build/json_schema_api.gni View 2 chunks +6 lines, -0 lines 0 comments Download
M build/precompile.h View 3 chunks +17 lines, -69 lines 0 comments Download
M build/toolchain/win/BUILD.gn View 2 chunks +2 lines, -6 lines 0 comments Download
M cc/BUILD.gn View 3 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/BUILD.gn View 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/browser/devtools/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/extensions/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/BUILD.gn View 1 chunk +4 lines, -1 line 0 comments Download
M chrome/common/extensions/api/BUILD.gn View 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/installer/util/BUILD.gn View 1 chunk +6 lines, -2 lines 0 comments Download
M chrome/test/BUILD.gn View 4 chunks +6 lines, -0 lines 0 comments Download
M chrome/utility/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M components/autofill/core/browser/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
M components/policy/core/common/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M content/child/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
M content/common/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M content/public/browser/BUILD.gn View 1 chunk +4 lines, -1 line 0 comments Download
M content/public/common/BUILD.gn View 1 chunk +4 lines, -1 line 0 comments Download
M content/shell/BUILD.gn View 1 chunk +6 lines, -2 lines 0 comments Download
M content/test/BUILD.gn View 3 chunks +7 lines, -1 line 0 comments Download
M extensions/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
M extensions/browser/BUILD.gn View 1 chunk +6 lines, -2 lines 0 comments Download
M extensions/common/BUILD.gn View 1 chunk +6 lines, -2 lines 0 comments Download
M extensions/renderer/BUILD.gn View 1 chunk +6 lines, -2 lines 0 comments Download
M gpu/BUILD.gn View 1 chunk +6 lines, -1 line 0 comments Download
M gpu/command_buffer/service/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M media/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M media/base/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M net/BUILD.gn View 3 chunks +18 lines, -6 lines 0 comments Download
M ppapi/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M ppapi/cpp/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M ppapi/proxy/BUILD.gn View 1 chunk +4 lines, -1 line 0 comments Download
M ppapi/shared_impl/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M ppapi/thunk/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/protocol/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M sandbox/win/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
M skia/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M storage/browser/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
M sync/BUILD.gn View 2 chunks +4 lines, -0 lines 0 comments Download
M ui/gfx/BUILD.gn View 1 chunk +6 lines, -2 lines 0 comments Download
M ui/gl/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M ui/message_center/BUILD.gn View 1 chunk +6 lines, -2 lines 0 comments Download
M ui/views/BUILD.gn View 2 chunks +8 lines, -2 lines 0 comments Download
M ui/wm/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
brettw
5 years, 4 months ago (2015-07-28 17:23:11 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1258273004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1258273004/20001
5 years, 4 months ago (2015-07-28 17:24:21 UTC) #4
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 4 months ago (2015-07-28 18:24:52 UTC) #5
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/bc8b2a244ab981b264a86f5f2a64464089e05e32 Cr-Commit-Position: refs/heads/master@{#340728}
5 years, 4 months ago (2015-07-28 18:25:37 UTC) #6
Dirk Pranke
5 years, 4 months ago (2015-07-28 19:08:13 UTC) #7
Message was sent while issue was closed.
lgtm. Shouldn't we have an arg to turn this off or on? I think we do for GYP.

Powered by Google App Engine
This is Rietveld 408576698