Closed Bug 785118 Opened 12 years ago Closed 12 years ago

use PARALLEL_DIRS in netwerk/

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla17

People

(Reporter: froydnj, Assigned: froydnj)

Details

Attachments

(1 file)

This doesn't look to be that difficult, and netwerk has quite a number of source files (200+ .cpp).
Attached patch patchSplinter Review
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.
Attachment #654676 - Flags: review?(mh+mozilla)
Oh, and this patch wins ~3% of build time on an m-c build (8 core/16 threads Xeon with SSD).
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.
(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?
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.
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.
We probably just need a generic way to express code generation in our Makefiles.
(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...
Attachment #654676 - Flags: review?(mh+mozilla) → review+
Assignee: nobody → nfroyd
https://hg.mozilla.org/mozilla-central/rev/63d6c36e40b8
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: