Last Comment Bug 785118 - use PARALLEL_DIRS in netwerk/
: use PARALLEL_DIRS in netwerk/
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: Nathan Froyd [:froydnj]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-23 09:56 PDT by Nathan Froyd [:froydnj]
Modified: 2012-08-24 20:01 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (4.09 KB, patch)
2012-08-23 10:14 PDT, Nathan Froyd [:froydnj]
mh+mozilla: review+
Details | Diff | Splinter Review

Description Nathan Froyd [:froydnj] 2012-08-23 09:56:01 PDT
This doesn't look to be that difficult, and netwerk has quite a number of source files (200+ .cpp).
Comment 1 Nathan Froyd [:froydnj] 2012-08-23 10:14:23 PDT
Created attachment 654676 [details] [diff] [review]
patch

We can't parallelize everything; build/ has to be built after everything else because it relies on everything else.

Not sure if this needs a necko peer review or not, feel free to bounce it if so.
Comment 2 Nathan Froyd [:froydnj] 2012-08-23 10:15:50 PDT
Oh, and this patch wins ~3% of build time on an m-c build (8 core/16 threads Xeon with SSD).
Comment 3 Gregory Szorc [:gps] 2012-08-23 10:37:57 PDT
Watch out for etld_data.inc in netwerk/dns. I had to one-off that in build-splendid because of race conditions. I'm not sure if it is just a dependency inside netwerk or whether other parts of the tree don't like when it's missing. If it's other parts of the tree, then PARALLEL_DIRS in netwerk should be safe.
Comment 4 Nathan Froyd [:froydnj] 2012-08-23 10:50:06 PDT
(In reply to Gregory Szorc [:gps] from comment #3)
> Watch out for etld_data.inc in netwerk/dns. I had to one-off that in
> build-splendid because of race conditions. I'm not sure if it is just a
> dependency inside netwerk or whether other parts of the tree don't like when
> it's missing. If it's other parts of the tree, then PARALLEL_DIRS in netwerk
> should be safe.

Hm, that seems strange.  etld_data.inc is only ever built and touched from within netwerk/dns/.  I haven't seen problems with it in a handful of builds, but that's one of the fun things about parallel programming, isn't it?  Do you happen to recall what the problem was and what you wound up doing about it?
Comment 5 Ted Mielczarek [:ted.mielczarek] 2012-08-23 11:00:05 PDT
There's no technical reason for netwerk/build to exist anymore, we could actually kill most of it since we don't support linking necko as a standalone library anymore. We still need the nsNetModule bits, but it doesn't have to depend on the other directories. It'd require some fiddling elsewhere to ensure that we link the right bits libxul at the end. We could probably create a netwerk/libs.mk or something that defines all the libraries that we want to link.
Comment 6 Gregory Szorc [:gps] 2012-08-23 11:11:16 PDT
My error was probably stemming from how build-splendid does building. Basically, I try to build all the C/CPP files in one pass without looking at the other dependencies in the Makefiles. Looking at the Makefile now, since nsEffectiveTLDService.cpp depends on etld_data.inc, that was breaking build-splendid.

There's only a handful of places where compiling cpp files depends on additional dependencies in Makefiles. If we had a build pass/tier that handled these one-offs, we could convert the *entire* C++ compilation tier to be fully parallel.
Comment 7 Ted Mielczarek [:ted.mielczarek] 2012-08-23 11:20:12 PDT
We probably just need a generic way to express code generation in our Makefiles.
Comment 8 Nathan Froyd [:froydnj] 2012-08-23 11:28:15 PDT
(In reply to Ted Mielczarek [:ted] from comment #5)
> There's no technical reason for netwerk/build to exist anymore

Filed bug 785158 for this.

(In reply to Ted Mielczarek [:ted] from comment #7)
> We probably just need a generic way to express code generation in our
> Makefiles.

I think this is not a bad idea, if you can enforce something like "this cpp depends on this file, which is constructed by running this (Python) script with these arguments".  We're doing something similar to etld_data.inc in telemetry in bug 781531 and I cringed just a little bit when I added the build rules to Makefile.in. ;)

Anyway, returning you to your regularly-scheduled netwerk bugs...
Comment 10 Ryan VanderMeulen [:RyanVM] 2012-08-24 20:01:02 PDT
https://hg.mozilla.org/mozilla-central/rev/63d6c36e40b8

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