#5532 HIGH Update.: Sugar shell consuming vast amounts of memory

[Zarro Boogs per Child]

#5532: Sugar shell consuming vast amounts of memory
-----------------------+----------------------------------------------------
  Reporter:  kimquirk  |       Owner:  tomeu   
      Type:  defect    |      Status:  new     
  Priority:  high      |   Milestone:  Update.1
 Component:  sugar     |     Version:          
Resolution:            |    Keywords:          
  Verified:  0         |    Blocking:          
 Blockedby:            |  
-----------------------+----------------------------------------------------

Comment(by marco):

 Review for fix_leaks_ps.patch:

 {{{
 +        self._s1 = None
 }}}

 I think we should merge _add_items in the constructor. Please also get rid
 of the commented out code while you are at it.

 {{{
 +            shell_model = self._shell.get_model()
 }}}

 We are doing this in so many places... I think we should store the model
 in an instance variable.

 {{{
 +        self._s1 = home_model.connect('active-activity-changed',
 }}}

 I think we should use descriptive names even if gets more verbose. I'm
 generally using hid as a suffix to indicate it's a handler id.

 {{{
          self._layout.remove(icon)
          del self._activities[activity_model.get_id()]
 +        icon.destroy()
 }}}

 I think destroyed items are removed automatically, so the remove would be
 unnecessary, can you verify please?

 {{{
 +    def __destroy_cb(self, activity_icon):
 +        for key, icon in self._icons.iteritems():
 +            icon.destroy()
 +
 +        self._icon.destroy()
 }}}

 I don't understand why this is necessary. Killing an item should also
 explicitly destroy all of its children automatically. If you just
 self._icons = None do we still leak?

 (Please make sure to pylint all the patches, /me trying to get used to
 always do it)

-- 
Ticket URL: <http://dev.laptop.org/ticket/5532#comment:14>
One Laptop Per Child <http://dev.laptop.org>
OLPC bug tracking system