[OPEN-ILS-DEV] PATCH: osrf_json_tools.c (speedup of jsonFormatString())

[Scott McKellar]

This patch provides a performance boost for the jsonFormatString
function:

1. Replaced the old _tabs function, which required the construction and
destruction of a growing_buffer, with a new append_indentation function,
which adds white space to an existing growing_buffer.  This change
eliminates a passel of mallocs and frees.

2. Removed the call to strlen() from the loop condition.

3. Replaced calls to buffer_fadd(), a fairly slow function, with calls
to OSRF_BUFFER_ADD_CHAR() and append_indentation().  Also: replaced a
call to buffer_add_char with the corresponding macro.

4. Eliminated a harmless but wasteful bug that sometimes added
indentation to the end of a line.

In my benchmarking, using a moderately complex JSON string 201
characters long, the new version was seven times as fast as the old.

NOTE: this patch depends on previous patches to utils.[ch], not yet
applied at this writing, that add a buffer_add_n function.  With a 
little rewriting we could use buffer_add() instead, but only at the
cost of several precious nanoseconds.

----------

Item 4 on the above list needs a little explanation.

The original version of jsonFormatString() included the following,
to be executed after outputting a ']' or '}' character:

    if(string[i+1] != ',') {
        tab = _tabs(depth);
        buffer_fadd( buf, "%s", tab );
        free(tab);
    }
    
...where tab is a string of blank spaces used for indentation.  The
format string in the buffer_fadd() call looked a little funny to me
because we were writing out the indentation without writing a newline
first.

After some head-scratching I figured out that, in well-formed JSON,
a '}' or ']' character (outside of a string literal) can be followed
*only* by a comma, white space, or another '} or ']' character.  The
snippet above writes useless blank spaces to the *end* of a line
instead of the beginning.  Nobody noticed because the extra white
space is invisible if you don't go looking for it.

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

This patch will admittedly have little impact.  The function in question
is called only from srfsh, where no one will know the difference, or
care.  I am like a medieval sculptor, carving intricate gargoyles that
no one will ever see from the street, because God can see them.

Scott McKellar
http://home.swbell.net/mck9/ct/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: osrf_json_tools_c_6.patch
Type: text/x-patch
Size: 2591 bytes
Desc: not available
Url : http://libmail.georgialibraries.org/pipermail/open-ils-dev/attachments/20081215/ebbcc8ef/attachment-0001.bin