[syslog-ng] Per-thread scratch buffers

[Balazs Scheidler]

On Mon, 2011-10-31 at 10:16 +0100, Gergely Nagy wrote:
> Balazs Scheidler <bazsi at 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).

> 
> The expected usage is to acquire a buffer when needed, and release it
> whenever we don't need it anymore.
> 
> Spice it up with some __tls_deref & similar, and we're pretty much done,
> I think.
> 
> What do you think? (Meanwhile, I'll go ahead and see if this works)
> 

-- 
Bazsi