DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On Fri, Mar 02, 2012 at 03:11:17PM +, Tim Bunce wrote: > > > > The second patch, which makes DBIS more efficient, is a lot more complex, > > and more likely to break things (especially as it's changing a bunch of > > macros that are directly #included by the DBD drivers. You may need to > > bump API version numbers; I don't understand that bit. > > 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. I finally got around to looking at DBD::Pg and was horrified to discover the number of DBIS calls made by hot paths in the code. Most are hidden behind various tracing macros. Even fetch calls 3 + 3 * num_of_fields! Then I looked at DBD::Oracle and discovered the same kind of thing. Which is particularly disappointing for me as I wrote the tracing mechanism it uses (though maybe that pre-dated thread support). Anyway, the upshot is that getting DBIS calls out of major drivers will require a fair bit of work. It's just grunt-work really, nothing complicated. As you noted with DBD::mysql, Dave, the performance gains are worth it. Any volunteers? Naturally I'll be happy to suggest an approach I think will work well (basically an extension of that used by DBD::Oracle). Dave, I'll get back to you about the DBIS changes soon, I hope. Tim.
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On Tue, 13 Mar 2012 21:44:06 +, Tim Bunce wrote: > On Fri, Mar 02, 2012 at 03:11:17PM +, Tim Bunce wrote: > > > > > > The second patch, which makes DBIS more efficient, is a lot more complex, > > > and more likely to break things (especially as it's changing a bunch of > > > macros that are directly #included by the DBD drivers. You may need to > > > bump API version numbers; I don't understand that bit. > > > > 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. > > I finally got around to looking at DBD::Pg and was horrified to discover > the number of DBIS calls made by hot paths in the code. Most are hidden > behind various tracing macros. Even fetch calls 3 + 3 * num_of_fields! When hidden, it won't stand out to the maintainers. How do we/I see where it happens? Do you have a (short) guide to check if my/a DBD (CSV, Unify, File) uses those too? > Then I looked at DBD::Oracle and discovered the same kind of thing. > Which is particularly disappointing for me as I wrote the tracing > mechanism it uses (though maybe that pre-dated thread support). > > Anyway, the upshot is that getting DBIS calls out of major drivers will > require a fair bit of work. It's just grunt-work really, nothing complicated. > As you noted with DBD::mysql, Dave, the performance gains are worth it. > > Any volunteers? Naturally I'll be happy to suggest an approach I think > will work well (basically an extension of that used by DBD::Oracle). > > Dave, I'll get back to you about the DBIS changes soon, I hope. -- H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/ using perl5.00307 .. 5.14 porting perl5 on HP-UX, AIX, and openSUSE http://mirrors.develooper.com/hpux/http://www.test-smoke.org/ http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On 13/03/12 21:44, Tim Bunce wrote: On Fri, Mar 02, 2012 at 03:11:17PM +, Tim Bunce wrote: The second patch, which makes DBIS more efficient, is a lot more complex, and more likely to break things (especially as it's changing a bunch of macros that are directly #included by the DBD drivers. You may need to bump API version numbers; I don't understand that bit. 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. On the other hand, I want whatever speed I can get so I am happy to recompile any DBDs to get it. Do people like me have to do without improvements for the sake of those who don't want to recompile their DBDs or can it be done in some other way or opt in. I finally got around to looking at DBD::Pg and was horrified to discover the number of DBIS calls made by hot paths in the code. Most are hidden behind various tracing macros. Even fetch calls 3 + 3 * num_of_fields! Then I looked at DBD::Oracle and discovered the same kind of thing. Which is particularly disappointing for me as I wrote the tracing mechanism it uses (though maybe that pre-dated thread support). I am happy to change DBD::Oracle or may be Yanick will. Anyway, the upshot is that getting DBIS calls out of major drivers will require a fair bit of work. It's just grunt-work really, nothing complicated. As you noted with DBD::mysql, Dave, the performance gains are worth it. Any volunteers? Naturally I'll be happy to suggest an approach I think will work well (basically an extension of that used by DBD::Oracle). Dave, I'll get back to you about the DBIS changes soon, I hope. Tim. I don't think DBD::ODBC uses any DBIS stuff but to be sure I'd echo Merijn's reply; if you could point out what to look for and an approach I'd happily change any DBD I could run and provide patches for it (currently I have ODBC, Pg, Oracle, CSV, SQLite, mysql and all those you get with DBI). Martin -- Martin J. Evans Easysoft Limited http://www.easysoft.com
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On Wed, Mar 14, 2012 at 08:24:36AM +0100, H.Merijn Brand wrote:
> On Tue, 13 Mar 2012 21:44:06 +, Tim Bunce
> wrote:
>
> > On Fri, Mar 02, 2012 at 03:11:17PM +, Tim Bunce wrote:
> > > >
> > > > The second patch, which makes DBIS more efficient, is a lot more
> > > > complex,
> > > > and more likely to break things (especially as it's changing a bunch of
> > > > macros that are directly #included by the DBD drivers. You may need to
> > > > bump API version numbers; I don't understand that bit.
> > >
> > > 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.
> >
> > I finally got around to looking at DBD::Pg and was horrified to discover
> > the number of DBIS calls made by hot paths in the code. Most are hidden
> > behind various tracing macros. Even fetch calls 3 + 3 * num_of_fields!
>
> When hidden, it won't stand out to the maintainers.
>
> How do we/I see where it happens? Do you have a (short) guide to check
> if my/a DBD (CSV, Unify, File) uses those too?
The slowness of the DBIS macro is only relevant for compiled drivers.
The DBIS macro is used to get a pointer to the DBI's struct dbistate_st.
In non-threaded perls it's simply a static variable initialised once.
In threaded perls it's a macro that has to do more expensive work.
For many years now all DBI handles have had their own pointer to
dbistate_st. Since almost all code in the driver has access to a handle
that's the best way to get the dbistate_st struct pointer.
Essentially it means changing all instances of DBIS to DBIc_DBISTATE(imp_xxh)
(where imp_xxh can be any of the imp_dbh, imp_sth etc variables).
Looking at DBIXS.h I see these other macros that use DBIS and thus
should be avoided in hot code:
DBILOGFP=> DBIc_LOGPIO(imp_xxh)
DBILOGMSG(...) => DBIc_DBISTATE(imp_xxh)->logmsg(...)
neatsvpv(...) => DBIc_DBISTATE(imp_xxh)->neatsvpv(...)
however, usually those are only called in tracing code so it may not be
worth the effort to change them.
The DBIh_COM(h) macro also uses DBIS but there's a chicken-and-egg
problem here since DBIh_COM is the underlying mechanism of the
D_imp_xxh(h) macros used to get the imp_xxh pointer. So it can't be
avoided. Yet more reason to apply Dave's changes.
Tim.
p.s. For the logic to control tracing I recommend the DBIc_TRACE macro
(or something driver-specific like it or built using it):
DBIc_TRACE(imp, flags, flag_level, fallback_level)
That's true if flags match the handle trace flags & handle trace
level >= flag_level, OR if handle trace_level > fallback_level
DBIc_TRACE(imp, 0, 0, 4) = if handle trace level >= 4
DBIc_TRACE(imp, DBDf_TRACE_FOO, 2, 4) = if tracing DBDf_TRACE_FOO & level>=2
OR level>=4
DBIc_TRACE(imp, DBDf_TRACE_FOO, 2, 0) = as above but never trace just due to
level
For example:
if (DBIc_TRACE(imp_xxh, DBIf_TRACE_SQL|DBIf_TRACE_xxx, 3, 7)) {
PerlIO_printf(DBIc_LOGPIO(imp_sth), "\tThe %s wibbled the %s\n", ...);
}
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On Wed, Mar 14, 2012 at 08:52:38AM +, Martin J. Evans wrote: > On 13/03/12 21:44, Tim Bunce wrote: > >On Fri, Mar 02, 2012 at 03:11:17PM +, Tim Bunce wrote: > >>> > >>>The second patch, which makes DBIS more efficient, is a lot more complex, > >>>and more likely to break things (especially as it's changing a bunch of > >>>macros that are directly #included by the DBD drivers. You may need to > >>>bump API version numbers; I don't understand that bit. > >> > >>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. > > On the other hand, I want whatever speed I can get so I am happy to > recompile any DBDs to get it. Do people like me have to do without > improvements for the sake of those who don't want to recompile their > DBDs or can it be done in some other way or opt in. Per my recent message, implicit usage of DBIS is higher than I'd thought. The DBI will certain get Dave's improvements in some form, and yes, drivers will need to be recompiled. In fact they may *have* to be recompiled. > I don't think DBD::ODBC uses any DBIS stuff I had a (very) quick look and it loosk good. It's even using DBIc_TRACE() :) There are cases where an imp_sth is passed as a parameter yet D_imp_xxh(sth) is being called. So you end up with imp_sth and imp_xxh pointers that point to the same thing. See dbd_st_fetch and dbd_bind_ph for example. Just delete the D_imp_xxh(sth); line and change all imp_xxh in the function to be imp_sth instead. There may be cases where the same applies with imp_dbh. There are other places where internal functions use the D_imp_xxh(h) initializer but that could be avoided by passing in imp_xxh from the caller. (And by *_xxh I mean _dbh, _sth - whichever is appropriate.) > but to be sure I'd echo > Merijn's reply; if you could point out what to look for and an > approach I'd happily change any DBD I could run and provide patches > for it (currently I have ODBC, Pg, Oracle, CSV, SQLite, mysql and all > those you get with DBI). Let me know if my reply to Merijn wasn't clear or complete enough. Tim.
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On 14/03/12 10:37, Tim Bunce wrote: On Wed, Mar 14, 2012 at 08:52:38AM +, Martin J. Evans wrote: On 13/03/12 21:44, Tim Bunce wrote: On Fri, Mar 02, 2012 at 03:11:17PM +, Tim Bunce wrote: The second patch, which makes DBIS more efficient, is a lot more complex, and more likely to break things (especially as it's changing a bunch of macros that are directly #included by the DBD drivers. You may need to bump API version numbers; I don't understand that bit. 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. On the other hand, I want whatever speed I can get so I am happy to recompile any DBDs to get it. Do people like me have to do without improvements for the sake of those who don't want to recompile their DBDs or can it be done in some other way or opt in. Per my recent message, implicit usage of DBIS is higher than I'd thought. The DBI will certain get Dave's improvements in some form, and yes, drivers will need to be recompiled. In fact they may *have* to be recompiled. I don't think DBD::ODBC uses any DBIS stuff I had a (very) quick look and it loosk good. It's even using DBIc_TRACE() :) I thought I was pretty up to date. There are cases where an imp_sth is passed as a parameter yet D_imp_xxh(sth) is being called. So you end up with imp_sth and imp_xxh pointers that point to the same thing. See dbd_st_fetch and dbd_bind_ph for example. Just delete the D_imp_xxh(sth); line and change all imp_xxh in the function to be imp_sth instead. There may be cases where the same applies with imp_dbh. Yeah, thanks, I'll fix those. There are other places where internal functions use the D_imp_xxh(h) initializer but that could be avoided by passing in imp_xxh from the caller. (And by *_xxh I mean _dbh, _sth - whichever is appropriate.) I'll do that too. but to be sure I'd echo Merijn's reply; if you could point out what to look for and an approach I'd happily change any DBD I could run and provide patches for it (currently I have ODBC, Pg, Oracle, CSV, SQLite, mysql and all those you get with DBI). Let me know if my reply to Merijn wasn't clear or complete enough. It was fine. I'll try and find time to sort Oracle out unless Yanick steps in. I also meant to change Oracle to use DBD trace flag etc which gets rid of loads of ora_verbose code all over the place but I've not had time as yet as the level or rts reported for Oracle keeps me pretty busy. Martin -- Martin J. Evans Easysoft Limited http://www.easysoft.com
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On 14/03/12 11:37, Martin J. Evans wrote: On 14/03/12 10:37, Tim Bunce wrote: On Wed, Mar 14, 2012 at 08:52:38AM +, Martin J. Evans wrote: On 13/03/12 21:44, Tim Bunce wrote: On Fri, Mar 02, 2012 at 03:11:17PM +, Tim Bunce wrote: The second patch, which makes DBIS more efficient, is a lot more complex, and more likely to break things (especially as it's changing a bunch of macros that are directly #included by the DBD drivers. You may need to bump API version numbers; I don't understand that bit. 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. On the other hand, I want whatever speed I can get so I am happy to recompile any DBDs to get it. Do people like me have to do without improvements for the sake of those who don't want to recompile their DBDs or can it be done in some other way or opt in. Per my recent message, implicit usage of DBIS is higher than I'd thought. The DBI will certain get Dave's improvements in some form, and yes, drivers will need to be recompiled. In fact they may *have* to be recompiled. Good. I don't think DBD::ODBC uses any DBIS stuff I had a (very) quick look and it loosk good. It's even using DBIc_TRACE() :) I thought I was pretty up to date. There are cases where an imp_sth is passed as a parameter yet D_imp_xxh(sth) is being called. So you end up with imp_sth and imp_xxh pointers that point to the same thing. See dbd_st_fetch and dbd_bind_ph for example. Just delete the D_imp_xxh(sth); line and change all imp_xxh in the function to be imp_sth instead. There may be cases where the same applies with imp_dbh. Yeah, thanks, I'll fix those. Those D_imp_xxh calls in dbd_st_fetch and dbd_bind_ph are simply there because those fns call DBIh_SET_ERR_CHAR and that takes an imp_xxh_t. I have removed them but then I need to cast imp_dbh/imp_sth to imp_xxh_t before calling DBIh_SET_ERR_CHAR so that then makes we wonder why imp_xxh_t is really required? just so you can pass an imp_dbh_t or an imp_sth_t to it? The only places DBD::ODBC using imp_xxh_t is in calls to DBIh_SET_ERR_CHAR and DBIc_TRACE (and a few calls to DBIc_TYPE). So are you effectively saying imp_xxh_t exists for the simple reason it could be either an imp_sth_t/imp_dbh_t and if that is so I presume I can just delete all D_imp_xxh macros where the imp_xxh is passed to DBIh_SET_ERR_CHAR/DBIc_TRACE, pass the real imp_*h and cast it and avoid a call to dbih_getcom2? There are other places where internal functions use the D_imp_xxh(h) initializer but that could be avoided by passing in imp_xxh from the caller. (And by *_xxh I mean _dbh, _sth - whichever is appropriate.) I'll do that too. That is a bigger job but one I've started. but to be sure I'd echo Merijn's reply; if you could point out what to look for and an approach I'd happily change any DBD I could run and provide patches for it (currently I have ODBC, Pg, Oracle, CSV, SQLite, mysql and all those you get with DBI). Let me know if my reply to Merijn wasn't clear or complete enough. It was fine. I'll try and find time to sort Oracle out unless Yanick steps in. I also meant to change Oracle to use DBD trace flag etc which gets rid of loads of ora_verbose code all over the place but I've not had time as yet as the level or rts reported for Oracle keeps me pretty busy. Martin Martin -- Martin J. Evans Easysoft Limited http://www.easysoft.com
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On 03/14/12 07:37, Martin J. Evans wrote: I'll try and find time to sort Oracle out unless Yanick steps in. I'll see if I can give it a go, but as of now that's more a pledge of goodwill than decent skills. The deep guts of DBI are still very murky to me. But I'll do my best. :-) Joy, `/anick
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On Wed, Mar 14, 2012 at 02:00:19PM +, Martin J. Evans wrote: > > > >>There are cases where an imp_sth is passed as a parameter yet > >>D_imp_xxh(sth) is being called. So you end up with imp_sth and imp_xxh > >>pointers that point to the same thing. See dbd_st_fetch and dbd_bind_ph for > >>example. Just delete the D_imp_xxh(sth); line and change all imp_xxh in > >>the function to be imp_sth instead. There may be cases where the same > >>applies with imp_dbh. > > > >Yeah, thanks, I'll fix those. > > Those D_imp_xxh calls in dbd_st_fetch and dbd_bind_ph are simply there > because those fns call DBIh_SET_ERR_CHAR and that takes an imp_xxh_t. > I have removed them but then I need to cast imp_dbh/imp_sth to > imp_xxh_t before calling DBIh_SET_ERR_CHAR so that then makes we > wonder why imp_xxh_t is really required? just so you can pass an > imp_dbh_t or an imp_sth_t to it? > > The only places DBD::ODBC using imp_xxh_t is in calls to > DBIh_SET_ERR_CHAR and DBIc_TRACE (and a few calls to DBIc_TYPE). So > are you effectively saying imp_xxh_t exists for the simple reason it > could be either an imp_sth_t/imp_dbh_t and if that is so I presume I > can just delete all D_imp_xxh macros where the imp_xxh is passed to > DBIh_SET_ERR_CHAR/DBIc_TRACE, pass the real imp_*h and cast it and > avoid a call to dbih_getcom2? Yes. imp_xxh_t gives access to those struct elements that are common to all handles. It's always safe to cast a imp_??h_t to imp_xxh_t. Tim.
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On 14/03/2012 10:20, Tim Bunce wrote:
On Wed, Mar 14, 2012 at 08:24:36AM +0100, H.Merijn Brand wrote:
On Tue, 13 Mar 2012 21:44:06 +, Tim Bunce
wrote:
On Fri, Mar 02, 2012 at 03:11:17PM +, Tim Bunce wrote:
The second patch, which makes DBIS more efficient, is a lot more complex,
and more likely to break things (especially as it's changing a bunch of
macros that are directly #included by the DBD drivers. You may need to
bump API version numbers; I don't understand that bit.
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.
I finally got around to looking at DBD::Pg and was horrified to discover
the number of DBIS calls made by hot paths in the code. Most are hidden
behind various tracing macros. Even fetch calls 3 + 3 * num_of_fields!
When hidden, it won't stand out to the maintainers.
How do we/I see where it happens? Do you have a (short) guide to check
if my/a DBD (CSV, Unify, File) uses those too?
The slowness of the DBIS macro is only relevant for compiled drivers.
The DBIS macro is used to get a pointer to the DBI's struct dbistate_st.
In non-threaded perls it's simply a static variable initialised once.
In threaded perls it's a macro that has to do more expensive work.
For many years now all DBI handles have had their own pointer to
dbistate_st. Since almost all code in the driver has access to a handle
that's the best way to get the dbistate_st struct pointer.
Essentially it means changing all instances of DBIS to DBIc_DBISTATE(imp_xxh)
(where imp_xxh can be any of the imp_dbh, imp_sth etc variables).
Looking at DBIXS.h I see these other macros that use DBIS and thus
should be avoided in hot code:
DBILOGFP=> DBIc_LOGPIO(imp_xxh)
DBILOGMSG(...) => DBIc_DBISTATE(imp_xxh)->logmsg(...)
DBD::ODBC had none of the above.
neatsvpv(...) => DBIc_DBISTATE(imp_xxh)->neatsvpv(...)
DBD::ODBC had a number of these however, the suggested change does not work:
original:
if (DBIc_TRACE(imp_sth, DBD_TRACING, 0, 4)) {
TRACE2(imp_sth, "ODBC_CLEAR_RESULTS '%s' => %s\n",
key, neatsvpv(value,0));
}
new:
if (DBIc_TRACE(imp_sth, DBD_TRACING, 0, 4)) {
TRACE2(imp_sth, "ODBC_CLEAR_RESULTS '%s' => %s\n",
key, (DBIc_DBISTATE(imp_sth)->neatsvpv(value,0)));
}
compiler error:
dbdimp.c: In function 'odbc_clear_result_set':
dbdimp.c:233: error: expected identifier before '(' token
dbdimp.c: In function 'odbc_db_STORE_attrib':
dbdimp.c:4672: warning: cast to pointer from integer of different size
dbdimp.c:4677: warning: cast to pointer from integer of different size
dmake: Error code 129, while making 'dbdimp.o'
I've not really looked into it yet.
however, usually those are only called in tracing code so it may not be
worth the effort to change them.
ok, but I'm happy to do it in my case.
I've now changed DBD::ODBC to remove all unnecessary calls to D_imp_whatever.
The DBIh_COM(h) macro also uses DBIS but there's a chicken-and-egg
problem here since DBIh_COM is the underlying mechanism of the
D_imp_xxh(h) macros used to get the imp_xxh pointer. So it can't be
avoided. Yet more reason to apply Dave's changes.
Tim.
p.s. For the logic to control tracing I recommend the DBIc_TRACE macro
(or something driver-specific like it or built using it):
DBIc_TRACE(imp, flags, flag_level, fallback_level)
I will change DBD::Oracle to use DBIc_TRACE at the weekend. I've started
getting rid of DBIS from DBD::Oracle but as you said it is a bit of a PITA to
do although not difficult.
That's true if flags match the handle trace flags& handle trace
level>= flag_level, OR if handle trace_level> fallback_level
DBIc_TRACE(imp, 0, 0, 4) = if handle trace level>= 4
DBIc_TRACE(imp, DBDf_TRACE_FOO, 2, 4) = if tracing DBDf_TRACE_FOO& level>=2
OR level>=4
DBIc_TRACE(imp, DBDf_TRACE_FOO, 2, 0) = as above but never trace just due
to level
For example:
if (DBIc_TRACE(imp_xxh, DBIf_TRACE_SQL|DBIf_TRACE_xxx, 3, 7)) {
PerlIO_printf(DBIc_LOGPIO(imp_sth), "\tThe %s wibbled the %s\n", ...);
}
Martin
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On 15/03/12 10:16, Martin J. Evans wrote:
On 14/03/2012 10:20, Tim Bunce wrote:
On Wed, Mar 14, 2012 at 08:24:36AM +0100, H.Merijn Brand wrote:
On Tue, 13 Mar 2012 21:44:06 +, Tim Bunce
wrote:
On Fri, Mar 02, 2012 at 03:11:17PM +, Tim Bunce wrote:
The second patch, which makes DBIS more efficient, is a lot more complex,
and more likely to break things (especially as it's changing a bunch of
macros that are directly #included by the DBD drivers. You may need to
bump API version numbers; I don't understand that bit.
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.
I finally got around to looking at DBD::Pg and was horrified to discover
the number of DBIS calls made by hot paths in the code. Most are hidden
behind various tracing macros. Even fetch calls 3 + 3 * num_of_fields!
When hidden, it won't stand out to the maintainers.
How do we/I see where it happens? Do you have a (short) guide to check
if my/a DBD (CSV, Unify, File) uses those too?
The slowness of the DBIS macro is only relevant for compiled drivers.
The DBIS macro is used to get a pointer to the DBI's struct dbistate_st.
In non-threaded perls it's simply a static variable initialised once.
In threaded perls it's a macro that has to do more expensive work.
For many years now all DBI handles have had their own pointer to
dbistate_st. Since almost all code in the driver has access to a handle
that's the best way to get the dbistate_st struct pointer.
Essentially it means changing all instances of DBIS to DBIc_DBISTATE(imp_xxh)
(where imp_xxh can be any of the imp_dbh, imp_sth etc variables).
Looking at DBIXS.h I see these other macros that use DBIS and thus
should be avoided in hot code:
DBILOGFP => DBIc_LOGPIO(imp_xxh)
DBILOGMSG(...) => DBIc_DBISTATE(imp_xxh)->logmsg(...)
DBD::ODBC had none of the above.
neatsvpv(...) => DBIc_DBISTATE(imp_xxh)->neatsvpv(...)
DBD::ODBC had a number of these however, the suggested change does not work:
original:
if (DBIc_TRACE(imp_sth, DBD_TRACING, 0, 4)) {
TRACE2(imp_sth, " ODBC_CLEAR_RESULTS '%s' => %s\n",
key, neatsvpv(value,0));
}
new:
if (DBIc_TRACE(imp_sth, DBD_TRACING, 0, 4)) {
TRACE2(imp_sth, " ODBC_CLEAR_RESULTS '%s' => %s\n",
key, (DBIc_DBISTATE(imp_sth)->neatsvpv(value,0)));
}
compiler error:
dbdimp.c: In function 'odbc_clear_result_set':
dbdimp.c:233: error: expected identifier before '(' token
dbdimp.c: In function 'odbc_db_STORE_attrib':
dbdimp.c:4672: warning: cast to pointer from integer of different size
dbdimp.c:4677: warning: cast to pointer from integer of different size
dmake: Error code 129, while making 'dbdimp.o'
I've not really looked into it yet.
however, usually those are only called in tracing code so it may not be
worth the effort to change them.
ok, but I'm happy to do it in my case.
I've now changed DBD::ODBC to remove all unnecessary calls to D_imp_whatever.
The DBIh_COM(h) macro also uses DBIS but there's a chicken-and-egg
problem here since DBIh_COM is the underlying mechanism of the
D_imp_xxh(h) macros used to get the imp_xxh pointer. So it can't be
avoided. Yet more reason to apply Dave's changes.
Tim.
p.s. For the logic to control tracing I recommend the DBIc_TRACE macro
(or something driver-specific like it or built using it):
DBIc_TRACE(imp, flags, flag_level, fallback_level)
I will change DBD::Oracle to use DBIc_TRACE at the weekend. I've started
getting rid of DBIS from DBD::Oracle but as you said it is a bit of a PITA to
do although not difficult.
That's true if flags match the handle trace flags& handle trace
level>= flag_level, OR if handle trace_level> fallback_level
DBIc_TRACE(imp, 0, 0, 4) = if handle trace level>= 4
DBIc_TRACE(imp, DBDf_TRACE_FOO, 2, 4) = if tracing DBDf_TRACE_FOO& level>=2 OR
level>=4
DBIc_TRACE(imp, DBDf_TRACE_FOO, 2, 0) = as above but never trace just due to
level
For example:
if (DBIc_TRACE(imp_xxh, DBIf_TRACE_SQL|DBIf_TRACE_xxx, 3, 7)) {
PerlIO_printf(DBIc_LOGPIO(imp_sth), "\tThe %s wibbled the %s\n", ...);
}
Martin
Argh, DBD::Oracle tracing is a mess wrt to this discussion:
ocitrace.h:
#define DBD_OCI_TRACEON (DBIS->debug >= 6 || dbd_verbose>=6)
#define DBD_OCI_TRACEFP (DBILOGFP)
#define OCIServerRelease_log_stat(sc,errhp,b,bl,ht,ver,stat)\
stat =OCIServerRelease(sc,errhp,b,bl,ht,ver);\
(DBD_OCI_TRACEON) \
? PerlIO_printf(DBD_OCI_TRACEFP,\
"%sOCIServerRelease(%p)=%s\n",\
OciTp, sc,oci_status_name(stat)),stat \
: stat
Every single OCI call uses DBD_OCI_TRACEON which in turn uses DBIS->debug and non have a
imp_xxx handle so this is a very large change. Ensuring the code at each point an OCI call
is made has an imp_xxh and getting the right one is going to be an awful job especially when
a load of funcs in oci
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On Thu, Mar 15, 2012 at 10:33:43AM +, Martin J. Evans wrote: > > Argh, DBD::Oracle tracing is a mess wrt to this discussion: > > ocitrace.h: > > #define DBD_OCI_TRACEON (DBIS->debug >= 6 || dbd_verbose>=6) > #define DBD_OCI_TRACEFP (DBILOGFP) > > #define OCIServerRelease_log_stat(sc,errhp,b,bl,ht,ver,stat)\ > stat =OCIServerRelease(sc,errhp,b,bl,ht,ver);\ > (DBD_OCI_TRACEON) \ > ? PerlIO_printf(DBD_OCI_TRACEFP,\ >"%sOCIServerRelease(%p)=%s\n",\ >OciTp, sc,oci_status_name(stat)),stat \ > : stat > > Every single OCI call uses DBD_OCI_TRACEON which in turn uses > DBIS->debug and non have a imp_xxx handle so this is a very large > change. Ensuring the code at each point an OCI call is made has an > imp_xxh and getting the right one is going to be an awful job > especially when a load of funcs in oci8.c don't even have a handle. I > don't see an easy way to automate this change so I'm not sure I've got > the stomach for this. If I do this will I really "see" some speed up > as it is a lot of work. Maybe just do the OCI* function calls that are used in the main fetch code path, i.e., dbd_st_fetch. Add an imp parameter -#define OCIServerRelease_log_stat(sc,errhp,b,bl,ht,ver,stat) +#define OCIServerRelease_log_stat(imp,sc,errhp,b,bl,ht,ver,stat) stat =OCIServerRelease(sc,errhp,b,bl,ht,ver);\ ! (DBIc_TRACE(imp,...)) \ ! ? PerlIO_printf(DBIc_LOGPIO(imp),\ "%sOCIServerRelease(%p)=%s\n",\ OciTp, sc,oci_status_name(stat)),stat \ : stat Tim.
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On 15/03/12 21:18, Tim Bunce wrote:
On Thu, Mar 15, 2012 at 10:33:43AM +, Martin J. Evans wrote:
Argh, DBD::Oracle tracing is a mess wrt to this discussion:
ocitrace.h:
#define DBD_OCI_TRACEON (DBIS->debug>= 6 || dbd_verbose>=6)
#define DBD_OCI_TRACEFP (DBILOGFP)
#define OCIServerRelease_log_stat(sc,errhp,b,bl,ht,ver,stat)\
stat =OCIServerRelease(sc,errhp,b,bl,ht,ver);\
(DBD_OCI_TRACEON) \
? PerlIO_printf(DBD_OCI_TRACEFP,\
"%sOCIServerRelease(%p)=%s\n",\
OciTp, sc,oci_status_name(stat)),stat \
: stat
Every single OCI call uses DBD_OCI_TRACEON which in turn uses
DBIS->debug and non have a imp_xxx handle so this is a very large
change. Ensuring the code at each point an OCI call is made has an
imp_xxh and getting the right one is going to be an awful job
especially when a load of funcs in oci8.c don't even have a handle. I
don't see an easy way to automate this change so I'm not sure I've got
the stomach for this. If I do this will I really "see" some speed up
as it is a lot of work.
Maybe just do the OCI* function calls that are used in the main fetch
code path, i.e., dbd_st_fetch. Add an imp parameter
-#define OCIServerRelease_log_stat(sc,errhp,b,bl,ht,ver,stat)
+#define OCIServerRelease_log_stat(imp,sc,errhp,b,bl,ht,ver,stat)
stat =OCIServerRelease(sc,errhp,b,bl,ht,ver);\
! (DBIc_TRACE(imp,...)) \
! ? PerlIO_printf(DBIc_LOGPIO(imp),\
"%sOCIServerRelease(%p)=%s\n",\
OciTp, sc,oci_status_name(stat)),stat \
: stat
Tim.
Last night I finished changing DBD::Oracle to eradicate all DBIS usage. I ran
into quite a few problems along the way but all DBIS usage is gone except for a
few calls in functions passed to Oracle. I've not moved to DBIc_TRACE yet but
when I get time I will do that and add the new DBD trace flag as well
(hopefully to replace ora_verbose which seems pointless to me simply adding yet
another test when no-one really uses it other than as on/off).
As a side note there is a phenomenal amount of tracing in DBD::Oracle which it
would be nice to noop the whole lot out for people who don't want the code
continually testing whether tracing is on - me.
A quick benchmark:
use DBI;
use strict;
use warnings;
use Benchmark;
my $h = DBI->connect("dbi:Oracle:host=xxx.easysoft.local;sid=test",
"xxx", "xxx", {RaiseError => 1});
if (@ARGV) {
eval {
$h->do(q/drop table dbis/);
};
setup($h);
}
timethese(10, {
'read' => sub {readit($h)},
'readperrow' => sub {readitperrow($h)}});
#readit($h);
sub setup {
$h->do(q/create table dbis (a int, b varchar(100))/);
$h->begin_work;
my $s = $h->prepare(q/insert into dbis values(?,?)/);
foreach (1..100) {
$s->execute($_, "the quick brown fox jumps over the lazy dog");
}
$h->commit;
print "Table populated\n";
}
sub readit {
my $h = shift;
my $s = $h->prepare(q/select * from dbis/);
$s->execute;
my $d = $s->fetchall_arrayref;
print "Read ", scalar(@$d), " rows\n";
}
sub readitperrow {
my $h = shift;
my $s = $h->prepare(q/select * from dbis/);
$s->execute;
my $rows = 0;
while(my $d = $s->fetchrow_arrayref) {
$rows++;
}
print "Read $rows rows\n";
}
perl 5.14.2
Perl without threads:
1.42:
read: 83 wallclock secs (37.55 usr + 5.77 sys = 43.32 CPU) @ 0.23/s (n=10)
readperrow 85 wallclock secs (39.53 usr + 4.98 sys = 44.51 CPU) @ 0.22/s
(n=10)
subversion trunk:
read: 85 wallclock secs (40.23 usr + 6.22 sys = 46.45 CPU) @ 0.22/s (n=10)
readperrow: 85 wallclock secs (40.06 usr + 5.32 sys = 45.38 CPU) @ 0.22/s
(n=10)
Perl with threads:
1.42
read: 128 wallclock secs (86.11 usr + 5.41 sys = 91.52 CPU) @ 0.11/s (n=10)
readperrow: 137 wallclock secs (95.33 usr + 4.86 sys = 100.19 CPU) @ 0.10/s
(n=10)
subversion trunk:
read: 94 wallclock secs (52.55 usr + 5.68 sys = 58.23 CPU) @ 0.17/s (n=10)
readperrow: 104 wallclock secs (62.74 usr + 5.06 sys = 67.80 CPU) @ 0.15/s
(n=10)
which only goes to remind me why I don't use a Perl with threads but if you do,
DBD::Oracle should be a fair bit faster now.
BTW, this change is literally thousands of lines of code so if you depend on
DBD::Oracle I'd get a copy of the subversion trunk and try it.
Martin
--
Martin J. Evans
Easysoft Limited
http://www.easysoft.com
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On Fri, Mar 16, 2012 at 02:18:00PM +, Martin J. Evans wrote: > > Perl with threads: > 1.42 > read: 128 wallclock secs (86.11 usr + 5.41 sys = 91.52 CPU) @ 0.11/s > (n=10) > readperrow: 137 wallclock secs (95.33 usr + 4.86 sys = 100.19 CPU) @ > 0.10/s (n=10) > > subversion trunk: > read: 94 wallclock secs (52.55 usr + 5.68 sys = 58.23 CPU) @ 0.17/s (n=10) > readperrow: 104 wallclock secs (62.74 usr + 5.06 sys = 67.80 CPU) @ > 0.15/s (n=10) > > which only goes to remind me why I don't use a Perl with threads but if you > do, DBD::Oracle should be a fair bit faster now. So 92 reduced to 58 cpu, and 100 reduced to 68. Those are impressive gains! > BTW, this change is literally thousands of lines of code so if you depend on > DBD::Oracle I'd get a copy of the subversion trunk and try it. Thanks for doing the work Martin! Tim.
Re: DBD::Pg, DBD::Oracle and others are slow with threads due to DBIS
On 12-03-16 10:18 AM, Martin J. Evans wrote: Last night I finished changing DBD::Oracle to eradicate all DBIS usage. Holy schmollee, you're a beast. I've synced the Git repo with your work (branch master with the raw goodness, branch DBIS-less for a version that can be directly be installed from the checkout). Joy, `/anick
