You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2004/10/18 05:32:14 UTC
[mp2 patch] anon subs B::Deparse deployment replacement
This patch replaces the B::Deparse logic for anonsubs with a different
approach.
I'd like to thanks Philippe and Dave Mitchell who helped me solve some
leakage and crashes problems.
Here is how anon-subs are now handled:
We have two ways anon-subs can be registered by:
A) at startup from httpd.conf:
PerlTransHandler 'sub { ... }'
B) run-time perl code
$r->push_handlers(PerlTransHandler => sub { .... });
In the case of non-threaded perl, we just compile A or grab B and
store it in the mod_perl struct and call it when it's used. No
problems here
In the case of threads, things get more complicated. we no longer can
store the CV value of the compiled anon-sub, since when perl_clone is
called each interpreter will have a different CV value. since we need
to be able to have 1 entry for each anon-sub across all interpreters a
different solution is needed. to remind in the case of named subs, we
just store the name of the sub and look its corresponding CV when we
need it.
The used solution: each process has a global counter, which always
grows. Every time a new anon-sub is encountered, a new ID is allocated
from that process-global counter and that ID is stored in the mod_perl
struct. The compiled CV is stored as
$PL_modglobal{ANONSUB}{$id} = CV;
when perl_clone is called, each clone will clone that CV value, but we
will still be able to find it, since we stored it in the hash. so we
retrieve the CV value, whatever it is and we run it.
That explanation can also be written and run in perl:
use threads;
our %h;
$h{x} = eval 'sub { print qq[this is sub @_\n] }';
$h{x}->("main");
threads->new(sub { $h{x}->(threads->self->tid)});
remaining issues with anon-subs:
- i'm not sure if I can use a static global variable as a mutex (see
XXX in the patch)
- should we do mutex locking only for threaded mpm? (see XXX in the
patch) in which case, it's probably going to be too expensive to
test modperl_threaded_mpm or not?). if all anonsub handlers are
compiled at run-time, there will be no overhead what so over at
run-time. so probably this can be just documented.
- when PerlScope is utilized we need to croak if the handler wasn't
compiled+registered before perl_clone, since if this happens we may
have one interpreter that compiles and registers the anon-sub
handler but another one is run
- we need more tests
Following the patch inlined and attached:
Index: src/modules/perl/mod_perl.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/mod_perl.c,v
retrieving revision 1.223
diff -u -r1.223 mod_perl.c
--- src/modules/perl/mod_perl.c 30 Sep 2004 03:39:24 -0000 1.223
+++ src/modules/perl/mod_perl.c 18 Oct 2004 03:27:44 -0000
@@ -280,6 +280,8 @@
newSVpv(ap_server_root_relative(p, "lib/perl"), 0));
#endif /* MP_COMPAT_1X */
+ modperl_handler_anon_init(aTHX_ p);
+
if (!modperl_config_apply_PerlRequire(s, scfg, perl, p)) {
exit(1);
}
Index: src/modules/perl/modperl_callback.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_callback.c,v
retrieving revision 1.80
diff -u -r1.80 modperl_callback.c
--- src/modules/perl/modperl_callback.c 30 Sep 2004 03:30:29 -0000 1.80
+++ src/modules/perl/modperl_callback.c 18 Oct 2004 03:27:44 -0000
@@ -67,20 +67,8 @@
if (MpHandlerANON(handler)) {
#ifdef USE_ITHREADS
- /* it's possible that the interpreter that is running the anon
- * cv, isn't the one that compiled it. so to be safe need to
- * re-eval the deparsed form before using it.
- * XXX: possible optimizations, see modperl_handler_new_anon */
- SV *sv = eval_pv(handler->name, TRUE);
- cv = (CV*)SvRV(sv);
+ cv = modperl_handler_anon_get(aTHX_ handler->mgv_obj);
#else
- /* the same interpreter that has compiled the anon cv is used
- * to run it */
- if (!handler->cv) {
- SV *sv = eval_pv(handler->name, TRUE);
- handler->cv = (CV*)SvRV(sv); /* cache */
- SvREFCNT_inc(handler->cv);
- }
cv = handler->cv;
#endif
}
@@ -90,7 +78,6 @@
cv = modperl_mgv_cv(gv);
}
else {
-
const char *name;
modperl_mgv_t *symbol = handler->mgv_cv;
Index: src/modules/perl/modperl_handler.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_handler.c,v
retrieving revision 1.28
diff -u -r1.28 modperl_handler.c
--- src/modules/perl/modperl_handler.c 15 Aug 2004 20:06:51 -0000 1.28
+++ src/modules/perl/modperl_handler.c 18 Oct 2004 03:27:44 -0000
@@ -43,6 +43,157 @@
return handler;
}
+/* How anon-subs are handled:
+ * We have two ways anon-subs can be registered
+ * A) at startup from httpd.conf:
+ * PerlTransHandler 'sub { ... }'
+ * B) run-time perl code
+ * $r->push_handlers(PerlTransHandler => sub { .... });
+ *
+ * In the case of non-threaded perl, we just compile A or grab B and
+ * store it in the mod_perl struct and call it when it's used. No
+ * problems here
+ *
+ * In the case of threads, things get more complicated. we no longer
+ * can store the CV value of the compiled anon-sub, since when
+ * perl_clone is called each interpreter will have a different CV
+ * value. since we need to be able to have 1 entry for each anon-sub
+ * across all interpreters a different solution is needed. to remind
+ * in the case of named subs, we just store the name of the sub and
+ * look its corresponding CV when we need it.
+ *
+ * The used solution: each process has a global counter, which always
+ * grows. Every time a new anon-sub is encountered, a new ID is
+ * allocated from that process-global counter and that ID is stored in
+ * the mod_perl struct. The compiled CV is stored as
+ * $PL_modglobal{ANONSUB}{$id} = CV;
+ * when perl_clone is called, each clone will clone that CV value, but
+ * we will still be able to find it, since we stored it in the
+ * hash. so we retrieve the CV value, whatever it is and we run it.
+ *
+ * that explanation can be written and run in perl:
+ *
+ * use threads;
+ * our %h;
+ * $h{x} = eval 'sub { print qq[this is sub @_\n] }';
+ * $h{x}->("main");
+ * threads->new(sub { $h{x}->(threads->self->tid)});
+ *
+ * XXX: more nuances will follow
+ */
+
+static int MP_anon_subs_counter;
+#if MP_THREADED
+/* XXX: is this correct at all? */
+perl_mutex MP_anon_subs_lock;
+#endif
+
+#if MP_THREADED
+static apr_status_t modperl_handler_anon_mutex_cleanup(void *data)
+{
+ MP_TRACE_g(MP_FUNC, "destroy lock for anonsubs");
+ MUTEX_DESTROY(&MP_anon_subs_lock);
+
+ return APR_SUCCESS;
+}
+#endif
+
+void modperl_handler_anon_init(pTHX_ apr_pool_t *p)
+{
+ modperl_modglobal_key_t *gkey =
+ modperl_modglobal_lookup(aTHX_ "ANONSUB");
+ MP_TRACE_h(MP_FUNC, "init $PL_modglobal{ANONSUB} = []");
+ MP_MODGLOBAL_STORE_HV(gkey);
+
+ MP_anon_subs_counter = 0;
+
+ /* XXX: should the mutex locking be needed only if
+ * modperl_threaded_mpm? */
+
+#if MP_THREADED
+ MUTEX_INIT(&MP_anon_subs_lock);
+
+ apr_pool_cleanup_register(p, NULL,
+ modperl_handler_anon_mutex_cleanup,
+ apr_pool_cleanup_null);
+#endif
+}
+
+static int modperl_handler_anon_next_id(void)
+{
+
+#ifdef USE_ITHREADS
+ MUTEX_LOCK(&MP_anon_subs_lock);
+#endif
+
+ ++MP_anon_subs_counter;
+
+#ifdef USE_ITHREADS
+ MUTEX_UNLOCK(&MP_anon_subs_lock);
+#endif
+
+ return MP_anon_subs_counter;
+}
+
+/* allocate and populate the anon handler sub-struct */
+MP_INLINE modperl_mgv_t *modperl_handler_anon_next(pTHX_ apr_pool_t *p)
+{
+ /* re-use modperl_mgv_t entry which is otherwise is not used
+ * by anon handlers */
+ modperl_mgv_t *anon =
+ (modperl_mgv_t *)apr_pcalloc(p, sizeof(*anon));
+
+ anon->name = apr_psprintf(p, "%d", modperl_handler_anon_next_id());
+ anon->len = strlen(anon->name);
+ PERL_HASH(anon->hash, anon->name, anon->len);
+
+ MP_TRACE_h(MP_FUNC, "[%s] new anon handler: '%s'",
+ modperl_pid_tid(p), anon->name);
+ return anon;
+}
+
+MP_INLINE void modperl_handler_anon_add(pTHX_ modperl_mgv_t *anon, CV *cv)
+{
+ modperl_modglobal_key_t *gkey =
+ modperl_modglobal_lookup(aTHX_ "ANONSUB");
+ HE *he = MP_MODGLOBAL_FETCH(gkey);
+ HV *hv;
+
+ if (!(he && (hv = (HV*)HeVAL(he)))) {
+ Perl_croak(aTHX_ "can't find ANONSUB top entry (get)");
+ }
+
+ SvREFCNT_inc(cv);
+ if (!(*hv_store(hv, anon->name, anon->len, (SV*)cv, anon->hash))) {
+ SvREFCNT_dec(cv);
+ Perl_croak(aTHX_ "hv_store of '%s' has failed!", anon->name);
+ }
+
+ MP_TRACE_h(MP_FUNC, "anonsub '%s' added", anon->name);
+}
+
+MP_INLINE CV *modperl_handler_anon_get(pTHX_ modperl_mgv_t *anon)
+{
+ modperl_modglobal_key_t *gkey =
+ modperl_modglobal_lookup(aTHX_ "ANONSUB");
+ HE *he = MP_MODGLOBAL_FETCH(gkey);
+ HV *hv;
+ SV *sv;
+
+ if (!(he && (hv = (HV*)HeVAL(he)))) {
+ Perl_croak(aTHX_ "can't find ANONSUB top entry (get)");
+ }
+
+ if ((he = hv_fetch_he(hv, anon->name, anon->len, anon->hash))) {
+ sv = HeVAL(he);
+ MP_TRACE_h(MP_FUNC, "anonsub get '%s'", anon->name);
+ }
+ else {
+ Perl_croak(aTHX_ "can't find ANONSUB's '%s' entry", anon->name);
+ }
+
+ return (CV*)sv;
+}
static
modperl_handler_t *modperl_handler_new_anon(pTHX_ apr_pool_t *p, CV *cv)
@@ -53,34 +204,16 @@
MpHandlerANON_On(handler);
#ifdef USE_ITHREADS
- /* XXX: perhaps we can optimize this further. At the moment when
- * perl w/ ithreads is used, we always deparse the anon subs
- * before storing them and then eval them each time they are
- * used. This is because we don't know whether the same perl that
- * compiled the anonymous sub is used to run it.
- *
- * A possible optimization is to cache the CV and use that cached
- * value w/ or w/o deparsing at all if:
- *
- * - the mpm is non-threaded mpm and no +Clone/+Parent is used
- * (i.e. no perl pools) (no deparsing is needed at all)
- *
- * - the interpreter that has supplied the anon cv is the same
- * interpreter that is executing that cv (requires storing aTHX
- * in the handler's struct) (need to deparse in case the
- * interpreter gets switched)
- *
- * - other cases?
- */
- handler->cv = NULL;
- handler->name = modperl_coderef2text(aTHX_ p, cv);
- MP_TRACE_h(MP_FUNC, "[%s] new deparsed anon handler:\n%s\n",
- modperl_pid_tid(p), handler->name);
+ handler->cv = NULL;
+ handler->name = NULL;
+ handler->mgv_obj = modperl_handler_anon_next(aTHX_ p);
+ modperl_handler_anon_add(aTHX_ handler->mgv_obj, cv);
#else
/* it's safe to cache and later use the cv, since the same perl
* interpeter is always used */
- handler->cv = cv;
+ handler->cv = cv;
handler->name = NULL;
+
MP_TRACE_h(MP_FUNC, "[%s] new cached cv anon handler\n",
modperl_pid_tid(p));
#endif
Index: src/modules/perl/modperl_handler.h
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_handler.h,v
retrieving revision 1.10
diff -u -r1.10 modperl_handler.h
--- src/modules/perl/modperl_handler.h 4 Mar 2004 06:01:07 -0000 1.10
+++ src/modules/perl/modperl_handler.h 18 Oct 2004 03:27:44 -0000
@@ -22,6 +22,11 @@
MP_HANDLER_ACTION_SET
} modperl_handler_action_e;
+void modperl_handler_anon_init(pTHX_ apr_pool_t *p);
+MP_INLINE modperl_mgv_t *modperl_handler_anon_next(pTHX_ apr_pool_t *p);
+MP_INLINE void modperl_handler_anon_add(pTHX_ modperl_mgv_t *anon, CV *cv);
+MP_INLINE CV *modperl_handler_anon_get(pTHX_ modperl_mgv_t *anon);
+
#define modperl_handler_array_new(p) \
apr_array_make(p, 1, sizeof(modperl_handler_t *))
Index: src/modules/perl/modperl_mgv.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_mgv.c,v
retrieving revision 1.38
diff -u -r1.38 modperl_mgv.c
--- src/modules/perl/modperl_mgv.c 15 Oct 2004 22:38:14 -0000 1.38
+++ src/modules/perl/modperl_mgv.c 18 Oct 2004 03:27:47 -0000
@@ -193,9 +193,35 @@
}
if (strnEQ(name, "sub ", 4)) {
- MP_TRACE_h(MP_FUNC, "handler is anonymous\n");
- MpHandlerANON_On(handler);
+ SV *sv;
+ CV *cv;
MpHandlerPARSED_On(handler);
+ MpHandlerANON_On(handler);
+
+ ENTER;SAVETMPS;
+ sv = eval_pv(name, TRUE);
+ if (!(SvROK(sv) && (cv = (CV*)SvRV(sv)) && (CvFLAGS(cv) &
CVf_ANON))) {
+
+ Perl_croak(aTHX_ "expected anonymous sub, got '%s'\n", name);
+ }
+
+#ifdef USE_ITHREADS
+ handler->cv = NULL;
+ handler->name = NULL;
+ handler->mgv_obj = modperl_handler_anon_next(aTHX_ p);
+ modperl_handler_anon_add(aTHX_ handler->mgv_obj, cv);
+ MP_TRACE_h(MP_FUNC, "[%s] new anon handler",
+ modperl_pid_tid(p));
+#else
+ SvREFCNT_inc(cv);
+ handler->cv = cv;
+ handler->name = NULL;
+ MP_TRACE_h(MP_FUNC, "[%s] new cached-cv anon handler",
+ modperl_pid_tid(p));
+#endif
+
+ FREETMPS;LEAVE;
+
return 1;
}
Index: src/modules/perl/modperl_perl_global.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_perl_global.c,v
retrieving revision 1.23
diff -u -r1.23 modperl_perl_global.c
--- src/modules/perl/modperl_perl_global.c 14 Oct 2004 00:02:05 -0000 1.23
+++ src/modules/perl/modperl_perl_global.c 18 Oct 2004 03:27:47 -0000
@@ -15,30 +15,14 @@
#include "mod_perl.h"
-static void modperl_perl_global_init(pTHX_ modperl_perl_globals_t *globals)
-{
- globals->env.gv = PL_envgv;
- globals->inc.gv = PL_incgv;
- globals->defout.gv = PL_defoutgv;
- globals->rs.sv = &PL_rs;
- globals->end.av = &PL_endav;
- globals->end.key = MP_MODGLOBAL_END;
-}
-
/* XXX: PL_modglobal thingers might be useful elsewhere */
-#define MP_MODGLOBAL_FETCH(gkey) \
- hv_fetch_he(PL_modglobal, (char *)gkey->val, gkey->len, gkey->hash)
-
-#define MP_MODGLOBAL_STORE_HV(gkey) \
- (HV*)*hv_store(PL_modglobal, gkey->val, gkey->len, \
- (SV*)newHV(), gkey->hash)
-
#define MP_MODGLOBAL_ENT(key) \
{key, "ModPerl::" key, MP_SSTRLEN("ModPerl::") + MP_SSTRLEN(key), 0}
static modperl_modglobal_key_t MP_modglobal_keys[] = {
MP_MODGLOBAL_ENT("END"),
+ MP_MODGLOBAL_ENT("ANONSUB"),
{ NULL },
};
@@ -64,6 +48,16 @@
}
return NULL;
+}
+
+static void modperl_perl_global_init(pTHX_ modperl_perl_globals_t *globals)
+{
+ globals->env.gv = PL_envgv;
+ globals->inc.gv = PL_incgv;
+ globals->defout.gv = PL_defoutgv;
+ globals->rs.sv = &PL_rs;
+ globals->end.av = &PL_endav;
+ globals->end.key = MP_MODGLOBAL_END;
}
/*
Index: src/modules/perl/modperl_perl_global.h
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_perl_global.h,v
retrieving revision 1.14
diff -u -r1.14 modperl_perl_global.h
--- src/modules/perl/modperl_perl_global.h 7 Apr 2004 02:56:30 -0000 1.14
+++ src/modules/perl/modperl_perl_global.h 18 Oct 2004 03:27:47 -0000
@@ -16,6 +16,13 @@
#ifndef MODPERL_PERL_GLOBAL_H
#define MODPERL_PERL_GLOBAL_H
+#define MP_MODGLOBAL_FETCH(gkey) \
+ hv_fetch_he(PL_modglobal, (char *)gkey->val, gkey->len, gkey->hash)
+
+#define MP_MODGLOBAL_STORE_HV(gkey) \
+ (HV*)*hv_store(PL_modglobal, gkey->val, gkey->len, \
+ (SV*)newHV(), gkey->hash)
+
typedef struct {
const char *name;
const char *val;
Index: src/modules/perl/modperl_types.h
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_types.h,v
retrieving revision 1.78
diff -u -r1.78 modperl_types.h
--- src/modules/perl/modperl_types.h 20 Sep 2004 18:14:48 -0000 1.78
+++ src/modules/perl/modperl_types.h 18 Oct 2004 03:27:47 -0000
@@ -176,13 +176,16 @@
typedef struct modperl_handler_t modperl_handler_t;
-struct modperl_handler_t{
+struct modperl_handler_t {
+ /* could be:
+ * - the lightweight gv for named subs
+ * - the lookup data in $PL_modperl{ANONSUB}
+ */
modperl_mgv_t *mgv_obj;
modperl_mgv_t *mgv_cv;
/* could be:
- * - a subroutine name
- * - a subroutine source code as a string (anon subs)
- * - NULL, when .cv is set (anon subs)
+ * - a subroutine name for named subs
+ * - NULL for anon subs
*/
const char *name;
CV *cv;
Index: src/modules/perl/modperl_util.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_util.c,v
retrieving revision 1.84
diff -u -r1.84 modperl_util.c
--- src/modules/perl/modperl_util.c 14 Oct 2004 18:25:01 -0000 1.84
+++ src/modules/perl/modperl_util.c 18 Oct 2004 03:27:47 -0000
@@ -700,86 +700,6 @@
return package;
}
-char *modperl_coderef2text(pTHX_ apr_pool_t *p, CV *cv)
-{
- dSP;
- int count;
- SV *bdeparse;
- SV *use;
- char *text;
- int tainted_orig;
-
- /* B::Deparse >= 0.61 needed for blessed code references.
- * 0.6 works fine for non-blessed code refs.
- * notice that B::Deparse is not CPAN-updatable.
- * 0.61 is available starting from 5.8.0
- */
-
- /*
- Perl_load_module(aTHX_ PERL_LOADMOD_NOIMPORT,
- newSVpvn("B::Deparse", 10),
- newSVnv(SvOBJECT((SV*)cv) ? 0.61 : 0.60));
- * Perl_load_module() was causing segfaults in the worker MPM.
- * this is a work around until we can find the problem with
- * Perl_load_module()
- * See: http://marc.theaimsgroup.com/?t=109684579900001&r=1&w=2
- */
- use = newSVpv("use B::Deparse ", 15);
- if (SvOBJECT((SV*)cv)) {
- sv_catpvn(use, "0.61", 3);
- }
- sv_catpvn(use, " ();", 4);
-
- tainted_orig = PL_tainted;
- TAINT_NOT;
- eval_sv(use, G_DISCARD);
- PL_tainted = tainted_orig;
- sv_free(use);
-
- ENTER;
- SAVETMPS;
-
- /* create the B::Deparse object */
- PUSHMARK(sp);
- XPUSHs(sv_2mortal(newSVpvn("B::Deparse", 10)));
- PUTBACK;
- count = call_method("new", G_SCALAR);
- SPAGAIN;
- if (count != 1) {
- Perl_croak(aTHX_ "Unexpected return value from B::Deparse::new\n");
- }
- if (SvTRUE(ERRSV)) {
- Perl_croak(aTHX_ "error: %s", SvPVX(ERRSV));
- }
- bdeparse = POPs;
-
- PUSHMARK(sp);
- XPUSHs(bdeparse);
- XPUSHs(sv_2mortal(newRV_inc((SV*)cv)));
- PUTBACK;
- count = call_method("coderef2text", G_SCALAR);
- SPAGAIN;
- if (count != 1) {
- Perl_croak(aTHX_ "Unexpected return value from "
- "B::Deparse::coderef2text\n");
- }
- if (SvTRUE(ERRSV)) {
- Perl_croak(aTHX_ "error: %s", SvPVX(ERRSV));
- }
-
- {
- STRLEN n_a;
- text = apr_pstrcat(p, "sub ", POPpx, NULL);
- }
-
- PUTBACK;
-
- FREETMPS;
- LEAVE;
-
- return text;
-}
-
SV *modperl_apr_array_header2avrv(pTHX_ apr_array_header_t *array)
{
AV *av = newAV();
Index: src/modules/perl/modperl_util.h
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_util.h,v
retrieving revision 1.68
diff -u -r1.68 modperl_util.h
--- src/modules/perl/modperl_util.h 14 Sep 2004 13:37:21 -0000 1.68
+++ src/modules/perl/modperl_util.h 18 Oct 2004 03:27:47 -0000
@@ -96,14 +96,6 @@
char *modperl_file2package(apr_pool_t *p, const char *file);
-/**
- * convert a compiled *CV ref to its original source code
- * @param p pool object (with a shortest possible life scope)
- * @param cv compiled *CV
- * @return string of original source code
- */
-char *modperl_coderef2text(pTHX_ apr_pool_t *p, CV *cv);
-
SV *modperl_apr_array_header2avrv(pTHX_ apr_array_header_t *array);
apr_array_header_t *modperl_avrv2apr_array_header(pTHX_ apr_pool_t *p,
SV *avrv);
Index: t/filter/both_str_req_add.t
===================================================================
RCS file: /home/cvs/modperl-2.0/t/filter/both_str_req_add.t,v
retrieving revision 1.6
diff -u -r1.6 both_str_req_add.t
--- t/filter/both_str_req_add.t 3 Aug 2004 16:16:19 -0000 1.6
+++ t/filter/both_str_req_add.t 18 Oct 2004 03:27:47 -0000
@@ -5,7 +5,7 @@
use Apache::TestRequest;
use Apache::TestUtil;
-plan tests => 1, need_min_module_version('B::Deparse', 0.6);
+plan tests => 1;
my $data = join ' ', 'A'..'Z', 0..9;
my $expected = lc $data; # that's what the input filter does
Index: t/hooks/push_handlers.t
===================================================================
RCS file: /home/cvs/modperl-2.0/t/hooks/push_handlers.t,v
retrieving revision 1.7
diff -u -r1.7 push_handlers.t
--- t/hooks/push_handlers.t 3 Aug 2004 16:16:20 -0000 1.7
+++ t/hooks/push_handlers.t 18 Oct 2004 03:27:47 -0000
@@ -5,7 +5,7 @@
use Apache::TestUtil;
use Apache::TestRequest;
-plan tests => 1, need_min_module_version('B::Deparse', 0.6);
+plan tests => 1;
my @refs = qw(conf conf1 conf2 coderef
full_coderef coderef1 coderef2 coderef3);
Index: t/hooks/TestHooks/push_handlers_blessed.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/t/hooks/TestHooks/push_handlers_blessed.pm,v
retrieving revision 1.2
diff -u -r1.2 push_handlers_blessed.pm
--- t/hooks/TestHooks/push_handlers_blessed.pm 3 Aug 2004 16:16:21 -0000 1.2
+++ t/hooks/TestHooks/push_handlers_blessed.pm 18 Oct 2004 03:27:47 -0000
@@ -19,7 +19,7 @@
sub handler {
my $r = shift;
- plan $r, tests => 1, need_min_module_version('B::Deparse', 0.61);;
+ plan $r, tests => 1;
my $sub = sub {
ok 1;
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: [mp2 patch] anon subs B::Deparse deployment replacement
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
>
>>Stas Bekman wrote:
>>
>>>This patch replaces the B::Deparse logic for anonsubs with a
>>>different approach.
>>
>>[...]
>>
>>>remaining issues with anon-subs:
>>
>>[...]
>>
>>>- we need more tests
>>
>>Joe, did you have a chance to write some tests for this issue, as
>>you've mentioned before that you've planned to?
>
>
> No tests from me yet- sorry I'm tied up this week, so I can't
> scrutinize the patch for a few more days. I'll try to make
> some time for testing the _17 release candidates on amd64.
Sure, take your time, Joe.
I hope to get that patch in after _17 release and then I hope people can
try writing tests trying to break it.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] anon subs B::Deparse deployment replacement
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
> Stas Bekman wrote:
> > This patch replaces the B::Deparse logic for anonsubs with a
> > different approach.
> [...]
> > remaining issues with anon-subs:
> [...]
> > - we need more tests
>
> Joe, did you have a chance to write some tests for this issue, as
> you've mentioned before that you've planned to?
No tests from me yet- sorry I'm tied up this week, so I can't
scrutinize the patch for a few more days. I'll try to make
some time for testing the _17 release candidates on amd64.
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] anon subs B::Deparse deployment replacement
Posted by Stas Bekman <st...@stason.org>.
Stas Bekman wrote:
> This patch replaces the B::Deparse logic for anonsubs with a different
> approach.
[...]
> remaining issues with anon-subs:
[...]
> - we need more tests
Joe, did you have a chance to write some tests for this issue, as you've
mentioned before that you've planned to?
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] anon subs B::Deparse deployment replacement
Posted by Stas Bekman <st...@stason.org>.
Sorry for taking so long to reply, I got back to work on this code now.
>>>> - i'm not sure if I can use a static global variable as a mutex (see
>>>> XXX in the patch)
>>
>>
>>> I don't think there is a problem with that. But couldn't the
>>> modperl_global.*
>>> stuff be used for this instead ?
>>
>>
>> I thought of that.
>>
>> 1) I'm not sure it's a good idea to use a single mutex for several
>> unrelated tasks. That'll slow things down.
>
>
> As far as I can see, each different modperl_global entrie defined with
> the use of MP_GLOBAL_DECL() gets it's own mutex.
that's correct. I've failed to see that behind multiple macros :(
>> 2) I'm not sure how to get hold of that modperl_global variable when
>> we need it.
>
>
> You don't need it. You can just :
> MP_GLOBAL_DECL(anonsub_cnt, int)
>
> And you then get modperl_global_get_anonsub_cnt() for free.
I don't need it for free, since I need to increment and do locking anyway,
so I did use the global type, but I've replaced the MP_GLOBAL_DECL with just:
/*** anon handlers code ***/
static modperl_global_t MP_global_anon_cnt;
void modperl_global_anon_cnt_init(apr_pool_t *p)
{
int *data = (int *)apr_pcalloc(p, sizeof(int));
*data = 0;
modperl_global_init(&MP_global_anon_cnt, p, (void *)data, "anon_cnt");
}
int modperl_global_anon_cnt_next(void)
{
int next;
/* XXX: inline lock/unlock? */
modperl_global_lock(&MP_global_anon_cnt);
next = ++*(int *)(MP_global_anon_cnt.data);
modperl_global_unlock(&MP_global_anon_cnt);
return next;
}
since this is all is needed. What do you think about XXX? should I replace
those calls with copy-n-pasted locking code? I see they aren't MP_INLINE
so I'm not sure whether the compiler will inline those for us.
>> [...]
>> as for APR_ANYLOCK, why doug has used the perl locking routines in
>> first place?
>
>
> Only reason I can see is that if you use Perl's locking routines, when they
> are not needed (i.e. no-ithreads), they are #defined as no-ops, while
> apr_anylock
> stuff does have a small overhead even if you pick the apr_anylock_none
> type (one
> pointer dereferencing + integer cmp).
so it's the best to stick with perl locking functions then.
Thanks Philippe!
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] anon subs B::Deparse deployment replacement
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Stas Bekman wrote:
> Philippe M. Chiasson wrote:
>
>>Stas Bekman wrote:
>>
>>>Stas Bekman wrote:
>>>
>>>>This patch replaces the B::Deparse logic for anonsubs with a different
>>>>approach.
>>>
>>>Any comments?
>>
>>Sorry, I thought I had replied to that one.
>>
>>Stas Bekman wrote:
>>
>>>- i'm not sure if I can use a static global variable as a mutex (see
>>> XXX in the patch)
>
>>I don't think there is a problem with that. But couldn't the
>>modperl_global.*
>>stuff be used for this instead ?
>
> I thought of that.
>
> 1) I'm not sure it's a good idea to use a single mutex for several
> unrelated tasks. That'll slow things down.
As far as I can see, each different modperl_global entrie defined with
the use of MP_GLOBAL_DECL() gets it's own mutex.
> 2) I'm not sure how to get hold of that modperl_global variable when we
> need it.
You don't need it. You can just :
MP_GLOBAL_DECL(anonsub_cnt, int)
And you then get modperl_global_get_anonsub_cnt() for free.
>
> [...]
> as for APR_ANYLOCK, why doug has used the perl locking routines in first
> place?
Only reason I can see is that if you use Perl's locking routines, when they
are not needed (i.e. no-ithreads), they are #defined as no-ops, while apr_anylock
stuff does have a small overhead even if you pick the apr_anylock_none type (one
pointer dereferencing + integer cmp).
--
--------------------------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] anon subs B::Deparse deployment replacement
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> Stas Bekman wrote:
>
>> Stas Bekman wrote:
>>
>>> This patch replaces the B::Deparse logic for anonsubs with a different
>>> approach.
>>
>>
>> Any comments?
>
>
> Sorry, I thought I had replied to that one.
>
> Stas Bekman wrote:
>
>> - i'm not sure if I can use a static global variable as a mutex (see
>> XXX in the patch)
> I don't think there is a problem with that. But couldn't the
> modperl_global.*
> stuff be used for this instead ?
I thought of that.
1) I'm not sure it's a good idea to use a single mutex for several
unrelated tasks. That'll slow things down.
2) I'm not sure how to get hold of that modperl_global variable when we
need it.
>> - should we do mutex locking only for threaded mpm? (see XXX in the
>> patch) in which case, it's probably going to be too expensive to
>> test modperl_threaded_mpm or not?).
>
>
> Well, first, you modperl_threaded_mpm isn't expensive anymore:
>
> static int MP_threaded_mpm = 0;
> int modperl_threaded_mpm(void)
> {
> return MP_threaded_mpm;
> }
>
> But, really, we should be able to reuse either some of the modperl_global.*
> stuff for this or the APR_ANYLOCK stuff so that we can get mutex locking
> for threaded mpms and near 0-overhead for prefork.
as for modperl_global_* please see above.
as for APR_ANYLOCK, why doug has used the perl locking routines in first
place?
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] anon subs B::Deparse deployment replacement
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Stas Bekman wrote:
> Stas Bekman wrote:
>
>>This patch replaces the B::Deparse logic for anonsubs with a different
>>approach.
>
> Any comments?
Sorry, I thought I had replied to that one.
Stas Bekman wrote:
> - i'm not sure if I can use a static global variable as a mutex (see
> XXX in the patch)
I don't think there is a problem with that. But couldn't the modperl_global.*
stuff be used for this instead ?
> - should we do mutex locking only for threaded mpm? (see XXX in the
> patch) in which case, it's probably going to be too expensive to
> test modperl_threaded_mpm or not?).
Well, first, you modperl_threaded_mpm isn't expensive anymore:
static int MP_threaded_mpm = 0;
int modperl_threaded_mpm(void)
{
return MP_threaded_mpm;
}
But, really, we should be able to reuse either some of the modperl_global.*
stuff for this or the APR_ANYLOCK stuff so that we can get mutex locking
for threaded mpms and near 0-overhead for prefork.
> if all anonsub handlers are
> compiled at run-time, there will be no overhead what so over at
> run-time. so probably this can be just documented.
> - when PerlScope is utilized we need to croak if the handler wasn't
> compiled+registered before perl_clone, since if this happens we may
> have one interpreter that compiles and registers the anon-sub
> handler but another one is run
> - we need more tests
--
--------------------------------------------------------------------------------
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
Re: [mp2 patch] anon subs B::Deparse deployment replacement
Posted by Stas Bekman <st...@stason.org>.
Stas Bekman wrote:
> This patch replaces the B::Deparse logic for anonsubs with a different
> approach.
Any comments?
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org