Geoffrey Young wrote:
I'd just like to see that refactored so there is no duplication between
perl-script and modperl handlers. Perhaps use some macros.


ok, I incorporated your suggestions - here is a new patch.

the only thing I didn't really see was how to refactor the modperl and
perl-script handlers any more than they already are. I mean, sure, we can
shuffle stuff around, but each approach I can think of exchanges (very)
little duplication for other things. for instance, I could move the call to
modperl_env_request_popualte to modperl_response_handler_run, but it would
mean that (worst case) perl-script would have 3 calls to the interpreter
selection logic instead of (worst case) two.

That's the problem with your latest patch. You should not unselect the interp before you finish the callback. Nested calls to select are NOOPs (i.e. they figure out that the interpreter was already selected and won't do it again), same goes for unselect. So you need to move that unselect untill after _run:


> +    /* default is -SetupEnv, add if PerlOption +SetupEnv */
> +    if (MpDirSETUP_ENV(dcfg)) {
> +        MP_dRCFG;
>
> +#ifdef USE_ITHREADS
> +        if (MpInterpPUTBACK(interp)) {
> +            /* PerlInterpScope handler */
> +            modperl_interp_unselect(interp);
> +            rcfg->interp = NULL;
> +        }
> +#endif
> +
>      }

Otherwise it's indeed wasteful.

Now, given that the select code is run anyway inside the callback, just take the select/unselect parts out of if (MpDirSETUP_ENV(dcfg)), similar to perl-script.

[...]

But this can
(and probably should) be done after you commit your current work.


yeah, I think that's what I'd like to do.  so, save any gaping holes in the
current patch (such as forgetting to return the interpreter to the pool :)
I'd like to just go forward with it.  then, we have a series of tests
checked in an a functionality we know is working, so someone can refactor
without fear of breaking anything.

sound ok?

With the fix above, yes. I haven't read through the rest of the patch, assuming that it's the same as the previous one.


If you post a new patch for mod_perl.c, please post only it.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to