New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 801925 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

gn doesn't use thin archives

Project Member Reported by thakis@chromium.org, Jan 15 2018

Issue description

gn's alink in build/toolchain/gcc_toolchain.gni uses rcsD (as of https://chromium.googlesource.com/chromium/src/+/52619d68cf150087682e2a70c4a221882aa59f47%5E%21/#F0) and doesn't use T (except, for some reason, on AIX).

This makes alink much less efficient than necessary.

$ ls --sort=size -hl $(find /usr/local/google/home/thakis/src/chrome/src/out/gn -name '*.a') | head -10
-rw-r----- 1 thakis eng  647M Oct 26 13:16 /usr/local/google/home/thakis/src/chrome/src/out/gn/obj/third_party/WebKit/Source/bindings/core/v8/libbindings_core_impl.a
-rw-r----- 1 thakis eng  633M Oct 26 13:16 /usr/local/google/home/thakis/src/chrome/src/out/gn/obj/third_party/WebKit/Source/bindings/modules/v8/libbindings_modules_impl.a
-rw-r----- 1 thakis eng  440M Oct 26 13:16 /usr/local/google/home/thakis/src/chrome/src/out/gn/obj/chrome/browser/libbrowser.a
-rw-r----- 1 thakis eng  282M Oct 26 13:16 /usr/local/google/home/thakis/src/chrome/src/out/gn/obj/chrome/browser/ui/libui.a
-rw-r----- 1 thakis eng  247M Oct 26 13:12 /usr/local/google/home/thakis/src/chrome/src/out/gn/obj/net/libnet.a
-rw-r----- 1 thakis eng  222M Oct 26 13:16 /usr/local/google/home/thakis/src/chrome/src/out/gn/obj/third_party/WebKit/Source/platform/libblink_platform.a
-rw-r----- 1 thakis eng  215M Oct 26 13:16 /usr/local/google/home/thakis/src/chrome/src/out/gn/obj/chrome/browser/extensions/libextensions.a
-rw-r----- 1 thakis eng  207M Oct 26 13:16 /usr/local/google/home/thakis/src/chrome/src/out/gn/obj/third_party/WebKit/Source/core/svg/libsvg.a
-rw-r----- 1 thakis eng  194M Oct 26 13:16 /usr/local/google/home/thakis/src/chrome/src/out/gn/obj/content/renderer/librenderer.a
-rw-r----- 1 thakis eng  182M Sep 29 16:15 /usr/local/google/home/thakis/src/chrome/src/out/gn/obj/media/cast/libcast_sender.a


Is that intentional? Arguably with gn's source_set, this isn't needed as much and it's a bit less of a problem in component builds, but static library builds are slower than necessary due to this.


(I don't know if complete_static_lib can be checked for in a toolchain. If so, we should pass T only if complete_static_lib isn't set.)
 
I don't think that's intentional.
Components: Build
> I don't know if complete_static_lib can be checked for in a toolchain. If so, we should pass T only if complete_static_lib isn't set.

dpranke@ do you know?

It would be nice to build with thin archives.  Adding -T reduced the size of my out dir from 2.4G to 1.8G, and speed up the build.

Cc: timbrown@chromium.org
dpranke: Could we just add config("thin_archive") { arflags = ["-T"] } and add that to the default configs, and then just remove the config everywhere complete_static_lib is set?
That sounds reasonable to me.
Owner: thomasanderson@chromium.org
Status: Started (was: Untriaged)
Going to try the approach from c#5.
Cc: brettw@chromium.org
I don't really like the approach in #c5 (as just commented on the CL). I think we might want to change GN instead to better differentiate the flags needed for complete libs.
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 20 2018

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

commit 8ce24e55e022b54554357ec6e01aeba36d4ae1bd
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Tue Mar 20 00:09:56 2018

Add thin_archive config to global build config

To enable thin archives on posix, the current plan is to:
1. Land this CL.
2. Remove thin_archive from third_party build files as appropriate.
3. Uncomment the arflags = [ "-T" ] that this CL adds.

If you want a standalone static library, you now need to both set
complete_static_lib=true and remove the thin_archive config. See the comment in
build/config/compiler/BUILD.gn for details.

