Closed
Bug 785118
Opened 12 years ago
Closed 12 years ago
use PARALLEL_DIRS in netwerk/
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla17
People
(Reporter: froydnj, Assigned: froydnj)
Details
Attachments
(1 file)
4.09 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
This doesn't look to be that difficult, and netwerk has quite a number of source files (200+ .cpp).
Assignee | ||
Comment 1•12 years ago
|
||
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)
Assignee | ||
Comment 2•12 years ago
|
||
Oh, and this patch wins ~3% of build time on an m-c build (8 core/16 threads Xeon with SSD).
Comment 3•12 years ago
|
||
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.
Assignee | ||
Comment 4•12 years ago
|
||
(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•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
||
We probably just need a generic way to express code generation in our Makefiles.
Assignee | ||
Comment 8•12 years ago
|
||
(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...
Updated•12 years ago
|
Attachment #654676 -
Flags: review?(mh+mozilla) → review+
Updated•12 years ago
|
Assignee: nobody → nfroyd
Assignee | ||
Comment 9•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/63d6c36e40b8
Status: NEW → ASSIGNED
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/63d6c36e40b8
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•