1011229 - enable discarding unused functions and data at link time on Linux platforms

Status: RESOLVED FIXED

(NSS :: Build, defect, P2)

Description

[Nathan Froyd [:froydnj]]

Attachment: add -ffunction-sections and -Wl,--gc-sections

Bug 611781 comment 4 suggests that there is a significant win from simply enabling unused functions and data to be thrown away at link time.

The attached patch implements this idea.  Sizes before:

[froydnj@cerebro nss.hg]$ find . -name libnss\*.so |xargs size
   text	   data	    bss	    dec	    hex	filename
1263022	  24560	   6024	1293606	 13bd26	./lib/nss/Linux3.13_x86_64_glibc_PTH_64_OPT.OBJ/libnss3.so
 165319	   2640	    168	 168127	  290bf	./lib/softoken/legacydb/Linux3.13_x86_64_glibc_PTH_64_OPT.OBJ/libnssdbm3.so
 149216	  23752	   1408	 174376	  2a928	./lib/util/Linux3.13_x86_64_glibc_PTH_64_OPT.OBJ/libnssutil3.so
   5410	    832	      8	   6250	   186a	./lib/sysinit/Linux3.13_x86_64_glibc_PTH_64_OPT.OBJ/libnsssysinit.so
 516509	 128440	     32	 644981	  9d775	./lib/ckfw/builtins/Linux3.13_x86_64_glibc_PTH_64_OPT.OBJ/libnssckbi.so

Sizes after:

[froydnj@cerebro nss.hg]$ find . -name libnss\*.so |xargs size
   text	   data	    bss	    dec	    hex	filename
1132109	  23724	   5960	1161793	 11ba41	./lib/nss/Linux3.13_x86_64_glibc_PTH_64_OPT.OBJ/libnss3.so
 127639	   2480	    160	 130279	  1fce7	./lib/softoken/legacydb/Linux3.13_x86_64_glibc_PTH_64_OPT.OBJ/libnssdbm3.so
 148472	  23720	   1408	 173600	  2a620	./lib/util/Linux3.13_x86_64_glibc_PTH_64_OPT.OBJ/libnssutil3.so
   5414	    832	      8	   6254	   186e	./lib/sysinit/Linux3.13_x86_64_glibc_PTH_64_OPT.OBJ/libnsssysinit.so
 510404	 128424	     32	 638860	  9bf8c	./lib/ckfw/builtins/Linux3.13_x86_64_glibc_PTH_64_OPT.OBJ/libnssckbi.so

The wins are significant for libnss3.so (~10%) and libnssdbm3.so (~25%).

There are no test regressions on x86-64 Linux.

Comment 1

[Mike Hommey [:glandium]]

Please do that in security/build instead, and do it only if it's enabled for gecko.

Comment 2

[Wan-Teh Chang]

Comment on attachment 8423408 [details] [diff] [review]
add -ffunction-sections and -Wl,--gc-sections

I found -ffunction-sections -fdata-sections documented in
the GCC manual as early as version 2.95.3:
https://gcc.gnu.org/onlinedocs/gcc-2.95.3/gcc_2.html#SEC10

So it should be fine to always use these flags.

DSO_CFLAGS may not be the best makefile variable for these
flags because -ffunction-sections -fdata-sections don't seem
to be related to dynamic shared objects. I think we should
add them to OS_CFLAGS or to OPTIMIZER.

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Mike Hommey [:glandium] from comment #1)
> Please do that in security/build instead, and do it only if it's enabled for
> gecko.

Why not improve the upstream situation for everybody, not just gecko?

Comment 4

[Ryan Sleevi]

Even better would be to list the symbols its dropping, and see if we can't remove them from source as well :)

Comment 5

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

I agree with Nathan that it is best to do it upstream so everybody benefits. and I agree with Ryan that it would be great to have a list of things that are getting dropped so we can delete that source code.

Also, I think we should do the same for Windows (where the vast majority of users are) at the same time.. AFAICT, that just means building with the LTO options for compiling and linking NSS.

Comment 6

[Nathan Froyd [:froydnj]]

(In reply to Ryan Sleevi from comment #4)
> Even better would be to list the symbols its dropping, and see if we can't
> remove them from source as well :)

You can see what got deleted from earlier experiments with Gecko's copy of NSS in bug 611781 comment 11.  I went with the --gc-sections approach for now, because it doesn't require as much testing, and because it's effective with future instances of dead code cropping up.

I am willing to go through and delete stuff, but I would prefer to defer that to a followup bug.

(In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO? for response) from comment #5)
> Also, I think we should do the same for Windows (where the vast majority of
> users are) at the same time.. AFAICT, that just means building with the LTO
> options for compiling and linking NSS.

Fixing this for Windows is next on my list.  Does the Windows linker not have a similar --gc-sections style flag?  You really have to turn on LTCG to get unreferenced function deletion?

(In reply to Wan-Teh Chang from comment #2)
> DSO_CFLAGS may not be the best makefile variable for these
> flags because -ffunction-sections -fdata-sections don't seem
> to be related to dynamic shared objects. I think we should
> add them to OS_CFLAGS or to OPTIMIZER.

I guess if you were compiling NSS statically and linking it statically, there would be benefit to having things split out into separate sections.  I will investigate moving them.

Comment 7

[Nathan Froyd [:froydnj]]

Attachment: nss-gc-sections.patch

Here's a patch which adds -ffunction-sections -fdata-sections to OS_CFLAGS instead.  I'm not sure it really makes much difference compared to the previous patch, though, since OS_CFLAGS includes DSO_CFLAGS.

Wan-Teh, which would you like to see landed?  The patch adding the flags to DSO_CFLAGS or the patch adding the flags to OS_CFLAGS?

Comment 8

[Wan-Teh Chang]

Comment on attachment 8423899 [details] [diff] [review]
nss-gc-sections.patch

r=wtc. I will check in this patch for you.

I know it doesn't matter whether -ffunction-sections -fdata-sections are
added to OS_CFLAGS directly or via DSO_CFLAGS. My suggestion was about
logical correctness DSO_CFLAGS is supposed to be the flags needed when
we're compiling NSS for shared libraries.

Comment 9

[Wan-Teh Chang]

(In reply to Nathan Froyd (:froydnj) from comment #6)
> 
> Fixing this for Windows is next on my list.  Does the Windows linker not
> have a similar --gc-sections style flag?

I believe the equivalent Visual C++ compiler flag is /Gy:
http://msdn.microsoft.com/en-us/library/xsa71f43.aspx

NSPR is using /Gy but NSS isn't. This would need to be added to
nss/coreconf/WIN32.mk.

The equivalent Windows linker flag seems to be /OPT:REF:
http://msdn.microsoft.com/en-us/library/bxwfs976.aspx

I see -OPT:REF in nss/coreconf/WIN32.mk but it's inside an ifdef.
Also, MSDN says /OPT:REF is the default in non-debug builds.

Comment 10

[Wan-Teh Chang]

Attachment: nss-gc-sections.patch by Nathan Froyd

I reordered the compiler flags in Nathan's patch and checked it in.

https://hg.mozilla.org/projects/nss/rev/9b43cad852e1

Comment 11

[Wan-Teh Chang]

Comment on attachment 8425166 [details] [diff] [review]
nss-gc-sections.patch by Nathan Froyd

Patch pushed to mozilla-central as part of NSS_3_16_2_BETA2:
https://hg.mozilla.org/mozilla-central/rev/980e7d78b358