fmoralesc / pastie

a simple *nix clipboard manager with application-indicator support
46 stars 7 forks source link

Memory leak #10

Closed hotice closed 14 years ago

hotice commented 14 years ago

After using Pastie 0.5 pre for a whole day, it uses ~300 mb of RAM. Screenshot: http://imgur.com/7O3Nm.png

fmoralesc commented 14 years ago

Can you send me (hel dot sheep at gmail dot com) your .clipboard_history file?

hotice commented 14 years ago

I actually can't because I had passwords on the clipboard (long story). But I also had files on the clipboard, could that have something to do with it? Also, after restarting Pastie it worked normally.

fmoralesc commented 14 years ago

Don't worry. File entries shouldn't be a problem because they contain nothing more than a list of files; pastie doesn't save the files themselves. So, those are quite light (unless, of course, you are copying hundreds or thousand of files from within a folder). I was thinking the problem might have been due to some images in the history. Every image item retains a copy of the entire image pixel array (so we can save it later, and then reconstruct it from the .clipboard_history file), so those entries can get quite large, depending on what you copy. Maybe there is duplicated data somewhere. The only thing I really need to know for now is whether you had images in your history, and if so, how large they were, how many items your history had and how large the .clipboard_history file was (so i can make an estimation of how many memory pastie should have used to retain the history).

fmoralesc commented 14 years ago

Also, how is it that the system monitor app shows pastie's name as "pastie"? Here it shows me "python", and I need to check the command line to identify it.

hotice commented 14 years ago

It used to be just "Python" in the first version but then when I started using the PPA it changed to "pastie". I can even kill it with "killall pastie".

The .clipboard_history file is at work and I'm at home now... I'll try to replicate it here but if I can't, I'll send in the details tomorrow.

fmoralesc commented 14 years ago

OK, thanks. I'll try to replicate it too.

hotice commented 14 years ago

OK I managed to get Pastie to 61mb of RAM. I copied some files, some images and most importantly, some pieces of the same image in GIMP. Like: open an image in GIMP, copy part of it, then paste it, copy another part and paste it and so on.

It took under one minute to do this :)

fmoralesc commented 14 years ago

gee, that's bad...

hotice commented 14 years ago

I've sent you the file you requested. It looks very screwed up...

fmoralesc commented 14 years ago

That's because it contains the binary data of the images encoded in base64, and because i was too lazy to make pastie pretify the XML envelope...

hotice commented 14 years ago

Then hopefully it's not as bad as I imagined. It can be fixed, right?

fmoralesc commented 14 years ago

Well, it can at least be made better. (As a last resort, we can keep only the labels in memory and use the .clipboard_history as a cache for the history contents, so every time an item is selected, it retrieves the data from disk instead of memory.) I'll check how this works for the glippy developers.

radioboy commented 14 years ago

I'm having the problem, too. Pastie is getting to use more than 1 GB of ram and keeps growing. :( As far as I remember I was not copying anything at the moment.

legluondunet commented 14 years ago

I have same problem.

fmoralesc commented 14 years ago

I'm working on it. It seems this is due to some reference cycles that obstruct python garbage collector.

fmoralesc commented 14 years ago

I've made some changes that should mitigate the problem. The code is in http://github.com/fmoralesc/pastie/tree/testing

Also, this problem might have some relation to https://bugs.launchpad.net/ubuntu/+source/indicator-application/+bug/569273 and https://bugs.launchpad.net/indicator-application/+bug/602920

fmoralesc commented 14 years ago

A more recent version (0.21 against 0.19) of the application indicators is available in ppa:ted/bugfix . Using this version, the menus memory is freed when the history is cleaned.

hotice commented 14 years ago

You can copy the fixed appindicator from that PPA in the Pastie PPA.

fmoralesc commented 14 years ago

That's a good idea, and I'll do it asap. Still, it's a partial fix for pastie. It's memory usage still grows as the history is used (much more slowly, though, if at all). I'm trying to figure out what's the deal with that, so I'm delaying the release of the 0.5 code.

hotice commented 14 years ago

By the way, I'm not sure if you're aware of this, but to copy a package from one PPA to another, all you have to do is select "View packages", then "Copy package", then copy it... all from within Launchpad.

