1321065 - Make --enable-profiling the default

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

Bug 1320312 caused a multi-day local build failure because the default/local build configuration differed from CI.

This could have been mitigated had local builds and automation builds used a similar configuration.

Per ted in bug 1321041 comment 1:

> I think we could make --enable-profiling the default, and have the beta and
> release mozconfigs --disable-profiling, instead of what we currently do
> (--enable-profiling for nightly+aurora). Historically --enable-profiling
> just made sure we built with a framepointer enabled, but I guess it has
> grown in scope over the years.

I agree with ted: let's make the no-mozconfig/default configuration match automation on the Nightly channel as much as possible by making --enable-profiling the default.

Comment 1

[(Away)]

> > (--enable-profiling for nightly+aurora).
I just double-checked with ted on irc: the aurora channel doesn't enable profiling.

Btw, the flip side of this change is that bustage in non-profiling configs will appear only on aurora merge day, with no advance warning available to an unsuspecting developer.

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1321065 - Default to --enable-profiling for nightly milestones.

Review commit: https://reviewboard.mozilla.org/r/96354/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96354/

Comment 3

[Gregory Szorc [:gps]]

(In reply to David Major [:dmajor] from comment #1)
> Btw, the flip side of this change is that bustage in non-profiling configs
> will appear only on aurora merge day, with no advance warning available to
> an unsuspecting developer.

Yup :/

We have a number of other things in this bucket (packaging, l10n repacks, etc), so we'll just add this one to the list of potential things that can fail on uplift day. The recourse is to perform aurora, beta, release, etc build configurations as part of regular CI on all channels, without actually shipping those bits to users. TaskCluster makes this *much* easier.

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8815459 [details]
Bug 1321065 - Default to --enable-profiling for nightly milestones.

https://reviewboard.mozilla.org/r/96354/#review96580

TIL --enable-profiling is js-specific. I figured that would be a global flag.

I was initially expecting a number of changes to in-tree mozconfigs to remove --enable-profiling usage. But since we're keying this off the milestone version and we may wish to perform different build configurations on different channels, it makes sense to be explicit in the in-tree mozconfigs.

Comment 5

[Mike Hommey [:glandium]]

(In reply to Gregory Szorc [:gps] from comment #4)
> Comment on attachment 8815459 [details]
> Bug 1321065 - Default to --enable-profiling for nightly milestones.
> 
> https://reviewboard.mozilla.org/r/96354/#review96580
> 
> TIL --enable-profiling is js-specific. I figured that would be a global flag.

It's not. It's in there because it's a s> https://reviewboard.mozilla.org/r/96354/#review96580hared option between js standalone builds and gecko builds.

> I was initially expecting a number of changes to in-tree mozconfigs to
> remove --enable-profiling usage. But since we're keying this off the
> milestone version and we may wish to perform different build configurations
> on different channels, it makes sense to be explicit in the in-tree
> mozconfigs.

Actually, over time, I want to remove the redundancy in the mozconfigs.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ad3151a0a79
Default to --enable-profiling for nightly milestones. r=gps

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for Android bustage:

https://hg.mozilla.org/integration/autoland/rev/8f217ad4bcd380c91e2c580fe56bba1e7dff6c60

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=7ad3151a0a797b2230a45483c1adabf4d896405b
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=7441120&repo=autoland

TEST-UNEXPECTED-FAIL | /home/worker/workspace/build/src/python/mozbuild/mozbuild/test/configure/lint.py | Lint.test_b2g, line 26: Missing @depends for `result`: '--help'

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

s/Android//

Comment 9

[Mike Hommey [:glandium]]

I forgot I'm only half-way through being done with --help dependencies. Also, the change in default for --enable-profiling breaks the validations in test_moz_configure.py.

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8815459 [details]
Bug 1321065 - Default to --enable-profiling for nightly milestones.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96354/diff/1-2/

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8815459 [details]
Bug 1321065 - Default to --enable-profiling for nightly milestones.

https://reviewboard.mozilla.org/r/96354/#review96602

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/10bb1ad646cd
Default to --enable-profiling for nightly milestones. r=gps

Comment 13

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/autoland/rev/698893ea26f8 - was ASan by any chance not --enable-profiling before this patch? https://treeherder.mozilla.org/#/jobs?repo=autoland&bugfiler&fromchange=739ac5ad4db5a20d7fdff847cbd675e4c7608a38&group_state=expanded&filter-searchStr=7c16b0d3d1ebda0cb66743a1f310210b92662b73&tochange=0fe1c7f17e6816412cd3c97cb3f055ed3a13af42 says you added a pretty good chance of blowing up to ASan gtest.

Comment 14

[Mike Hommey [:glandium]]

So, went through all the logs and the builds without --enable-profiling, currently are the ones using those mozconfigs:
  browser/config/mozconfigs/linux32/debug
  browser/config/mozconfigs/linux32/debug
  browser/config/mozconfigs/linux64/artifact
  browser/config/mozconfigs/linux64/debug
  browser/config/mozconfigs/linux64/debug-asan
  browser/config/mozconfigs/linux64/debug-static-analysis-clang
  browser/config/mozconfigs/linux64/nightly-asan
  browser/config/mozconfigs/linux64/opt-static-analysis-clang
  browser/config/mozconfigs/linux64/valgrind
  browser/config/mozconfigs/macosx64/debug
  browser/config/mozconfigs/macosx64/debug-static-analysis
  browser/config/mozconfigs/macosx64/nightly
  browser/config/mozconfigs/macosx64/opt-static-analysis
  mobile/android/config/mozconfigs/android-api-15/debug

From all those, it looks like really only asan would benefit from an exception for --enable-profiling being the default.

Funny thing to note in there, browser/config/mozconfigs/macosx64/nightly is not browser/config/mozconfigs/macosx-universal/nightly and has differences. So turning off universal builds might pick unwanted changes. I'll comment in the relevant bug about this so that someone looks more thoroughly.

Comment 15

[Mike Hommey [:glandium]]

Comment on attachment 8815459 [details]
Bug 1321065 - Default to --enable-profiling for nightly milestones.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96354/diff/2-3/

Comment 16

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f6cc35920bf2
Backed out changeset 10bb1ad646cd for ASan gtest failures

Comment 17

[Gregory Szorc [:gps]]

Comment on attachment 8815459 [details]
Bug 1321065 - Default to --enable-profiling for nightly milestones.

https://reviewboard.mozilla.org/r/96354/#review96894

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/800a887c705e
Default to --enable-profiling for nightly milestones. r=gps

Comment 19

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/800a887c705e