BUG= 801925 
R=thakis,dpranke
TBR=sky

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Icd1d3389da13106f46de3468475f0102a88c6211
Reviewed-on: https://chromium-review.googlesource.com/954344
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544221}
[modify] https://crrev.com/8ce24e55e022b54554357ec6e01aeba36d4ae1bd/build/config/BUILDCONFIG.gn
[modify] https://crrev.com/8ce24e55e022b54554357ec6e01aeba36d4ae1bd/build/config/compiler/BUILD.gn
[modify] https://crrev.com/8ce24e55e022b54554357ec6e01aeba36d4ae1bd/chrome/installer/gcapi_mac/BUILD.gn
[modify] https://crrev.com/8ce24e55e022b54554357ec6e01aeba36d4ae1bd/components/cronet/ios/BUILD.gn
[modify] https://crrev.com/8ce24e55e022b54554357ec6e01aeba36d4ae1bd/media/cast/BUILD.gn
[modify] https://crrev.com/8ce24e55e022b54554357ec6e01aeba36d4ae1bd/ppapi/cpp/BUILD.gn

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 21 2018

The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium/+/da89ac4c49013ce8d2d1d81e389cd37c0ee2ef27

commit da89ac4c49013ce8d2d1d81e389cd37c0ee2ef27
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Wed Mar 21 03:56:15 2018

Remove thin_archive config from complete static libraries

Following CL [1], it is required to remove the thin_archive config
everywhere complete_static_lib is set.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/954344

BUG= chromium:801925 
R=thestig

Change-Id: Ia142aa65c375f0ab42346599d07167c38332c357
Reviewed-on: https://pdfium-review.googlesource.com/28890
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

[modify] https://crrev.com/da89ac4c49013ce8d2d1d81e389cd37c0ee2ef27/BUILD.gn

Project Member

Comment 11 by bugdroid1@chromium.org, Mar 21 2018

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

commit eea617251e0df799fabf45dd898054647e75601b
Author: pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Mar 21 06:19:21 2018

Roll src/third_party/pdfium/ 40d37b1a8..da89ac4c4 (1 commit)

https://pdfium.googlesource.com/pdfium.git/+log/40d37b1a8077..da89ac4c4901

$ git log 40d37b1a8..da89ac4c4 --date=short --no-merges --format='%ad %ae %s'
2018-03-21 thomasanderson Remove thin_archive config from complete static libraries

Created with:
  roll-dep src/third_party/pdfium
BUG= chromium:801925 


The AutoRoll server is located here: https://pdfium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


TBR=dsinclair@chromium.org

Change-Id: Ib4c12f7135e08e2391edfffb2a4b729265ade551
Reviewed-on: https://chromium-review.googlesource.com/972663
Reviewed-by: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#544646}
[modify] https://crrev.com/eea617251e0df799fabf45dd898054647e75601b/DEPS

Project Member

Comment 12 by bugdroid1@chromium.org, Mar 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/5a925d8a892bf8b4f9622cdb8819799bbe1350a6

commit 5a925d8a892bf8b4f9622cdb8819799bbe1350a6
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Wed Mar 21 07:38:37 2018

Remove thin_archive config from complete static libraries

Following CL [1], it is required to remove the thin_archive config
everywhere complete_static_lib is set.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/954344

BUG= chromium:801925 
R=machenbach

Change-Id: Id75e06543545924771820500c53df3d5ee58774b
Reviewed-on: https://chromium-review.googlesource.com/972550
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52089}
[modify] https://crrev.com/5a925d8a892bf8b4f9622cdb8819799bbe1350a6/gni/v8.gni

Project Member

Comment 13 by bugdroid1@chromium.org, Mar 22 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/36fc5e10ba81851737550f91d297133af96216c4

commit 36fc5e10ba81851737550f91d297133af96216c4
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Thu Mar 22 15:28:34 2018

Remove thin_archive config from complete static libraries

Following CL [1], it is required to remove the thin_archive config
everywhere complete_static_lib is set.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/954344

BUG= chromium:801925 

Change-Id: I4af26e4e35ad045758a93ca679edfe0241de5b32
Reviewed-on: https://webrtc-review.googlesource.com/63525
Reviewed-by: Patrik Höglund <phoglund@webrtc.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#22571}
[modify] https://crrev.com/36fc5e10ba81851737550f91d297133af96216c4/BUILD.gn

Project Member

Comment 14 by bugdroid1@chromium.org, Mar 23 2018

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

