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 +0000, Martin J. Evans wrote:
>>> On 13/03/12 21:44, Tim Bunce wrote:
>>>> On Fri, Mar 02, 2012 at 03:11:17PM +0000, 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