Make --enable-profiling the default

RESOLVED FIXED in Firefox 53

Status

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: gps, Assigned: glandium)

Tracking

unspecified
mozilla53
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(1 attachment)

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.
> > (--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.
Assignee: nobody → mh+mozilla
(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 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.
Attachment #8815459 - Flags: review?(gps) → review+
(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.
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/7ad3151a0a79
Default to --enable-profiling for nightly milestones. r=gps
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'
Flags: needinfo?(mh+mozilla)
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.
Flags: needinfo?(mh+mozilla)
Comment on attachment 8815459 [details]
Bug 1321065 - Default to --enable-profiling for nightly milestones.

https://reviewboard.mozilla.org/r/96354/#review96602
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/10bb1ad646cd
Default to --enable-profiling for nightly milestones. r=gps
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.
Backout by philringnalda@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f6cc35920bf2
Backed out changeset 10bb1ad646cd for ASan gtest failures
Comment on attachment 8815459 [details]
Bug 1321065 - Default to --enable-profiling for nightly milestones.

https://reviewboard.mozilla.org/r/96354/#review96894
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/800a887c705e
Default to --enable-profiling for nightly milestones. r=gps
https://hg.mozilla.org/mozilla-central/rev/800a887c705e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Duplicate of this bug: 1340725
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.