Closed Bug 407794 Opened 12 years ago Closed 12 years ago

Nightlies should not override the module-specific build settings.

Categories

(Firefox Build System :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: sayrer, Assigned: sayrer)

References

Details

(Keywords: perf)

Attachments

(2 files, 1 obsolete file)

This is bad. At least, it's bad that we do this in nightlies and release builds. It's unlikely that a global setting is good. For example, sqlite should be built with very high optimization, not "-O2".

Nightlies should not override the module-specific build settings.
Flags: blocking1.9?
OS: Mac OS X → All
Hardware: PC → All
sqlite would like to be built with:

-O3 on gcc
-xO5 on sun
-Ox on msvc

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/db/sqlite3/src/Makefile.in&rev=1.30#59
Summary: --enable-optimize overrides module-specific optimization settings → Nightlies should not override the module-specific build settings.
I suggested in another bug that we switch to using
--enable-optimize
--enable-debugger-info-modules

This won't quite work on Linux, afaik, since we need to explicitly set -gstabs+, but we might be able to work around that in other ways.
I believe the correct solution is to add

export CFLAGS=-gstabs+
export CXXFLAGS=-gstabs+

to the mozconfig, and remove the --enable-optimize flag(s)

Neither --enable-debugger-info-modules nor MOZ_DEBUG_SYMBOLS affects NSPR on non-windows correctly
Keywords: perf
Attachment #292478 - Flags: review?(ted.mielczarek)
Comment on attachment 292478 [details] [diff] [review]
use the right settings for each tier1 by default, so modules can override

Could you add comments to the mozconfigs so that nobody undoes this later?  Just something like
# Don't add explicit optimize flags here, set them in configure.in, see bug 407794.  Also a comment above the CFLAGS/CXXFLAGS explaining that this is for Breakpad would be helpful.
Attachment #292478 - Flags: review?(ted.mielczarek) → review+
Assignee: nobody → sayrer
Attachment #292478 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #292484 - Flags: approval1.9?
Comment on attachment 292484 [details] [diff] [review]
address ted's comments

go go sayre
Attachment #292484 - Flags: approval1.9? → approval1.9+
This had to be backed out due to a test failure on Linux.

It seemed to hurt mac performance on some tests and improve it on others. Makes sense: shifting to -Os in a few too many places.
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
Depends on: 408258
Attachment #293018 - Flags: review?(ted.mielczarek)
Comment on attachment 293018 [details] [diff] [review]
patch with wallpaper for various issues

This contains a workaround for bug 408258, since we should stop shipping builds with that problem immediately. It reverts the OS X default optimization to O2, since -Os that regressed Tp and Tdhtml slightly. And since -Os is no longer the default for spidermonkey on mac, this incorporates the already-reviewed patch from bug 407600.
Attachment #293018 - Flags: review?(ted.mielczarek) → review+
Attachment #293018 - Flags: approval1.9?
Comment on attachment 293018 [details] [diff] [review]
patch with wallpaper for various issues

it blocks, no need to approve
Attachment #293018 - Flags: approval1.9?
Depends on: 408418, 408422
This works. Filed bugs on issues uncovered by this change:

bug 408258 
bug 408418
bug 408422
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Blocks: 410414
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.