[kepler-dev] change proposal for ptolemy.vergil.tree.EntityTreeModel class.
Christopher Brooks
cxh at eecs.berkeley.edu
Wed Dec 15 17:50:49 PST 2010
Hi Jianwu,
Sorry for the delay.
I folded the change in, the diff is below:
< @version $Id: EntityTreeModel.java 59133 2010-09-21 01:10:44Z cxh $
---
> @version $Id: EntityTreeModel.java 59240 2010-09-24 17:37:16Z cxh $
189,205c189
< int i = 0;
< int size = _listenerList.size();
< while (i < size){
< Object _listener =
((WeakReference)_listenerList.get(i)).get();
< if (_listener == null)
< {
< _listenerList.remove(i);
< size--;
< }
< else {
< if (_listener == listener){
< _listenerList.remove(i);
< size--;
< }
< i++;
< }
< }
---
> _listenerList.remove(new WeakReference(listener));
_Christopher
> Hi Christopher,
>
> I have to bother you about the class again. Changes made by Derik
> (http://bugzilla.ecoinformatics.org/show_bug.cgi?id=5095#c13 and c14) is
> to make reporting suite working. But now Kepler has memory leak again. I
> tried different ways in order to get rid of unused TableauFrame objects
> when a window is closed. But none of them work except changing the
> _listenerList of ptolemy.vergil.tree.EntityTreeModel back to be
> WeakReference.
>
> The attached EntityTreeModel is the same with the last one I sent
> to you. Please have a check. Thanks.
>
> Best wishes
>
> Sincerely yours
>
> Jianwu Wang
> jianwu at sdsc.edu
> http://users.sdsc.edu/~jianwu/
>
> Assistant Project Scientist
> Scientific Workflow Automation Technologies (SWAT) Laboratory
> San Diego Supercomputer Center
> University of California, San Diego
> San Diego, CA, U.S.A.
>
>
> On 9/27/2010 2:58 PM, Christopher Brooks wrote:
>> Hi Jianwu,
>> I'm not sure if I responded to this, but I folded your change in to the
>> trunk and into the 8.0 branch.
>>
>> No worries about folding speculative changes like this in. Tracking
>> down
>> memory leaks is a real art, and there are many false paths.
>>
>> Thanks,
>>
>> _Christopher
>>
>> On 9/24/10 6:01 PM, Jianwu Wang wrote:
>>> Hi Christopher,
>>>
>>> I did more explicit cleanup in Kepler code and found the current leak
>>> is gone even without the changes for
>>> ptolemy.vergil.tree.EntityTreeModel class. Another issue is that I
>>> found another error in my
>>> changes: '_listenerList.remove(new WeakReference(listener));' will
>>> not really work. I attached a new version for this class.
>>>
>>> So there are two options for it now. We can either remove all my
>>> changes, or use the attached version. I also saw many similar
>>> listeners in Ptolemy, such as _valueListeners in
>>> ptolemy.kernel.util.ConfigurableAttribute and
>>> ptolemy.data.expr.Variable, which do not use weak reference. It might
>>> be good to keep them the same.
>>>
>>> Sorry for the mess up. I should dig more in Kepler before changing
>>> Ptolemy code.
>>>
>>> On 9/24/10 10:39 AM, Christopher Brooks wrote:
>>>> Hi Jianwu,
>>>> Thanks, I folded this into the devel trunk and the 8.0 branch.
>>>>
>>>> _Christopher
>>>>
>>>> On 9/24/10 9:54 AM, Jianwu Wang wrote:
>>>>> Hi Christopher,
>>>>>
>>>>> Since WeakReference.get() could return null, we'd better check it
>>>>> after using this function. So I changed this class a little big
>>>>> more. Please find new class from the attachment and check it in if
>>>>> you
>>>>> think it is proper. Thanks.
>>>>>
>>>>> On 9/20/10 6:14 PM, Christopher Brooks wrote:
>>>>>> Hi Jianwu,
>>>>>>
>>>>>> Your change looks ok to me. The diff between what you sent and what
>>>>>> is in the tree is below:
>>>>>>
>>>>>> bash-3.2$ diff ~/Downloads/EntityTreeModelNew.java
>>>>>> EntityTreeModel.java
>>>>>> 30d29
>>>>>> < import java.lang.ref.WeakReference;
>>>>>> 64c63
>>>>>> < public class EntityTreeModelNew implements TreeModel {
>>>>>> ---
>>>>>> > public class EntityTreeModel implements TreeModel {
>>>>>> 70c69
>>>>>> < public EntityTreeModelNew(NamedObj root) {
>>>>>> ---
>>>>>> > public EntityTreeModel(NamedObj root) {
>>>>>> 82c81
>>>>>> < _listenerList.add(new WeakReference(listener));
>>>>>> ---
>>>>>> > _listenerList.add(listener);
>>>>>> 176c175
>>>>>> < _listenerList.remove(new WeakReference(listener));
>>>>>> ---
>>>>>> > _listenerList.remove(listener);
>>>>>> 188c187
>>>>>> < TreeModelListener listener = (TreeModelListener)
>>>>>> ((WeakReference)listeners.next()).get();
>>>>>> ---
>>>>>> > TreeModelListener listener = (TreeModelListener) listeners.next();
>>>>>> 263c262
>>>>>> < private List _listenerList = new LinkedList<WeakReference>();
>>>>>> ---
>>>>>> > private List _listenerList = new LinkedList();
>>>>>>
>>>>>>
>>>>>> I folded in the change and added a comment:
>>>>>>
>>>>>> /** Add a listener to this model.
>>>>>> * @param listener The listener to add.
>>>>>> * @see #removeTreeModelListener(TreeModelListener)
>>>>>> */
>>>>>> public void addTreeModelListener(TreeModelListener listener) {
>>>>>> // In http://bugzilla.ecoinformatics.org/show_bug.cgi?id=4801,
>>>>>> // Jianwu Wang found that the _listenerList attribute in
>>>>>> // ptolemy.vergil.tree.EntityTreeModel was leaking memory.
>>>>>> // This attribute keeps holding some references, so that
>>>>>> // instances of EntityTreeModel and its sub-classes can not be
>>>>>> // GCed when a window is open and closed.
>>>>>> //
>>>>>> // When it and its sub-classes are used in Kepler,
>>>>>> // addTreeModelListener() and removeTreeModelListener()
>>>>>> // functions of EntityTreeModel class are called implicitly.
>>>>>> // Jianwu did not find a good way to clean up after a window
>>>>>> // is closed. Changing the _listenerList attribute using
>>>>>> // WeakReference fixe the leak.
>>>>>> _listenerList.add(new WeakReference(listener));
>>>>>> }
>>>>>>
>>>>>> I updated the 8.0 release branch, thanks for the patch!
>>>>>>
>>>>>> Derik, I think you would want this in the 2.1 release. I'm still
>>>>>> working on deterimining
>>>>>> if ptolemy-8.0.0 is what should be shipped with Kepler 2.1.
>>>>>>
>>>>>> _Christopher
>>>>>>
>>>>>>
>>>>>> On 9/20/10 4:27 PM, Jianwu Wang wrote:
>>>>>>> As I mentioned at
>>>>>>> http://bugzilla.ecoinformatics.org/show_bug.cgi?id=4801, I found
>>>>>>> one class might need to be changed for memory leak. It's the
>>>>>>> _listenerList attribute in
>>>>>>> ptolemy.vergil.tree.EntityTreeModel. One memory leak I'm tracing
>>>>>>> down is that this attribute keeps holding some references, so
>>>>>>> that instances of EntityTreeModel and its sub-classes can not be
>>>>>>> GCed
>>>>>>> when a window is open and closed.
>>>>>>>
>>>>>>> When it and its sub-classes are used in Kepler,
>>>>>>> addTreeModelListener() and removeTreeModelListener() functions of
>>>>>>> EntityTreeModel class are called implicitly. I haven't find good
>>>>>>> ways to clean
>>>>>>> up after a window is closed.
>>>>>>>
>>>>>>> If changing the _listenerList attribute using WeakReference, it
>>>>>>> will be able to fix the leak.
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
More information about the Kepler-dev
mailing list