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

[Christopher Brooks]

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.
>>>>
>>>
>>>
>>
>
>

-- 
Christopher Brooks, PMP                       University of California
CHESS Executive Director                      US Mail: 337 Cory Hall
Programmer/Analyst CHESS/Ptolemy/Trust        Berkeley, CA 94720-1774
ph: 510.643.9841 fax:510.642.2718	      (Office: 545Q Cory)
home: (F-Tu) 707.665.0131 cell: 707.332.0670