Last Comment Bug 304785 - Replacing NSPR variables in config/ is slow
: Replacing NSPR variables in config/ is slow
: fixed1.8
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: x86 OS/2
-- minor (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
: Gregory Szorc [:gps] (away until 2017-03-20)
Depends on:
  Show dependency treegraph
Reported: 2005-08-16 01:49 PDT by Peter Weilbacher
Modified: 2005-09-28 06:51 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Faster version for (1.88 KB, patch)
2005-08-16 01:56 PDT, Peter Weilbacher
chase: review+
Details | Diff | Splinter Review
Updated edit patch (1.50 KB, patch)
2005-08-19 21:32 PDT, Steven H. Levine
chase: review+
asa: approval1.8b5+
Details | Diff | Splinter Review

Description User image Peter Weilbacher 2005-08-16 01:49:24 PDT
Near the end of the configure run (line 7135 of mozilla/ a message
    configure: warning: Recreating 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/ 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
Comment 1 User image Peter Weilbacher 2005-08-16 01:56:54 PDT
Created attachment 192807 [details] [diff] [review]
Faster version for

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
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 User image Steven H. Levine 2005-08-18 21:54:25 PDT
Regarding your hack comment, there is a better way if you get the urge.  Unix
shells do sane quoting.  This means that

  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"' }'

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 User image Mike Kaply [:mkaply] 2005-08-19 08:15:59 PDT
Peter, do you want to try Steven's suggestion before I check this in?
Comment 4 User image Peter Weilbacher 2005-08-19 08:55:49 PDT
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 User image Steven H. Levine 2005-08-19 21:32:56 PDT
Created attachment 193250 [details] [diff] [review]
Updated 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 User image Peter Weilbacher 2005-08-20 01:29:45 PDT
Comment on attachment 193250 [details] [diff] [review]
Updated 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 User image Chase Phillips 2005-08-20 12:07:15 PDT
Comment on attachment 193250 [details] [diff] [review]
Updated edit patch

Yeah, even better.
Comment 8 User image Mike Kaply [:mkaply] 2005-09-15 07:16:18 PDT
Fix checked in.
Comment 9 User image Peter Weilbacher 2005-09-16 14:11:40 PDT
Comment on attachment 193250 [details] [diff] [review]
Updated 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 User image Asa Dotzler [:asa] 2005-09-27 21:11:01 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.