commit 2220fd02de569658459cc203adb4f13ee7cf08e6
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Mar 23 16:08:57 2018

Roll src/third_party/webrtc/ 29e7bee33..06e363a6e (13 commits)

https://webrtc.googlesource.com/src.git/+log/29e7bee3308d..06e363a6e608

$ git log 29e7bee33..06e363a6e --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG=chromium:None,chromium:801925


The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng;master.tryserver.chromium.win:win-msvc-dbg
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Ic393e84eccf47403f6854ff5fc5ad73538c1d504
Reviewed-on: https://chromium-review.googlesource.com/977890
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#545469}
[modify] https://crrev.com/2220fd02de569658459cc203adb4f13ee7cf08e6/DEPS

Project Member

Comment 15 by bugdroid1@chromium.org, Apr 6 2018

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

commit 40a8d84a54ac47d3299194c5377aebd71714f1f0
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Fri Apr 06 20:34:58 2018

Enable thin archive builds on POSIX.

With the following gn args:
use_goma = true
is_debug = false
is_component_build = false

Reduces the size of the out directory from 2.4G to 1.8G and reduces
build time from 1m36s to 1m20s.

BUG= 801925 , 829956 

Change-Id: Ie426ae7b16385a166147bcb149694df7c93c2e14
Reviewed-on: https://chromium-review.googlesource.com/978648
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548918}
[modify] https://crrev.com/40a8d84a54ac47d3299194c5377aebd71714f1f0/build/config/compiler/BUILD.gn
[modify] https://crrev.com/40a8d84a54ac47d3299194c5377aebd71714f1f0/build/toolchain/gcc_ar_wrapper.py
[modify] https://crrev.com/40a8d84a54ac47d3299194c5377aebd71714f1f0/build/toolchain/gcc_toolchain.gni

Status: Fixed (was: Started)
Project Member

Comment 17 by bugdroid1@chromium.org, Apr 13 2018

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

commit c38bba2dcbdf40ac601dcedf6ead8163ee2e784c
Author: Nico Weber <thakis@chromium.org>
Date: Fri Apr 13 02:06:22 2018

mac, ios: Unconfuse lldb after thin_archive change.

We accidentally passed -T to the static archive tool on mac and ios.
The static archive tool on mac and ios is not ar but a mac-specific
tool called libtool, and its -T flag means something completely different.

Bug:  801925 
Change-Id: I44dacbb8696cceb26ac7a5de86a2e8a0de638c00
Reviewed-on: https://chromium-review.googlesource.com/1010513
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550479}
[modify] https://crrev.com/c38bba2dcbdf40ac601dcedf6ead8163ee2e784c/build/config/compiler/BUILD.gn

Project Member

Comment 18 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c38bba2dcbdf40ac601dcedf6ead8163ee2e784c

commit c38bba2dcbdf40ac601dcedf6ead8163ee2e784c
Author: Nico Weber <thakis@chromium.org>
Date: Fri Apr 13 02:06:22 2018

mac, ios: Unconfuse lldb after thin_archive change.

We accidentally passed -T to the static archive tool on mac and ios.
The static archive tool on mac and ios is not ar but a mac-specific
tool called libtool, and its -T flag means something completely different.

Bug:  801925 
Change-Id: I44dacbb8696cceb26ac7a5de86a2e8a0de638c00
Reviewed-on: https://chromium-review.googlesource.com/1010513
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550479}
[modify] https://crrev.com/c38bba2dcbdf40ac601dcedf6ead8163ee2e784c/build/config/compiler/BUILD.gn

Project Member

Comment 19 by bugdroid1@chromium.org, May 9 2018

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

commit 1ed851337f4781d2504bdcc03f11d1ffb5526a3a
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Wed May 09 22:01:13 2018

Enable thin archives for ChromeOS builds

LLVM has been updated in the ChromeOS sysroot, so it should be safe to enable
thin archives now.

BUG= 801925 

Change-Id: I7dc2492937a26adc4f9af61abccbdb086e3a38cb
Reviewed-on: https://chromium-review.googlesource.com/1050980
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557336}
[modify] https://crrev.com/1ed851337f4781d2504bdcc03f11d1ffb5526a3a/build/config/compiler/BUILD.gn
[modify] https://crrev.com/1ed851337f4781d2504bdcc03f11d1ffb5526a3a/build/toolchain/gcc_toolchain.gni

Sign in to add a comment