Skip to content
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 #31

Merged
merged 4 commits into from
Mar 23, 2017
Merged

Memory leak #31

merged 4 commits into from
Mar 23, 2017

Conversation

thejohnhoffer
Copy link
Contributor

@thejohnhoffer thejohnhoffer commented Mar 23, 2017

Found and fixed memory leak at line 156 of UtilityLayer/Keywords.py due to passing list of database table ['neuron','synapse'] by reference instead of by values.

The reference to this list gets passed as the LIST keyword of the NamelessStruct RUNTIME.DB.TABLE. The internals of the NamelessStruct in Line 19 of UtilityLayer/Structures.py add each keyword argument NAME to the LIST keyword before adding unique vaules for self.LIST.

Although in NamelessStruct self.LIST contains only unique values, the original reference passed as the keyword argument LIST continues to accumulate values. This issue can be fixed by using python's [:] notation to create a copy of the list.

@@ -153,7 +153,7 @@ def __init__(self):
# ALL THE DATABASE RUNTIME TERMS
self.DB = NamelessStruct(
TABLE = NamelessStruct(
LIST = _table_list,
LIST = _table_list[:],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not want to write this for each list in UtilityLayer/Keywords.py, since it will be very easy to forget. The dereferencing with [:] should instead happen in UtilityLayer/Structures.py, where it will only need to be written once in the class definition of NamelessStruct.

# Begin making the list
all_list = _keywords.get('LIST',[])
# Begin a new list of all keyword NAMES
all_list = _keywords.get('LIST',[])[:]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Start accumulating keyword names in a copy of the LIST keyword.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Begin making the list
all_list = _keywords.get('LIST',[])
# Begin a new list of all keyword NAMES
all_list = _keywords.get('LIST',[])[:]
# Add other keywords to list
for key in _keywords:
keyval = _keywords[key]
setattr(self,key,keyval)
# Put all lists into list
Copy link
Contributor Author

@thejohnhoffer thejohnhoffer Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The few changes not shown above here just improve code legibility. They have no functional effect.

@thejohnhoffer thejohnhoffer merged commit c4881f7 into update_v2 Mar 23, 2017
@thejohnhoffer thejohnhoffer deleted the memory-leak branch March 23, 2017 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants