Re: Changes to make DBIS more efficient
On Fri, Mar 02, 2012 at 05:04:29PM +, Dave Mitchell wrote:
On Fri, Mar 02, 2012 at 03:23:30PM +, Dave Mitchell wrote:
I'd be happy in principle, although some guidance would be welcome
Also in particular, what needs to continue happening across an API
change? Is it just that a driver, compiled and installed against an old
DBI, must continue to work if the DBI is upgraded?
I've now attached a revised DBIS patch. The only difference is that
the new DBI continues to store a pointer to the dbi_state struct in
$DBI::_dbistate, so that DBD modules compiled against the old DBI can
continue to retrieve the struct the old way. Once recompiled, they'll
start to use the new method.
IIUC, this means that there are no binary or backwards compatibility
issues, and no API version numbers need bumping.
--
The optimist believes that he lives in the best of all possible worlds.
As does the pessimist.
From 0fb5fd6a7099c5fc40bd0b9d40b8b34d748533b5 Mon Sep 17 00:00:00 2001
From: David Mitchell da...@iabyn.com
Date: Fri, 10 Feb 2012 13:11:13 +
Subject: [PATCH] under ithreads, make DBIS efficient for DBD::*
rather than the slow looking up of $DBI::_dbistate on every use of DBIS,
convert DBIS into a call to a C-level function that returns the address of
the dbi_state struct.
Since the C-level function is only directly callable from DBI, store its
address in an XSUB, so that the DBD:* modules can retrieve the function's
address and cache it in a static var.
We continue to store the dbi_state struct address within $DBI::_dbistate
too, so that DBD modules compiled against an older DBI will continue to
work if the DBI is upgraded but the DBD not recompiled.
---
DBI.pm |4 +---
DBI.xs | 39 +++
DBIXS.h | 39 +++
3 files changed, 47 insertions(+), 35 deletions(-)
diff --git a/DBI.pm b/DBI.pm
index 4e02e8c..614ffae 100644
--- a/DBI.pm
+++ b/DBI.pm
@@ -519,10 +519,8 @@ END {
sub CLONE {
-my $olddbis = $DBI::_dbistate;
_clone_dbis() unless $DBI::PurePerl; # clone the DBIS structure
-DBI-trace_msg(sprintf CLONE DBI for new thread %s\n,
- $DBI::PurePerl ? : sprintf((dbis %x - %x),$olddbis,
$DBI::_dbistate));
+DBI-trace_msg(CLONE DBI for new thread\n);
while ( my ($driver, $drh) = each %DBI::installed_drh) {
no strict 'refs';
next if defined {DBD::${driver}::CLONE};
diff --git a/DBI.xs b/DBI.xs
index 64dc5b1..a9e9000 100644
--- a/DBI.xs
+++ b/DBI.xs
@@ -127,8 +127,6 @@ char *neatsvpv _((SV *sv, STRLEN maxlen));
SV * preparse(SV *dbh, const char *statement, IV ps_return, IV ps_accept, void
*foo);
static meth_types get_meth_type(const char * const name);
-DBISTATE_DECLARE;
-
struct imp_drh_st { dbih_drc_t com; };
struct imp_dbh_st { dbih_dbc_t com; };
struct imp_sth_st { dbih_stc_t com; };
@@ -310,13 +308,20 @@ typedef struct {
START_MY_CXT
-#if defined(MULTIPLICITY) || defined(PERL_OBJECT) || defined(PERL_CAPI)
-# undef DBIS
-# define DBIS (MY_CXT.dbi_state)
-#endif
+#undef DBIS
+#define DBIS (MY_CXT.dbi_state)
#define g_dbi_last_h(MY_CXT.dbi_last_h)
+/* allow the 'static' dbi_state struct to be accessed from other files */
+dbistate_t**
+_dbi_state_lval(pTHX)
+{
+dMY_CXT;
+return (MY_CXT.dbi_state);
+}
+
+
/* --- */
static void *
@@ -521,15 +526,12 @@ dbi_bootinit(dbistate_t * parent_dbis)
dbistate_t* DBISx;
DBISx = (struct dbistate_st*)malloc_using_sv(sizeof(struct dbistate_st));
-
-/* publish address of dbistate so dynaloaded DBD's can find it,
- * taking care to store the value in the same way it'll be used
- * to avoid problems on some architectures, for example see
- * http://rt.cpan.org/Public/Bug/Display.html?id=32309
- */
-sv_setiv(get_sv(DBISTATE_PERLNAME, GV_ADDMULTI), 0); /* force SvIOK */
DBIS = DBISx;
-DBIS_PUBLISHED_LVALUE = DBISx;
+
+/* make DBIS available to DBD modules the old (= 1.618) way,
+ * so that unrecompiled DBD's will still work against a newer DBI */
+sv_setiv(get_sv(DBI::_dbistate, GV_ADDMULTI),
+PTR2IV(MY_CXT.dbi_state));
/* store version and size so we can spot DBI/DBD version mismatch */
DBIS-check_version = check_version;
@@ -547,12 +549,6 @@ dbi_bootinit(dbistate_t * parent_dbis)
DBIS-thr_owner = PERL_GET_THX;
#endif
-DBISTATE_INIT; /* check DBD code to set DBIS from DBISTATE_PERLNAME */
-
-if (DBIS_TRACE_LEVEL 9) {
-sv_dump(DBISTATE_ADDRSV);
-}
-
/* store some function pointers so DBD's can call our functions */
DBIS-getcom = dbih_getcom;
DBIS-clearcom= dbih_clearcom;
@@ -4346,6 +4342,9 @@ BOOT:
(void)cv;
(void)items; /* avoid 'unused variable' warning */
dbi_bootinit(NULL);
+/* make this sub into a fake XS so it can bee seen by DBD::* modules;
+ * never actually call it as an XS sub, or it will crash and
Re: Changes to make DBIS more efficient
Perfect! Applied. Many thanks Dave.
Tim.
On Wed, Apr 18, 2012 at 11:48:31AM +0100, Dave Mitchell wrote:
On Fri, Mar 02, 2012 at 05:04:29PM +, Dave Mitchell wrote:
On Fri, Mar 02, 2012 at 03:23:30PM +, Dave Mitchell wrote:
I'd be happy in principle, although some guidance would be welcome
Also in particular, what needs to continue happening across an API
change? Is it just that a driver, compiled and installed against an old
DBI, must continue to work if the DBI is upgraded?
I've now attached a revised DBIS patch. The only difference is that
the new DBI continues to store a pointer to the dbi_state struct in
$DBI::_dbistate, so that DBD modules compiled against the old DBI can
continue to retrieve the struct the old way. Once recompiled, they'll
start to use the new method.
IIUC, this means that there are no binary or backwards compatibility
issues, and no API version numbers need bumping.
--
The optimist believes that he lives in the best of all possible worlds.
As does the pessimist.
From 0fb5fd6a7099c5fc40bd0b9d40b8b34d748533b5 Mon Sep 17 00:00:00 2001
From: David Mitchell da...@iabyn.com
Date: Fri, 10 Feb 2012 13:11:13 +
Subject: [PATCH] under ithreads, make DBIS efficient for DBD::*
rather than the slow looking up of $DBI::_dbistate on every use of DBIS,
convert DBIS into a call to a C-level function that returns the address of
the dbi_state struct.
Since the C-level function is only directly callable from DBI, store its
address in an XSUB, so that the DBD:* modules can retrieve the function's
address and cache it in a static var.
We continue to store the dbi_state struct address within $DBI::_dbistate
too, so that DBD modules compiled against an older DBI will continue to
work if the DBI is upgraded but the DBD not recompiled.
---
DBI.pm |4 +---
DBI.xs | 39 +++
DBIXS.h | 39 +++
3 files changed, 47 insertions(+), 35 deletions(-)
diff --git a/DBI.pm b/DBI.pm
index 4e02e8c..614ffae 100644
--- a/DBI.pm
+++ b/DBI.pm
@@ -519,10 +519,8 @@ END {
sub CLONE {
-my $olddbis = $DBI::_dbistate;
_clone_dbis() unless $DBI::PurePerl; # clone the DBIS structure
-DBI-trace_msg(sprintf CLONE DBI for new thread %s\n,
- $DBI::PurePerl ? : sprintf((dbis %x - %x),$olddbis,
$DBI::_dbistate));
+DBI-trace_msg(CLONE DBI for new thread\n);
while ( my ($driver, $drh) = each %DBI::installed_drh) {
no strict 'refs';
next if defined {DBD::${driver}::CLONE};
diff --git a/DBI.xs b/DBI.xs
index 64dc5b1..a9e9000 100644
--- a/DBI.xs
+++ b/DBI.xs
@@ -127,8 +127,6 @@ char *neatsvpv _((SV *sv, STRLEN maxlen));
SV * preparse(SV *dbh, const char *statement, IV ps_return, IV ps_accept,
void *foo);
static meth_types get_meth_type(const char * const name);
-DBISTATE_DECLARE;
-
struct imp_drh_st { dbih_drc_t com; };
struct imp_dbh_st { dbih_dbc_t com; };
struct imp_sth_st { dbih_stc_t com; };
@@ -310,13 +308,20 @@ typedef struct {
START_MY_CXT
-#if defined(MULTIPLICITY) || defined(PERL_OBJECT) || defined(PERL_CAPI)
-# undef DBIS
-# define DBIS (MY_CXT.dbi_state)
-#endif
+#undef DBIS
+#define DBIS (MY_CXT.dbi_state)
#define g_dbi_last_h(MY_CXT.dbi_last_h)
+/* allow the 'static' dbi_state struct to be accessed from other files */
+dbistate_t**
+_dbi_state_lval(pTHX)
+{
+dMY_CXT;
+return (MY_CXT.dbi_state);
+}
+
+
/* --- */
static void *
@@ -521,15 +526,12 @@ dbi_bootinit(dbistate_t * parent_dbis)
dbistate_t* DBISx;
DBISx = (struct dbistate_st*)malloc_using_sv(sizeof(struct dbistate_st));
-
-/* publish address of dbistate so dynaloaded DBD's can find it,
- * taking care to store the value in the same way it'll be used
- * to avoid problems on some architectures, for example see
- * http://rt.cpan.org/Public/Bug/Display.html?id=32309
- */
-sv_setiv(get_sv(DBISTATE_PERLNAME, GV_ADDMULTI), 0); /* force SvIOK */
DBIS = DBISx;
-DBIS_PUBLISHED_LVALUE = DBISx;
+
+/* make DBIS available to DBD modules the old (= 1.618) way,
+ * so that unrecompiled DBD's will still work against a newer DBI */
+sv_setiv(get_sv(DBI::_dbistate, GV_ADDMULTI),
+PTR2IV(MY_CXT.dbi_state));
/* store version and size so we can spot DBI/DBD version mismatch */
DBIS-check_version = check_version;
@@ -547,12 +549,6 @@ dbi_bootinit(dbistate_t * parent_dbis)
DBIS-thr_owner = PERL_GET_THX;
#endif
-DBISTATE_INIT; /* check DBD code to set DBIS from DBISTATE_PERLNAME */
-
-if (DBIS_TRACE_LEVEL 9) {
-sv_dump(DBISTATE_ADDRSV);
-}
-
/* store some function pointers so DBD's can call our functions */
DBIS-getcom = dbih_getcom;
DBIS-clearcom= dbih_clearcom;
@@ -4346,6 +4342,9 @@ BOOT:
Re: Changes to make DBIS more efficient
Am 02.03.2012 16:11, schrieb Tim Bunce: [...] However, I'd still like to make the change. So I'm thinking it might be best to support *both* mechanisms for at least a few releases before removing the old one. The gives end users a safe zone where they can upgrade the DBI and then later upgrade compiled drivers after they've been changed. Probably Makefile.PL's CheckConflicts() can be used to warn about old DBD's as we know about them ... /Jens
Re: Changes to make DBIS more efficient
On Fri, Mar 02, 2012 at 03:11:17PM +, Tim Bunce wrote: On Fri, Feb 10, 2012 at 03:27:24PM +, Dave Mitchell wrote: I'm concerned that changing the API, and thus forcing all compiled drivers to be recompiled at the same time the DBI is installed, isn't worth the gain. Especially as drivers shouldn't be using DBIS in any hot code anyway. However, I'd still like to make the change. So I'm thinking it might be best to support *both* mechanisms for at least a few releases before removing the old one. The gives end users a safe zone where they can upgrade the DBI and then later upgrade compiled drivers after they've been changed. Would you be okay with reworking the change along those lines? I'd be happy in principle, although some guidance would be welcome, as I'm not currently clear what within my patch does (and doesn't) constitute an API change. Is it the addition of an extra XS function? Is the fix as easy as something along the lines of #if DBIXS_REVISION NNN # define DBIS the-new-way ... and other stuff #else # define DBIS the-old-way ... and other stuff #endif ??? -- Britain, Britain, Britain! Discovered by Sir Henry Britain in sixteen-oh-ten. Sold to Germany a year later for a pfennig and the promise of a kiss. Destroyed in eighteen thirty-forty two, and rebuilt a week later by a man. This we know. Hello. But what of the people of Britain? Who they? What do? And why? -- Little Britain
Re: Changes to make DBIS more efficient
On Fri, Mar 02, 2012 at 03:23:30PM +, Dave Mitchell wrote: I'd be happy in principle, although some guidance would be welcome Also in particular, what needs to continue happening across an API change? Is it just that a driver, compiled and installed against an old DBI, must continue to work if the DBI is upgraded? -- You're so sadly neglected, and often ignored. A poor second to Belgium, When going abroad. -- Monty Python, Finland
