-
-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Memory leak for vectors and matrices #112
Comments
Thanks for the report. I confirm the behaviour on my machine as well. |
Interestingly, this leaks memory
but this does not:
though they seem to result in the same PARI object. |
Yes these end up with the same pari object. But these two functions allocating memory in different ways. |
I'm a bit confuse as to way We create the item, put it in the cache, then increase the reference count and then set the pari coefficient. But I don't know why we need to increase the reference count. It is stored in the cache and it will be around as long as the vector/matrix is around or until the index gets set to something new. Either way it should be fine. Increasing the reference count by |
Be careful that inside |
Yes, instead of |
Ok, my previous idea was nonsense. However, the solution seems to be rather simple. The problem is that the matrix/vector is still on the stack when we do the item assignment. Assigning entries to a vector/matrix on the stack sounds like a bad idea. This is how I figure (in sage):
The above does not leak memory. However, when you comment |
When I run
Is this related? |
I don't think this is a bug. It is not exactly a feature, but it is a consequence of something which is regarded as a feature. It is also not really a memory leak. If you watch the memory usage while this loop is running you will see it grow to about 75% of physical memory, then decay and stabilize at about 20% of physical memory. That is the python garbage collector at work. After the last sync of cypari and cypari2, cypari2 changed their memory management scheme for Gen objects. In cypari 1.4.1 all GENs live on the pari stack. Pari has a notion of "cloning" GENs which means copying a GEN to the heap, and "uncloning" heap based GENs which means freeing them. Cypari2 makes use of this, and supports Gen objects whose GEN is either on the heap or on the stack. Intermediate values when evaluating an expression generally stay on the stack. But containers, like matrices, are moved to the heap as soon as an entry is assigned a value. And when the stack gets to be more than half full, all GENs on the stack are moved to the heap. This is a way of dealing with Pari's suggested strategy of cleaning, or "repiling" the stack during a long computation. Essentially, cypari2 has delegated its management of GENs to the python garbage collector, at least for the heap based GENs. Since this loop involves creating lots of matrices which are immediately destroyed, it generates a very large number of heap based Gens whose GENs must be deallocated by python's garbage collector. The garbage collector allows the memory footprint to grow quite large before becoming more aggressive and taming the growth. It is worth noting that the stack design provides much more efficient deallocation than could ever be possible with a reference counting garbage collection scheme. If you have a few million dead matrices on the stack you can deallocate all of them in one machine instruction just by setting the stack pointer before the first one. But a reference counting garbage collector will have to call free() millions of times in order to deallocate them. However, since every GEN is wrapped by a Gen object, which must be deallocated by the python garbage collector, it is not really possible for cypari(2) to take full advantage of this efficiency. |
I retract my statement that this is not a bug. By adding some print statements in the Gen
For some reason (which is probably important to understand) the case of a pari Gen created from a python list is different. It behaves normally.
|
An addendum. While the refcount behavior above looks wrong for the pari int Gens, it does not cause the same kind of memory issues to replace |
If you think through what happens when you repeatedly call At the beginning of each iteration the bottom of the pari stack is a matrix with a refcount of 1, held by the variable M. (Actually held by the namespace dict and assigned to the name M.) When the assignment expression is evaluated two things happen, presumably atomically. First a new matrix Gen is created and assigned to stackbottom. The matrix Gen whose GEN is immediately above the bottom now has refcount 2. One reference is held by the variable M and one by the new matrix Gen on the bottom, whose Second, the matrix Gen on the bottom is assigned to the varible M. This sets its refcount to 1, and reduces the refcount of the matrix above from 2 to 1. The steady state is that there is a long chain of matrix Gens whose GEN is on the pari stack. Each Gen in the chain has refcount 1 held by the Gen whose GEN is directly below its GEN on the pari stack. Eventually the GENs belonging to the Gens in this chain fill the pari stack to half of its maximum size. At that point all GENS on the pari stack are moved to the heap (i.e. cloned). When a Gen in the long chain is moved to the heap its refcount is reduced to 0, so it gets destroyed immediately after it is moved to the heap. The docstring for pari_instance.stacksizemax says: If that is correct it also explains why memory grows under this loop to a (large) fixed fraction of total memory but never reaches OOM. It is not clear to me how this should be fixed. One option might be to always create new Gens on the heap instead of on the stack. However, pari will always create a new Gen on the pari stack. So this would mean immediately cloning the GEN when a new Gen is created. That costs the time required for the copy. I assume that the tactic of leaving GENs on the stack as long as possible was intended to increase performance by avoiding these copies. That performance increase would be lost. On the other hand, in this particular context, the copy is happening anyway. It just gets delayed until the stack is half full. Another option might be to do away with the heap altogether. That is probably not a good idea. |
If what I said above is correct, it is definitely not the whole story since it does not account for the fact that many Gens are moved to the heap immediately. |
I can now finish the story. The discussion above explains why the loop |
Oh, and the fix is to call A.fixGen() as soon as a matrix A is created. |
With version 2.1.2, the following causes cypari2 to allocate memory at several GB per minute:
Observed in Python 3.8 and 3.9 as part of SageMath 9.3-9.5 on both linux (gcc) and macOS (clang).
For what it's worth, the other cypari does not seem to have this problem.
Also filed at the SageMath trac.
The text was updated successfully, but these errors were encountered: