977951 - Linking all compiled tests in "./mach build binaries" slows down the build significantly

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

When I run ./mach build binaries it used to take less than 6 seconds to complete (after I'd only changed a single C++ file in content/, which is AWESOME by the way). But recently we started to relink all compiled test executables, which greatly slows down the build, down to about 60 seconds. This is only going to get worse and worse as we add more compiled unit tests (which we *should* do).

Why can't we either:
1. Link the compiled test binaries right before we run them, so we don't pay the price of linking them until we actually need them, or
2. have a "build binaries" target that doesn't relink the compiled tests. I get that we may not want this to be the default target, as it could confuse people.

Comment 1

[Gregory Szorc [:gps]]

I had a bug somewhere where I suggested we not compile C++ tests unless the user actually runs them or they are needed (for packaging, etc)...

Comment 2

[Mike Hommey [:glandium]]

(In reply to Chris Pearce (:cpearce) from comment #0)
> But recently we started to relink all compiled test
> executables, which greatly slows down the build, down to about 60 seconds.

It has been this way for a very long time. Are you sure what has changed is not that linking those takes longer now for some reason?

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Mike Hommey [:glandium] from comment #2)
> (In reply to Chris Pearce (:cpearce) from comment #0)
> > But recently we started to relink all compiled test
> > executables, which greatly slows down the build, down to about 60 seconds.
> 
> It has been this way for a very long time. Are you sure what has changed is
> not that linking those takes longer now for some reason?

Really? On Windows? No, I'm not sure that something else slowed down linking of the tests. But I do recall ./mach build binaries running in < 6 seconds up until recently.

Comment 4

[Gregory Szorc [:gps]]

Please submit the output of |mach build binaries|, timestamps and all.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: output of `./mach build binaries`

Ur, so I guess from the timestamps we can actually see that most time is spent linking libxul... Sorry, my mistake. I guess incremental linking stopped helping so much over the last few weeks. :(

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I don't think we're doing incremental linking anymore.

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8383451 [details]
output of `./mach build binaries`

There's something weird, though:
0:07.21 xul.lib.desc

we shouldn't be building a fake lib for xul.

Comment 8

[Mike Hommey [:glandium]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #6)
> I don't think we're doing incremental linking anymore.

incremental linking was reenabled a few months ago.

Comment 9

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #7)
> Comment on attachment 8383451 [details]
> output of `./mach build binaries`
> 
> There's something weird, though:
> 0:07.21 xul.lib.desc
> 
> we shouldn't be building a fake lib for xul.

Ah, side effect of bug 974216. Just in case, can you check if backing that out changes something?

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

Wow. Backing out bug 974216 dropped my build time back down to < 8s:

cpearce@MIDAS ~/src/mozilla/purple
$ touch content/media/MediaDecoderStateMachine.cpp

cpearce@MIDAS ~/src/mozilla/purple
$ ./mach build binaries
 0:00.18 C:/mozilla-build/msys/bin/sh.exe -c c:/mozilla-build/mozmake/mozmake.EXE -C c:/Users/cpearce/src/mozilla/purple/objdir -j8 -s backend.RecursiveMakeBackend
 0:00.37 C:/mozilla-build/msys/bin/sh.exe -c c:/mozilla-build/mozmake/mozmake.EXE -j8 -s binaries
 0:00.77 From dist/public: Kept 0 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:00.78 From dist/sdk: Kept 0 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:00.79 From dist/private: Kept 0 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:00.89 From dist/idl: Kept 1148 existing; Added/updated 2; Removed 0 files and 0 directories.
 0:00.92 From dist/bin: Kept 49 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:01.10 From dist/include: Kept 4001 existing; Added/updated 1; Removed 0 files and 0 directories.
 0:02.94 From _tests: Kept 15881 existing; Added/updated 13; Removed 0 files and 0 directories.
 0:03.52 Unified_cpp_content_media1.obj
 0:03.65 libGLESv2.dll
 0:03.91 libEGL.dll
 0:06.70 Unified_cpp_content_media1.cpp
 0:06.71 content_media.lib.desc
 0:07.01 gklayout.lib.desc
 0:07.20 xul.dll
Your build was successful!

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

Sorry, more like 10s:

$ touch content/media/MediaDecoderStateMachine.cpp && time ./mach build binaries
 0:00.18 C:/mozilla-build/msys/bin/sh.exe -c c:/mozilla-build/mozmake/mozmake.EXE -C c:/Users/cpearce/src/mozilla/purple/objdir -j8 -s backend.RecursiveMakeBackend
 0:00.37 C:/mozilla-build/msys/bin/sh.exe -c c:/mozilla-build/mozmake/mozmake.EXE -j8 -s binaries
 0:00.77 From dist/public: Kept 0 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:00.78 From dist/sdk: Kept 0 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:00.79 From dist/private: Kept 0 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:00.89 From dist/idl: Kept 1148 existing; Added/updated 2; Removed 0 files and 0 directories.
 0:00.92 From dist/bin: Kept 49 existing; Added/updated 0; Removed 0 files and 0 directories.
 0:01.11 From dist/include: Kept 4001 existing; Added/updated 1; Removed 0 files and 0 directories.
 0:02.96 From _tests: Kept 15881 existing; Added/updated 13; Removed 0 files and 0 directories.
 0:03.53 Unified_cpp_content_media1.obj
 0:03.67 libGLESv2.dll
 0:03.91 libEGL.dll
 0:06.76 Unified_cpp_content_media1.cpp
 0:06.77 content_media.lib.desc
 0:07.03 gklayout.lib.desc
 0:07.22 xul.dll
Your build was successful!

real    0m10.960s
user    0m0.000s
sys     0m0.015s

Comment 12

[Mike Hommey [:glandium]]

Attachment: Don't force build a static libxul

Comment 13

[Mike Hommey [:glandium]]

Comment on attachment 8383487 [details] [diff] [review]
Don't force build a static libxul

Can you try this patch instead of the backout?

Comment 14

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8383487 [details] [diff] [review]
Don't force build a static libxul

Yup, that maintains a nice ~10s ./mach build binaries for me. Awesome! :)

Comment 15

[Chris Pearce [:cpearce (Not reading bugmail)]]

Can this land? :)

Comment 16

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/765770122c29

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/765770122c29