[KinoSearch] Orderly global destruction

[Marvin Humphrey]

On Feb 20, 2008, at 8:04 PM, Nathan Kurz wrote:

> On 2/20/08, Marvin Humphrey <marvin at rectangular.com> wrote:
>> It's kind of
>> pointless to create and then immediately destroy the outer RV, but we
>> have to do things that way because there are no public Perl C API
>> functions that allow for the direct creation of an inner object.
>
> It's been a while since I've done this, but isn't the inner object
> just a SV with the IV field pointing to the C object?  If so, could
> you create it directly?  I think the exact type of SV shouldn't matter
> much, as everything (I think ) can store an IV.
>
> A little quick web searching seems to show that current idiom is
> something like sv_set_iv(sv, PTR2IV(ptr)).  Is the problem that PTR2IV
> isn't public?  If so, I'd guess that would have to be just an
> oversight, as traditionally it was just a (IV) cast.

PTR2IV is documented in perlguts.  It's not in perlapi, but it should  
be.  Using it is safe.

However, it's not that simple.  These are the changes to trunk/perl/xs/ 
KinoSearch/Obj.c that it would really take to switch to direct creation:

+    /* Code cribbed from Perl_sv_bless, in sv.c. */
+    stash = gv_stashpvn(self->_->class_name, self->_->class_name_len,  
TRUE);
+    inner_obj = newSV(0);
+    SvOBJECT_on(inner_obj);
+    PL_sv_objcount++;
+    SvUPGRADE(inner_obj, SVt_PVMG);
+    SvSTASH_set(inner_obj, (HV*)SvREFCNT_inc(stash));
+    sv_setiv(inner_obj, PTR2IV(self));
+    self->ref.native = inner_obj;

As the comment indicates, the code was derived from Perl_sv_bless in  
sv.c, which gets called via pp_bless in pp.c.  It was a PITA to figure  
out what was needed and what wasn't, which is partly why I hadn't  
gotten to this before.  Perl_sv_bless has barely changed from 5.8.3 to  
5.10.0, so I think that at least that code would work smoothly across  
all Perl versions supported by KS.

However, these are non-public API invocations:

   * SvOBJECT_on is undocumented, but it just sets a flag.
   * I have no idea what PL_sv_objcount is for, so I'm cargo-culting  
there.

We also have to modify kino_Doc_to_native, in trunk/perl/xs/KinoSearch/ 
Doc.c:

+    HV *stash
+        = gv_stashpvn(self->_->class_name, self->_->class_name_len,  
true);
+    Gv_AMupdate(stash);
+

What that does is turn on overloading.  I think it only needs to be  
called once per class, but we have to keep invoking it in case of  
subclassing.

Gv_AMupdate is undocumented.  I extracted it from the Gv_AMG  macro,  
also undocumented, used by sv_bless:

     if (Gv_AMG(stash))
         SvAMAGIC_on(sv);
     else
         (void)SvAMAGIC_off(sv);

Gv_AMG is defined in sv.h:

   #define Gv_AMG(stash)    (PL_amagic_generation && Gv_AMupdate(stash))

 From my testing, it looks like as soon as there's any class that  
turns on overloading, PL_amagic_generation turns positive and stays  
positive, causing Gv_AMupdate(stash) to be invoked with each call to  
sv_bless().

I should also mention that KS currently uses the macro SvAMAGIC_on in  
Doc.c, even though it's undocumented.  That was what it took to get  
overloading working for Doc under Perl 5.8.x.

> The solution you described seems like it would work fine, but maybe
> the direct creation approach would sidestep the need for it.

The complexity resides in the concept.  We're only talking about a few  
lines of code, but writing them takes a fairly deep understanding of  
the Perl code base.

The disappointing thing is that after all the work that it took to  
disentangle sv_bless and suss out the direct creation approach, I see  
absolutely no difference in the benchmarks.  Creating and destroying  
that extra RV just doesn't seem to matter.

So, we could commit the attached patch implementing direct creation,  
and the conceptual basis of kino_Obj_create would marginally less  
wacky, but to make that happen we have to use a bunch of undocumented  
Perl internal functions.  I appreciate the nudge that it took to get  
me to fully explore this path, but it looks like a dead end.

Marvin Humphrey
Rectangular Research
http://www.rectangular.com/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: no_rv.diff
Type: application/octet-stream
Size: 1930 bytes
Desc: not available
Url : http://www.rectangular.com/pipermail/kinosearch/attachments/20080221/17314a1a/no_rv.obj
-------------- next part --------------