Closed
Bug 603214
Opened 14 years ago
Closed 14 years ago
Build NSPR in parallel
Categories
(NSPR :: NSPR, enhancement)
NSPR
NSPR
Tracking
(Not tracked)
RESOLVED
FIXED
4.8.7
People
(Reporter: khuey, Assigned: khuey)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
This patch drops the time to build NSPR on my system from 35s to 7.5s (500% increase in speed). The only complex target I saw is nspr4, if my eyes aren't deceiving me all the rest only use object files from one directory.
Attachment #482156 -
Flags: superreview?(wtc)
Attachment #482156 -
Flags: review?(ted.mielczarek)
Comment 1•14 years ago
|
||
khuey: Thanks for the patch. Please add a comment to explain why adding a 'build' makefile target fixes the problem. +export:: + $(MAKE) -C . build You should be able to omit "-C .", right? Can you write this makefile rule as follows? export:: build
Assignee | ||
Comment 2•14 years ago
|
||
No. In a parallel make, all of the dependencies of a target are executed concurrently (up to the N maximum jobs from -jN). The instructions for a target, are however, executed sequentially. By moving the final linking of nspr4 to an additional target we can ensure that final linking occurs after we loop through the other directories (since rules.mk gets included before this).
Assignee | ||
Comment 3•14 years ago
|
||
Attachment #482156 -
Attachment is obsolete: true
Attachment #482702 -
Flags: superreview?(wtc)
Attachment #482702 -
Flags: review?(ted.mielczarek)
Attachment #482156 -
Flags: superreview?(wtc)
Attachment #482156 -
Flags: review?(ted.mielczarek)
Comment 4•14 years ago
|
||
khuey: thanks for the explanation. I understand it now. I also compared your fix with the previous attempt in bug 123423. Both patches have the same idea: recurse into $(DIRS) before building $(TARGETS). I'm glad you made it work. I edited your comment and removed "-C .", which I believe is unnecessary. Please review this patch. I think we can fix this in a more elegant way, but your fix is fine (most bang for the buck).
Attachment #482702 -
Attachment is obsolete: true
Attachment #482736 -
Flags: superreview?(ted.mielczarek)
Attachment #482736 -
Flags: review?(khuey)
Attachment #482702 -
Flags: superreview?(wtc)
Attachment #482702 -
Flags: review?(ted.mielczarek)
Comment 5•14 years ago
|
||
Should I change the comment to say what I summarized in comment 4? We use a 'build' target here to ensure that we build $(TARGETS) after recursing into $(DIRS) to create the object files in a parallel build. ... or We use a 'build' target here to ensure that we build $(TARGETS) after looping over $(DIRS) to create the object files in a parallel build. ...
Assignee | ||
Comment 6•14 years ago
|
||
Comment on attachment 482736 [details] [diff] [review] Patch v3 by Kyle Huey I prefer the second comment.
Attachment #482736 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 7•14 years ago
|
||
(In reply to comment #5) > We use a 'build' target here to ensure that we build $(TARGETS) > after looping over $(DIRS) to create the object files in a > parallel build. ... This one.
Comment 8•14 years ago
|
||
Patch checked in on the NSPR trunk (NSPR 4.8.7). Checking in Makefile.in; /cvsroot/mozilla/nsprpub/Makefile.in,v <-- Makefile.in new revision: 1.25; previous revision: 1.24 done Checking in pr/src/Makefile.in; /cvsroot/mozilla/nsprpub/pr/src/Makefile.in,v <-- Makefile.in new revision: 1.60; previous revision: 1.59 done
Attachment #482736 -
Attachment is obsolete: true
Attachment #482736 -
Flags: superreview?(ted.mielczarek)
Updated•14 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.8.7
You need to log in
before you can comment on or make changes to this bug.
Description
•