905271 - Build ICU with -jN

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Justin Lebar (not reading bugmail)]

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.

Comment 1

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v1

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?

Comment 2

[Mike Hommey [:glandium]]

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.

Comment 3

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v2

Comment 4

[Mike Hommey [:glandium]]

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.

Comment 5

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v3

Comment 6

[Mike Hommey [:glandium]]

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+ :)

Comment 7

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e8bcb131ca7c

Thanks, Mike.

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e8bcb131ca7c

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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

[Ryan VanderMeulen [:RyanVM]]

No occurrences so far post-backout.

Comment 11

[Justin Lebar (not reading bugmail)]

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.

Comment 12

[Justin Lebar (not reading bugmail)]

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.

Comment 13

[Mike Hommey [:glandium]]

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.

Comment 14

[Justin Lebar (not reading bugmail)]

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

Comment 15

[Justin Lebar (not reading bugmail)]

Now without tests: https://tbpl.mozilla.org/?tree=Try&rev=93ea0d674630

Comment 16

[Justin Lebar (not reading bugmail)]

That try run shows red on Linux, so I guess I give up.  :(

Comment 17

[Mike Hommey [:glandium]]

It's still worth fixing.

Comment 18

[Mike Hommey [:glandium]]

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.

Comment 19

[Justin Lebar (not reading bugmail)]

\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

[Ryan VanderMeulen [:RyanVM]]

Does it matter than comment 18 was on OSX?

Comment 21

[Mike Hommey [:glandium]]

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

[Mike Hommey [:glandium]]

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

[Mike Hommey [:glandium]]

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.

Comment 24

[(not currently active) Ted Mielczarek]

bug 1239083 fixes this by using moz.build files to build ICU instead of the upstream build system.