sindex() and strstr() //Re: To Paul D. Smith: "make -j" as a great tool for SMP and cluster parallelization

[Markus Mauhart]

"Khamenia, Valery" <address@hidden> wrote in message
news:address@hidden
> Dear Paul,
>
>   I can't get through your spam filters probably,
>   so I try to contact you via this mail list.
>
>   I use "make -j" as a great tool for launch
>   parallel computations on SSI cluster. I use it
>   in biotech field, but it is of course worth of
>   doing the same in other fields too.
>
>   All is quite fine, but the "make" is inadequately
>   slow with its substitution references. I've made
>   a patch to "sindex" function and that makes a *great*
>   speed up. It was a bit discussed here:
>
>   http://savannah.gnu.org/patch/?func=detailitem&item_id=2903
>
>   For your convenience the final version of sindex function
>   is attached in "p.c" file to this email also. I hope the
>   code in this small patch is very clear.
>
>   To test the speed up one could use the test "Makefile",
>   which is also attached to this email.
>
>   A lot of users surely will be very thankful to you if you
>   could apply this patch.


Valery, your sindex()-patch needs two fixes.

1st it has a different semantic as the original sindex(), and 2nd
w.r.t. performance it is incompatible with the subst_expand()-patch
that you did mention in ...
http://savannah.gnu.org/patch/index.php?func=detailitem&item_id=2777
... and which has been incorporated in make381beta1.

The difference in semantic is that for slen==0 or small[0]==0, the
original sindex(big,blen,small,slen) would return 0, while your version
returns big.

W.r.t. performance, you have to replace "if(blen)" with "if(blen && big[blen])",
and for sanity do the same with "small/slen"; otherwise your version of
sindex() - when combined with the subst_expand()-patch/make381beta1 - will
again be extremely slow in your own benchmark. Additionally, at least for
small, I'd use 'alloca' for the temporary copy like it is done at many other
places inside make.


OTOH the whole patch is redundant once Paul follows your previous suggestion
to removes sindex() and switches to strstr().  Allthough sindex() has a more
sophisticated semantic than strstr() (sindex() can deal with strings w/o term
zero), that case is never used in make381beta1: either blen/slen are compile
time constants > 0 equal to strlen(string litteral), or they are values that
have been just computed via strlen(big/small).
The only place were this is not obvious is the sindex()-call inside 
subst_expand(),
"sindex(t, tlen, subst, slen)". Here tlen comes from locally computed strlen(),
but slen comes from a subst_expand()-parameter and hence might be the length of
a not-zero-terminated substring, in theory at least ;-) But in practice all
calls of subst_expend() use strlen(pattern) for parameter slen, therefore
a more economic version of subst_expand() could remove its parameter slen
completely and instead compute a local variable slen on its own, and it could
replace "sindex(t, tlen, subst, slen)" with "slen ? strstr(t,subst) : 0"
cause at this point we know that tlen==strlen(t) and slen==strlen(subst).

-> So that's what I have done with my local copy of make381beta1, remove
function sindex(), remove subst_expand()'s parameter slen, and instead
use "small[0] ? strstr(big,small) : 0" instead of sindex(..).


Regards,
Markus.