search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 482 forks source link

Memory leak in BasisExchangeMatroid.components #28498

Closed edd8e884-f507-429a-b577-5d554626c0fe closed 5 years ago

edd8e884-f507-429a-b577-5d554626c0fe commented 5 years ago

As reported in this ask question, there is a memory leak in BasisExchangeMatroid.components, see:

sage: G = [g for g in graphs.nauty_geng("12 16:16 -Ct")]
sage: for _ in range(20):
....:     for g in G:
....:         A = g.incidence_matrix()
....:         M = Matroid(A, ring = GF(2))
....:         if M.is_connected():
....:             pass
....:     print(get_memory_usage())
....:     
8539.23046875
8543.7421875
8547.73828125
8551.734375
8555.73046875
8559.59765625
8563.59375
8566.9453125
8570.94140625
8574.9375
8578.2890625
8582.28515625
8586.15234375
8589.6328125
8593.62890625
8597.49609375
8600.9765625
8604.84375
8608.83984375
8612.3203125

The memory leak is because sig_free is missing.

CC: @sagetrac-Rudi @jdemeyer

Component: memleak

Author: Thierry Monteil

Branch/Commit: e9ed458

Reviewer: Vincent Delecroix

Issue created by migration from https://trac.sagemath.org/ticket/28498

edd8e884-f507-429a-b577-5d554626c0fe commented 5 years ago

Branch: u/tmonteil/memory_leak_in_basisexchangematroid_components

edd8e884-f507-429a-b577-5d554626c0fe commented 5 years ago
comment:2

After the fix:

sage: G = [g for g in graphs.nauty_geng("12 16:16 -Ct")]
sage: for _ in range(20):
....:     for g in G:
....:         A = g.incidence_matrix()
....:         M = Matroid(A, ring = GF(2))
....:         if M.is_connected():
....:             pass
....:     print(get_memory_usage())
....:     
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375
8462.484375

I am not sure whether this should be doctested.

New commits:

db6182b#28498 : remove trailing spaces
e9ed458#28498 : fix memleak
edd8e884-f507-429a-b577-5d554626c0fe commented 5 years ago

Commit: e9ed458

videlec commented 5 years ago
comment:4

Nice catch! Such memory leaks should be doctested for each code snippet (ie running the same code 100 times should not increase memory usage). I am not sure it makes sense at the level of this single function.

videlec commented 5 years ago

Reviewer: Vincent Delecroix

156bf7a1-cdbf-4d2b-a578-797e3af0730c commented 5 years ago
comment:6

Sorry for making this mistake in the first place, and thanks for fixing!

slel commented 5 years ago

Description changed:

--- 
+++ 
@@ -32,5 +32,5 @@
 8612.3203125

-Siegfried is missing. +The memory leak is because sig_free is missing.

fchapoton commented 5 years ago
comment:8

moving milestone to 9.0 (after release of 8.9)

vbraun commented 5 years ago

Changed branch from u/tmonteil/memory_leak_in_basisexchangematroid_components to e9ed458