Closed Bug 977951 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox Build System :: General, defect)

29 Branch
x86_64
Windows 8.1
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla30

People

(Reporter: cpearce, Assigned: glandium)

References

Details

Attachments

(2 files)

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.
Summary: Should have a "./mach build binaries" target that doesn't relink all test executables → Linking all compiled tests in "./mach build binaries" slows down the build significantly
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)...
Component: mach → Build Config
(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?
(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.
Please submit the output of |mach build binaries|, timestamps and all.
Flags: needinfo?(cpearce)
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. :(
Flags: needinfo?(cpearce)
I don't think we're doing incremental linking anymore.
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.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #6)
> I don't think we're doing incremental linking anymore.

incremental linking was reenabled a few months ago.
(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?
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!
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
Attachment #8383487 - Flags: review?(gps)
Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
Blocks: 974216
Comment on attachment 8383487 [details] [diff] [review]
Don't force build a static libxul

Can you try this patch instead of the backout?
Attachment #8383487 - Flags: feedback?(cpearce)
Attachment #8383487 - Flags: review?(gps) → review+
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! :)
Attachment #8383487 - Flags: feedback?(cpearce) → feedback+
Can this land? :)
https://hg.mozilla.org/mozilla-central/rev/765770122c29
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.