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

Issue 1261073004: Reduce DefaultConcurrentLinks from phys/4GB to phys/5GB. (Closed)

Created:
5 years, 4 months ago by brucedawson
Modified:
5 years, 4 months ago
Reviewers:
brettw
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Reduce DefaultConcurrentLinks from phys/4GB to phys/5GB. This is copied from the gyp repo, ninja.py: https://codereview.chromium.org/1255813005 VS 2015 uses about 20% more memory when linking. This caused a severe page-fault storm on my development machine and subsequent tests showed that linking could use ~63 GB of RAM (15 parallel links on a 64 GB machine). This suggests that VS 2013 would also be using a lot of RAM (52.5 GB?) such that very little was left for other applications or disk cache. Linker working sets vary wildly but with VS 2013 there are six links that use over 6 GB of working set and with VS 2015 there are seven. This change reduces parallel links by 20% (increase RAM-per-linker by 25%). The value can be overridden by setting GYP_LINK_CONCURRENCY. Tests with VS 2013 showed no statistically significant change in build times when link concurrency was reduced from 15 to 10 or 12. Further reductions may be prudent when we switch to VS 2015. BUG=440500 R=brettw@chromium.org Committed: https://crrev.com/ad86c6fcef0fbdf26da9165ae8bbbd3748201afc Cr-Commit-Position: refs/heads/master@{#342892}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -1 line) Patch
M build/toolchain/get_concurrent_links.py View 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 6 (1 generated)
brucedawson
This change was already made in the gyp repo. It appears to improve VS 2013 ...
5 years, 4 months ago (2015-07-28 18:39:24 UTC) #1
brettw
lgtm
5 years, 4 months ago (2015-07-29 20:17:59 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1261073004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1261073004/1
5 years, 4 months ago (2015-08-11 20:39:43 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 4 months ago (2015-08-11 21:05:21 UTC) #5
commit-bot: I haz the power
5 years, 4 months ago (2015-08-11 21:06:05 UTC) #6
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/ad86c6fcef0fbdf26da9165ae8bbbd3748201afc
Cr-Commit-Position: refs/heads/master@{#342892}

Powered by Google App Engine
This is Rietveld 408576698