[OPEN-ILS-DEV] PATCH: osrf_json_object.c (tweaks)

Scott McKellar mck9 at swbell.net
Sat Dec 8 13:52:39 EST 2007


These patches tidy up a few things and introduce a modest performance
boost.  Summary:

1. I eliminated the jsonInternalParserHandler variable as unnecessary.

2. I added the const qualifier here and there.  In particular,
the "handler" member of jsonParserContext now points to a const
json_parserHandler.

3. Upon allocating a jsonParserContext or jsonInternalParser, I
initialize all members explicitly.

4. I use a statically allocated jsonParserContext and
jsonInternalParser
instead of allocating and freeing them dynamically.

Details:

1. In the original code, the jsonInternalParserHandler variable was 
initialized to point to jsonInternalParserHandlerStruct, and never
updated to point anywhere else.  I eliminated this pointer and just
took the address of jsonInternalParserHandlerStruct where needed.

Once I no longer needed different names to distinguish between pointer
and pointee, I shortened the pointee's name by removing the "Struct" 
from the end.  So what was the name of the pointer is now the name of 
the instance itself.

2. When we create a jsonParserContext we supply a pointer to a set
of function pointers.  The jsonParsetContext then carries a copy of
this pointer.

However the parser never has any business changing those function
pointers, nor should it free the set (which in practice is statically
allocated anyway).  So I treated the set as const (the set as a whole,
not the individual function pointers.)  This change involved constness
tweaks in several places.

3. No further comment...

4. Now here's the performance tweak.

When we parse a JSON string, the original code mallocs a
jsonParserContext and a jsonInternalParser.  When it's done parsing,
it frees each of these objects.

These two objects don't really need to be allocated dynamically, 
because we never have more than one in use at a time.  So my first
impulse was to declare static instances at file scope and reuse them.
That way we would avoid the overhead of allocating and freeing them
every time we parse a JSON string (which we do a lot).

However Bill appears to have ambitious plans for the parsing code.
In particular we use function pointers to specify how to handle the
various lexical events, so that we could specify other behaviors
if we ever wanted to.  We're not really using this flexibility for
anything yet, but I didn't want to make it needlessly difficult to
do so in the future.

So I went ahead and declared static instance of these objects, but I
used a couple of booleans to keep track of whether they're in use.
If I need to allocate an instance and the static one is already
in use, I allocate another one with malloc().  Likewise, when it's 
time to deallocate an instance, I call free() only if the pointer
doesn't point to the static instance.

In practice the mallocs and frees are currently unreachable, but
they're there if they're ever needed.

I wrote a simple benchmark program that repeatedly parses a minimal
JSON string.  With this benchmark, the tweaked version runs about
6% faster than the original.  In real-world use the boost will be 
scarcely visible at best, but it's an easy tweak, and every little
bit helps.

Note that this tweak is distinctly NOT thread-safe.

---------

The patch for osrf_json.h is independent of another recent patch to
the same file that hasn't been applied.  I believe that either patch
will work, with or without the other, but I am not a patch wrangler.

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: osrf_json_h_6.patch
Type: text/x-patch
Size: 1454 bytes
Desc: 456546133-osrf_json_h_6.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20071208/625e939f/osrf_json_h_6.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: osrf_json_parser_c_2.patch
Type: text/x-patch
Size: 4018 bytes
Desc: 2564361677-osrf_json_parser_c_2.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20071208/625e939f/osrf_json_parser_c_2.bin


More information about the Open-ils-dev mailing list