On Tue, Jan 24, 2006 at 02:23:04AM -0800, Gisle Aas wrote:
> I suggest this patch (relative to 1.50) to get PERL_NO_GET_CONTEXT
> going. Any reason not to?
It's been a while since I looked into the issues so I'm not sure off-hand.
The two big issues are 1) wanting to avoid having to recompile drivers,
and 2) remaining backwards compatible with perl 5.6.x.
Has this patch been tested in both those directions?
I think it's time to move on from 5.6 now so I'm think that the next
DBI release will be the last that officially supports 5.6.
I'll send a note to dbi-users and see what the reaction is :)
Having done that I'm happy to be a little more radical for a while.
(So I'm probably saying that I'll add this patch to 1.52 not 1.51.)
> Further, is it really problematic to start passing 'my_perl' as
> argument to these functions? Are they ever called from drivers?
Quite a few (see struct dbistate_st in DBIXS.h):
char * (*neat_svpv) _((SV *sv, STRLEN maxlen));
imp_xxh_t * (*getcom) _((SV *h)); /* see DBIh_COM macro */
void (*clearcom) _((imp_xxh_t *imp_xxh));
SV * (*event) _((SV *h, const char *name, SV*, SV*));
int (*set_attr_k) _((SV *h, SV *keysv, int dbikey, SV *valuesv));
SV * (*get_attr_k) _((SV *h, SV *keysv, int dbikey));
AV * (*get_fbav) _((imp_sth_t *imp_sth));
SV * (*make_fdsv) _((SV *sth, const char *imp_class, STRLEN
imp_size, const char *col_name));
int (*bind_as_num) _((int sql_type, int p, int s, int *t, void
*v));
int (*hash) _((const char *string, long i));
SV * (*preparse) _((SV *sth, char *statement, IV ps_return, IV
ps_accept, void *foo));
int (*logmsg) _((imp_xxh_t *imp_xxh, const char *fmt, ...));
int (*set_err_sv) _((SV *h, imp_xxh_t *imp_xxh, SV *err, SV
*errstr, SV *state, SV *method));
int (*set_err_char) _((SV *h, imp_xxh_t *imp_xxh, const char *err,
IV err_i, const char *errstr, const char *state, const char *method));
int (*bind_col) _((SV *sth, SV *col, SV *ref, SV *attribs));
> The RoadMap file seems to indicate that there would be code changes needed
> for the drivers to support PERL_NO_GET_CONTEXT.
If a my_perl param was simply added to those functions then drivers
would break until their code was changed to add the extra argument.
In some cases the calls are made from macros provided by the DBI so
a simple rebuild of the same driver source would fix those. I don't
know off-hand how many that applies to. It might be all of them,
if we're lucky.
I'd like to avoid forcing drivers to be changed, or even recompiled.
Installing a new DBI shouldn't ever break old code. Also, for the
driver authors, it should be easy for a driver to build against old and
new DBI APIs. Those requirements make it tricky. Still doable, just
more thought/work.
For example, an extra set of entry points could be added that have the
my_perl param and the DBI macros changed to use them. That way old code
uses the old entry points and the new faster entry points can be used
by simply rebuilding the driver. (Assuming all are called via
DBI-supplied macros.)
> I'm interested in this because ActivePerl is always compiled with
> threads enabled and without it we get gazillions of calls to
> ((PerlInterpreter*)pthread_getspecific((*Perl_Gthr_key_ptr(((void
> *)0))))) in DBI.o.
Understood - hence the item in the roadmap. Many unix distro perls are
also built with threads/multiplicity and would benefit from this.
Same goes for using mod_perl in apache 2.
I'd *very* interested in discussions/patches to help this issue.
I don't have time to lead the work myself but I'll certainly respond
quickly to any questions or patches :)
Tim.
p.s. The size of struct dbistate_st can't change so the new func
pointers will need to go into a new structure hanging off struct
dbistate_st via a pointer using the space reserved by 'void *pad2[5];'
> --Gisle
>
> Index: DBI.xs
> --- DBI.xs.~1~ Tue Jan 24 11:10:37 2006
> +++ DBI.xs Tue Jan 24 11:10:37 2006
> @@ -8,6 +8,7 @@
> */
>
> #define IN_DBI_XS 1 /* see DBIXS.h */
> +#define PERL_NO_GET_CONTEXT
>
> #include "DBIXS.h" /* DBI public interface for DBD's written in C */
>
> @@ -188,6 +189,7 @@
> check_version(const char *name, int dbis_cv, int dbis_cs, int need_dbixs_cv,
> int drc_s,
> int dbc_s, int stc_s, int fdc_s)
> {
> + dTHX;
> dPERINTERP;
> static const char msg[] = "you probably need to rebuild the DBD driver
> (or possibly the DBI)";
> (void)need_dbixs_cv;
> @@ -207,7 +209,8 @@
> static void
> dbi_bootinit(dbistate_t * parent_dbis)
> {
> -INIT_PERINTERP;
> + dTHX;
> + INIT_PERINTERP;
>
> Newz(dummy, DBIS, 1, dbistate_t);
>
> @@ -287,6 +290,7 @@
> char *
> neatsvpv(SV *sv, STRLEN maxlen) /* return a tidy ascii value, for debugging
> only */
> {
> + dTHX;
> dPERINTERP;
> STRLEN len;
> SV *nsv = Nullsv;
> @@ -405,6 +409,7 @@
> static int
> set_err_char(SV *h, imp_xxh_t *imp_xxh, const char *err_c, IV err_i, const
> char *errstr, const char *state, const char *method)
> {
> + dTHX;
> char err_buf[28];
> SV *err_sv, *errstr_sv, *state_sv, *method_sv;
> if (!err_c) {
> @@ -421,6 +426,7 @@
> static int
> set_err_sv(SV *h, imp_xxh_t *imp_xxh, SV *err, SV *errstr, SV *state, SV
> *method)
> {
> + dTHX;
> dPERINTERP;
> SV *h_err;
> SV *h_errstr;
> @@ -518,6 +524,7 @@
> static char *
> mkvname( HV *stash, const char *item, int uplevel) /* construct a variable
> name */
> {
> + dTHX;
> STRLEN lna;
> SV *sv = sv_newmortal();
> sv_setpv(sv, HvNAME(stash));
> @@ -565,6 +572,7 @@
> static int
> dbih_logmsg(imp_xxh_t *imp_xxh, const char *fmt, ...)
> {
> + dTHX;
> dPERINTERP;
> va_list args;
> #ifdef I_STDARG
> @@ -582,6 +590,7 @@
> static int
> set_trace_file(SV *file)
> {
> + dTHX;
> dPERINTERP;
> STRLEN lna;
> const char *filename;
> @@ -623,6 +632,7 @@
> static IV
> parse_trace_flags(SV *h, SV *level_sv, IV old_level)
> {
> + dTHX;
> IV level;
> if (!level_sv || !SvOK(level_sv))
> level = old_level; /* undef: no change */
> @@ -652,6 +662,7 @@
> static int
> set_trace(SV *h, SV *level_sv, SV *file)
> {
> + dTHX;
> dPERINTERP;
> D_imp_xxh(h);
> int RETVAL = DBIS->debug; /* Return trace level in effect now */
> @@ -679,6 +690,7 @@
> static SV *
> dbih_inner(SV *orv, const char *what)
> { /* convert outer to inner handle else croak(what) if what is not null */
> + dTHX;
> dPERINTERP;
> MAGIC *mg;
> SV *ohv; /* outer HV after derefing the RV */
> @@ -746,6 +758,7 @@
> static imp_xxh_t *
> dbih_getcom2(SV *hrv, MAGIC **mgp) /* Get com struct for handle. Must be
> fast. */
> {
> + dTHX;
> dPERINTERP;
> imp_xxh_t *imp_xxh;
> MAGIC *mg;
> @@ -793,6 +806,7 @@
> static SV *
> dbih_setup_attrib(SV *h, imp_xxh_t *imp_xxh, char *attrib, SV *parent, int
> read_only, int optional)
> {
> + dTHX;
> dPERINTERP;
> STRLEN len = strlen(attrib);
> SV **asvp;
> @@ -838,6 +852,7 @@
> static SV *
> dbih_make_fdsv(SV *sth, const char *imp_class, STRLEN imp_size, const char
> *col_name)
> {
> + dTHX;
> dPERINTERP;
> D_imp_sth(sth);
> const STRLEN cn_len = strlen(col_name);
> @@ -860,6 +875,7 @@
> static SV *
> dbih_make_com(SV *p_h, imp_xxh_t *p_imp_xxh, const char *imp_class, STRLEN
> imp_size, STRLEN extra, SV* imp_templ)
> {
> + dTHX;
> dPERINTERP;
> static const char *errmsg = "Can't make DBI com handle for %s: %s";
> HV *imp_stash;
> @@ -967,6 +983,7 @@
> static void
> dbih_setup_handle(SV *orv, char *imp_class, SV *parent, SV *imp_datasv)
> {
> + dTHX;
> dPERINTERP;
> SV *h;
> char *errmsg = "Can't setup DBI handle of %s to %s: %s";
> @@ -1113,6 +1130,7 @@
> static void
> dbih_dumpcom(imp_xxh_t *imp_xxh, const char *msg, int level)
> {
> + dTHX;
> dPERINTERP;
> SV *flags = sv_2mortal(newSVpv("",0));
> STRLEN lna;
> @@ -1178,9 +1196,10 @@
> static void
> dbih_clearcom(imp_xxh_t *imp_xxh)
> {
> +
> + dTHX;
> dPERINTERP;
> dTHR;
> - dTHX;
> int dump = FALSE;
> int debug = DBIS_TRACE_LEVEL;
> int auto_dump = (debug >= 6);
> @@ -1284,6 +1303,7 @@
> static AV *
> dbih_setup_fbav(imp_sth_t *imp_sth)
> {
> + dTHX;
> dPERINTERP;
> int i;
> AV *av;
> @@ -1329,6 +1349,7 @@
> }
>
> if (DBIc_is(imp_sth, DBIcf_TaintOut)) {
> + dTHX;
> dTHR;
> TAINT; /* affects sv_setsv()'s called within same perl statement */
> }
> @@ -1342,6 +1363,7 @@
> static int
> dbih_sth_bind_col(SV *sth, SV *col, SV *ref, SV *attribs)
> {
> + dTHX;
> dPERINTERP;
> D_imp_sth(sth);
> AV *av;
> @@ -1412,6 +1434,7 @@
> static int
> dbih_set_attr_k(SV *h, SV *keysv, int dbikey, SV *valuesv)
> {
> + dTHX;
> dPERINTERP;
> dTHR;
> D_imp_xxh(h);
> @@ -1650,6 +1673,7 @@
> static SV *
> dbih_get_attr_k(SV *h, SV *keysv, int dbikey)
> {
> + dTHX;
> dPERINTERP;
> dTHR;
> D_imp_xxh(h);
> @@ -1971,6 +1995,7 @@
> static SV * /* find attrib in handle or its parents */
> dbih_find_attr(SV *h, SV *keysv, int copydown, int spare)
> {
> + dTHX;
> D_imp_xxh(h);
> SV *ph;
> STRLEN keylen;
> @@ -1997,6 +2022,7 @@
> static SV *
> dbih_event(SV *hrv, const char *evtype, SV *a1, SV *a2)
> {
> + dTHX;
> /* We arrive here via DBIh_EVENT* macros (see DBIXS.h) called from
> */
> /* DBD driver C code OR $h->event() method (in DBD::_::common) */
> /* XXX VERY OLD INTERFACE/CONCEPT MAY GO SOON */
> @@ -2038,6 +2064,7 @@
> static char *
> dbi_caller(long *line)
> {
> + dTHX;
> register I32 cxix;
> register PERL_CONTEXT *cx;
> register PERL_CONTEXT *ccstack = cxstack;
> @@ -2078,6 +2105,7 @@
> static char *
> log_where(int trace_level, SV *buf, int append, char *suffix)
> {
> + dTHX;
> dTHR;
> if (!buf) {
> buf = sv_2mortal(newSV(80));
> @@ -2116,6 +2144,7 @@
> static void
> clear_cached_kids(SV *h, imp_xxh_t *imp_xxh, const char *meth_name, int
> trace_level)
> {
> + dTHX;
> dPERINTERP;
> if (DBIc_TYPE(imp_xxh) <= DBIt_DB &&
> DBIc_CACHED_KIDS((imp_drh_t*)imp_xxh)) {
> if (DBIc_TRACE_LEVEL(imp_xxh) > trace_level)
> @@ -2165,6 +2194,7 @@
> #define DBIprof_FIRST_CALLED 5
> #define DBIprof_LAST_CALLED 6
> #define DBIprof_max_index 6
> + dTHX;
> double ti = t2 - t1;
> const char *path[DBIprof_MAX_PATH_ELEM+1];
> int idx = -1;
> @@ -2312,6 +2342,7 @@
> static void
> dbi_profile_merge(SV *dest, SV *increment)
> {
> + dTHX;
> AV *d_av, *i_av;
> SV *tmp;
> double i_nv;
> @@ -3205,6 +3236,7 @@
> we add support for odbc escape sequences.
> */
>
> + dTHX;
> int idx = 1;
>
> char in_quote = '\0';
> End of Patch.
