[Bioperl-l] A better fix for Species.pm memory leak, please REVIEW.
cjfields at illinois.edu
Tue Nov 25 00:57:31 EST 2008
On Nov 24, 2008, at 10:59 PM, George Hartzell wrote:
>> In this case, when the last ref to the Bio::Species is released it
>> should be garbage collected and trigger DESTROY (which is set up in
>> Bio::Root::Root). This in turn triggers any cleanup methods, such as
>> calling DESTROY on the Tree and Taxon and get rid of memory leaks
>> wouldn't require weaken/is_weak).
> How will this interact with the places that Bio::Species unhooks the
> various cleanup mechanisms that Tree sets up. There are comments that
> talk about the sub ref screwing with someone's use of Storable. It
> seems like if the new Species has cleanup code registered then there's
> a problem with whom-ever had a problem with it before.
The deleted root cleanup methods are for the internal
Bio::Tree::Tree. Apparently code refs don't persist using Storable or
BioSQL (though I thought they could using B::Deparse?).
Anyway, for now I have everything set up in Bio::Species::DESTROY,
which calls SUPER::DESTROY(), then the Tree and Taxon cleanup methods
prior to explicitly deleting them. I also removed all weaken/is_weak
and the Scalar::Utils requirement from Bio::Species. It passes all
tests in the test suite. Also, I didn't see any significant memory
issues using Rutger's original file and the test file in the above bug
report, but it's worth another run through with more data..
>> I'm perfectly willing to test that out if no one else is (I am
>> on a few other bugs at the moment but I can probably get to it in the
>> next day or two). Not to tread on any toes, but I would like to get
>> this one taken care of prior to 1.6 and these are fairly nasty memory
>> issues. If there is any consolation it shouldn't be too hard to re-
>> rig the methods to delegate appropriately.
>>> I don't really feel hot or bothered about this fix vs. the
>>> of weaken and stashing the subspecies. It depends on what the
>>> intended for Species and its tree, neither are particularly sexy and
>>> if Species is really on its way out the door it may not matter.....
>> I don't think it will matter post-1.6, but we'll have to support
>> Bio::Species for the 1.6 release series. So it will have to be
>> maintained for the short term, until 1.7 comes out.
> I can't poke at this stuff much until the weekend. If you can kick
> it, go for it!
As you might have guessed I already have this set up. If there aren't
objections I'll commit the code tomorrow for testing. We can role
back if there are significant issues.
More information about the Bioperl-l