Stas Bekman wrote:
Philippe M. Chiasson wrote:
Stas Bekman wrote:
Philippe M. Chiasson wrote:
Stas Bekman wrote:
[...]

I've traced it down to this line :

       handler = modperl_handler_new(p, handler_name);

So basically, to be able to use the existing modperl_handler_* framework to
run <perl> sections, I am creating a brand new handler for each section,
creating that behaviour.

But can you arrange for it not to think that it wasn't resolved yet, keeping things as they are?

Not sure I follow you there. In most case, there will be only one handler, but
there might be multiple ones, and I'd want to make sure to create one and only
one handler for each of them.

Only one? That doesn't seem to be the case. It creates a new handler on every <Perl> section.

What I am saying is that in most cases, there would only be a need for a single handler 'Apache::PerlSections' and it would be invoked once per <Perl> sections.

The module Apache::PerlSections is loaded only one, it's the tracing message that was misleading.

Not only the tracing message, also the fact that it creates a new modperl_handler_t each time around. That's the inneficiency I was refering to.

How do you see that as possible ?

I'll fix the tracing. But I've also played a bit to speed things up a bit and save some memory (see [1]), but I'm not sure whether this is worth the maintenance overhead.

Thats definitely a -1 from my point of view. It's duplicating a lot of code that should stay in modperl_handler.c

I believe that the <Perl> sections handler(s) should probably be stashed
in modperl_config_srv_t->handlers_per_srv just like the other kind of
handlers and be retrieved with modperl_handler_lookup_handlers().

IMO, that makes for a much more generic solution, and if there are 50 <Perl>
sections (and a few <Perl> sections with their own custom handlers), there
would be only a few modperl_handler_t created.

In any case, why the handler is not freed when <Perl> section is done?

That's a certain _leak_ right there.

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.

If it is then this effort is certainly moot.

See above, but I think there is certainly a better solution.

<Pseudo-code>
if (!modperl_handler_lookup_handlers()) {
 modperl_handler_new();
 modperl_cmd_push_handlers();
}
</Pseudo-code>

I'll submit a patch to illustrate this later on.

[1]
Index: src/modules/perl/modperl_cmd.c
===================================================================
--- src/modules/perl/modperl_cmd.c    (revision 123453)
+++ src/modules/perl/modperl_cmd.c    (working copy)
@@ -554,6 +554,20 @@
         SV *saveconfig = MP_PERLSECTIONS_SAVECONFIG_SV;
         AV *args = Nullav;

+        /* resolve the handler here w/o going through
+         * modperl_handler_resolve, so it will be faster and memory
+         * will be saved (e.g. handler dup is avoided)
+         */
+        modperl_require_module(aTHX_ handler->name, TRUE);
+        handler->mgv_obj = modperl_mgv_new(p);
+        handler->mgv_obj->len = strlen(handler->name);
+        handler->mgv_obj->name =
+            apr_pstrndup(p, handler->name, handler->mgv_obj->len);
+        handler->mgv_cv = modperl_mgv_compile(aTHX_ p, handler->name);
+        modperl_mgv_append(aTHX_ p, handler->mgv_cv, "handler");
+        MpHandlerPARSED_On(handler);
+        MpHandlerMETHOD_On(handler);
+
         modperl_handler_make_args(aTHX_ &args,
                                   "Apache::CmdParms", parms,
                                   "APR::Table", options,


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

Attachment: signature.asc
Description: OpenPGP digital signature



Reply via email to