389781 - NSS should be built size-optimized in browser builds on Linux, Windows, and Mac

Status: RESOLVED FIXED

(NSS :: Build, defect, P2)

Description

[Julien Pierre]

The Mozilla foundation is concerned about our code size increase in NSS 3.12 due mainly to new features. We can offset a very large part of that increase by using compiler flags accordingly on the specific platforms they are concerned about. The size optimization should be the default except for mozilla/security/nss/lib/freebl, which should keep building the way it is.

For Solaris/JES we should continue to build all directories as we do today. We can use one of our Sun ifdef's like BUILD_SUN_PKGS to make sure the size optimization doesn't happen on the platforms that overlap (Windows, Linux) with Mozilla's list.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Size optimized for Win95 builds, perhaps, but not for WinNT builds, IMO.

Comment 2

[Julien Pierre]

Good point, Nelson, that's another way to look at it. But maybe someday JES will switch to the WIN95 flavor. So it's probably better to test BUILD_SUN_PKG still.

Comment 3

[Robert Relyea]

I would be inclined to size optimize certain directories in NSS by default on all platforms (libpkix, certdb, cms, pkcs12, pkcs7, pki) while keeping certain 'core' directories optimized for speed (ssl, freebl, softoken, pk11wrap), assuming that such a split would preserve our existing SSL server performance.

bob

Comment 4

[Julien Pierre]

Bob,

We aren't currently measuring performance, so we prefer not to change the compiler flags for our Sun builds for that reason. I suggested that we make the change only in Mozilla client builds of NSS. For the client, the performance impact should be much more limited than for server. I think optimizing freebl for speed alone would be sufficient. Does Mozilla have performance regression tests that involve NSS / SSL pages so we can make sure they are not impacted ?

Comment 5

[Christophe Ravel]

List of compiler flags to use for code size optimization:

Linux - gcc (from man gcc):
   Current optimization: -O2
   Suggested optimize for size: -Os

       -Os Optimize for size.  -Os enables all -O2 optimizations that do not
           typically increase code size.  It also performs further optimiza-
           tions designed to reduce code size.

MacOSX - gcc (from man cc):
    Current optimization: -O2
    Suggested optimize for size: -Oz

       -Os Optimize for size, but not at the expense of speed.  -Os enables
           all -O2 optimizations that do not typically increase code size.
           However, instructions are chosen for best performance, regardless
           of size.  To optimize solely for size on Darwin, use -Oz (APPLE
           ONLY).

           The following options are set for -O2, but are disabled under -Os:
           -falign-functions  -falign-jumps  -falign-loops -falign-labels
           -freorder-blocks  -freorder-blocks-and-partition
           -fprefetch-loop-arrays

           When optimizing with -Os or -Oz (APPLE ONLY) on Darwin, any func-
           tion up to 30 ``estimated insns'' in size will be considered for
           inlining.  When compiling C and Objective-C sourcefiles with -Os or
           -Oz on Darwin, functions explictly marked with the "inline" keyword
           up to 450 ``estimated insns'' in size will be considered for inlin-
           ing.  When compiling for Apple POWERPC targets, -Os and -Oz (APPLE
           ONLY) disable use of the string instructions even though they would
           usually be smaller, because the kernel can't emulate them correctly
           in some rare cases.  This behavior is not portable to any other gcc
           environment, and will not affect most programs at all.  If you
           really want the string instructions, use -mstring.

       -Oz (APPLE ONLY) Optimize for size, regardless of performance.  -Oz
           enables the same optimization flags that -Os uses, but -Oz also
           enables other optimizations intended solely to reduce code size.
           In particular, instructions that encode into fewer bytes are pre-
           ferred over longer instructions that execute in fewer cycles.  -Oz
           on Darwin is very similar to -Os in FSF distributions of GCC.  -Oz
           employs the same inlining limits and avoids string instructions
           just like -Os.

