I am fine to change that, if there's a consensus about that. We will also need to change it in tools/clang/scripts/update.py:
https://cs.chromium.org/chromium/src/tools/clang/scripts/update.py?q=clang/scripts/updat&sq=package:chromium&l=402

  need_gold_plugin = 'LLVM_DOWNLOAD_GOLD_PLUGIN' in os.environ or (
      sys.platform.startswith('linux') and
      'buildtype=Official' in os.environ.get('GYP_DEFINES', '') and
      'branding=Chrome' in os.environ.get('GYP_DEFINES', ''))

Peter, Nico, Hans -- any objections in dropping branding=Chrome check?

SGTM

Sounds fine.

Since we're now using LTO for release, I think it's also fine to bundle the plugin by default tbh. (...but if we're moving off it and to lld soon, then might as well wait for that)

sgtm

> From a practical viewpoint, the current code, as is, means that at least four different configurations (debug, release, release+official, release+official+branding) have four different performance characteristics, which makes debugging and perf work a big headache.

If you like headaches, check out  http://crbug.com/673025  ;-)

Okay, I will make the change soon. That is currently blocked on 'CFI Linux ToT' bot being red in a sense that I want to fix it before doing anything else.

The fix is under review: https://codereview.chromium.org/2621193003/

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

commit eed4ea8fb01d92a623f3d912d3ea904e7740e13f
Author: krasin <krasin@chromium.org>
Date: Wed Jan 11 18:52:59 2017

Don't condition on the branding when choosing LTO and CFI.

BUG= 678915 

Review-Url: https://codereview.chromium.org/2621193003
Cr-Commit-Position: refs/heads/master@{#442969}

[modify] https://crrev.com/eed4ea8fb01d92a623f3d912d3ea904e7740e13f/build/config/sanitizers/sanitizers.gni
[modify] https://crrev.com/eed4ea8fb01d92a623f3d912d3ea904e7740e13f/build/toolchain/toolchain.gni
[modify] https://crrev.com/eed4ea8fb01d92a623f3d912d3ea904e7740e13f/tools/clang/scripts/update.py

thanks a lot!

Wow I did not know branding introduced LTO. Is that why my release+official builds are now extremely slow?

I'm not very familiar with LTO, and I have been doing benchmarks / profiling without it turned on. Does it change the performance characteristics of Chrome significantly? If not I would love to have a flag that turns this off, as linking is taking many orders of magnitude more time for me now, with tcmalloc complaining about huge allocations (in the GBs).

The reasoning in  issue 673025  makes sense to me (is_official_build should include every optimization). Still, it seems like there should be a place for a dev-friendly build that is relatively optimized, to enable things like "smart-ish" local profiling for finding hotspots.

Perf bots just don't cut it for *discovering* hot spots.

Yes, there's a flag that turns this off. Setting allow_posix_link_time_opt=false in GN flags will disable it.

Enabled LTO (happened back in May 2016, but conditioned on the branding as well) was motivated by two things:

1. Whole program devirtualization. By our measures, tens of thousand virtual calls are devirtualized in that mode bringing measurable improvements in some cases. See some stats here: https://bugs.chromium.org/p/chromium/issues/detail?id=634139&desc=2#c22

2. Enabling Control Flow Integrity that makes it harder to conduct some particular attacks. See https://www.chromium.org/developers/testing/control-flow-integrity for more details.

We are also actively working on ThinLTO and bringing it to Chrome that should significantly speed up the link time. See  https://crbug.com/660216  (but most of the work happens on the LLVM side at the moment)

> Still, it seems like there should be a place for a dev-friendly build that is relatively optimized, to enable things like "smart-ish" local profiling for finding hotspots.

Isn't that release builds (is_debug=false)? Or you feel there is something missing there that doesn't help you finding hot spots?

> Perf bots just don't cut it for *discovering* hot spots.
agree, perf bots are for discovering regressions not hot spots.

> Isn't that release builds (is_debug=false)? Or you feel there is something missing there that doesn't help you finding hot spots?

I don't think I know enough of the underlying optimizations to be sure if is_debug=false is good enough. I mostly just trusted some comment I probably read in chromium-dev that said release+is_official_build should be used for perf profiling :)

My goal would be to have the most optimized build that builds in a reasonable time for fast iterative development. If is_official_build minus LTO has different perf characteristics from is_debug=false I would prefer that because it still builds reasonably quickly.

> I don't think I know enough of the underlying optimizations to be sure if is_debug=false is good enough. I mostly just trusted some comment I probably read in chromium-dev that said release+is_official_build should be used for perf profiling :)

It is true. Now that krasin fixed this bug, release+official, to the best of my knowledge, is perf-representative (not yet on Windows due to  Issue 673025 ). So if you need to do accurate perf profiling use that and, yes, it will take some time to build what we ship. But if you do profiling very likely you'll build just once. Actually why not pulling just binary and symbols from GCS?

If you want to tradeoff build+link time for accuracy just use release without official. That won't have LTO and will save you link time. I searched a bit around and I couldn't spot any other major perf tweaks in release-but-not-official. 

Having said that, for profiling I'd just go for release+official or GCS prebuilts+symbols

Thank you primiano for the help, I'll follow your advice.

+1, release (is_debug=false) is "builds fast, still reasonably optimized". is_official_build is "full optimizations, possibly unreasonable build time".

(there's work underway to make is_official_build not take forever, but for now it is what it is)