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

Issue 673025 link

Starred by 8 users

Issue metadata

Status: WontFix
Owner:
Closed: Feb 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocked on:
issue 589665
issue 680795


Participants' hotlists:
PGO-bugs


Sign in to add a comment

The is_official_build=true + is_chrome_branded=true build config doesn't match what we ship on Windows

Project Member Reported by h...@chromium.org, Dec 9 2016

Issue description

In other words, the "official" build config isn't really the official build config.

This means, for example, that the chromium.perf Windows bots are measuring performance on the wrong kind of builds, which seems bad.

It regressed when we started shipping PGO+WPO builds on Windows without making that the official build mode.

(It's also higly confusing for developers who want to evaluate the performance of some change.)
 

Comment 1 by h...@chromium.org, Dec 9 2016

(One way to look at it is that it seems we did A/B testing with regular official builds (A) vs WPO+PGO builds (B), and afterwards instead of enabling WPO+PGO for the A builds, we just kept shipping the B variant.)
I agree that we should make this change. Maybe full_wpo_on_official should default to on for official builds, or else a new flag could be created with the opposite sense. Or maybe doing partial LTCG is just not a useful scenario anymore - I'm not sure what it really represents, so I'm fine with removing the partial LTCG option.

This would mean that local official builds would be full-LTCG instead of partial. It would not mean that local official builds were PGO - I think that might be pushing things a bit too far.

Labels: -Pri-0 Pri-1
I agree too, but I don't think this is P0.
Cc: benhenry@chromium.org dtu@chromium.org
I'm not convinced that the 'is_official_build' flag should enable PGO:
  - Presently a PGO build of Chrome takes ~6h, I don't expect developers to do this kind of build locally.
  - To me the 'is_official_build + is_chrome_branded' build is used by some developers to test the integration of a Chrome specific feature (crash upload, DRM stuffs that I know nothing about etc).
  - Not so many people care about measuring performance locally, they rely on the perf (try)bots to do this, so the solution might be to only update these bots.
  - full_wpo_on_official is really expensive too, it has a huge impact on build time and this will affect a lot the unittests... 

One potential approach might be to update the current partial WPO config with the data from a PGO build (i.e. check which files are optimized for speed/size in a PGO build and do the same thing in the partial WPO build).

So there's no ideal solution here... We could maybe add a new flag ('is_shipping_build' or something similar), but I'm not expecting a lot of people to use this flag (it requires to do 2 builds and to run the benchmarking script between these) so I don't know if it's worth it...

On at least two occasions I have tried to reproduce a bug that "only occurs on official builds" and found that the bug actually requires "full_wpo_on_official". So, while full_wpo_on_official increases build times over just is_official_build, the fact that full_wpo_on_official is off by default on official builds has actually *cost* me time because I have to endure a full official build and then redo the build with full_wpo_on_official, usually after wasting some time figuring out the difference.

So, I think that is_official_build should imply full_wpo_on_official. I don't think that it should imply PGO because that is impractical.

Perhaps we should have an option to have an is_official_build without full_wpo_on_official, but I'm really not sure that that is a useful or meaningful configuration anymore. It is an arbitrary middle point between a release build and an official build that is of historical significance but I don't believe that it's arbitrary middle position has any value anymore.

So, having thought about this I am proposing having "full_wpo_on_official = is_official_build", and calling that close-enough-but-no-closer.

Comment 7 by thakis@chromium.org, Dec 12 2016

The perf waterfall must test what we ship. The perf waterfall should just do a regular official build, else things will be so confusing that nobody understands what's going on. Hence, we need to turn on PGO in is_official_build builds.

Maybe profile generation and profile use could be done independently, and official builds would download preexisting profiles from somewhere. Then it wouldn't take 6h probably?

But this is a serious bug, and needs attention.

Comment 8 by h...@chromium.org, Dec 12 2016

The most immediate concern is that the perf bots are not measuring peformance on kind of builds we ship.



But the broader issue seems important too.

> - Presently a PGO build of Chrome takes ~6h, I don't expect developers to do this kind of build locally.

I think already most developers don't do official builds. But I don't think we should work around the build time by making the is_official build config not actually do official builds.

> - To me the 'is_official_build + is_chrome_branded' build is used by some developers to test the integration of a Chrome specific feature (crash upload, DRM stuffs that I know nothing about etc).

I thought is_chrome_branded should be enough to test integration of Chrome-specific features (I see from the .gn files that this doesn't seem to be entirely true..)

> - Not so many people care about measuring performance locally, they rely on the perf (try)bots to do this, so the solution might be to only update these bots.

Right. The perf bots are the most important. Developers don't usually do official builds.

> So there's no ideal solution here... We could maybe add a new flag ('is_shipping_build' or something similar), but I'm not expecting a lot of people to use this flag (it requires to do 2 builds and to run the benchmarking script between these) so I don't know if it's worth it...

is_shipping_build sounds like another name for what is_official used to mean :-)



Maybe what we want to make is_official + is_chrome_branded match the configuration we ship again, and then expose options to turn off PGO, WPO, etc. for developers who want to debug things and don't want to wait for the full official build.
Blockedon: 589665
Yes, this is a serious issue and I agree that ideally is_official_build should imply that you're building the build that's being shipped to the users. There's a lot of things that has to be done in order to get there, I was in MTV to discuss this last week and I'm presently opening all the corresponding crbugs. Here's the current list of blockers:

- For 'full_wpo_on_official = is_official_build', we tried this before and it had a HUGE impact on build time, and at the time /LTCG:Incremental didn't worked. I've added a GN flag to retry this, if it now works then I'll get rid of the partial WPO config (I agree that this config doesn't make sense, it's just convenient to keep the build time reasonable). Tracking bug is  crbug.com/589665 . If it doesn't work then we'll add more bots.

- For the idea of generating the profile database on a separate builder: I've proposed this before and we just need to build the infrastructure for this and confirm that there's no performance impact. I'll open the crbug later today.

- We want to switch the Perf Win bots to PGO soon-ish, the build time is the main issue but we'll just add more bots for now, the tracking bug is crbug.com/673369.

- The tracking bug for the PGO build time issue is crbug.com/650432 (there's some blocking bug associated to it and I'm adding more).

The main constraint right now is that I'm the only one maintaining this configuration, so I'm trying to address these issues in priority order but the priority often depends on who you're talking to.
Cc: hendr...@chromium.org jbau...@chromium.org vmi...@chromium.org mar...@chromium.org
 Issue 548454  has been merged into this issue.
Cc: -vmi...@chromium.org -hendr...@chromium.org -mar...@chromium.org -jbau...@chromium.org
I'll assign this issue to me, but feel free to raise your hand if you think that you can help.
Owner: sebmarchand@chromium.org
Cc: primiano@chromium.org
> The perf waterfall must test what we ship. The perf waterfall should just do a regular official build, else things will be so confusing that nobody understands what's going on. Hence, we need to turn on PGO in is_official_build builds.

I couldn't agree more.

I don't think we should play the game where every new optimization that is too long becomes yet_another_gn_configuration_that_makes_the_previous_one_less_official = true.
We already have the notion of debug vs release vs "performance release" (i.e. is_official_build).
IMHO either we make the decision that PGO is so beneficial that is worth making many is_official_build developers unhappy (so we keep it tied to is_official_build) OR we should not use it until it becomes acceptably fast (in terms of build-time).
Blockedon: 680795
Labels: Pri-3
Lowering the priority of the PGO bugs as we've switched the official build to Clang. I'll close these bugs once we decide that we won't support the VS build anymore.
Status: WontFix (was: Available)
WontFix'ing this issue as the switch to Clang make it obsolete.

Sign in to add a comment