Closed Bug 905271 Opened 6 years ago Closed 4 years ago

Build ICU with -jN

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: justin.lebar+bug, Unassigned)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
Attached patch Patch, v1 (obsolete) — Splinter Review
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 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-
Attached patch Patch, v2Splinter Review
Attachment #790342 - Attachment is obsolete: true
Attachment #790555 - Flags: review?(mh+mozilla)
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-
Attached patch Patch, v3 (obsolete) — Splinter Review
Attachment #790555 - Attachment is obsolete: true
Attachment #790556 - Flags: review?(mh+mozilla)
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+
Attachment #790556 - Attachment is obsolete: true
Attachment #790556 - Flags: review?(mh+mozilla)
Attachment #790555 - Attachment is obsolete: false
https://hg.mozilla.org/mozilla-central/rev/e8bcb131ca7c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Assignee: general → justin.lebar+bug
Target Milestone: mozilla26 → ---
No occurrences so far post-backout.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
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)
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)
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
That try run shows red on Linux, so I guess I give up.  :(
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → WONTFIX
It's still worth fixing.
Assignee: justin.lebar+bug → general
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Blocks: es-intl
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.
\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.
Does it matter than comment 18 was on OSX?
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 :-/
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.
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: general → nobody
bug 1239083 fixes this by using moz.build files to build ICU instead of the upstream build system.
Depends on: 1239083
Status: REOPENED → RESOLVED
Closed: 6 years ago4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.