Closed
Bug 905271
Opened 11 years ago
Closed 8 years ago
Build ICU with -jN
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: justin.lebar+bug, Unassigned)
References
Details
Attachments
(1 file, 2 obsolete files)
1.67 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
Right now ICU builds with -j1. But in my (extremely limited) testing, it is very happy to build with -jN. I ran it about ten times and it compiled fine every time. Building with -j1 takes 30s, while -j16 takes 7s on my four-physical-core machine. That's an ideal speedup.
Reporter | ||
Comment 1•11 years ago
|
||
This is almost surely the wrong way to do it, because a) It doesn't respect -j1, if you pass that to toplevel make for whatever reason b) It doesn't respect -jN for very large N, if you pass that to toplevel make because e.g. you have a distcc cluster. But I don't see how else to do it, because in js/src/Makefile.in, MOZ_MAKE_FLAGS is empty. Halp?
Attachment #790342 -
Flags: review?(mh+mozilla)
Comment 2•11 years ago
|
||
Comment on attachment 790342 [details] [diff] [review] Patch, v1 Review of attachment 790342 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/Makefile.in @@ +250,5 @@ > ICU_LIB_RENAME = $(foreach libname,$(ICU_LIB_NAMES),\ > cp -p intl/icu/lib/s$(libname)$(ICU_LIB_SUFFIX).lib intl/icu/lib/$(libname).lib;) > +else > + cores=$(shell $(PYTHON) -c 'import multiprocessing; print(2*multiprocessing.cpu_count())') > + ICU_GMAKE_OPTIONS=-j$(cores) That ought to be simpler, and after a small test, it appears it is: just use $(MAKE) instead of $(GMAKE) when invoking ICU's build system. Which would break in pymake, so you just need to make that conditional to ifdef .PYMAKE.
Attachment #790342 -
Flags: review?(mh+mozilla) → review-
Reporter | ||
Comment 3•11 years ago
|
||
Attachment #790342 -
Attachment is obsolete: true
Attachment #790555 -
Flags: review?(mh+mozilla)
Comment 4•11 years ago
|
||
Comment on attachment 790555 [details] [diff] [review] Patch, v2 Review of attachment 790555 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/Makefile.in @@ +239,5 @@ > > ifdef ENABLE_INTL_API > > ifeq ($(OS_ARCH),WINNT) > + # ICU doesn't work with pymake. Can you add that it's because pymake doesn't support order-only prerequisites? @@ +245,5 @@ > + ICU_MAKE = $(GMAKE) > + else > + ICU_MAKE = $(MAKE) > + endif > + You still need -j1 on windows, because gmake likes to dead-lock with -jN.
Attachment #790555 -
Flags: review?(mh+mozilla) → review-
Reporter | ||
Comment 5•11 years ago
|
||
Attachment #790555 -
Attachment is obsolete: true
Attachment #790556 -
Flags: review?(mh+mozilla)
Comment 6•11 years ago
|
||
Comment on attachment 790555 [details] [diff] [review] Patch, v2 Review of attachment 790555 [details] [diff] [review]: ----------------------------------------------------------------- <glandium> jlebar: pymake doesn't set variables that could affect make's behaviour, so in practice, you don't need -j1 when going from pymake to make <glandium> jlebar: the problem stays for make -> make, but i don't think we allow windows builds with make <jlebar> glandium: and anyway those probably do not have -jN. <jlebar> glandium: otherwise they are going to have a bad time. <glandium> jlebar: so, all in all, if you copy/paste this conversation in the bug, you have my r+ :)
Attachment #790555 -
Flags: review- → review+
Reporter | ||
Updated•11 years ago
|
Attachment #790556 -
Attachment is obsolete: true
Attachment #790556 -
Flags: review?(mh+mozilla)
Reporter | ||
Updated•11 years ago
|
Attachment #790555 -
Attachment is obsolete: false
Reporter | ||
Comment 7•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e8bcb131ca7c Thanks, Mike.
Comment 8•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e8bcb131ca7c
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Reporter | ||
Updated•11 years ago
|
Assignee: general → justin.lebar+bug
Target Milestone: mozilla26 → ---
Comment 9•11 years ago
|
||
Backed out under suspicion of causing increased build flakiness. These just showed up in the last day and this certainly seems like the most-likely candidate. https://hg.mozilla.org/mozilla-central/rev/1ed5a88cd4d0 https://tbpl.mozilla.org/php/getParsedLog.php?id=26598874&tree=Try https://tbpl.mozilla.org/php/getParsedLog.php?id=26596233&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=26582726&tree=Mozilla-Inbound https://secure.pub.build.mozilla.org/buildapi/self-serve/mozilla-inbound/rev/bc51f3691c07
Comment 10•11 years ago
|
||
No occurrences so far post-backout.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 11•11 years ago
|
||
Ouch, I forgot to do $(GMAKE) -j1. That's almost certainly the cause of the windows bustage. But one of these is Linux64 bustage. https://tbpl.mozilla.org/php/getParsedLog.php?id=26582726&tree=Mozilla-Inbound Is make segfaulting on Linux, too? I don't know what to make of this.
Reporter | ||
Comment 12•11 years ago
|
||
glandium, do you think it's possible that we need to install a newer version of make on the Linux buildslaves? If so, I could do -jN only if you have the appropriate version, but that carries risk of build breakage that doesn't show up on tbpl.
Flags: needinfo?(mh+mozilla)
Comment 13•11 years ago
|
||
The only rational explanation i have is that the ICU build system doesn't work well with -jN, and the programs it builds that are used to generate some of its code are broken by make -jN. A possibility is two make processes running the same command tipping on each other's foot, suggesting problematic dependencies.
Flags: needinfo?(mh+mozilla)
Reporter | ||
Comment 14•11 years ago
|
||
In the hopes that the linux64 failure was an unrelated fluke, I've pushed to try with gmake -j1. Let's see what we can see... https://tbpl.mozilla.org/?tree=Try&rev=a6e4a0e1cb4b
Reporter | ||
Comment 15•11 years ago
|
||
Now without tests: https://tbpl.mozilla.org/?tree=Try&rev=93ea0d674630
Reporter | ||
Comment 16•11 years ago
|
||
That try run shows red on Linux, so I guess I give up. :(
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → WONTFIX
Comment 17•11 years ago
|
||
It's still worth fixing.
Assignee: justin.lebar+bug → general
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Comment 18•11 years ago
|
||
So, it turns out those linux crashes *are* happening on buildbots without this patch applied: https://tbpl.mozilla.org/php/getParsedLog.php?id=27149133&tree=Mozilla-Central Which means it's unrelated and this can land.
Reporter | ||
Comment 19•11 years ago
|
||
\o/ Would you mind landing this as pushed to try in comment 15, Mike? I may not be able to do it in a timely fashion.
Comment 20•11 years ago
|
||
Does it matter than comment 18 was on OSX?
Comment 21•11 years ago
|
||
mmmm on local testing, i can reproduce those crashes quite quickly when building ICU in parallel, but after a half hour trying and retrying and retrying, nothing happened on a -j1 build :-/
Comment 22•11 years ago
|
||
At this point, my guess is that there *are* a race condition and missing dependencies in the icu build system. Because the only way i can manage to kind of reproduce the crash is if i truncate some of the intermediate files.
Comment 23•11 years ago
|
||
That being said, that doesn't tell why it fails on osx without -jN... or maybe make on osx is not GNU's and its behavior makes it inherit the -jN? I don't know.
Assignee | ||
Updated•10 years ago
|
Assignee: general → nobody
Comment 24•8 years ago
|
||
bug 1239083 fixes this by using moz.build files to build ICU instead of the upstream build system.
Depends on: 1239083
Updated•8 years ago
|
Status: REOPENED → RESOLVED
Closed: 11 years ago → 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•