Windows - cl.exe (from MSDN at http://msdn2.microsoft.com/en-us/library/fwkeyyhe(VS.71).aspx)
    Current optimization: -O2
    Suggested optimize for size: -O1

        /O1 (Minimize Size)
          Equivalent to: /Og /Os /Oy /Ob2 /Gs /GF /Gy
          Creates the smallest code in the majority of cases.

        /O2 (Maximize Speed)
 	  Equivalent to: /Og /Oi /Ot /Oy /Ob2 /Gs /GF /Gy
          Creates the fastest code in the majority of cases. (Default setting for release builds)

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Christophe, 
I gather that there's a implicit question in your previous comment.
The answer is that, for gcc, I believe we want -Os, but not -Oz.
Also, as you know. we want this change only for SOME directories, 
not for all.  Let's start with just nss/lib/libpkix.  Julien may have 
other ideas for this.

Comment 7

[Christophe Ravel]

Thanks for your input.

Here is more about the on-going plan:

- There will be a global variable (ALLOW_OPT_CODE_SIZE) that can turn on and off optimization for code size in NSS. By default, it is turned on, except when BUILD_SUN_PKG=1. This default setting can be overridden at the make command line level.

- In each directory, a variable (OPT_CODE_SIZE) can be set (in manifest.mn) to enable code size optimization in this directory and its sub-directories. By default, there is no code size optimization.

So, for a given directory, the code is going to be optimized for size if both the global variable (ALLOW_OPT_CODE_SIZE) and the local variable (OPT_CODE_SIZE) are set to 1.

The definition of the optimization flag is defined in coreconf/$(OS_TARGET).mk using the OPTIMIZER variable.
- for gcc (Windows, Linux, MacOSX): -Os
- for cl.exe (Windows): -O1
I'll keep the current definition of OPTIMIZER when code size optimization is not requested.

I am currently working on setting code size optimization for the following directories:
- libpkix
- certdb
- cms (smime)
- pkcs12
- pkcs7
- pki
as suggested by Bob.
This can easily be changed.

I'll provide numbers (code size) for different options (libpkix only, 6 directories above) so we can make a choice.

Comment 8

[Christophe Ravel]

Some results on Linux 32 bit:

Without optimization:
    * libnss3.so: 1,328,467
    * libsmime3.so: 190,262
TOTAL code size:  3,237,431

Code size optimization (-Os) for pkix:
    * libnss3.so: 1,172,850 (Gain: 155,617 = 11.71%)
    * other libs: unchanged.
TOTAL code size: 3,081,814 (Gain: 155,617 = 4.81%)

Code size optimization (-Os) for libpkix, certdb, cms, pkcs12, pkcs7, pki:
    * libnss3.so: 1,148,274 (Gain: 180,193 = 13.56%)
    * libsmime3.so: 179,152 (Gain: 11,110 = 5.84%)
    * other libs: unchanged.
TOTAL code size:  3,046,128 (Gain: 191,303 = 5.91%)

Notes:
- Total code size = all NSS libraries (.so)
- sizes are in bytes.

Comment 9

[Julien Pierre]

Christophe, thanks for those measurements.

I assume you were comparing "optimized for speed" against "optimized for size", not "without optimization" (ie, debug) against "code size optimization".
Can you confirm that ?

Also, when you said you built -Os for pkix, did you mean lib/pkix or lib/libpkix ?
Either way, we don't want to do just that. We want to build every directory of every library optimized for size, except for lib/freebl .

One more question, did you compare file sizes or code size as reported by the "size" command ? It's the later we are concerned with .

Comment 10

[Christophe Ravel]

(In reply to comment #9)
> Christophe, thanks for those measurements.
> 
> I assume you were comparing "optimized for speed" against "optimized for size",
> not "without optimization" (ie, debug) against "code size optimization".
> Can you confirm that ?

Yes, that's correct. My previous report was ambiguous on this.
Instead of "without optimization" one should read "without code size optimization".

> 
> Also, when you said you built -Os for pkix, did you mean lib/pkix or
> lib/libpkix ?

I really meant libpkix (lib/libpkix).

> Either way, we don't want to do just that. We want to build every directory of
> every library optimized for size, except for lib/freebl .
Well, we don't know yet.

Bob says: libpkix, certdb, cms, pkcs12, pkcs7, pki
Nelson says: libpkix
Julien says: everything but freebl

I could measure this 3rd option and come back with the results so we can make an informed decision.

> 
> One more question, did you compare file sizes or code size as reported by the
> "size" command ? It's the later we are concerned with .

It is the file size (output of ls -l). I thought that Mozilla.org was concerned by the download size (the file size).
I can also measure the code size as you suggest.
 

Comment 11

[Julien Pierre]

Christophe,

It's true there isn't agreement yet on which directories.
I think only freebl is really performance sensitive as far as the client is concerned, if anything. But without performance tests on their part, we can only guess.

I don't think doing just lib/libpkix makes sense. It will not achieve the size reductions we need to meet mozilla's size goals.

Mozilla cares about the code size (output of size). They strip the binaries on their own after the NSS build before packaging the client products. The stripped file sizes will closely match the code size. And their concern isn't just download size but RAM usage as well.

Comment 12

[Wan-Teh Chang]

Using -Os for everything but lib/freebl is a good idea.
lib/freebl already uses hand-picked optimization flags
on Solaris, and we only do that in that directory.

Comment 13

[Nelson Bolyard (seldom reads bugmail)]

For clients, optimizing for size instead of speed makes sense.
For servers, it often does not. If server performance is important, 
then optimizing for size will not be in their best interest.

The worrisome case (IMO) is on platforms where the same set 
of NSS shared libs are used for clients and servers.  
Call this the "system NSS" case.  I think we may be seeing 
that on Solaris or Linux soon (if not already).

Comment 14

[Christophe Ravel]

Additional results - still on Linux 32 bit:

Without any code size optimization:
TOTAL file size:  3,237,431
TOTAL code size:  2,629,596

Code size optimization (-Os) for all sub-directories under nss/lib, except nss/lib/freebl:
Total file size: 2812219 (Gain: 425,212 = 13.13%)
TOTAL code size:  2202713 (Gain: 1,034,718 = 31.96%)

Notes:
- file size is the output of "ls -l"
- code size is the output of "size", first column (text section)

Comment 15

[Nelson Bolyard (seldom reads bugmail)]

Christophe:  Thanks for the work you're doing on this bug.

A few requests:
1) when using the output of "size", please report the sum of the first
two columns, rather than only the first column.  We care about the data
segment too.
2) Since we're looking at size reductions and not increases, please 
report them as "reduction", not as "gain" (which implies increase).

