[OPEN-ILS-DEV] PATCH: utils.c (faster buffer_add_char)

[Scott McKellar]

This patch provides a faster implementation of buffer_add_char().  

It also replaces malloc() with calloc() in safe_calloc(), and removes
the memset().  The behavior should be identical, but there is a chance
that calloc() will be trivially more efficient, depending on how the
standard library is written.

The original implementation of buffer_add_char() was a thin wrapper 
around buffer_add(), and relied on buffer_add() to expand the buffer
as needed.  The new version does the buffer expansion for itself.  I
pulled the buffer expansion code out into a separate function called
by both buffer_add() and buffer_add_char().

I ran a simple benchmark program that built a string of 100 characters
by successive calls to buffer_add_char().  For this benchmark, on my
machine, the new version was just over five times as fast as the old.

The timing difference will vary somewhat with the length of the
receiving string.  Because buffer_add() uses strcat() to append the
new string, its run time is linearly related to the length of the
original string (assuming no buffer expansion).  The new version 
runs in constant time (assuming no buffer expansion).

This new version will give a minor speed boost to any program that
calls buffer_add_char().  However I have seen a number of places
that call buffer_add() to add a single character, for example:

    buffer_add( buffer, "}" );

This usage is faster than the old implementation of buffer_add_char()
because it does exactly the same thing but eliminates a layer of
function call.  However I expect it to be slower than the new version.
One of the items on my janitorial agenda will be to replace this
usage with proper calls to the new buffer_add_char().

--------------

I have three further suggestions for the handling of growing_buffers.
I didn't want to make these changes on my own without discussion.

1. buffer_add_char() should return void instead of int.  So far as I
can tell, no code looks at the return value anyway -- nor would it
make sense to, since it always returns the same value.

2. It's also doubtful whether we ever check the return value of
buffer_add(), buffer_fadd(), or buffer_reset(), though I haven't 
looked.  We could simplify the code a bit by letting them return void.

3. If the amount of buffer space needed is greater than 
BUFFER_MAX_SIZE, we not only issue an error message, but also free
the original buffer.  I don't see any reason to do so, unless we
want to make it nearly impossible for the client code to continue.
We could get approximately the same result more gracefully by calling
exit() or abort().  Alternatively we could leave the original buffer
unchanged, and rely on the client code to check the return value
(which it seldom or never does in practice).

4. If we ask buffer_add to append an empty string, it returns 0,
but if we ask it to append a non-empty string, it returns the length
of the newly augmented string.  If we never look at the return value
then it doesn't matter what we return.  However it would be more
consistent to return the original string length if the string to be
appended is empty.  Appending an empty string is not an error, it's
just a special case.

Scott McKellar
http://home.swbell.net/mck9/ct/

Developer's Certificate of Origin 1.1 By making a contribution to
this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license indicated
in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source license
and I have the right under that license to submit that work with
modifications, whether created in whole or in part by me, under the
same open source license (unless I am permitted to submit under a
different license), as indicated in the file; or

(c) The contribution was provided directly to me by some other person
who certified (a), (b) or (c) and I have not modified it; and

(d) In the case of each of (a), (b), or (c), I understand and agree
that this project and the contribution are public and that a record
of the contribution (including all personal information I submit
with it, including my sign-off) is maintained indefinitely and may
be redistributed consistent with this project or the open source
license indicated in the file.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: utils_c_6.patch
Type: text/x-patch
Size: 3584 bytes
Desc: 4057215802-utils_c_6.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20071209/b0782089/utils_c_6.bin