Ticket #4241 (closed defect: fixed)

Opened 4 years ago

Last modified 4 years ago

[with patch; positive review] magma -- memory is never freed in the interface when MagmaElement's are deleted

Reported by: was Owned by: was
Priority: major Milestone: sage-3.2
Component: interfaces Keywords:
Cc: Work issues:
Report Upstream: Reviewers:
Authors: Merged in:
Dependencies: Stopgaps:

Description

Observe:

sage: a = magma('10000')
sage: a.name()
'_sage_[1]'
sage: del a
sage: magma.eval('_sage_[1]')
'10000'

Whenever anybody ever creates a MagmaElement? via the Magma interface, it doesn't get deleted. This is because possible (1) the clear method in magma.py is commented out, and/or (2) the _available_var list that gets appended to in (1) isn't actually used by magma.py, so e.g., _sage_[1] in the example above never gets re-used.

Attachments

sage-4241.patch Download (2.8 KB) - added by was 4 years ago.

Change History

comment:1 Changed 4 years ago by was

  • Status changed from new to assigned

comment:2 Changed 4 years ago by was

Here's a vivid illustration of the memory leakage, which of course we know is there by reading the code:

sage: a = [magma('3^100000') for _ in range(1000)]
sage: magma.GetMemoryUsage()
42917912
sage: del a
sage: magma.GetMemoryUsage()
42917912
sage: a = [magma('3^100000') for _ in range(1000)]
sage: magma.GetMemoryUsage()
69103640
sage: del a
sage: magma.GetMemoryUsage()
69103640

comment:3 Changed 4 years ago by was

Without patch:

sage: a = [magma('3^100000') for _ in range(1000)]; del a;magma.GetMemoryUsage() 
42917912
sage: a = [magma('3^100000') for _ in range(1000)]; del a;magma.GetMemoryUsage() 
94192176
sage: a = [magma('3^100000') for _ in range(1000)]; del a;magma.GetMemoryUsage()
121287216

With patch:

sage: a = [magma('3^100000') for _ in range(1000)]; del a;magma.GetMemoryUsage()
40817200
sage: a = [magma('3^100000') for _ in range(1000)]; del a;magma.GetMemoryUsage()
41820720
sage: a = [magma('3^100000') for _ in range(1000)]; del a;magma.GetMemoryUsage()
41820720
sage: a = [magma('3^100000') for _ in range(1000)]; del a;magma.GetMemoryUsage()
41820720
sage: a = [magma('3^100000') for _ in range(1000)]; del a;magma.GetMemoryUsage()
41820720

}}}

Changed 4 years ago by was

comment:4 Changed 4 years ago by was

  • Summary changed from magma -- memory is never freed in the interface when MagmaElement's are deleted to [with patch; needs review] magma -- memory is never freed in the interface when MagmaElement's are deleted

comment:5 Changed 4 years ago by mabshoff

  • Summary changed from [with patch; needs review] magma -- memory is never freed in the interface when MagmaElement's are deleted to [with patch; positive review] magma -- memory is never freed in the interface when MagmaElement's are deleted

Patch looks good to me. There is a spelling error in the new docstring: "clearlying" _ i will fix it in the patch I will apply.

Cheers,

Michael

comment:6 Changed 4 years ago by mabshoff

  • Status changed from assigned to closed
  • Resolution set to fixed
  • Milestone changed from sage-3.2.1 to sage-3.2

Merged in Sage 3.2.alpha1

Note: See TracTickets for help on using tickets.