304785 - Replacing NSPR variables in config/autoconf.mk is slow

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Peter Weilbacher]

Near the end of the configure run (line 7135 of mozilla/configure.in) a message
appears
    configure: warning: Recreating autoconf.mk with updated nspr-config output
after which the configure process seems to hang for some time while using 100%
CPU. On my relatively fast machine this takes about 1 minute but others said (in
netscape.public.mozilla.os2) that it takes tens of minutes on their machines, so
this is worth improving on.

The "hang" is due to a perl loop used to replace the variables NSPR_LIBS and
NSPR_CFLAGS in config/autoconf.mk. This loop is very slow on platforms that are
not as effective at spawning shells (like OS/2) as nspr-config is called for
every line in autoconf.mk.

Comment 1

[Peter Weilbacher]

Attachment: Faster version for configure.in

This alleviates the problem by using the nspr-config call just once, before the
perl loop, to input it into a variable that in turn is used in the perl
one-liner.
 
I didn't find any other instance of this export + perl $ENV hack so there
probabl y is a better way, but the patch doesn't break anything on Linux while
on OS/2 the time it takes to do this variable replacement is down from one
minute to a fraction of a second.

Comment 2

[Steven H. Levine]

Regarding your hack comment, there is a better way if you get the urge.  Unix
shells do sane quoting.  This means that

  v='d'
  echo 'abc'$v'def'

will do what you expect, so you can get rid of the export and replace the perl
invocation with 

  $PERL -pi.bak -e 's {^NSPR_LIBS\s*=.*} {NSPR_LIBS = '"$NSPR_LIBS_REPL"' }'
config/autoconf.mk

I suspect autoconf style would prefer the variable to be lowercased too.

I'd need to test but the double quotes are unlikely to be required.

Comment 3

[Mike Kaply [:mkaply]]

Peter, do you want to try Steven's suggestion before I check this in?

Comment 4

[Peter Weilbacher]

Now I tried it but it doesn't work. With normall commands Steven's trick works
but perl somehow interprets the $\(DIST\) and whatever I try with escaping and
quoting it always gives me CFLAGS like "-I430 430DIST)/include/nspr". So, I
would vote to check in the patch as is.

Comment 5

[Steven H. Levine]

Attachment: Updated autoconf.mk edit patch

I neglected to avoid excess variable interpolation.  The revised patch should
handle all cases as long as nspr_config does not output a value containing one
or more single quotes.

FWIW, $( is a variable that represents the read gid of the current process. 
Since OS/2 doesn't do gids, perl always supplies a 0 value.

Comment 6

[Peter Weilbacher]

Comment on attachment 193250 [details] [diff] [review]
Updated autoconf.mk edit patch

Yes, that works for me both on OS/2 and Linux with bash or ash as shell. chase,
do you like this one better, too?

Comment 7

[Chase Phillips]

Comment on attachment 193250 [details] [diff] [review]
Updated autoconf.mk edit patch

Yeah, even better.

Comment 8

[Mike Kaply [:mkaply]]

Fix checked in.

Comment 9

[Peter Weilbacher]

Comment on attachment 193250 [details] [diff] [review]
Updated autoconf.mk edit patch

Mike, thanks for the checkin.

We probably have to do quite a lot of builds on the 1.8 branch, so it would be
nice to get this in there, too.

Comment 10

[Asa Dotzler [:asa]]

Mike or Peter, time is running out on 1.8b5 and I suspect we're soon going to
start withdrawing the approvals for non-blocking bugs to minimize risk. If this
still needs to land on the branch, please do so ASAP and add the "fixed1.8"
keyword. Thanks.