# memory leak in astshim isSeries function

XMLWordPrintable

## Details

• Type: Bug
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
3
• Sprint:
• Team:

## Description

Running the attached script results in the failure

 ERROR: RuntimeError: AST: Error at line 51 in file src/detail/utils.cc.AssocId(WinMap): There are too many AST Objects in use at once. [__main__] Traceback (most recent call last):  File "cause_failure_afwCameraGeom.py", line 13, in   pt = getCenterPixel('R:2,2 S:1,1')  File "cause_failure_afwCameraGeom.py", line 7, in getCenterPixel  centerPoint = camera[name].getCenter(FOCAL_PLANE) RuntimeError: AST: Error at line 51 in file src/detail/utils.cc.AssocId(WinMap): There are too many AST Objects in use at once. 

It is possible that this is expected/desirable behavior, given that I am needlessly calling makeCameraSys over and over again on the same inputs. I was under the impression, however, that these would get deleted upon exiting the getCenterPixel method defined in the script.

## Attachments

1. cause_failure_afwCameraGeom.py
0.4 kB
2. memory_leak.cc
0.4 kB

## Activity

Hide
Russell Owen added a comment - - edited

I have narrowed it down to the attached astshim example code: "memory_leak.cc". Put a copy in examples/ and run scons examples to build it. Then run it while watching memory.

ParallelMap also shows the problem, but CmpMap does not. SeriesMap and ParallelMap are trivial subclasses of CmpMap. They are unusual in that all three wrap the same AST class: AstCmpMap. All other wrapped AST objects have only one wrapper class.

It is the AST mappings contained in the compound mapping that are leaking, based on looking at getNObject. I don't yet know if the astshim wrapper classes for those contained mappings are also leaking.

Show
Russell Owen added a comment - - edited I have narrowed it down to the attached astshim example code: "memory_leak.cc". Put a copy in examples/ and run scons examples to build it. Then run it while watching memory. ParallelMap also shows the problem, but CmpMap does not. SeriesMap and ParallelMap are trivial subclasses of CmpMap. They are unusual in that all three wrap the same AST class: AstCmpMap. All other wrapped AST objects have only one wrapper class. It is the AST mappings contained in the compound mapping that are leaking, based on looking at getNObject . I don't yet know if the astshim wrapper classes for those contained mappings are also leaking.
Hide
Russell Owen added a comment -

The leak was in free function isSeries: it created two AstMapping * in order to call astDecompose and did not astAnnul them.

I expanded unit testing of memory, especially for compound objects, so such errors are more likely to be caught.

I also cleaned up a few other things.

Show
Russell Owen added a comment - The leak was in free function isSeries : it created two AstMapping * in order to call astDecompose and did not astAnnul them. I expanded unit testing of memory, especially for compound objects, so such errors are more likely to be caught. I also cleaned up a few other things.
Hide
Tim Jenness added a comment -

Looks good. Thanks. I created the PR to make it easier to review.

Show
Tim Jenness added a comment - Looks good. Thanks. I created the PR to make it easier to review.
Hide
Russell Owen added a comment - - edited

Thank you for the review. Sorry I forgot to make the pull request. I filed DM-12006 to improve testing of copyPolymorphic.

Show
Russell Owen added a comment - - edited Thank you for the review. Sorry I forgot to make the pull request. I filed DM-12006 to improve testing of copyPolymorphic.
Hide
Russell Owen added a comment -

One more note, for posterity: it turns out that calling TransformMap.transform and the similar Camera.transform and Detector.transform has significant overhead because it makes a deep copy of the transform before applying it. If one wishes to transform many points it is far more efficient to get the transform first, using getTransform(fromsys, tosys) and then call applyForward(point-or-even-better-a-list-of-points) on that.

Show
Russell Owen added a comment - One more note, for posterity: it turns out that calling TransformMap.transform and the similar Camera.transform and Detector.transform has significant overhead because it makes a deep copy of the transform before applying it. If one wishes to transform many points it is far more efficient to get the transform first, using getTransform(fromsys, tosys) and then call applyForward(point-or-even-better-a-list-of-points) on that.

## People

• Assignee:
Russell Owen
Reporter:
Scott Daniel
Reviewers:
Tim Jenness
Watchers:
Paul Price, Russell Owen, Scott Daniel, Tim Jenness