Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2190813002:
Add LC_ALL=C to gcc linker wrappers to speed up links (Closed)
DescriptionAdd LC_ALL=C to gcc linker wrappers to speed up links
This speeds up an Android component build of libchrome.cr.so from
35s -> 10s on my z620.
BUG=632230
Committed: https://crrev.com/52f2a85089d3e3a85cee112a6ee6c7423394644b
Cr-Commit-Position: refs/heads/master@{#408549}
Patch Set 1 #
Total comments: 2
Patch Set 2 : target linker step onnly #
Messages
Total messages: 29 (16 generated)
Description was changed from ========== Add LC_ALL=C to gcc linker wrappers to speed up links This speeds up an Android component build of libchrome.cr.so from 35s -> 10s on my z620. BUG=632230 ========== to ========== Add LC_ALL=C to gcc linker wrappers to speed up links This speeds up an Android component build of libchrome.cr.so from 35s -> 10s on my z620. BUG=632230 ==========
agrieve@chromium.org changed reviewers: + dpranke@chromium.org, pasko@chromium.org
The CQ bit was checked by agrieve@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/07/28 02:00:04, commit-bot: I haz the power wrote: > Dry run: CQ is trying da patch. Follow status at > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm. hopefully this is safe on mac as well ...
https://codereview.chromium.org/2190813002/diff/1/build/toolchain/gcc_solink_... File build/toolchain/gcc_solink_wrapper.py (right): https://codereview.chromium.org/2190813002/diff/1/build/toolchain/gcc_solink_... build/toolchain/gcc_solink_wrapper.py:104: os.environ['LC_ALL'] = 'C' bring it back to the original after linking? I am not as confident that: * CollectTOC/UpdateTOC/strip would behave correctly with modified locale * whatever someone else adds later in the wrapper won't do harm under this non-default environment just to be extra safe..
https://codereview.chromium.org/2190813002/diff/1/build/toolchain/gcc_solink_... File build/toolchain/gcc_solink_wrapper.py (right): https://codereview.chromium.org/2190813002/diff/1/build/toolchain/gcc_solink_... build/toolchain/gcc_solink_wrapper.py:104: os.environ['LC_ALL'] = 'C' On 2016/07/28 10:03:56, pasko wrote: > bring it back to the original after linking? > > I am not as confident that: > * CollectTOC/UpdateTOC/strip would behave correctly with modified locale > * whatever someone else adds later in the wrapper won't do harm under this > non-default environment > > just to be extra safe.. Done.
The CQ bit was checked by agrieve@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm, thank you
mcgrathr@chromium.org changed reviewers: + mcgrathr@chromium.org
LC_ALL=C should be safe and conceivably a speedup for everything. The only potential downside is that error messages from various things will be in English instead of the developer's chosen language.
On 2016/07/28 18:25:28, Roland McGrath wrote: > LC_ALL=C should be safe and conceivably a speedup for everything. The only > potential downside is that error messages from various things will be in English > instead of the developer's chosen language. Thanks for the sanity check Roland! Going to just check in as-is since there's not really much speed-up to be had other than in the link step anyways.
The CQ bit was checked by agrieve@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org Link to the patchset: https://codereview.chromium.org/2190813002/#ps20001 (title: "target linker step onnly")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add LC_ALL=C to gcc linker wrappers to speed up links This speeds up an Android component build of libchrome.cr.so from 35s -> 10s on my z620. BUG=632230 ========== to ========== Add LC_ALL=C to gcc linker wrappers to speed up links This speeds up an Android component build of libchrome.cr.so from 35s -> 10s on my z620. BUG=632230 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Add LC_ALL=C to gcc linker wrappers to speed up links This speeds up an Android component build of libchrome.cr.so from 35s -> 10s on my z620. BUG=632230 ========== to ========== Add LC_ALL=C to gcc linker wrappers to speed up links This speeds up an Android component build of libchrome.cr.so from 35s -> 10s on my z620. BUG=632230 Committed: https://crrev.com/52f2a85089d3e3a85cee112a6ee6c7423394644b Cr-Commit-Position: refs/heads/master@{#408549} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/52f2a85089d3e3a85cee112a6ee6c7423394644b Cr-Commit-Position: refs/heads/master@{#408549}
Message was sent while issue was closed.
thakis@chromium.org changed reviewers: + thakis@chromium.org
Message was sent while issue was closed.
That's really cool, thanks. Did we file a gold bug about this?
Message was sent while issue was closed.
On 2016/07/29 18:00:13, Nico wrote: > That's really cool, thanks. Did we file a gold bug about this? I didn't. Just re-asked on original mailing-list thread.
Message was sent while issue was closed.
On 2016/08/05 20:26:27, agrieve wrote: > On 2016/07/29 18:00:13, Nico wrote: > > That's really cool, thanks. Did we file a gold bug about this? > > I didn't. Just re-asked on original mailing-list thread. Filed a bug just now: https://sourceware.org/bugzilla/show_bug.cgi?id=20447 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
