seanjensengrey / cogen

Automatically exported from code.google.com/p/cogen
MIT License
0 stars 0 forks source link

Completed operations not removed from timeouts list #11

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Steps to reproduce:

1. Queue an operation with a timeout far in the future
2. Let the operation succeed
3. The operation will still be in the Scheduler.timeouts list

If you happen to create a large number of such operations, your app will
leak memory due to these old operations still being on the timeouts list. 
It seems to be that process_op should check to see if the operation it just
processed is on the timeouts list, and remove it if so.

Original issue reported on code.google.com by wondercl...@gmail.com on 8 Apr 2009 at 12:18

GoogleCodeExporter commented 8 years ago
This could be implemented in TimedOperation.cleanup - however, removing 
arbitrary
items from the timeouts heapq might impact performance.

Original comment by ionel...@gmail.com on 8 Apr 2009 at 6:28

GoogleCodeExporter commented 8 years ago
That's true, if the heapq is large.  One possibility would be to maintain a 
counter
of the number of stale items on the timeouts heapq.  Whenever an operation is
completed, it could increment this counter if it has a timeout set.  It would be
decremented as timeouts are processed.  If the counter gets large, it could 
initiate
a pass over the heapq to remove all stale operations.  It would probably be 
best to
define "large" as a percentage of the total size of the heapq -- say 70%.  In 
this
case, it will probably be more efficient to traverse the heapq, copying only 
live
operations into a new heapq, and then discarding the old heapq.  This would 
allow
stale operations to live for a while, but they will be cleaned up periodically, 
which
solves the problem of ever-growing memory usage in my scenario.

An alternative heuristic would be to look at how far in the future the timeout 
is,
and if the timeout is soon, don't worry about it, but if it's a while, better to
clean it up and free the memory.  What "far in the future" means probably 
depends on
the application, and so should be tweakable by the programmer.

Or you could say that the overhead of the extra bookkeeping isn't worth it for 
most
applications, and just provide a method on Scheduler for the programmer to 
manually
trigger a traversal of the heapq, for those rare apps that need such a feature.

Original comment by wondercl...@gmail.com on 8 Apr 2009 at 7:34

GoogleCodeExporter commented 8 years ago
It looks like it's a general problem with coroutines that generate large 
amounts of
operations with a timeout so I'm adding a fix in trunk that removes the 
timeouts on
the operation's finalize method. 

Test and report if you have any problems or performance issues.

Original comment by ionel...@gmail.com on 26 Apr 2009 at 4:48

GoogleCodeExporter commented 8 years ago

Original comment by ionel...@gmail.com on 27 Apr 2009 at 5:57