This refers to an earlier question I asked here, but it's urgent, and specific, enough to warrant a separate thread. Our version of webwork has been springing some massive memory leaks -- the server's completely crashed twice in the past two days -- and it seems as if it's due to the custom macros I wrote over the summer buckling under the fall semester load.
I think I've pinpointed the problem to be my hand-rolled traversing of the parse tree in our simplification checker. The gist is fairly simple: given a Formula object, the checker walks the parse tree and accumulates enough data along the way to (heuristically) determine whether the student's answer is simplified enough compared to the correct answer. For example, it counts the number of operations, it checks to see whether fractions are unreduced, etc.
I'm guessing that the problem is the following: because I was unable to completely understand the construction of the parse tree, I was instantiating a new (anonymous) Formula object at every node to iterate the analysis. Specifically, the code to pull out the child node looks something like this (assuming a BOP for ease of use):
my $node = shift;
my @children = ();
$_ = $node->{tree}->class;
return () if /Number/;
return () if /Variable/;
if (/BOP/) {
push @children, Formula($node->{tree}->{lop});
push @children, Formula($node->{tree}->{rop});
}
On the one hand, this guarantees that each iterate will continue to be a Formula object, which is nice; on the other hand, as I mentioned, this requires producing an anonymous Formula object for every single node in the tree which may be causing grief when 500+ students hit at the same time.
The questions I have are thus:
1) In your estimation, is there any advantage in having the parse-tree traversal operating on Formula objects instead of Parser objects? I thought there was, but I'm not sure any more.
2) Given mod_perl's shonky garbage collection, is using Formula objects like this a Bad Thing?
3) If this doesn't seem to be the problem, are there any other kinds of programming issues -- random number generation, function pointer calls, whatever -- that are known to produce massive memory leaks?
4) [Random] Is there a reason why the "equation" field of a subformula/subparser doesn't seem to update when you descend the tree? For example, the following code:
$node = Formula( "a^4 * [b^(3/2)]" );
my $r = $node->{tree}->{rop};
warn $r->{rop}->{equation};
Produces "a^4 * [b^(3/2)]", not "3/2" as it seems like it ought to.
[We're running WW 2.x, but I'm not sure what x right now -- the server's about to buckle again, it seems -- and we've got 2+G memory running on a Debian server, if that makes a difference.]
First, I suspect that you are right, the creation of a large number of Formula object may be the source of your memory leak (see below).
In answer to your questions:
1) There only reason I can think of for wanting to turn the nodes into full Formula objects is to be able to use the overloaded perl operators that are part of the Value package. Most notably, the equality operator would allow you to compare two sub-formulas. But unless you need that, I don't really see a need to do it.
2) I suspect that it is a Bad Thing. The Formula structure has a reference loop that I think may be the cause of the memory leak, and so make lots of these may well be a bad idea.
4) The equation field is a pointer to the top level Parser object (which is really what the Formula object is). It is needed by the nodes for various reasons (like finding the Context of the Formula, issuing error reports that highlight the location in the student's input string, and so on). If you want the string version of the parse tree from a note down, use $node->string. E.g., in your example $r->{rop}->string will produce "3/2" if that's what you want, whereas $r->{rop}{equation} is actually a pointer to $node (which happens to stringify as the original equation string, but it's really the full Formula structure; do warn ref($r->{rop}{equation}) to see this). It is this reference that is probably the source of the memory leak, but unfortunately, it is absolutely essential.
I have always been a little worried about this reference, but have never actually investigated it (ignorance is bliss). Partly that's because I really don't know how to go about it, and partly because if I were sure it was the problem, I'd be obliged to change it, and that would mean rewriting the guts of the parser, which I really don't want to do.
I think the usual solution at the moment is to set some of the resource limits on the httpd child processes so that they die before they eat up your whole machine. It's been a while since I remember hearing the discussion, but as I recall, you want to reduce the MaxKeepAliveRequests or MaxRequestPerChild setting so that the children die off more often. It may also be possible to use the Apache::Resource module to set the RLIMIT_DATA size, but I've never tried that.
Davide
In answer to your questions:
1) There only reason I can think of for wanting to turn the nodes into full Formula objects is to be able to use the overloaded perl operators that are part of the Value package. Most notably, the equality operator would allow you to compare two sub-formulas. But unless you need that, I don't really see a need to do it.
2) I suspect that it is a Bad Thing. The Formula structure has a reference loop that I think may be the cause of the memory leak, and so make lots of these may well be a bad idea.
4) The equation field is a pointer to the top level Parser object (which is really what the Formula object is). It is needed by the nodes for various reasons (like finding the Context of the Formula, issuing error reports that highlight the location in the student's input string, and so on). If you want the string version of the parse tree from a note down, use $node->string. E.g., in your example $r->{rop}->string will produce "3/2" if that's what you want, whereas $r->{rop}{equation} is actually a pointer to $node (which happens to stringify as the original equation string, but it's really the full Formula structure; do warn ref($r->{rop}{equation}) to see this). It is this reference that is probably the source of the memory leak, but unfortunately, it is absolutely essential.
I have always been a little worried about this reference, but have never actually investigated it (ignorance is bliss). Partly that's because I really don't know how to go about it, and partly because if I were sure it was the problem, I'd be obliged to change it, and that would mean rewriting the guts of the parser, which I really don't want to do.
I think the usual solution at the moment is to set some of the resource limits on the httpd child processes so that they die before they eat up your whole machine. It's been a while since I remember hearing the discussion, but as I recall, you want to reduce the MaxKeepAliveRequests or MaxRequestPerChild setting so that the children die off more often. It may also be possible to use the Apache::Resource module to set the RLIMIT_DATA size, but I've never tried that.
Davide
Ok, I've rewritten the parser traversal to use only Parser objects, not Formulas. For the most part it makes better code too, e.g. I hadn't realized that methods like class were in both Value and Parser. I've contacted our sysadmin about imposing stricter resource limits, hopefully they'll be in place by tomorrow morning. Here's to hoping this resolves the issue; I'll post back here when we see what's up.
A thought occurs to me: if the circular references really are the problem, there are a couple of solutions I know of, though have never tried myself:
1) The module Scalar:Utils has incorporated the <a href="http://search.cpan.org/~gbarr/Scalar-List-Utils-1.19/lib/Scalar/Util.pm">WeakRef</a> module which decreases the reference count of a given object. This is supposedly the standard way in Perl 5.6 or higher.
2) One can write an explicit destructor that deletes the references when the object passes out of scope, which is more what I'm used to; I think there are subtleties involved in using this with mod_perl, though.
3) There's also the module <a href="http://search.cpan.org/dist/Data-Structure-Util/lib/Data/Structure/Util.pm">Data::Structure::Utils </a> that I just found on CPAN that might do the trick.
I think all of these would enable you to keep the bulk of your code intact, which would be the optimal solution.
As an aside: does anyone know how to simulate a heavy load (e.g. creating 500 virtual students all logging in at once and making mistakes) without actually farming the job out to a bunch of undergraduates? I'd like to ensure we don't have memory issues like this again, but I can't figure out how to find them in the first place.
1) The module Scalar:Utils has incorporated the <a href="http://search.cpan.org/~gbarr/Scalar-List-Utils-1.19/lib/Scalar/Util.pm">WeakRef</a> module which decreases the reference count of a given object. This is supposedly the standard way in Perl 5.6 or higher.
2) One can write an explicit destructor that deletes the references when the object passes out of scope, which is more what I'm used to; I think there are subtleties involved in using this with mod_perl, though.
3) There's also the module <a href="http://search.cpan.org/dist/Data-Structure-Util/lib/Data/Structure/Util.pm">Data::Structure::Utils </a> that I just found on CPAN that might do the trick.
I think all of these would enable you to keep the bulk of your code intact, which would be the optimal solution.
As an aside: does anyone know how to simulate a heavy load (e.g. creating 500 virtual students all logging in at once and making mistakes) without actually farming the job out to a bunch of undergraduates? I'd like to ensure we don't have memory issues like this again, but I can't figure out how to find them in the first place.
Thanks, Rob, these look like just what I need. I'll try to find time to try this out soon, but it looks like it should do the trick. These types of references occur in only two places , if I remember correctly, so it should not be hard to weaken them.
Davide
Davide
Well, your suggestion of Scalar::Util::weaken worked out perfectly. It was easy to modify the parser items to weaken their link back to the main Formula object. There was also a similar recursive reference within the Context object, and that was also easily fixed. So I think the MathObjects no longer leak memory. I have submitted the changes to the CVS archive, and you should be able to update pg without updating webwork2.
Thanks for the suggestion; it was a big help. I had been worried about those recursive references for a long time, so it is nice to have them fixed.
Davide
Thanks for the suggestion; it was a big help. I had been worried about those recursive references for a long time, so it is nice to have them fixed.
Davide