[syslog-ng] Per-thread scratch buffers

[Balazs Scheidler]

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 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).

After I've posted this I noticed the implementation in follow-up
patches. Those comments came after this, so read them too :)

-- 
Bazsi