Comment 16

[Christophe Ravel]

(In reply to comment #13)
    > The worrisome case (IMO) is on platforms where the same set 
    > of NSS shared libs are used for clients and servers.  
    > Call this the "system NSS" case.  I think we may be seeing 
    > that on Solaris or Linux soon (if not already).

    So far, we have no plan to optimize for code size on Solaris (see bug title).

    By default, the code would be optimized for size:
    - in the directories where OPT_CODE_SIZE=1
    - for the platforms that use this feature (defined in coreconf/$(OS_TARGET).mk)
    Except:
    - when BUILD_SUN_PKG=1
    - when ALLOW_OPT_CODE_SIZE=0 is set on the make command line
    In the 2 exceptions above, no code size optimization is performed. The code is
    optimized for performance (the current behavior).

    Redhat (or other Linux distributions) would have to choose if they want the
    code size optimized NSS or the performance optimized NSS for their system
    version of NSS.

    Alternative solution:
    We could reverse the default and have code size optimization enabled only when
    ALLOW_OPT_CODE_SIZE=1 is specified on the make command line. The Firefox build
    (and other client application builds) would have to explicitly use this flag.

    Comments ?

Comment 17

[Robert Relyea]

I would vote to make CODE_SIZE optimization an option (off by default).

There is a define called MOZILLA_BUILD which we can use to turn CODE_SIZE on by default.

bob

Comment 18

[Nelson Bolyard (seldom reads bugmail)]

Yes, I agree with Bob.  I'd rather not add yet another make/environment 
variable to change the build.  Since the space optimized builds will be
for mozilla, let's use one of the existing variables that signify builds
for mozilla.  I think MOZILLA_BUILD is one, and there are likely others.

Comment 19

[Julien Pierre]

Using one of the mozilla build macros is fine.
As far as platforms which ship with NSS, the platform vendor has to make the call whether raw performance or file/memory size matters more. I think for Sun it is clearly the former.

Comment 20

[Christophe Ravel]

New numbers including Nelson's comment about code size (text + data instead of text), still on Linux 32 bits:

Without any code size optimization:
TOTAL file size:  3,237,431
TOTAL code size:  2,734,864

Code size optimization (-Os) for all sub-directories under nss/lib, except
nss/lib/freebl:
TOTAL file size: 2,812,219 (Reduction: 425,212 = 13.13%)
TOTAL code size: 2,307,953 (Reduction: 426,911 = 15.61%)

I'll change the default behavior with the following:
- Code size optimization if off, except if:
    * MOZILLA_SECURITY_BUILD is defined
  or
    * ALLOW_OPT_CODE_SIZE=1 is passed to the make command line


What stays the same:
- Code size optimization actually happens:
    * code size optimization is on (see above)
  and
    * code is in a directory or sub-directory where OPT_CODE_SIZE=1
  and
    * platform uses this feature (defined in coreconf/$(OS_TARGET).mk). Currently Windows, Linux and MacOSX

Comment 21

[Christophe Ravel]

Results for MacOSX x86 (10.4.10):

Without any code size optimization:
TOTAL file size: 3,187,168
TOTAL code size: 2,605,056

Code size optimization (-Os) for all sub-directories under nss/lib, except
nss/lib/freebl:
TOTAL file size: 3,139,364 (Reduction: 47,804 = 1.50%)
TOTAL code size: 2,564,096 (Reduction: 40,960 = 1.57%)

Some files have actually increased their size:
libsqlite3.dylib: Increase from 366,736 to 377,424 (+2.91%)
libssl3.dylib: Increase from 174,356 to 194,304 (+11.44%)

Code size optimization (-Oz) for all sub-directories under nss/lib, except
nss/lib/freebl:
TOTAL file size: 2,889,384 (Reduction: 297,784 = 9.34%)
TOTAL code size: 2,310,144 (Reduction: 294,912 = 11.32%)

1 file has actually increased its size:
libssl3.dylib: Increase from 174,356 to 186,740 (+7.10%)

libsqlite3.dylib has its size reduced to 294,912 (-8.86%) with this -Oz flag.

Comments:
- The man page for gcc on MacOSX says:
    "-Oz on Darwin is very similar to -Os in FSF distributions of GCC."
So I think we should use -Oz on MacOSX instead of -Os as suggested by Nelson.
- libssl3 doesn't get optimized correctly for code size on MacOSX. I don't know the reason why. But in the light of the numbers, we should not try to optimize it for code size on this platform.

Comment 22

[Christophe Ravel]

Results for Windows 32 bit (2003):

Without any code size optimization:
TOTAL file size: 2,011,136

With code size optimization (-O1) for all sub-directories under ns/lib, except ns/lib/freebl:
TOTAL file size: 1,769,472 (Reduction: 241,664 = 12.02%)

Note: according to Julien, the file size is the code size on Windows optimized build because the libraries are automatically stripped.

Comment 23

[Julien Pierre]

Thanks for making these measurements, Christophe !

The Mac result is a little disturbing. Was this an x86 Mac or PPC Mac ? It is odd that libssl3 would increase in size with -Oz rather than decreased. That is probably a bug worth reporting to Apple.
I agree that we should use -Oz on Mac rather than -Os. I think the 12 KB increase for libssl3 is small enough that it's not worth making a special case for it - the bug should just be reported to Apple and hopefully they will take care of it in the next compiler update.

The Windows result is better than I expected. I wonder how Microsoft gets such small code sizes compared to gcc.

Comment 24

[Christophe Ravel]

I ran all.sh with the code size optimized bits.

- all is green on Linux and Windows
- I have issues with MacOSX: there are a lot of SSL test error (with the regular code and with the code optimized for size). I guess there is a trick I am missing for this platform. Does anyone have a tip to run all.sh correctly on MacOSX ?

Comment 25

[Christophe Ravel]

Output of test on MacOSX: 1 PASSED 2 FAILED in the extract below

Server Args: -r_-r
tstclnt -p 8443 -h Christophe-iMac.local -f -d ../client \
        -T -n TestUser50 -w nss  < /Users/chravel/Documents/Sun/ws/mozilla/security/nss/tests/ssl/sslreq.dat
subject DN: CN=Christophe-iMac.local,E=[e-mail],O=BOGUS NSS,L=Mountain View,ST=California,C=US
issuer  DN: CN=NSS Test CA,O=BOGUS NSS,L=Mountain View,ST=California,C=US0 cache hits; 1 cache misses, 0 cache not reusableHTTP/1.0 200 OK^MServer: Generic Web Server^MDate: Tue, 26 Aug 1997 22:10:05 GMT^M
Content-type: text/plain^M
^M
GET / HTTP/1.0^M
^M
EOF^M^M
^M
kill -0 1001 >/dev/null 2>/dev/nullselfserv with PID 1001 found at Fri Aug 10 12:02:23 PDT 2007
ssl.sh: SSL3 Require client auth (client auth)(cert TestUser50 - not revoked) produced a returncode of 0, expected is 0 PASSEDServer Args: -r_-r
tstclnt -p 8443 -h Christophe-iMac.local -f -d ../client \
        -T -n TestUser51 -w nss  < /Users/chravel/Documents/Sun/ws/mozilla/security/nss/tests/ssl/sslreq.dat
tstclnt: unable to connect (poll): Connection refused by peer.
kill -0 1001 >/dev/null 2>/dev/nullselfserv with PID 1001 found at Fri Aug 10 12:02:24 PDT 2007
ssl.sh: SSL3 Require client auth (client auth)(cert TestUser51 - not revoked) produced a returncode of 1, expected is 0 FAILEDServer Args: -r_-r
tstclnt -p 8443 -h Christophe-iMac.local -f -d ../client \
        -T -n TestUser52 -w nss  < /Users/chravel/Documents/Sun/ws/mozilla/security/nss/tests/ssl/sslreq.dat
tstclnt: unable to connect (poll): Connection refused by peer.
kill -0 1001 >/dev/null 2>/dev/nullselfserv with PID 1001 found at Fri Aug 10 12:02:24 PDT 2007
ssl.sh: SSL3 Require client auth (client auth)(cert TestUser52 - not revoked) produced a returncode of 1, expected is 0 FAILED

Note: this is the code without code size optimization.

Comment 26

[Christophe Ravel]

There seems to be a problem with the CRL creation. I created a bug:
Bug 391721 – cert.sh does not report errors in Creating CA CRL

Comment 27

[Christophe Ravel]

Code size optimization results on MacOSX PPC 32 bit (10.4.10):

Without any code size optimization:
TOTAL file size: 3,291,592
TOTAL code size: 2,703,360

Code size optimization (-Oz) for all sub-directories under nss/lib, except
nss/lib/freebl:
TOTAL file size: 3,194,728 (Reduction: 96,864 = 2.94%)
TOTAL code size: 2,613,248 (Reduction: 90,112 = 3.33%)

1 file has actually increased its size:
libssl3.dylib: Increase from 195,140 to 215,096 (Gain: 20,480 = +13.16%)
 

Comment 28

[Christophe Ravel]

(In reply to comment #26)
> There seems to be a problem with the CRL creation. I created a bug:
> Bug 391721 – cert.sh does not report errors in Creating CA CRL
> 

It looks like we have this "bad database" on all platforms. So it doesn't explain the SSL errors on MacOSX.

Comment 29

[Julien Pierre]

Christophe, your issue with all.sh is separate from this one, so you should file another bug and cc someone else with Mac OS X access/expertise. I think Glen and maybe Bob or Kai.

Comment 30

[Christophe Ravel]

Attachment: Optimize code size for Windows, Linux and MacOSX

- Code size optimization if off, except if:
    * MOZILLA_SECURITY_BUILD is defined
  or
    * ALLOW_OPT_CODE_SIZE=1 is passed to the make command line

- Code size optimization actually happens:
    * code size optimization is on (see above)
  and
    * code is in a directory or sub-directory where OPT_CODE_SIZE=1 in manifest.mn
  and
    * platform uses this feature (defined in coreconf/$(OS_TARGET).mk).
Currently Windows, Linux and MacOSX

Code size optimization flag:
Linux / gcc: -Os
Windows / gcc: -Os
Windows / cl: -O1
MacOSX / gcc: -Oz

Optimize the code for size in the following directories (all nss/lib except nss/lib/freebl):
nss/lib/asn1
nss/lib/base
nss/lib/certdb
nss/lib/certhigh
nss/lib/ckfw
nss/lib/crmf
nss/lib/cryptohi
nss/lib/dev
nss/lib/jar
nss/lib/libpkix
nss/lib/nss
nss/lib/pk11wrap
nss/lib/pkcs12
nss/lib/pkcs7
nss/lib/pki
nss/lib/pki1
nss/lib/smime
nss/lib/softoken
nss/lib/sqlite
nss/lib/ssl
nss/lib/util

Comment 31

[Christophe Ravel]

I was able to run all.sh successfully on MacOSX x86 and PPC. Thanks to Wan-Teh and Glen for their help.

On MacOSX:
  HOST=localhost
  DOMSUF=localdomain
in /etc/hosts:
  127.0.0.1  localhost localhost.localdomain

Cisco VPN must not be running when the tests are running.

Comment 32

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 276544 [details] [diff] [review]
Optimize code size for Windows, Linux and MacOSX

Please reverse the names ALLOW_OPT_CODE_SIZE and OPT_CODE_SIZE
so that ALLOW_OPT_CODE_SIZE is what gets defined in makefiles,
and OPT_CODE_SIZE is what may be defined on the command line.
The makefiles will decide whether or not code size optimization
is allowed for their respective directories.  The command line
option will instruct gmake to do it, (or not).
This change will affect all files that you patched. 

>+# Optimization of code for size
>+# ALLOW_OPT_CODE_SIZE
>+# =1: The code can be optimized for size.
>+#     The code is actually optimized for size only if OPT_CODE_SIZE=1
>+#     in a given source code directory (in manifest.mn)
>+# =0: Never optimize the code for size.
>+#
>+# Default value = 0 unless MOZILLA_SECURITY_BUILD is defined.
>+# Can be overwritten from the make command line.

The default value is undefined, not zero.  

>+# This part of the code, including all sub-dirs, can be optimized for size
>+export OPT_CODE_SIZE = 1

Remove the word "export" from this declaration in all the makefiles 
where you added it.  This is a make command that assigns a value to a make 
variable, not a shell command.

Comment 33

[Christophe Ravel]

export is a make command. Here is the documentation for this command:

5.7.2 Communicating Variables to a Sub-make

Variable values of the top-level make can be passed to the sub-make through the environment by explicit request. These variables are defined in the sub-make as defaults, but do not override what is specified in the makefile used by the sub-make makefile unless you use the `-e' switch (see Summary of Options).

To pass down, or export, a variable, make adds the variable and its value to the environment for running each command. The sub-make, in turn, uses the environment to initialize its table of variable values. See Variables from the Environment.

Except by explicit request, make exports a variable only if it is either defined in the environment initially or set on the command line, and if its name consists only of letters, numbers, and underscores. Some shells cannot cope with environment variable names consisting of characters other than letters, numbers, and underscores.

The value of the make variable SHELL is not exported. Instead, the value of the SHELL variable from the invoking environment is passed to the sub-make. You can force make to export its value for SHELL by using the export directive, described below. See Choosing the Shell.

The special variable MAKEFLAGS is always exported (unless you unexport it). MAKEFILES is exported if you set it to anything.

make automatically passes down variable values that were defined on the command line, by putting them in the MAKEFLAGS variable. See Options/Recursion.

Variables are not normally passed down if they were created by default by make (see Variables Used by Implicit Rules). The sub-make will define these for itself.

If you want to export specific variables to a sub-make, use the export directive, like this:

     export variable ...

If you want to prevent a variable from being exported, use the unexport directive, like this:

     unexport variable ...

In both of these forms, the arguments to export and unexport are expanded, and so could be variables or functions which expand to a (list of) variable names to be (un)exported.

As a convenience, you can define a variable and export it at the same time by doing:

     export variable = value

has the same result as:

     variable = value
     export variable

and

     export variable := value

has the same result as:

     variable := value
     export variable

Likewise,

     export variable += value

is just like:

     variable += value
     export variable

Comment 34

[Nelson Bolyard (seldom reads bugmail)]

OK, I stand corrected.  Leave the "export" words in there.  
But the rest of the comments still stand.

Comment 35

[Christophe Ravel]

Attachment: This patch takes Nelson's comments into account.

Comment 36

[Julien Pierre]

Comment on attachment 276694 [details] [diff] [review]
This patch takes Nelson's comments into account.

I haven't tested this, but it looks OK. I wonder if we can just set ALLOW_OPT_CODE_SIZE=1 in mozilla/security/nss/lib/manifest.mn and unset it in mozilla/security/nss/lib/freebl/manifest.mn . That would change fewer files. But it should be equivalent.

Comment 37

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 276694 [details] [diff] [review]
This patch takes Nelson's comments into account.

>+# Optimization of code for size
>+# OPT_CODE_SIZE
>+# =1: The code can be optimized for size.
>+#     The code is actually optimized for size only if ALLOW_OPT_CODE_SIZE=1
>+#     in a given source code directory (in manifest.mn)
>+# =0: Never optimize the code for size.
>+#
>+# Default value = 0 unless MOZILLA_SECURITY_BUILD is defined.

Default value is undefined.  (Is it not?)
In some arithmetic shell expressions, an undefined shell variable is 
evaluated as if it had the value zero.  But not all shell expressions
do that.

Comment 38

[Christophe Ravel]

No. Default value is set to 0 (if undefined):

+ifndef OPT_CODE_SIZE
+ifdef MOZILLA_SECURITY_BUILD
+OPT_CODE_SIZE = 1
+else
+OPT_CODE_SIZE = 0
+endif
+endif

Comment 39

[Christophe Ravel]

Committed on NSS trunk.

/cvsroot/mozilla/security/coreconf/Darwin.mk,v  <--  Darwin.mk
new revision: 1.19; previous revision: 1.18
done
Checking in coreconf/Linux.mk;
/cvsroot/mozilla/security/coreconf/Linux.mk,v  <--  Linux.mk
new revision: 1.30; previous revision: 1.29
done
Checking in coreconf/WIN32.mk;
/cvsroot/mozilla/security/coreconf/WIN32.mk,v  <--  WIN32.mk
new revision: 1.25; previous revision: 1.24
done
Checking in coreconf/command.mk;
/cvsroot/mozilla/security/coreconf/command.mk,v  <--  command.mk
new revision: 1.12; previous revision: 1.11
done
Checking in nss/lib/asn1/manifest.mn;
/cvsroot/mozilla/security/nss/lib/asn1/manifest.mn,v  <--  manifest.mn
new revision: 1.6; previous revision: 1.5
done
Checking in nss/lib/base/manifest.mn;
/cvsroot/mozilla/security/nss/lib/base/manifest.mn,v  <--  manifest.mn
new revision: 1.10; previous revision: 1.9
done
Checking in nss/lib/certdb/manifest.mn;
/cvsroot/mozilla/security/nss/lib/certdb/manifest.mn,v  <--  manifest.mn
new revision: 1.12; previous revision: 1.11
done
Checking in nss/lib/certhigh/manifest.mn;
/cvsroot/mozilla/security/nss/lib/certhigh/manifest.mn,v  <--  manifest.mn
new revision: 1.6; previous revision: 1.5
done
Checking in nss/lib/ckfw/manifest.mn;
/cvsroot/mozilla/security/nss/lib/ckfw/manifest.mn,v  <--  manifest.mn
new revision: 1.10; previous revision: 1.9
done
Checking in nss/lib/crmf/manifest.mn;
/cvsroot/mozilla/security/nss/lib/crmf/manifest.mn,v  <--  manifest.mn
new revision: 1.4; previous revision: 1.3
done
Checking in nss/lib/cryptohi/manifest.mn;
/cvsroot/mozilla/security/nss/lib/cryptohi/manifest.mn,v  <--  manifest.mn
new revision: 1.6; previous revision: 1.5
done
Checking in nss/lib/dev/manifest.mn;
/cvsroot/mozilla/security/nss/lib/dev/manifest.mn,v  <--  manifest.mn
new revision: 1.12; previous revision: 1.11
done
Checking in nss/lib/jar/manifest.mn;
/cvsroot/mozilla/security/nss/lib/jar/manifest.mn,v  <--  manifest.mn
new revision: 1.6; previous revision: 1.5
done
Checking in nss/lib/libpkix/manifest.mn;
/cvsroot/mozilla/security/nss/lib/libpkix/manifest.mn,v  <--  manifest.mn
new revision: 1.3; previous revision: 1.2
done
Checking in nss/lib/nss/manifest.mn;
/cvsroot/mozilla/security/nss/lib/nss/manifest.mn,v  <--  manifest.mn
new revision: 1.9; previous revision: 1.8
done
Checking in nss/lib/pk11wrap/manifest.mn;
/cvsroot/mozilla/security/nss/lib/pk11wrap/manifest.mn,v  <--  manifest.mn
new revision: 1.19; previous revision: 1.18
done
Checking in nss/lib/pkcs12/manifest.mn;
/cvsroot/mozilla/security/nss/lib/pkcs12/manifest.mn,v  <--  manifest.mn
new revision: 1.6; previous revision: 1.5
done
Checking in nss/lib/pkcs7/manifest.mn;
/cvsroot/mozilla/security/nss/lib/pkcs7/manifest.mn,v  <--  manifest.mn
new revision: 1.5; previous revision: 1.4
done
Checking in nss/lib/pki/manifest.mn;
/cvsroot/mozilla/security/nss/lib/pki/manifest.mn,v  <--  manifest.mn
new revision: 1.15; previous revision: 1.14
done
Checking in nss/lib/pki1/manifest.mn;
/cvsroot/mozilla/security/nss/lib/pki1/manifest.mn,v  <--  manifest.mn
new revision: 1.10; previous revision: 1.9
done
Checking in nss/lib/smime/manifest.mn;
/cvsroot/mozilla/security/nss/lib/smime/manifest.mn,v  <--  manifest.mn
new revision: 1.9; previous revision: 1.8
done
Checking in nss/lib/softoken/manifest.mn;
/cvsroot/mozilla/security/nss/lib/softoken/manifest.mn,v  <--  manifest.mn
new revision: 1.32; previous revision: 1.31
done
Checking in nss/lib/sqlite/manifest.mn;
/cvsroot/mozilla/security/nss/lib/sqlite/manifest.mn,v  <--  manifest.mn
new revision: 1.3; previous revision: 1.2
done
Checking in nss/lib/ssl/manifest.mn;
/cvsroot/mozilla/security/nss/lib/ssl/manifest.mn,v  <--  manifest.mn
new revision: 1.16; previous revision: 1.15
done
Checking in nss/lib/util/manifest.mn;
/cvsroot/mozilla/security/nss/lib/util/manifest.mn,v  <--  manifest.mn
new revision: 1.14; previous revision: 1.13
done

Comment 40

[Wan-Teh Chang]

Comment on attachment 276694 [details] [diff] [review]
This patch takes Nelson's comments into account.

If you change the sense of the ALLOW_OPT_CODE_SIZE variable to
DISALLOW_OPT_CODE_SIZE, then you only need to set/export
DISALLOW_OPT_CODE_SIZE to 1 in nss/lib/freebl/manifest.mn.
This also has the advantage that if we ever add a new
subdirectory under ns/lib, we don't need to set/export
any variable in its manifest.mn file to get the appropriate
default (allow optimizing for code size).  This also applies
to the manifest.mn files under nss/cmd, dbm, and jss that
this patch didn't modify.

Comment 41

[Robert Relyea]

The define 'MOZILLA_SECURITY_BUILD' does not mean build for a mozilla client.
The correct define is MOZILLA_CLIENT.

Because of this, mozilla clients are not getting this option turned on.

Comment 42

[Wan-Teh Chang]

It's easier to modify Mozilla's mozilla/security/manager/Makefile.in
and add this line:

DEFAULT_GMAKE_FLAGS += OPT_CODE_SIZE=1

This is also a better fix -- to replace product-specific flags with
feature flags.

Comment 43

[Nelson Bolyard (seldom reads bugmail)]

I agree with Wan-Teh's suggestion in comment 42.  
Should we open a separate bug for that, and mark this resolved/fixed again?

Comment 44

[Wan-Teh Chang]

We should also fix mozilla/security/coreconf/command.mk.

Either change "ifdef MOZILLA_SECURITY_BUILD" to
"ifdef MOZILLA_CLIENT", or change that block of code to

 68 ifndef OPT_CODE_SIZE
 72 OPT_CODE_SIZE = 0
 74 endif

I prefer the latter.

Comment 45

[Kai Engert [:KaiE:]]

(In reply to comment #43)
> I agree with Wan-Teh's suggestion in comment 42.  
> Should we open a separate bug for that, and mark this resolved/fixed again?

As this bug requests about building in browser, I guess it's ok to use this bug, even though it's a PSM patch, and your backend changes were in NSS, but I don't mind.

Comment 46

[Reed Loden [:reed]]

I'd be worried about who would triage this component for approval requests. You may do better with a bug in PSM that a driver is accountable for rather than NSS, which isn't listed on the drivers page.

Comment 47

[Kai Engert [:KaiE:]]

(In reply to comment #46)
> I'd be worried about who would triage this component for approval requests. You
> may do better with a bug in PSM that a driver is accountable for rather than
> NSS, which isn't listed on the drivers page.

Good point. Bug 400759 filed, marking dependent, resolving this one as fixed again.

Comment 48

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch proposed by Wan-Teh

The patch for PSM bug 400759 seems to obviate this change, 
but perhaps this is a good idea anyway.

Comment 49

[Wan-Teh Chang]

Comment on attachment 285905 [details] [diff] [review]
patch proposed by Wan-Teh

r=wtc.

Comment 50

[Nelson Bolyard (seldom reads bugmail)]

Checking in command.mk; new revision: 1.13; previous revision: 1.12

Comment 51

[Kai Engert [:KaiE:]]

If you would like to see the results from the Mozilla client builds, please cc yourself to bug 400759, where comments are being posted...