Last Comment Bug 407794 - Nightlies should not override the module-specific build settings.
: Nightlies should not override the module-specific build settings.
Status: RESOLVED FIXED
: perf
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: P2 normal (vote)
: ---
Assigned To: Robert Sayre
:
Mentors:
Depends on: 408258 408418 408422
Blocks: 410414 420759
  Show dependency treegraph
 
Reported: 2007-12-10 13:03 PST by Robert Sayre
Modified: 2008-03-03 13:02 PST (History)
17 users (show)
mtschrep: blocking1.9+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
use the right settings for each tier1 by default, so modules can override (4.11 KB, patch)
2007-12-10 14:15 PST, Robert Sayre
ted: review+
Details | Diff | Review
address ted's comments (5.18 KB, patch)
2007-12-10 14:51 PST, Robert Sayre
mtschrep: approval1.9+
Details | Diff | Review
patch with wallpaper for various issues (5.95 KB, patch)
2007-12-13 15:40 PST, Robert Sayre
ted: review+
Details | Diff | Review

Description Robert Sayre 2007-12-10 13:03:37 PST
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.
Comment 1 Robert Sayre 2007-12-10 13:08:02 PST
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
Comment 2 Ted Mielczarek [:ted.mielczarek] 2007-12-10 13:18:45 PST
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.
Comment 4 Ted Mielczarek [:ted.mielczarek] 2007-12-10 13:25:36 PST
Linux is actually goofier, fwiw:
http://lxr.mozilla.org/mozilla/source/tools/tinderbox-configs/firefox/linux/mozconfig#14
Only Win32 doesn't screw with this:
http://lxr.mozilla.org/mozilla/source/tools/tinderbox-configs/firefox/win32/mozconfig
Comment 5 Benjamin Smedberg [:bsmedberg] 2007-12-10 13:51:25 PST
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
Comment 6 Robert Sayre 2007-12-10 14:15:55 PST
Created attachment 292478 [details] [diff] [review]
use the right settings for each tier1 by default, so modules can override
Comment 7 Ted Mielczarek [:ted.mielczarek] 2007-12-10 14:24:32 PST
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.
Comment 8 Robert Sayre 2007-12-10 14:51:15 PST
Created attachment 292484 [details] [diff] [review]
address ted's comments
Comment 9 Mike Schroepfer 2007-12-10 14:59:33 PST
Comment on attachment 292484 [details] [diff] [review]
address ted's comments

go go sayre
Comment 10 Robert Sayre 2007-12-11 01:03:50 PST
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.
Comment 11 Robert Sayre 2007-12-13 15:40:02 PST
Created attachment 293018 [details] [diff] [review]
patch with wallpaper for various issues
Comment 12 Robert Sayre 2007-12-13 15:43:38 PST
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.
Comment 13 Robert Sayre 2007-12-13 19:34:20 PST
Comment on attachment 293018 [details] [diff] [review]
patch with wallpaper for various issues

it blocks, no need to approve
Comment 14 Robert Sayre 2007-12-14 14:29:26 PST
This works. Filed bugs on issues uncovered by this change:

bug 408258 
bug 408418
bug 408422

Note You need to log in before you can comment on or make changes to this bug.