On Mon, 17 Nov 2008 23:32:21 -0500, Scott McKellar <[EMAIL PROTECTED]> wrote:
This patch is a rewrite of the jsonObjectToJSON and jsonObjectToJSONRaw
functions. It is dependent on my previous patch to utils.c and utils.h,
adding the new buffer_append_uescape function.
One purpose is to replace a call to the uescape function with a call to
the faster buffer_append_uescape function. The other purpose is to
introduce a faster way to translate a jsonObject into a string.
(Also in one spot I broke up a very long string literal into several
smaller pieces so that it wouldn't wrap around in the editor.)
In the existing jsonObjectToJSON function, we receive a pointer to a
jsonObject and return a string of JSON. However we don't translate the
original jsonObject directly. Instead, we create a modified clone of the
original, inserting an additional JSON_HASH node wherever we find a
classname. Then we translate the clone, and finally destroy it.
It always struck me as an egregious waste to create and destroy a whole
parallel object just so that we could turn it into a string. So I looked
for a way to eliminate the cloning.
The result is a modification of add_json_to_buffer(), a local function
that recursively traverses and translates the jsonObject. When it sees a
classname (and it has been asked to expand classnames), the new version
inserts additional gibberish into the output text and then continues the
traversal, without modifying or copying the original jsonObject at all.
In my benchmark, this new approach was faster than the original by a
factor of about 5. When I combined this change with the use of the new
buffer_append_uencode function, it was faster by a factor of about 7.2.
The benchmark used a moderately complex jsonObject about 5 or 6 levels
deep, with both hashes and arrays, with classnames at several levels.
The performance gain will no doubt depend on the contents of the
jsonObject,but I haven't tried to isolate the variables.
The new version is a bit trickier and harder to read than the old. In my
opinion the speedup is worth the obscurity, because a lot of places in
Evergreen will benefit.
----------------
With this change, the jsonObjectEncodeClass function, which had been
called only from jsonObjectToJSON, is no longer called from anywhere.
It's the function that created the modified copy of the original
jsonObject. I see no reason to keep it around. After a decent interval,
if no one objects I shall submit a patch to eliminate it.
Agreed.
Likewise the jsonObjectToJSONRaw function is no longer called from
anywhere. It translates a jsonObject to JSON without expanding
classnames.
In this rewritten version it is identical to jsonObjectToJSON except that
it calls add_json_to_buffer() with a different parameter value.
If we eliminate jsonObjectToJSONRaw, I can simplify add_json_to_buffer()
a bit. However we might someday want to be able to translate a
jsonObject
without expanding classnames. If nobody minds, I'll leave this function
where it is.
Also agreed. We (or other code using OpenSRF) may need the Raw() version
in the future.
This code path is used /a lot/, so the speedups are greatly appreciated.
I've run into one issue with these last 2 patches, in particular, there
seems to be a subtle difference in uescape() and buffer_append_uescape().
The majority of the JSON parsing succeeds, but parsing certain
bibliographic data (MARC records) always results in NULL when attempting
to encode the containing object (biblio_record entry objects). If I
replace the call to buffer_append_uescape() with a call to uescape() in
osrf_json_object.c, then all is well.
I would send you some sample data to test, but I've yet to recreate the
issue outside of a running system. For now, I'm adding some logging to
see if I can track it down. I just wanted to pass this on in case any
lightbulbs went off. I'll let you if I find something...
Thanks, Scott
-b
--
Bill Erickson
| VP, Software Development & Integration
| Equinox Software, Inc. / The Evergreen Experts
| phone: 877-OPEN-ILS (673-6457)
| email: [EMAIL PROTECTED]
| web: http://esilibrary.com
