Mailing-List: contact lucy-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: lucy-dev@incubator.apache.org
Received-SPF: pass (athena.apache.org: local policy)
Date: Wed, 2 Mar 2011 22:30:01 -0800
From: Marvin Humphrey <marvin@rectangular.com>
To: lucy-dev@incubator.apache.org
Message-ID: <20110303063001.GA5707@rectangular.com>
References: <20110303030834.GB5339@rectangular.com>
 <AANLkTikZv8FqYnnjx797GWFmhOcfVxRCr4vxONAhjztF@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <AANLkTikZv8FqYnnjx797GWFmhOcfVxRCr4vxONAhjztF@mail.gmail.com>
User-Agent: Mutt/1.5.18 (2008-05-17)
Subject: Re: [lucy-dev] Host overriding of all non-final methods

On Wed, Mar 02, 2011 at 09:28:07PM -0800, Nathan Kurz wrote:
> On Wed, Mar 2, 2011 at 7:08 PM, Marvin Humphrey <marvin@rectangular.com> wrote:
> > For "final" methods, we have two options. �We can fail silently, as we do now.
> > In this case, there will be different behavior when a method is invoked from
> > the host (the illegal host override method fires) vs. when it is invoked from
> > within the Lucy core (the "final" method definition fires).
> >
> > The other option is to throw an exception at runtime when an attempt to
> > override a final method is detected. �Dynamic VTable objects are built lazily,
> > so the error would occur the first time the constructor for the problematic
> > class gets invoked.
> 
> Orient me for a moment:  this would have to be caught in the host
> language, and would need to be written this way for every language
> that there are bindings?  

The code that walks the host language OO structure needs to be custom-coded
for each binding.  The function VTable_novel_host_methods() fills that role.
Here's the implementing code for our Perl bindings, taken from from
perl/lib/Lucy.pm:

    sub novel_host_methods {
        my ( undef, $package ) = @_;
        no strict 'refs';
        my $stash   = \%{"$package\::"};
        my $methods = Lucy::Object::VArray->new(
            capacity => scalar keys %$stash );
        while ( my ( $symbol, $glob ) = each %$stash ) {
            next if ref $glob;
            next unless *$glob{CODE};
            $methods->push( Lucy::Object::CharBuf->new($symbol) );
        }
        return $methods;
    }

> Or is there somewhere that this could be caught by Lucy core?  

Checking to see whether a method is final can be done in the Lucy core, in
core/Lucy/Object/VTable.c -- so it doesn't need to be reimplemented for each
host.  There's some crude introspection metadata hanging off of each VTable;
we can store booleans in that metadata indicating which methods are final and
check the method names returned by novel_host_methods() to see whether an
illegal override has happened.  If we really want to.

> And there aren't many final methods are there?

Correct.  There are presently only three classes that have final methods.

Most methods on InStream and OutStream are final, because InStream and
OutStream are very performance sensitive -- they always show up in profiling
data.  The classes aren't final, so you can subclass them and add *more*
methods, e.g. to integrate additional decoders -- but you can't redefine
existing methods.

Then there's an internal class called InverterEntry.  It's a final class, so
all its methods are final.

> Only on extremely performance sensitive paths?

Right.  But even then, I don't recall that we got a lot of juice out of making
those methods final.  Our compiler isn't smart enough to extract the method
body from the source C file and inline it into another C file, like a HotSpot
compiler might do with a final method in Java.  The Clownfish compiler just
aliases the method to the implementing function, avoiding the double-deref of
retrieving the method pointer from the vtable.  There's not a lot to be saved
there.

Arguably, we don't even need the "final" keyword.  We'd should benchmark to
confirm my recollection about the performance implications, but I'll bet we
could remove it with no immediate impact on Lucy.  If we were doing fancier
stuff in our compiler we might, but as it stands, I think we're going to need
to declare code "inline" and put it into header files if we want the C
compiler to do perform aggressive inlining optimizations.

The file core/Lucy/Util/NumberUtils.cfh contains many such inline functions.  I
anticipate using those within posting decoders to operate directly on mmap'd
buffers.  Perhaps that's a better way forward given that we're always going to
be working within the constraints of a C compiler that operates file-by-file.

> > Right this moment I'm feeling lazy, so inertia is favoring the first option for
> > final methods, "fail silently." :) �Failing at runtime when illegal method
> > overriding from the host is detected isn't as friendly as failing at compile
> > time when an illegal override is found in a .cfh Clownfish header file, though.
> 
> That seems like the wrong kind of lazy. :)  I think the right kind of
> lazy is to solve it once by brute force:  ASSERT(! $method->final).
> But since true macros are hard in Perl, I'd be happy with adding an
> 'if DEBUG' to that so that it can get optimized away at compile if one
> wants it to be.  But you never really got on the ASSERT train, did
> you?

Haha, that's true.  When code can be verified via unit tests, I prefer that,
since it stores the noise out of band in a unit test file.  I'm not a fan of
the way asserts pollute the main code base.

Marvin Humphrey