dbs [Thu, 20 Nov 2008 01:27:16 +0000 (01:27 +0000)]
Force Class::DBI install, as dependencies have started making test t/11 (triggers) fail;
We don't use trigger support in Class::DBI, so forcing the install should be okay.
erickson [Tue, 18 Nov 2008 22:23:29 +0000 (22:23 +0000)]
2 Patches from Scott McKellar, with slight modification:
This patch adds a new function buffer_append_uescape(), a streamlined
replacement for uescape(). The optimizations are as follows:
1. As described in an earlier post, the new function appends the uescaped
text to an existing growing_buffer, rather than into an internal one that
it must create and destroy. It thereby saves two mallocs and two frees.
In addition, the calling code doesn't have to do a buffer_add().
2. Because it doesn't create an internal growing_buffer, it doesn't need
to do a strlen() to determine how big a buffer to allocate.
3. Since the new version doesn't have a boolean parameter, it doesn't have
to test the boolean.
4. For characters < 128 (i.e. ASCII characters), I rearranged the order of
the tests so that non-control characters are recognized immediately. In
uescape() we first go through a switch/case looking for several specific
control characters like '\b' and '\n'. In practice most characters are
not control characters, so this rearrangement saves a few CPU cycles.
5. For control characters, uescape() uses buffer_fadd() to format the
character into hex. Now, buffer_fadd is slow because it uses vsnprintf()
twice, once to determine a buffer size and once to do the formatting. In
turn vsnprintf() is slow because it has to parse the format string. In
this case we don't need vsnprintf() because we already know exactly how
big the buffer needs to be, and the formatting is simple. I eliminated
buffer_fadd() and formatted the hex manually with a little bit-twiddling.
Some of these optimizations could be applied to uescape(), but I haven't
bothered, partly because I wanted a clean comparison for benchmarking
purposes and partly because I expect uescape() to disappear from use
(though I am leaving it available).
=====
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.
erickson [Mon, 17 Nov 2008 03:17:50 +0000 (03:17 +0000)]
Patch from Scott McKellar:
This patch is mostly a couple of tweaks to the growing_buffer code, loosely
related to my previous patch to utils.h. There is also a small tweak to
uescape().
1. in buffer_add() I replaced strcat() with strcpy() for appending the new
string. Since we already know where the end of the old string is, we don't
need to ask strcat() to find it for us.
2. In buffer_reset(), the old code contains the following:
osrf_clearbuf( gb->buf, sizeof(gb->buf) );
The evident intent is to clear the buffer. However sizeof(gb->buf) is not
the size of the buffer, it's the size of the pointer to the buffer. We
were clearing only the first four bytes or so. I changed the line to:
osrf_clearbuf( gb->buf, gb->size );
3. Also in buffer_reset(), I added a line to populate the first byte of
the buffer with a nul, to ensure that the length of the (empty) string matches the n_used member.
4. In uescape(), we were examining the contents of string[] without first
verifying that string was not NULL. The result would be undefined
behavior if string were ever NULL. I added a couple of lines to treat
a NULL pointer as if it were a pointer to an empty string.
erickson [Mon, 17 Nov 2008 02:56:40 +0000 (02:56 +0000)]
Patch from Scott McKellar:
This patch fixes a bug in the OSRF_BUFFER_ADD_CHAR macro.
Like the corresponding buffer_add_char function, this macro appends a
specified character to a growing_buffer. Unlike the function, however, the
existing version of the macro does not also append a terminal nul.
This bug had gone unnoticed because, most of the time, the rest of the
buffer is already filled with nuls, left over from the initial creation of
the growing_buffer. I stumbled across the problem when, in the course of
writing a test harness for some other changes, I called buffer_reset()
in order to reuse an existing growing_buffer instead of destroying and
re-creating it.
With debugging turned on, buffer_reset() fills the buffer with exclamation
points, leaving a nul only in the very last byte. Later, if we use
buffer_add() or buffer_fadd() to extend the string stored in the
growing_buffer, it uses strcat() to append the new characters. The result
is a buffer overflow.
Actually buffer_reset() should place a nul in the first byte of the buffer.
Tomorrow I shall submit a patch to that effect.
dbs [Mon, 27 Oct 2008 05:07:06 +0000 (05:07 +0000)]
Clean up the source tree a little more:
* Delete setup.py.in (as we're not modifying it)
* Make math_client.py be modified with SYSCONFDIR location per other scripts
(although slightly longer term we'll need to stop modifying all of these
in place, because that doesn't work after the first ./configure run)
* Add a few files to automake's tracking so that make dist is a little happier
erickson [Fri, 24 Oct 2008 16:31:07 +0000 (16:31 +0000)]
the pool cleanup handler which was thought to only run on top-level child process exit is running on cloned processes cleanup. this is how mod_cgi runs scripts. disabling cleanup for now. note: this cleanup is new to 1.0
erickson [Mon, 13 Oct 2008 20:44:50 +0000 (20:44 +0000)]
io::socket::inet, somewhere between version 1.29 and 1.31, requires the peerport to be explicitly cast to an int. also updated error handling to use correct error var
added a configurable startup pause delay. after opensrf.settings has been started (which will always be the first service to start), pause the top-level process for the configured amount of time before starting any more services
config file, context, and cache server now have apache config settings. keeping a static version of the translator on hand to reduce a layer of malloc/free
This script provides per-service control of OpenSRF Perl processes. In other words, you can
stop/start/restart individual services. Note that you can only stop services that have
been started with this script.
TODO
Add start_all, stop_all, and restart_all to manage all host-specific Perl services
Give Apache modules the support they need to work
Push the apxs compile stage into the local install steps as it seems to require root privileges to write during build
Commit autotools patch from Kevin Beswick
(adjusted slightly for Bill's objson API compatibility layer removal)
Enclosed is a patch to update the OpenSRF autotools implementation. It
fixes most bugs such as:
-correctly replacing hardcoded directory paths in various files
-correctly implementing clean, and uninstall make targets
-fixes building src/c-apps modules without the lib prefix
-builds the src/gateway apache modules with apxs
-fixed the naming of the opensrf-c binary
Also, it implements more of the autotools features:
-rolling a tarball with make dist
-enables VPATH (parallel) builds
-checking a distribution with make distcheck
-------------------------------------------
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.
(d) 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(s) involved.
Merge the following patches from Kevin Beswick:
* fixed location to copy apachetools.h from ...
* another typo...
* fixed directory error in copying of header file.
* move one more copy instruction for a header file.
* create the perldir and jsdir
* changed the place where headers are copied... fixed an error which caused them to be copied wrong.
* fixed a capitalization typo issue in src/Makefile.am
* updated opensrf.xml.example to use C math and dbmath implementation rather than Perl
* fixed make distcheck problems -- builddir needed to be changed to srcdir
* fixed directory replacement in .c file problem stopped the command added in the previous revision from running multiple times, fixed the location of the file to execute the command on.
* changed where the directory replacement happens for osrf_json_gateway.c (it was after it was compiled rather than before)
* corrected another error with installing header files ( can't install directories recursively through the 'prefix_PRIMARY = files' apparently)
* went back to old way of copying perlmods and javascript. it will still be included in dist due to EXTRA_DIST in root makefile
* fix install of src/javascript and src/perlmods
* nobase_dist_lib_DATA defined twice... oops!
* fixed problem with order of execution of targets (install-data-local was being executed before files were copied. resulted in an error)
* fixed make dist, and changed ways that files are copied to their installed locations
* fixed path substitution for the rest of the files with hardcoded paths to ensure correct default functionality of opensrf
A little more autotools love:
* Avoid the use of tmp dir by using noinst_ prefix for timejson
* Remove DEF_LDLIBS usage (it enabled compiling, but did not link to the
libraries) and replace with specific LDADD and LIBADD options