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

[Scott McKellar]

This patch mainly tweaks some const-correctness, and introduces an
optimization to reduce the use of malloc() and free().  Summary:

1. I removed a const qualifier from the first parameter of
jsonObjectGetKey().

2. I added a const qualifier to the parameter of jsonBoolIsTrue().

3. When constructing a jsonObject, I explicitly initialize all the
members.

4. I introduced a linked list of jsonObjects that have been allocated
from the heap but are not currently in use.

5. I added a new function jsonObjectFreeUnused() that frees all the
jsonObjects in the free list, returning them to the heap.

Details:

1. As we have discussed before, jsonObjectGetKey() shouldn't accept
a const pointer to a jsonObject if it returns a non-const pointer
to the innards of the object, because the client code can then use
the non-const pointer to modify the innards of a supposedly const
object.

We now have a similar function jsonObjectGetKeyConst() that accepts
and returns const pointers.  I spent the last couple of dozen patches
replacing the old function with the new one wherever appropriate, so
that we can remove the const from the old one.

With one exception, I have verified that all the files that reference
jsonObjectGetKey() still compile.

The exception is object.c, which contains its own implementation of
jsonObjectGetKey().  I changed that version to remove the const, but
the file doesn't compile for other reasons.  For example, in line 71
it references the "next" member of a jsonObjectNode, but that struct
doesn't have a member by that name.  It looks like there's some kind
of disconnect between the C file and the headers it #includes.

Depending on what's going on with object.c, you may or may not want
to apply the patch to it.

2. I should have constified jsonBoolIsTrue() in a previous patch, 
but I missed it somehow.

3. Explicitly initializing all members of a jsonObject reduces our
reliance on the memset() in safe_malloc() (or in this case the
OSRF_MALLOC macro).  It is also necessary because now we sometimes
bypass the memset() by reusing previously allocated jsonObjects, as
described below.

4. Many of our applications are constantly building up and tearing
down jsonObjects.  I wince when I think of all the churning of
memory through malloc() and free().  Some of that churn can't be
avoided, but we can avoid some of it by maintaining a list of
jsonObjects that have been malloc'd but are not currently in use.
When we need to allocate a jsonObject, we check the free list first,
and malloc a new one only if the free list is empty.

I wrote a little benchmarking program that creates and destroys lots
of jsonObjects in a loop.  The use of a free list speeds up the 
program by a factor of about 3.5.

In a real program, of course, the speedup is likely not to be 
noticeable amid other overhead.  However it's an easy optimization
to do, and maybe it will help.

Admittedly, this optimization may increase the memory footprint of
an application, because we hold on to memory that we're not using.
My instinct is that this effect is not likely to be significant, but
if it is, we can mitigate it by judicious use of the 
jsonObjectFreeUnused function.  Depending on how glibc does its
memory management, there may be countervailing benefits such as a
reduction in memory fragmentation.

5. jsonObjectFreeUnused() takes the jsonObjects on the free list and
calls free() for each of them, leaving the free list empty.  This
function may or may not be useful, but it's there if needed.  For
example we might call it at the end of a program, but it would 
arguably be a waste of time, because the operating system will take
back the memory held by the process anyway upon exit.

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_5.patch
Type: text/x-patch
Size: 1470 bytes
Desc: 495364516-osrf_json_h_5.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20071202/83169681/osrf_json_h_5.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: osrf_json_object_c_4.patch
Type: text/x-patch
Size: 4039 bytes
Desc: 4103061089-osrf_json_object_c_4.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20071202/83169681/osrf_json_object_c_4.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: object_c_2.patch
Type: text/x-patch
Size: 484 bytes
Desc: 3195117548-object_c_2.patch
Url : http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20071202/83169681/object_c_2.bin