I hope that helps :)

fmoralesc commented 14 years ago

Thanks! I was just going to ask you that ;)

hotice commented 14 years ago

After the appindicator-cli update came from the PPA, it seems this bug has been fixed. I've copied random files for 5 minutes and Pastie only got an 0.1 extra MB in memory usage. I will continue to monitor it to see if that changes...

hotice commented 14 years ago

It seems it's not fixed afterall. I did some heavy copy/paste and it didn't leak any memory but after a day's usage, it used around ~400 MB of RAM again :(

fmoralesc commented 14 years ago

I see. Can you try using this new package? http://github.com/downloads/fmoralesc/pastie/pastie_0.5.0-1ubuntu1_all.deb It's the same version number as the previous one, but it contains the new code (so you'll have to remove the previous version before installing this). It's still experimental, but should fix the mayor leak. Notice, though, that somehow memory still grows on time. I'm still working on that.

hotice commented 14 years ago

Sure, I'll install it right away.

hotice commented 14 years ago

After about 3 hours of using it, Pastie used a max of 8.2 MB of RAM. Right now the amount of RAM decreased to 5.5. I will watch it closely tomorrow too and see if it still leaks memory - hopefully it won't.

hotice commented 14 years ago

By the way, I updated the Pastie 0.5-pre post on WebUpd8 with the link you provided above since the old one doesn't work anymore.

fmoralesc commented 14 years ago

OK. I had pulled it down because the problem was so serious I thought it was better to prevent people from downloading it for the moment. I should have told you, maybe.

fmoralesc commented 14 years ago

BTW, are you using 32 bits? I'm using 64bits and pastie's memory usage at start with one item is 11.7 MB. I read somewhere python tends to use more memory in 64bit systems

hotice commented 14 years ago

Yes, 32bit.

hotice commented 14 years ago

I used Pastie at work the whole day. The first time I installed the new version it used ~30-40 MB of RAM. I then deleted the .clipboard_history file and Pastie never went past 9 MB of RAM. In fact, most of the day it was around ~3.4 MB of RAM.

So could the .clipboard_history file affect the memory usage? Also, weird that even though after deleting .clipboard_history, I did some heavy copy/paste, the amount of RAM used didn't increase too much as you can read above...

fmoralesc commented 14 years ago

The memory usage depends on the type of contents you have. Text and files are very lightweight, but images are heavier. Besides this, if you have images in the history, pastie has to reconstruct them from the data in the history file when it loads, so if you had images saved into your previous history file, that might be the reason of the difference in ram usage. If not, then it's really weird. Still, I'm glad the changes seem to be working.

hotice commented 14 years ago

Well I keep copying images and the memory does not increase.

I've just set Pastie history to 40, copied about 40 images and Pastie only reached 9.9 MB.

fmoralesc commented 14 years ago

:) That's excellent news. (On the other hand, I just copied 10 images and pastie's memory usage is 28MB. This difference seems to be due the 64bits architecture vs the 32 bits.)

fmoralesc commented 14 years ago

I've cleaned up the code and fixed some issues with the recovery procedures. A new testing package is at http://github.com/downloads/fmoralesc/pastie/pastie_0.5.1-1ubuntu1_all.deb I'll wait for confirmation about this bug being fixed and then release via the PPA.

hotice commented 14 years ago

Using the old version it worked ok. Only once it went to 17 MB of RAM, I have no idea why. But most of the time it was around 3-4 MB of RAM. I tested it on 2 computers.

I'll try the new version now.

fmoralesc commented 14 years ago

After testing, I've come to the decision to release 0.5.2 via the PPA. I'm closing this issue.

hotice commented 14 years ago

Great!

fmoralesc commented 14 years ago

:) Thanks for your help.

hotice commented 14 years ago

Glad I could help!

hotice commented 14 years ago

I noticed something, maybe it can help improve Pastie memory usage even more:

Pastie doesn't use a lot of RAM, no matter how many files there are in its history. But when I take a screenshot with an application like Shutter, the image is copied to the clipboard (like an actual image, not a file - and show up in Pastie like this: "[1280x1024]"). That makes Pastie use some 10 MB extra of RAM (aprox.)