On Fri, Jul 11, 2014 at 2:03 PM, Jan Kaluža <[email protected]> wrote: > On 07/11/2014 01:23 PM, Yann Ylavic wrote: >> >> On Fri, Jul 11, 2014 at 1:07 PM, Jan Kaluža <[email protected]> wrote: >>> >>> On 07/11/2014 12:53 PM, Yann Ylavic wrote: >>>> >>>> >>>> Hi Jan, >>>> >>>> On Fri, Jul 11, 2014 at 11:17 AM, Jan Kaluža <[email protected]> wrote: >>>>> >>>>> >>>>> I've updated mod_journald to latest trunk and added documentation. You >>>>> can >>>>> check the patch against trunk at >>>>> >>>>> >>>>> <http://people.apache.org/~jkaluza/patches/mod_journald/0001-mod_journald.patch>. >>>> >>>> >>>> >>>> +static apr_pool_t *journald_info_get_pool(const ap_errorlog_info *info) >>>> +{ >>>> + if (info->s && info->s->process && info->s->process->pool) >>>> + return info->s->process->pool; >>>> + if (info->pool) >>>> + return info->pool; >>>> + if (info->c && info->c->pool) >>>> + return info->c->pool; >>>> + if (info->r && info->r->pool) >>>> + return info->r->pool; >>>> + return 0; >>>> +} >>>> >>>> Shouldn't this be in the reverse order? >>> >>> >>> >>> Hm, I think my original intention here was to try as first the pools >>> which >>> have the biggest chance to be available. Since I'm creating subpool of >>> the >>> pool returned by this method, it should be OK to use even >>> s->process->pool. >>> Maybe it's useless try to optimize things... >> >> >> When a subpool is created, a lock is acquired/released on the parent >> pool, so maybe for threaded MPMs here we'd better not hold it process >> wide for every log? >> > > Didn't know that. I've updated the patch and the order is now reversed. I'm > thinking about not creating subpool in case when r->pool is available, but > I'm not sure if more complicated code is worth doing it (not sure if the > performance penalty of r->pool subpool creation is big enough). I will do > some measurement with CustomLog.
I don't think it's suitable to allocate all the request logs on its pool (that can be huge), I prefer the current subpool implementation.
