[kepler-dev] change proposal for ptolemy.vergil.tree.EntityTreeModel class.

Jianwu Wang jianwu at sdsc.edu
Wed Dec 15 23:13:30 PST 2010 

Hi Christopher,

     No problem. Thanks for your help.

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 12/15/2010 5:50 PM, Christopher Brooks wrote:
> 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