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

Issue 591723003: Revert of Enable incremental linking for static_library too (Closed)

Created:
6 years, 3 months ago by scottmg
Modified:
6 years, 3 months ago
Reviewers:
jam
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Revert of Enable incremental linking for static_library too (patchset #2 id:20001 of https://codereview.chromium.org/502993005/) Reason for revert: Does not appear to work well for waterfall workload, see number of instances of "performing full link" in http://build.chromium.org/p/chromium.win/builders/Win%20Builder/builds/2780/steps/compile/logs/stdio Might still be worth investigating if it's worthwhile on trybots where less things might change per-job? But doesn't seem that likely. Original issue's description: > Enable incremental linking for static_library too > > Timings on z620: > > Without: > > GYP_DEFINES=component=static_library chromium_win_pch=0 incremental_chrome_dll=0 > > touch content\app\main.cc (causes chrome.dll, chrome_child.dll, and unit_tests.exe to relink) > tim ninja -C out\Release unit_tests > [19/19] LINK_EMBED unit_tests.exe > > real: 4m46.828s > > With: > > GYP_DEFINES=component=static_library branding=Chrome chromium_win_pch=0 incremental_chrome_dll=1 > > touch content\app\content_main.cc && tim ninja -C out\Release unit_tests > ninja: Entering directory `out\Release' > [19/19] LINK_EMBED unit_tests.exe > > real: 0m43.625s > > This is basically best case, and the actual performance on bots will differ based on > their config and the number of files that change from run-to-run, but it seems worth > enabling to see what happens, at least. > > R=jam@chromium.org > BUG=404809, 402270 > > Committed: https://crrev.com/6922bc1d4b0a2f4604598082a877d3add6d86d1c > Cr-Commit-Position: refs/heads/master@{#295786} TBR=jam@chromium.org NOTREECHECKS=true NOTRY=true BUG=404809, 402270 Committed: https://crrev.com/f01a9aee4db445d8f920ae191f27f0e52c76b50c Cr-Commit-Position: refs/heads/master@{#295815}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -23 lines) Patch
M build/common.gypi View 2 chunks +5 lines, -16 lines 0 comments Download
M chrome/chrome_dll.gypi View 1 chunk +0 lines, -7 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
scottmg
Created Revert of Enable incremental linking for static_library too
6 years, 3 months ago (2014-09-19 23:47:44 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/591723003/1
6 years, 3 months ago (2014-09-19 23:48:46 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1) as 32ce35cf3723e9692b8a078469b60abf54e955fe
6 years, 3 months ago (2014-09-19 23:50:51 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1) as af68551e3cb4e6e5b321d601f586b8dab916384c
6 years, 3 months ago (2014-09-19 23:51:14 UTC) #4
commit-bot: I haz the power
6 years, 3 months ago (2014-09-19 23:51:22 UTC) #5
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/f01a9aee4db445d8f920ae191f27f0e52c76b50c
Cr-Commit-Position: refs/heads/master@{#295815}

Powered by Google App Engine
This is Rietveld 408576698