perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philippe M. Chiasson" <go...@ectoplasm.org>
Subject Re: [mp2] why Apache::PerlSections gets resolved so many times?
Date Sun, 02 Jan 2005 07:39:01 GMT
Stas Bekman wrote:
> Philippe M. Chiasson wrote:
> 
>>>>> Is parms->pool freed when config phase is over?
>>>>
>>>> Well, AFAIK, parms->pool is process->pconf, and that doesn't get wiped
>>>> until the process terminates.
>>>
>>> So we need to use a temp pool instead.
>>
>> Yes, most certainly, and that would have to be parms->temp_pool
>> [...]
> smells tabs to me :)

That was quick cut-n-paste of a svn diff ;-)

Now, for the real patch:

Index: src/modules/perl/modperl_cmd.c
===================================================================
--- src/modules/perl/modperl_cmd.c	(revision 123883)
+++ src/modules/perl/modperl_cmd.c	(working copy)
@@ -243,7 +243,7 @@
 
 MP_CMD_SRV_DECLARE(post_config_requires)
 {
-    apr_pool_t *p = parms->pool;
+    apr_pool_t *p = parms->temp_pool;
     apr_finfo_t finfo;
     MP_dSCFG(parms->server);
 
@@ -420,7 +420,7 @@
 
 MP_CMD_SRV_DECLARE(perl)
 {
-    apr_pool_t *p = parms->pool;
+    apr_pool_t *p = parms->temp_pool;
     const char *endp = ap_strrchr_c(arg, '>');
     const char *errmsg;
     char *code = "";
@@ -473,7 +473,7 @@
 
 MP_CMD_SRV_DECLARE(perldo)
 {
-    apr_pool_t *p = parms->pool;
+    apr_pool_t *p = parms->temp_pool;
     server_rec *s = parms->server;
     apr_table_t *options;
     modperl_handler_t *handler = NULL;

>> And that's just for <Perl> sections, and a cursory glance suggests 
>> that there
>> could be a bunch of other configuration directives that mistakenly use 
>> parms->pool.
>>
>> I'll have a look and report later.

I've looked carefully over modperl_cmd.c and I think the 3 places highlighted
in my patch (strangely, it's all my code) seem to be the only places where
using parms->temp_pool instead of parms->pool improves memory usages.
 
> and please log that onto todo/release so we don't forget to review the 
> code.

Not sure what you'd wanted to have logged, but I've just finished reviewing
the code, so commit and forget ?

Also, note this entry in todo/features_optimizations:

* consider using the temp pool for things that are needed only during
  the configuration and can be dropped before the actual serving is
  started. that pool gets destroyed right after the post-config phase
  is over.

>>> It's about time we should start thinking about optimizations. I'd 
>>> check other startup thingies that may use the wrong pool which wastes 
>>> memory at run-time.

Yup, as noted above. I've only looked over modperl_cmd.c so far, but there
are quite a lot more to possibly look at including all modperl_startup()
related things.
 
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5

Mime
View raw message