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.
