On Mon, 2011-10-31 at 21:38 +0100, Balazs Scheidler wrote:
On Mon, 2011-10-31 at 10:16 +0100, Gergely Nagy wrote:
Balazs Scheidler <bazsi@balabit.hu> writes:
The alternative idea I was playing with was to create a new API for managing per-thread scratch buffers that could be reused even between multiple independent portions of syslog-ng.
The way it would work: - scratch buffers are a numbered array of GString buffers, allocated for _each_ thread.
Any reason we want per-thread scratch buffers? Apart from not needing to use a lock in this case..
that's the point, and to keep GString instances around.
As a first step in implementing this, I had something like the following in mind:
static GTrashStack *scratch_buffers;
GString *scratch_buffer_acquite (void)
a typo in the function name.
{ GString *s;
s = g_trash_stack_pop (scratch_buffers); if (!s) s = g_string_new (NULL); else g_string_set_size (s, 0); return s; }
are you sure that once you get your GString instance back from GTrashStack, it'll be a valid GString?
GTrashStack seems to corrupt the first pointer of each data block pushed to it (which is the "str" pointer in this case).
void scratch_buffer_release (GString *s) { g_trash_stack_push (s); }
void scratch_buffers_reset (void) { GString *s;
while ((s = g_trash_stack_pop (scratch_buffers)) != NULL) g_string_free (s, TRUE); }
I wouldn't want to free the GString instances, but reuse them. This is the other point: to avoid having to allocate new GString instances all the time.
The implementation I had in mind was: - dynamically sized array of GString instances (per-thread) - a stack pointer that contains the index within the array where we are at. Items below the pointer are used, above are not. - acquire: if there are unused items in the array, return it and increase the pointer. if there are no unused items, allocate new and store it in the array. - release: if the item to be freed is the one handed out last, decrement the pointer, else do nothing. - reset: reset the pointer to 0.
This way: - the number of GString instances depend on their usage. - GStrings are only allocated rarely, if the parallel number of GString buffers increase for some reason. - no need for locking as scratch buffers are per-thread
Of course it only shows its potential if all the similar cases already in the code are converted to scratch buffers (log_template_format, template functions, value-pairs, etc).
After I've posted this I noticed the implementation in follow-up patches. Those comments came after this, so read them too :) -- Bazsi