[Top] [All Lists]

Re: [PATCH] remove bashisms from xfstests

To: Christian Kujau <lists@xxxxxxxxxxxxxxx>
Subject: Re: [PATCH] remove bashisms from xfstests
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 7 Jan 2010 09:36:31 +1100
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <alpine.DEB.2.01.1001061114340.3483@xxxxxxxxxxxxxxxxxx>
References: <alpine.DEB.2.01.1001030125040.3483@xxxxxxxxxxxxxxxxxx> <20100106164844.GB4209@xxxxxxxxxxxxx> <alpine.DEB.2.01.1001061114340.3483@xxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
On Wed, Jan 06, 2010 at 11:24:10AM -0800, Christian Kujau wrote:
> On Wed, 6 Jan 2010 at 11:48, Christoph Hellwig wrote:
> > > 2) convert "let..." into something (hopefully) more portable
> > 
> > Dave converted these away from expr due to performance reasons.  I'd
> > like too see a prove that performance hasn't regressed due to this
> > change.
> That's why I hesitated to just change all scripts to /bin/bash, but didn't 
> dare to say so, because of the inevitable "but today's computers are fast 
> enough" reactions :-)

FYI - the problem I came across was to do with running xfsqa under UML,
where forking a new process is an expensive operation (I measured it
at about 300ms) and so using bash built-in expressions for
incrementing variables is a major win in terms of test run time.
IIRC, one test loops 100,000 times and the runtime went from half an
hour to 10s just by using "let n=n+1" instead of expr...

> > > 3) replace 'a == b' with 'a = b' in bourne shell scripts
> > 
> > This looks fine.  If you want feel free to submit these as a separate
> > first patch so that we have the large pile sorted out.
> OK, will do.
> > I fear this might cause some overhead in the shell.  What about the
> > following instead:
> > 
> > for i in `seq 0 39`; do
> >     ln -s $o symlink_$i
> >     o=symlink_$i
> > done
> I didn't go for seq(1), as I feared this wouldn't be available on every 
> machine.

seq(1) is already used in several tests, so it is ok to use in
more. :)


Dave Chinner

<Prev in Thread] Current Thread [Next in Thread>