On Fri, Sep 10, 2010 at 03:32:14PM +0200, Stefan Sperling wrote:
> On Thu, Sep 09, 2010 at 10:58:06PM -0000, stefan2_at_apache.org wrote:
> > Author: stefan2
> > Date: Thu Sep 9 22:58:06 2010
> > New Revision: 995603
> >
> > URL: http://svn.apache.org/viewvc?rev=995603&view=rev
> > Log:
> > Try really hard to help the compiler generate good code
> > for this frequently called function.
> >
> > * subversion/libsvn_subr/svn_string.c
> > (svn_stringbuf_appendbyte): reformulate
> >
> > Modified:
> > subversion/branches/performance/subversion/libsvn_subr/svn_string.c
> >
> > Modified: subversion/branches/performance/subversion/libsvn_subr/svn_string.c
> > URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/svn_string.c?rev=995603&r1=995602&r2=995603&view=diff
> > ==============================================================================
> > --- subversion/branches/performance/subversion/libsvn_subr/svn_string.c (original)
> > +++ subversion/branches/performance/subversion/libsvn_subr/svn_string.c Thu Sep 9 22:58:06 2010
> > @@ -403,8 +403,11 @@ svn_stringbuf_appendbyte(svn_stringbuf_t
> > apr_size_t old_len = str->len;
> > if (str->blocksize > old_len + 1)
> > {
> > - str->data[old_len] = byte;
> > - str->data[old_len+1] = '\0';
> > + char *dest = str->data;
> > +
> > + dest[old_len] = byte;
> > + dest[old_len+1] = '\0';
> > +
> > str->len = old_len+1;
> > }
> > else
> > @@ -412,7 +415,8 @@ svn_stringbuf_appendbyte(svn_stringbuf_t
> > /* we need to re-allocate the string buffer
> > * -> let the more generic implementation take care of that part
> > */
> > - svn_stringbuf_appendbytes(str, &byte, 1);
> > + char b = byte;
> > + svn_stringbuf_appendbytes(str, &b, 1);
> > }
> > }
> >
> >
>
> If you're expecting this function to stay written as it now,
> you need to add a comment explaining why it is laid out the way it is.
> Otherwise people might change it, and the resulting generated code will
> be less optimal again.
>
> That said, I have my doubts whether tweaking C code in order to tune the
> output of certain compilers in certain versions is a useful thing to do.
> Will this still work as expected when new compiler versions are released?
> It seems you'd rather want to fix the optimizer of the compiler.
This has been merged into trunk now, so I'm bumping this thread to
repeat my question above.
Stefan
Received on 2010-09-15 09:47:54 CEST