I'm trying to see if I could do it in a cleaner way, and I found it!
On partial updates, AltBrowser is able to cleanly delete the node morphs
because it sends #noteRemovalOfAll: aCollection to MorphTreeMorph, which
has the selection testing and removal code.
Which means changing
MorphTreeMorph>>updateContentsWithPreviouslyExpanded: aNodeList
nodeList := nil.
self noteRemovalOfAll: self allNodeMorphs. "<-- Changed"
(self nodeList isNil or: [ self nodeList isEmpty ])
ifTrue: [
nodeList := nil.
^ self emptySelection ].
self addSubmorphsFromNodeList: self currentNodelist previouslyExpanded:
aNodeList
It solves the problem.
However, I don't like this that much overall, because delete is never
sent to the node morphs themselves, and this seems very wrong to me.
Thierry
Le 30/06/2014 16:56, Goubier Thierry a écrit :
It would solve the selection kept while updating the full list, but it
won't solve partial updates. You would also be apparently deselecting
and reselecting when appending elements to the list.
Thierry
Le 30/06/2014 16:15, Nicolai Hess a écrit :
How about this change, if it rebuilds the whole list, we can savely
remove the selection, or not?
MorphTreeMorph>>
addSubmorphsFromNodeList: aNodeList previouslyExpanded: expandedNodeList
| morphList |
morphList := OrderedCollection new.
self
addMorphsTo: morphList
from: aNodeList
withExpandedItems: expandedNodeList
atLevel: 0.
self insertNewMorphs: morphList.
self listManager emptySelection. "<---
added"
self listManager updateSelectionFromModel.
self roots do: [:r | r updateChildrenRecursively].
self updateColumnMorphs
2014-06-30 15:35 GMT+02:00 Goubier Thierry <[email protected]
<mailto:[email protected]>>:
Ok, I have the test case showing the problem.
| c w t |
c := ClassTreeExample new.
w := c openOn: Collection.
t := c dependents last.
t expandAll.
t selectAll.
c updateList.
t listManager selectedMorphList do: [ :each | self assert: ( t
allNodeMorphs includes: each ) ]
Fail on the final assert. Switching to single selection hold the
same error.
Interestingly, reselecting cleans the selectedMorphList.
| c w t |
c := ClassTreeExample new.
w := c openOn: Collection.
t := c dependents last.
t expandAll.
t selectAll.
c updateList.
t setSelectedMorph: t allNodeMorphs first.
t listManager selectedMorphList do: [ :each | self assert: ( t
allNodeMorphs includes: each ) ]
This one has no error.
Changing the way Nautilus maintain the selection would work. Forcing
selectedMorphList to be cleaned up on addition / removal in the list
would be nice too (but possible update code can delete the node
morphs by itself).
Thierry
Le 30/06/2014 10:15, Johan Brichau a écrit :
Hi Nicolai,
Do you have a method selected in the browser?
As the memory leak happens via the selectedMorphs instvar, it
requires a package to be selected (and maybe even a method).
Before and after the load. I do this and notice the increase.
Smalltalk garbageCollect.
MorphTreeNodeMorph allInstances size.
It really only becomes problematic (i.e. out-of-mem) when you
load large projects. Maybe MOOSE will qualify?
Yes, I was astonished as well when I noticed that every update
was rebuilding the entire set of Morphs. It's the easiest
solution of course but it does impose a lot of overhead.
First, I will see how to solve that leaking problem. Then I will
take a look at the update itself.
I can only spend some of my free time on this, so I will
continue tonight.
best,
Johan
On 30 Jun 2014, at 09:09, Nicolai Hess <[email protected]
<mailto:[email protected]>> wrote:
2014-06-29 22:31 GMT+02:00 Johan Brichau <[email protected]
<mailto:[email protected]>>:
On 27 Jun 2014, at 14:00, Goubier Thierry
<[email protected] <mailto:[email protected]>>
wrote:
It seems to depend on the Nautilus window state. If
you've just opened it, then nothing seems to be amiss.
If you start to select things in it, you start to see
time spent in updateClassView, but why?
I just found that the instances are being kept by the
MorphTreeListManager#__selectedMorphs instvar. So, that
observation is correct: you need to have a package selected.
What seems to happen is that on every package load, the
Nautilus package tree is updated (which means a
PackageTreePackageNodeModel is created for each package in
the image via #asNautilusNodeWithModel:).
This update does not clear the selectedMorphs list and just
this single reference seem to keep the entire package tree
model and its Morphs in memory.
There were 496 morphs in the selectedMorphs list and when I
cleaned that, all trailing Morphs and PackageNodeModelNodes
(and all the other garbage) could be gc'ed.
On to investigating the growth of the selectedMorphs
variable...
Johan
Yes, we had quite some bugs with this package tree update
in the past. What I don't understand is, why
is the whole tree removed and rebuild, maybe this is a
common strategy in Morphic, update a list ore tree
will always rebuild the whole morph structure? But it
happens even if the structure is the same and
just this little package/dirty package icon is updated.
Another issue is this "selected package/package selection".
The tree (and for example the
category list and/or method list" as well), stores the
selection on multiple ways, in the model
(PackageTreeNautilus), the
UI (PackageTreeNautilusUi) and the treelist morph. Once as a
SelectedTreeNode, a Package from Nautilus Model and
once as a PackgeTreeSelection/__PackageTreeTagSelection.
I find it pretty confusing.
BTW, I still can not reproduce this memory leak behavior. I
tried this:
[ Gofer new smalltalkhubUser: 'ObjectProfile'
project: 'Roassal2';
package: 'ConfigurationOfRoassal2';
load.
(Smalltalk at: #ConfigurationOfRoassal2) load ] timeToRun.
with and without an open SystemBrowser. But the load time and
memory consumption is the same.
Nicolai
--
Thierry Goubier
CEA list
Laboratoire des Fondations des Systèmes Temps Réel Embarqués
91191 Gif sur Yvette Cedex
France
Phone/Fax: +33 (0) 1 69 08 32 92
<tel:%2B33%20%280%29%201%2069%2008%2032%2092> / 83 95
--
Thierry Goubier
CEA list
Laboratoire des Fondations des Systèmes Temps Réel Embarqués
91191 Gif sur Yvette Cedex
France
Phone/Fax: +33 (0) 1 69 08 32 92 / 83 95
