Re: speeding up XS_DBI_dispatch()

On Wed, Jan 25, 2012 at 04:14:07PM +0000, Dave Mitchell wrote:
> On Tue, Jan 24, 2012 at 08:42:28PM +0000, Tim Bunce wrote:
> 
> Given that my hacky cache for fetch() lookup reduced overall execution
> time by 16.5%, that reduces overall XS_DBI_dispatch percentage from 33.2%
> to 16.7%, i.e. nearly halves the dispatch time.

Nice.

> > The guts of the DBI are ancient and baroque with many dusty corners
> > were dragons may, or may not, be lurking.
> 
> Oh dear :-)
> 
> Just out of curiosity (since I'm in the middle of trying to understand
> the code in XS_DBI_dispatch), what is the main purpose of having all
> functions share a big dispatch fn which calls the inner method, rather
> than having a separate function for each?

Almost all the functionality in the dispatcher (tracing, RaiseError,
Callbacks, Profiling etc.) would need to be duplicated in each method.

> > > * To what extent (if any) is DBI allowed to cheat when it comes to
> > >   bypassing the perl API (in particular I'm thinking of the method
> > >   invalidation stuff which I don't think has always been part of the API).
> > 
> > The DBI is always allowed to cheat in the name of performance!
> > My simple but effective xsbypass hack doesn't seem to have caused
> > problems in the years it's been there (though it's a pity threaded perls
> > can't use it).
> 
> Just out of curiosity, why can't threaded perls use it?

To be honest I can't remember now.  It's disabled
#if (defined USE_THREADS || defined PERL_CAPI || defined PERL_OBJECT)
Can you think of a reason? All tests pass. Maybe I was just being conservative.

> PS - I'm specifically being paid only to fix a performance issue on
> non-threaded builds, so I won't be looking at threaded issues. But
> dPERINTERP looks like bad news on threaded builds.

The dPERINTERP stuff was added by ActiveState years ago to support
MULTIPLICITY. I don't remember the details now. I do recall that driver
authors are encouraged to avoid using the DBIS macro due to the cost.
There's rarely a need now as DBI handles carry a pointer to the
dbi_state structure. The DBI::DBD docs say

    The names C<dbis> and C<DBIS>, which were used in previous versions of
    this document, should be replaced with the C<DBIc_STATE(imp_xxh)> macro.

Hopefully most compiled drivers are following that advice now.
Ah, actually that's wrong! It should say DBIc_DBISTATE(imp_xxh).
Fixed in r15098.

[goes to look at the DBI.xs sources] Oh, wow. There are more dPERINTERP's
left than I thought. I've just eliminated a bunch of them in r15099.
Several were in hot paths, including fetchrow_arrayref and handle creation
and destruction. That's a useful win for threaded perl.

Thanks Dave!

Tim.