[Bioperl-l] A better fix for Species.pm memory leak, please REVIEW.
cjfields at illinois.edu
Sat Nov 22 23:22:25 EST 2008
On Nov 22, 2008, at 6:21 PM, George Hartzell wrote:
> I dug back into the Species.pm memory leak and my less-than-ideal fix
> for it.
> First, in my defense, my previous fix only causes failures in
> t/Species.t if network tests are enabled. It passes the generic
> ones. I *thought* I'd run the tests before I committed :).
> The basic cause of the problem is that when species() calls
> Bio::Tree::Tree->new(-node=> $species_taxon), $species_taxon is a copy
> of $self. The tree ends up adding that node onto the classification
> list and it gets linked it into the descendent links, which results in
> a cycle.
> If that lousy english description left you scratching your head,
> here's a hacked-to-pieces version of R Voss's original test case.
> You'll need Devel::Cycle (YAY LINCOLN!) to run it. Find yourself a
> genbank file (I used gbpri21.seq, from the original bug report), put
> it into a directory somewhere and do something like
> Anyway. Here's a fix. Instead of adding $self to the tree, make a
> new species node w/ $self's important bits and add that instead.
> There's a patch below that gets rid of my weaken and Sendu's
> fix-for-the-fix and then Does The Right Thing (I hope).
> It seems to pass all of the Species.t tests.
> The take home exam questions are: Are there any other missing
> important bits?
The only thing I can suggest is to make sure it deals with the
original problem (memory leaks, per bug 2594):
I'm not quite sure why we are attaching a Bio::Tree::Tree to the
Bio::Species, so I need to go back and review the changes from the
1.5.2 release (I'm sure there is a good reason, just can't recall and
haven't had time to look).
However, we aren't planning on retaining Bio::Species too much longer
anyway (we'll be moving towards a simpler Bio::Taxon-based system
after the 1.6 release and will be deprecating Bio::Species
eventually). Any fixes should probably take this into consideration.
> I'll commit it if someone'll review it.
I vote to go ahead and commit this (the solution seems sound), but
Sendu is the one who would be best to vet as he refactored this. If
there isn't word by the middle of the week you can go ahead and make
the commit. Thanks George!
More information about the Bioperl-l