Closed Bug 304785 Opened 20 years ago Closed 20 years ago

Replacing NSPR variables in config/autoconf.mk is slow

Categories

(Firefox Build System :: General, defect)

x86
OS/2
defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mozilla, Unassigned)

Details

(Keywords: fixed1.8)

Attachments

(1 file, 1 obsolete file)

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.
Attached patch Faster version for configure.in (obsolete) — Splinter Review
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.
Attachment #192807 - Flags: review?(chase)
Attachment #192807 - Flags: review?(chase) → review+
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.
Peter, do you want to try Steven's suggestion before I check this in?
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.
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 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?
Attachment #193250 - Flags: review?(chase)
Comment on attachment 193250 [details] [diff] [review] Updated autoconf.mk edit patch Yeah, even better.
Attachment #193250 - Flags: review?(chase) → review+
Fix checked in.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Attachment #192807 - Attachment is obsolete: true
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.
Attachment #193250 - Flags: approval1.8b5?
Attachment #193250 - Flags: approval1.8b5? → approval1.8b5+
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.
Keywords: fixed1.8
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: