Ticket #8423 (new defect)

Opened 6 years ago

Last modified 5 years ago

The freeform view leaks an icon on every move

Reported by: bemasc Owned by: marco
Priority: blocker Milestone: 8.2.0 (was Update.2)
Component: sugar Version: not specified
Keywords: relnote, polish:8.2.0 Cc: eben, gregorio
Action Needed: communicate Verified: no
Deployments affected: Blocked By: #7579
Blocking:

Description

I have written a simpler, faster replacement for grid.py. In the process of doing so, I have discovered a bug somewhere else in the stack. Every time the user drags+drops an icon in the freeform view, the view registers a Move of an existing icon, followed by an Add of a new (invisible!) icon at the same location... and more strange stuff from there.

I don't know where the code is that handles drag and drop, but it's clearly messed up, and it's probably part of the reason why the spread view doesn't work so well. It's also a memory leak, since a new icon is created and stored in memory every time the user moves an existing icon on the home view.

Sample log (from my hacked-up grid.py) showing a correct Move, followed immediately by an Add to the same location.

1221112905.758911 ERROR root: move <ActivityIcon object at 0x9b23cd4 (SugarFavoriteActivityIcon at 0x9d06358)> to (x,y)=(136,13)
1221112905.762867 ERROR root: energy 0 for <ActivityIcon object at 0x9b23cd4 (SugarFavoriteActivityIcon at 0x9d06358)>
1221112905.764855 ERROR root: energy of <ActivityIcon object at 0x9b23cd4 (SugarFavoriteActivityIcon at 0x9d06358)> on drop is 0
1221112905.818773 ERROR root: add <ActivityIcon object at 0x9b466e4 (SugarFavoriteActivityIcon at 0x9d87f60)> at (x,y)=(136,13)
1221112905.822634 ERROR root: energy 169 for None
1221112905.847948 ERROR root: move <_MyIcon object at 0x9ba41e4 (CanvasIcon at 0x9be8ad0)> to (x,y)=(131,74)
1221112905.851802 ERROR root: energy 0 for <_MyIcon object at 0x9ba41e4 (CanvasIcon at 0x9be8ad0)>
1221112905.853788 ERROR root: energy of <_MyIcon object at 0x9ba41e4 (CanvasIcon at 0x9be8ad0)> on drop is 0
1221112905.856174 ERROR root: move <CurrentActivityIcon object at 0x9ba42fc (CanvasIcon at 0x9be8b48)> to (x,y)=(143,114)
1221112905.859533 ERROR root: energy 0 for <CurrentActivityIcon object at 0x9ba42fc (CanvasIcon at 0x9be8b48)>
1221112905.862273 ERROR root: energy of <CurrentActivityIcon object at 0x9ba42fc (CanvasIcon at 0x9be8b48)> on drop is 0
1221112906.038640 ERROR root: energy 169 for <ActivityIcon object at 0x9b23cd4 (SugarFavoriteActivityIcon at 0x9d06358)>
1221112906.044504 ERROR root: energy 169 for <ActivityIcon object at 0x9b466e4 (SugarFavoriteActivityIcon at 0x9d87f60)>

My question is: where's the code that tells the spreadlayout to add, move, etc.?

Attachments

grid.py.diff (8.2 kB) - added by bemasc 6 years ago.
Diff of grid.py (it's a big diff...)
grid.py (6.1 kB) - added by bemasc 6 years ago.
New grid.py (if you don't like the diff)
grid.py.diff2 (1.9 kB) - added by bemasc 6 years ago.
Add new energy function with no plateaus.
favoritesview.py.diff (1.0 kB) - added by bemasc 6 years ago.
New patch to favoritesview to fix memory leaks and unnecessary icon creation
grid.py-eben.diff (14.0 kB) - added by Eben 6 years ago.

Change History

Changed 6 years ago by bemasc

(12:37:53 PM) bemasc: marcopg: every time the user drags and drops an icon, this calls FavoritesLayout.move_icon()
(12:38:55 PM) bemasc: and RandomLayout.move_icon(), which calls Grid.move()
(12:39:14 PM) bemasc: that moves the icon, which somehow sends an ActivityChanged signal to the registry
(12:40:57 PM) bemasc: that activity-changed signal triggers FavoritesView._activity_changed_cb
(12:41:14 PM) bemasc: which calls self._add_activity()
(12:41:34 PM) bemasc: and self._add_activity makes a new ActivityIcon and adds it to the grid!
(12:41:51 PM) marcopg: bemasc: ok please add info on the ticket, looks like you tracked it down

Changed 6 years ago by gregorio

  • blockedby 7579 added

Changed 6 years ago by gregorio

  • keywords relnote added

Changed 6 years ago by bemasc

Diff of grid.py (it's a big diff...)

Changed 6 years ago by bemasc

New grid.py (if you don't like the diff)

Changed 6 years ago by bemasc

I have attempted to fix this in the course of rewriting grid.py. Please test the above patches.

These patches also remove the only use of numpy in the shell, thus saving several megabytes of memory.

I have attempted to reproduce the behavior of the previous grid.py as closely as possible, since I am not sure what the ideal desired behaviors are, and I find the structure of the drawing system extremely confusing.

Changed 6 years ago by bemasc

Add new energy function with no plateaus.

Changed 6 years ago by bemasc

  • keywords relnote, blocks?:8.2.0 added; relnote removed

I've just posted a patch that replaces the energy and force functions with ones that behave identically except in the case of total containment. In the case of one icon entirely inside another, these functions have no plateau, and still naturally push the icons away from each other. (It even breaks the symmetry if they are perfectly centered.)

I read something on IRC about a potential memory leak in grid.py, but I didn't quite understand (marco?). Given the large amount of memory that I'm told is saved by removing numpy from the shell, I think this should go in 8.2.0. I think it's more "polish" than "blocker", but easy enough at this point to be worthwhile.

Changed 6 years ago by marco

#8372 is another possible approach to save memory without changing the algorithm.

bemasc, I'm a bit worried about the performance consequences of your approach. I tested it yesterday using the test.py I sent on the mailing (thread about SpreadLayout) and it *seemed* to be much heavier than the current approach. It's just a feeling so far, so it would be good to collect some data before we make a decision.

Changed 6 years ago by bemasc

New patch to favoritesview to fix memory leaks and unnecessary icon creation

Changed 6 years ago by bemasc

I've added a new patch to favoritesview.py. I have tested this patch with the old grid.py. I believe it, by itself, kills the memory leak on moves, removes the weird invisble-icons-in-the-grid bug, and prevents icon objects from being discarded and recreated on every move.

Changed 6 years ago by Eben

Changed 6 years ago by gregorio

  • cc gregorio added
  • keywords polish:8.2.0 added; blocks?:8.2.0 removed
  • priority changed from normal to blocker
  • milestone changed from Not Triaged to 8.2.0 (was Update.2)

Hi Guys,

This is polish, but its also a bad problem. Fix it you can after solving the worse memory issues.

Thanks,

Greg S

Changed 6 years ago by marco

  • blockedby 7579 removed
Note: See TracTickets for help on using tickets.