apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Cliff Woolley <jwool...@virginia.edu>
Subject Re: cvs commit: apr-util/buckets apr_brigade.c
Date Mon, 13 May 2002 22:52:37 GMT
On Mon, 13 May 2002, Greg Stein wrote:

> Ouch...
>
> -1 on this commit, pending further explanation. I'm loathe to see all of
> that extra code go in there for a dubious performance benefit. Are you
> *truly* saving any time? I see a while() loop in there copying data.
> Isn't it almost always faster to let the compiler produce an optimized
> memcpy() or somesuch? Brian? Let's see some details/numbers here, or
> else please revert.

FWIW, I was also very hesitant about this patch, due to complexity
concerns as well as the fact that I thought we might be ignoring the flush
parameter.  I investigated the latter, and I now believe this patch is
correct (or at least as correct as apr_brigade_write itself), though the
complexity does seem questionable.  I understand the concern: why loop
through the entire string to find the NULL only to loop through it again
to copy.  And no, the compiler won't do an optimized memcpy here because
the length is not known at compile time.  So it's a valid optimization.
At the same time, the brigade_write() family can be tricky to maintain at
times.  It's much better than it used to be (now that
check_brigade_flush() has been burned at the stake), though.

So the real question is this: is it ever possible for strlen()+memcpy() to
be faster than the while loop used here?  I believe the answer is no.

+0 on the optimization.

--Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



Mime
View raw message