Closed
Bug 783357
Opened 13 years ago
Closed 13 years ago
use PARALLEL_DIRS in dom/
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: froydnj, Assigned: froydnj)
References
Details
Attachments
(2 files, 1 obsolete file)
17.86 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
1.43 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
dom/ has grown pretty big, almost as big as content/. content/ does PARALLEL_DIRS. dom/ doesn't. Let's fix that.
Fresh build before PARALLEL_DIRS:
real 10m46.380s
user 75m31.311s
sys 6m47.187s
Fresh build with attached patch:
real 10m9.947s
user 77m9.975s
sys 6m52.453s
so ~5% improvement.
Guessing there are some subtle ordering issues which one run doesn't turn up, though...
Attachment #652529 -
Flags: feedback?(khuey)
Comment 1•13 years ago
|
||
I'm pretty sure we can't do interfaces/ in parallel.
![]() |
Assignee | |
Comment 2•13 years ago
|
||
Yeah, I do see some random build failures on try. May look at those today.
![]() |
Assignee | |
Updated•13 years ago
|
Attachment #652529 -
Flags: feedback?(khuey)
![]() |
Assignee | |
Comment 3•13 years ago
|
||
(In reply to :Ms2ger from comment #1)
> I'm pretty sure we can't do interfaces/ in parallel.
So it *looks* like the only thing preventing this from happening is that, e.g. domstubs.idl is included by a bunch of different files, thus requiring interfaces/base to be built before everything else. (And other examples, but domstubs is the most prominent one that I've seen.)
dom/interfaces/base/Makefile.in gets around some circularity by appending to XPIDL_FLAGS; would it be reasonable to add the necessary XPIDL_FLAGS+= bits to the appropriate directories so it at least *looks* like we are declaring our dependencies, rather than requiring serialization to sort them out for us?
fiddling with XPIDL_FLAGS (like LOCAL_INCLUDES) is the right way to proceed.
![]() |
Assignee | |
Comment 5•13 years ago
|
||
Here's the boring patch that adds XPIDL_FLAGS where necessary. This whole patch was generated by a script, the logic is something like:
for each directory with idl files:
determine #includes
find where the #includes live
write appropriate XPIDL_FLAGS bits to the directory's Makefile.in
so I am reasonably confident it won't produce random build failures. I assumed that only dependencies in dom/ mattered; other deps should be taken care of by top-level serialization, IIUC.
Attachment #654617 -
Flags: review?(khuey)
![]() |
Assignee | |
Comment 6•13 years ago
|
||
This patch flips dom/Makefile.in to use PARALLEL_DIRS in all the appropriate places.
Attachment #652529 -
Attachment is obsolete: true
Attachment #654618 -
Flags: review?(khuey)
![]() |
Assignee | |
Comment 7•13 years ago
|
||
CC'ing gps in case he has insights on this from build-splendid work.
Comment on attachment 654617 [details] [diff] [review]
add XPIDL_FLAGS where necessary
Review of attachment 654617 [details] [diff] [review]:
-----------------------------------------------------------------
XPIDL_FLAGS += \
directory \
$(NULL)
Even on the single line ones please.
Attachment #654617 -
Flags: review?(khuey) → review+
Comment on attachment 654618 [details] [diff] [review]
PARALLEL_DIRS magic for dom/
Review of attachment 654618 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/Makefile.in
@@ +43,2 @@
> interfaces/apps \
> $(NULL)
Just move this up into the other set of PARALLEL_DIRS.
Attachment #654618 -
Flags: review?(khuey) → review+
![]() |
Assignee | |
Comment 10•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb4219f3723d
https://hg.mozilla.org/integration/mozilla-inbound/rev/544b0cbdbb09
Assignee: nobody → nfroyd
Status: NEW → ASSIGNED
Comment 11•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fb4219f3723d
https://hg.mozilla.org/mozilla-central/rev/544b0cbdbb09
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Comment 12•13 years ago
|
||
So how about we back this out until we can do it right?
Comment 13•13 years ago
|
||
Yes, please back this out until we can fix the build issue.
![]() |
Assignee | |
Comment 14•13 years ago
|
||
FWIW, there is a patch for the build issue in bug 786703.
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•