autoconf-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: parallel autotest [1/3]: Refactor testsuite driver loop.


From: Eric Blake
Subject: Re: parallel autotest [1/3]: Refactor testsuite driver loop.
Date: Thu, 29 May 2008 22:27:54 +0000 (UTC)
User-agent: Loom/3.14 (http://gmane.org/)

Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de> writes:

> > > +at_stop_file=$at_suite_dir/at-stop
> > 
> > Looks like a good idea.  Please mention it in the ChangeLog entry.
> 
> No problem.  I didn't mean to add it as public interface though,
> not sure if that's what you intend it to be.

No - we already document that anything in the at- filesystem namespace is 
reserved for internal workings.  I merely wanted the string 'at_stop_file' in 
the ChangeLog, to make grepping for its introduction easier.

> > 
> > Do we still need the at- prefix on files that live under $at_helper_dir?
> 
> No, I think we don't.

Then I'm okay with you dropping the at- prefix within the job-dirs.  Note that 
my earlier comment about the at- filesystem namespace still applies to this 
situation: since the job-dirs live inside $at_helper_dir by the name at-groups, 
the various helper files are still off-limits to test groups by virtue of their 
parent directory.

> 
> > It 
> > seems a bit inconsistent to have $at_job_dir/at-stdout vs. 
$at_job_dir/pass, 
> > but I'm not sure whether the latter should be at-pass or whether we can 
> > simplify the former to stdout.
> 
> I think we should be able to simplify.  However, we can do that in a
> separate patch, I think.  Or right away, I don't mind.

At this point, keeping the commits separate is acceptable (one patch to 
refactor, another to rename at- files), since they are logically distinct 
changes.  But it is your call if you want to squash those into one rebased 
patch, since all access to those helper files goes through the shell variables 
that your patch already touched.

> > 
> > Are we consistent on capitalization of warning: vs. WARNING:?
> 
> I think this change makes it consistent with the rest of Autoconf, yes.

Aha.  AS_MESSAGE does indeed print in all caps.  I wonder, though, if that is a 
violation of GNU Coding Standards.  At any rate, cleaning it up would be a 
separate followon patch, and shouldn't hold up the current commit.

> > 1. Keep the file name as the result, but the contents become the group id.  
> > This avoids a pipe, but still forks:
> > 
> > echo "$at_group" > "$at_job_dir/$at_res"
> > ...
> > at_pass_list=`cat /dev/null */pass 2>/dev/null`
> 
> This needs to read hundreds of files; which can easily be slower than a
> fork.  For example, with warm caches, on NFS your command takes 100ms,
> while mine takes 8ms.  The cold cache cases are a lot worse, of course.
> I'd be interested to hear whether things are different on w32.
> 
> More generally, counting forks is not the only thing for efficiency;
> I/O is expensive, too.

Good point.  I didn't really test, so I don't have numbers to back this up; I 
was just typing as I thought about it, trying to see if it was possible to get 
a speedup.  Therefore, don't take this particular criticism as a rejection of 
your approach, so much as a thought experiment to see if there are any possible 
improvements (and there might not be).

At any rate, your approach is already an improvement for non-XSI shells, as 
there are fewer expr calls to work around the lack of $(()).

> 
> The fact that this forks once per test for slow shells makes
> this really unattractive.

I agree - 2 is a non-starter.

> > 3. Like 2, but rework the loop variable to avoid needing at_func_dirname:
> > 
> > touch "$at_job_dir/result-$at_res"
> > ...
> > at_pass_list=
> > ...
> > for at_group in $at_groups; do
> >   case $at_group/result-* in
> 
> The word after 'case' does not undergo filename expansion.
> That can be fixed, but still it would result in one filename
> expansion step be done for each test.  I think that is slightly
> slower than doing just one filename expansion over all files.
> 
> >     *-pass) at_pass_list="$at_pass_list $at_group" ;;
> 
> Still quadratic in number of passed tests.

Can anybody think of a non-forking option 4?  Use five 'test -f' instead of 
a 'case'?  Otherwise, I'm inclined to concede that your approach is probably 
the fastest across all shells (only five list varibles to populate, so O(1) 
pathname expansions sed invocations regardless of number of test groups; and 
list creation via a single pathname expansion instead of quadratic variable 
reassignment).  You are right that anything that has to read the files (rather 
than just using readdir() or access() to test for their existence) will 
probably be slower.  On the other hand, since each file lives in a different 
directory, you are still hitting a lot of inodes to to the pathname expansion.

-- 
Eric Blake






reply via email to

[Prev in Thread] Current Thread [Next in Thread]