Re: speeding up XS_DBI_dispatch()

On Mon, Jan 30, 2012 at 12:48:32PM +0000, Dave Mitchell wrote:
> On Sun, Jan 29, 2012 at 10:06:58PM +0000, Tim Bunce wrote:
> > On Sun, Jan 29, 2012 at 06:12:50PM +0000, Dave Mitchell wrote:
> > > The code itself (see diff below) just attaches some magic to the outer CV
> > > that caches the GV of the corresponding inner method.
> > 
> > Any reason for not using the existing 'internal method attribute'
> > mechanism? _install_method() already allocates a structure and
> > attaches it the the CV using CvXSUBANY(cv).any_ptr.  You could add the
> > three elements of method_cache_t directly into dbi_ima_t.  Then, since
> > the dispatch code already has the structure pointer (ima) handy, your
> > cache code could avoid the mg_findext call and use ima->method_cache
> > directly.
> 
> Well, the original reason was that I wasn't aware of dbi_ima_t :-)
> 
> However, on further reflection, I'm not sure that would work, as
> ima->method_cache would contain reference-counted SV pointers that would
> need to be freed when the CV is freed, and be duped when the CV is cloned
> in a new thread.  But there is no specific mechanism to allow this to
> happen, so I'd have to mess with CLONE (and maintain a list of special CVs
> that need cloning), and attach magic that has a free() method to ensure
> that ima is freed when the CV is. Which is all beginning to sound rather
> complex.

*nods*

> So I'm tempted to stick with the magic approach. I could optimise
> it to assume the cache magic is always the first one (and if not, take a
> slower route that finds and then moves it to the top).

That would be neat. Thanks.

dbih_getcom2 does the first part of that when looking up DBI hande
magic. I never added the "move to the top" because I figured it would be
very rare for magic to get added to the inner hash of a DBI handle.
It seems even less likely that someone would add magic to a DBI method,
but you never know. Unless you can think of a plausible case I'd be
happy if you just added the short-cut+fallback and skipped the shuffle.

> That's also caused me to realise that my current code isn't threadsafe, as
> it it doesn't dup the cache entries. Easily fixed, though.

Ah. I scratched my head for a while looking for threadafe issues and
missed that one. I'm glad you found it.

Thanks again Dave.

Tim.