Linux: Enable multi-threading with the gold linker.

Linux: Enable multi-threading with the gold linker.

BUG=161942
Committed: https://crrev.com/226488abee2adbdb6e54e5bfca1c3b2f52f2da2d
Cr-Commit-Position: refs/heads/master@{#314991}

[Lei Zhang, 2015-02-05 05:05:23]

thestig@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2015-02-05 05:28:21]

lgtm (bystanders: see bug for why this is landing again)

Do you have numbers on how much this helps?

[Lei Zhang, 2015-02-05 06:11:20]

On 2015/02/05 05:28:21, Nico wrote:
> lgtm (bystanders: see bug for why this is landing again)
> 
> Do you have numbers on how much this helps?

I don't have recent numbers. I can do some benchmarking tomorrow.

[tzik, 2015-02-05 07:15:22]

tzik@chromium.org changed reviewers:
+ tzik@chromium.org

[tzik, 2015-02-05 07:15:23]

https://codereview.chromium.org/894463003/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/894463003/diff/1/build/common.gypi#newcode4403
build/common.gypi:4403: '-Wl,--thread-count=4',
Could you update corresponding part in build/config/compiler/BUILD.gn too?

[Lei Zhang, 2015-02-05 23:22:08]

On 2015/02/05 06:11:20, Lei Zhang wrote:
> On 2015/02/05 05:28:21, Nico wrote:
> > lgtm (bystanders: see bug for why this is landing again)
> > 
> > Do you have numbers on how much this helps?
> 
> I don't have recent numbers. I can do some benchmarking tomorrow.

Given:
- shared_library / debug build
- mechanical hard drive
- delete out/Debug/chrome and sync before each run
- time is for running the link command directly, not running ninja

With a cold cache: (drop the cache between syncing / linking)
- no threading: 116 s
- 4 threads: 118 s

With a hot cache:
- no threading: 11.7 s
- 4 threads: 10.4 s

Evan's measurements from before showed a 25 s vs 19 s.

[Lei Zhang, 2015-02-05 23:24:46]

New patchsets have been uploaded after l-g-t-m from thakis@chromium.org

[Lei Zhang, 2015-02-05 23:27:31]

And of course the target audience will likely have a warm cache when building.
I'm guessing the multi-threaded case causes more random hard drive seeking,
which is why it's slower in the cold cache case.

https://codereview.chromium.org/894463003/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/894463003/diff/1/build/common.gypi#newcode4403
build/common.gypi:4403: '-Wl,--thread-count=4',
On 2015/02/05 07:15:22, tzik wrote:
> Could you update corresponding part in build/config/compiler/BUILD.gn too?

Done.

[tzik, 2015-02-06 02:24:06]

lgtm

[Lei Zhang, 2015-02-06 06:24:19]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2015-02-06 06:25:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/894463003/20001

[commit-bot: I haz the power, 2015-02-06 07:15:02]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-06 07:15:52]

Patchset 2 (id:??) landed as
https://crrev.com/226488abee2adbdb6e54e5bfca1c3b2f52f2da2d
Cr-Commit-Position: refs/heads/master@{#314991}

[Lei Zhang, 2015-02-06 21:44:21]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/909583002/ by thestig@chromium.org.

The reason for reverting is: Several CrOS folks are reporting linker errors
locally, even though the CrOS buildbots are green..

[tfarina, 2015-02-16 16:03:46]

tfarina@chromium.org changed reviewers:
+ tfarina@chromium.org

[tfarina, 2015-02-16 16:03:47]

Lei, could this be the cause of my errors in
https://codereview.chromium.org/920443003/?

I think this kind of change deserves a PSA on chromium-dev anyway. Sorry if it
was sent and missed it though.

[Lei Zhang, 2015-02-17 20:16:21]

On 2015/02/16 16:03:47, tfarina wrote:
> Lei, could this be the cause of my errors in
> https://codereview.chromium.org/920443003/

Yes, it's because you forced the gold flags on. Someone else had the same
problem, so we are asking the Android NDK folks to enable threading for their
toolchain.

> I think this kind of change deserves a PSA on chromium-dev anyway. Sorry if it
> was sent and missed it though.

There wasn't a PSA because:
a) The CL got reverted.
b) If you don't force GYP flags on, you wouldn't have hit that problem.
c) A change like this should "just work" behind the scenes and require no user
action.