On Fri, Mar 27, 2009 at 08:21:54AM -0400, Michael McCandless wrote:
> what if the Lucy obj never held onto the reference to the mirror host obj?
> (And the Lucy obj did its own reference counting, separately from the
> host's GC).
What you've laid out is *exactly* the same scheme Peter Karman used in SWISH3,
the same scheme Dave Balmain used in Ferret, and the same scheme I used in
KinoSearch for objects which subclass FastObj.
Since this algorithm seems to be an intuitive winner, and since I'd like to
just solve this problem and move on, I took a stab at converting KS over to
using it exclusively. Switching over to something that everybody else seems to
grok would be great -- indeed, I was rooting for my cached-host-object algo to
lose. Sacrificing a little performance for the sake of an implementation that
other people might have less difficulty understanding, maintaining, and
modifying would be just fine with me.
The results were disappointing. The cached-host-object algo has certain
advantages we should be reluctant to discard.
> When we cross the bridge, Lucy to host, we would make a new host
> wrapper obj each time. These host mirror objs are very lightweight
> wrappers, right?
FWIW, the cached-host-object algo wins the tight loop performance contest.
Below my sig, you'll find a patch that applies against KinoSearch svn revision
4361, and a benchmarking script. The patch gives TermQuery a member var named
"thing", plus Set_Thing() and Get_Thing() methods. The loop compares the
performance of calling $term_query->get_thing from Perl-space when "thing" is a
VArray (which subclasses FastObj) vs. when "thing" is a Hash (which subclasses
Obj).
The only difference in those loops is the behavior of FastObj_to_host (which
creates a new host object every time) vs. Obj_to_host (which exploits a cached
host object). The results:
mar...@smokey:~/projects/ks4361/perl $ perl -Mblib time_callbacks.pl
Rate no_cached_object cached_object
no_cached_object 1.93/s -- -66%
cached_object 5.71/s 196% --
On its own, this performance gap probably doesn't justify sacrificing clarity
of implementation. It only matters for host-language subclassing, where
performance is going to be heavily compromised anyway -- and if your method
does something non-trivial, this effect is going to get swamped.
However...
> But what do we lost by not retaining a permanent host obj wrapper?
We lose convenient use of the "inside-out-object" pattern. :(
The "inside-out object" design pattern is a technique that has been increasing
in popularity within the Perl community for a few years now -- Perl 5.10
provides the Hash::Util::FieldHash class specifically to support it:
http://perldoc.perl.org/Hash/Util/FieldHash.html#The-Inside-out-Technique
The inside-out technique is the *only* way we will be able to attach arbitary
data to a pure-Perl subclass of a Lucy object. Within the KinoSearch
distribution, all of the pure-Perl KSx classes depend on it --
KSx::Search::MockScorer, KSx::Simple, KSx::Remote::SearchServer,
KSx::Remote::SearchClient, etc -- and it is essential for making pure-Perl
subclassing of Lucy feasible.
Here's example code from the docs for KinoSearch::Obj which illustrates the
technique, attaching a "foo" variable to our "MyObj" subclass.
package MyObj;
use base qw( KinoSearch::Obj );
# Inside-out member var.
my %foo;
sub new {
my ( $class, %args ) = @_;
my $foo = delete $args{foo};
my $self = $class->SUPER::new(%args);
$foo{$$self} = $foo;
return $self;
}
sub get_foo {
my $self = shift;
return $foo{$$self};
}
sub DESTROY {
my $self = shift;
delete $foo{$$self};
$self->SUPER::DESTROY;
}
The problem is that DESTROY method. It is vital, as the Hash::Util::FieldHash
documentation explains:
When a normal object dies, anything stored in the object body is
garbage-collected by perl. With inside-out objects, Perl knows nothing
about the data stored in field hashes by a class, but these must be
deleted when the object goes out of scope. Thus the class must provide a
DESTROY method to take care of that.
Under the cached-host-object algorithm, that DESTROY method is called only
once, when the Lucy object has really had its refcount fall to 0. However, if
we create host wrappers for each callback, DESTROY gets called *multiple*
times, and the attached data gets zapped too soon.
It is techically possible to address the problem like so:
sub DESTROY {
my $self = shift;
if ( $self->get_refcount == 1 ) {
delete $foo{$$self};
}
$self->SUPER::DESTROY;
}
However, having to explain to the user that DESTROY will be called many times
for each Lucy object -- and having to expose get_refcount and explain why it's
needed -- makes for a stupid and ugly API.
> (Note that if it's a real performance problem, then in certain cases
> you could eg explicitly retain a reference at the top of the loop, do
> the loop, then drop the reference at the end).
This is a novel idea. I'd really like to get to a place where someone besides
me would feel comfortable going in and making such an optimization.
I've at least made some progress on the simplification front, which I'll
explain in another post.
Marvin Humphrey
##############################################
# Benchmark Obj_to_host vs. FastObj_to_host. #
##############################################
use strict;
use warnings;
use KinoSearch;
use Benchmark qw( cmpthese );
my $foo_query = KinoSearch::Search::TermQuery->new(
field => 'content',
term => 'foo',
);
my $bar_query = KinoSearch::Search::TermQuery->new(
field => 'content',
term => 'bar',
);
$foo_query->set_thing( KinoSearch::Util::VArray->new( capacity => 16 ) );
$bar_query->set_thing( KinoSearch::Util::Hash->new( capacity => 16 ) );
cmpthese(
10,
{ no_cached_object => sub { $foo_query->get_thing for 1 .. 100000 },
cached_object => sub { $bar_query->get_thing for 1 .. 100000 },
}
);
#############################################
# Patch to add "thing" member to TermQuery. #
#############################################
Index: ../core/KinoSearch/Search/TermQuery.bp
===================================================================
--- ../core/KinoSearch/Search/TermQuery.bp (revision 4361)
+++ ../core/KinoSearch/Search/TermQuery.bp (working copy)
@@ -12,6 +12,7 @@
CharBuf *field;
Obj *term;
+ Obj *thing;
static incremented TermQuery*
new(const CharBuf *field, const Obj *term);
@@ -33,6 +34,12 @@
Obj*
Get_Term(TermQuery *self);
+ public void
+ Set_Thing(TermQuery *self, Obj *thing);
+
+ public Obj*
+ Get_Thing(TermQuery *self);
+
public incremented Compiler*
Make_Compiler(TermQuery *self, Searchable *searchable, float boost);
Index: ../core/KinoSearch/Search/TermQuery.c
===================================================================
--- ../core/KinoSearch/Search/TermQuery.c (revision 4361)
+++ ../core/KinoSearch/Search/TermQuery.c (working copy)
@@ -29,6 +29,7 @@
Query_init((Query*)self, 1.0f);
self->field = CB_Clone(field);
self->term = Obj_Clone(term);
+ self->thing = NULL;
return self;
}
@@ -55,6 +56,7 @@
{
DECREF(self->field);
DECREF(self->term);
+ DECREF(self->thing);
Query_destroy((Query*)self);
}
@@ -63,6 +65,15 @@
Obj*
TermQuery_get_term(TermQuery *self) { return self->term; }
+Obj*
+TermQuery_get_thing(TermQuery *self) { return self->thing; }
+void
+TermQuery_set_thing(TermQuery *self, Obj *thing)
+{
+ DECREF(self->thing);
+ self->thing = thing ? INCREF(thing) : NULL;
+}
+
bool_t
TermQuery_equals(TermQuery *self, Obj *other)
{
Index: ../perl/lib/KinoSearch/Search/TermQuery.pm
===================================================================
--- ../perl/lib/KinoSearch/Search/TermQuery.pm (revision 4361)
+++ ../perl/lib/KinoSearch/Search/TermQuery.pm (working copy)
@@ -41,7 +41,7 @@
END_CONSTRUCTOR
{ "KinoSearch::Search::TermQuery" => {
- bind_methods => [qw( Get_Field )],
+ bind_methods => [qw( Get_Field Set_Thing Get_Thing )],
make_constructors => ["new"],
make_pod => {
synopsis => $synopsis,
