search

blue-systems / plasma-issues

plasma 5.6 - 5.xx (ongoing)
0 stars 0 forks source link

Wallpapers: has a memory leak, changing the wallpapers a few times and "apply" them pumps plasmashell each time higher #125

Closed star-buck closed 5 years ago

star-buck commented 6 years ago

currently at over 480MB: screenshot_20180223_194834

star-buck commented 6 years ago

just applying the same 2 wallpapers back and forth like 5 times brings it up to this: screenshot_20180223_195136

star-buck commented 6 years ago

current system: screenshot_20180223_195305

llelectronics commented 6 years ago

Maybe a Qt Bug. On Neptune with Plasma 5.12 but Qt 5.7 I don't see a big memory usage change when changing wallpapers. It runs in about 180MB plus minus depending on the wallpaper. Never even touching 200MB

eikehein commented 6 years ago

David mentioned during a recent meeting that he's done a full rewrite of the image wallpaper code. It's not been posted for review yet.

star-buck commented 6 years ago

seems not a Qtbug, is also happening with Plasma 5.12 and Qt5.10 as on Pinebook img: Download any wallpaper via GHNS, then set method to "Scaled and Cropped", then switch between at least 3 wallpapers and simply hit "apply" without closing the config window while having KSYSGUARD opened will show increasingly memory for plasmashell each time you hit "apply".

star-buck commented 6 years ago

@eikehein : that doesnt help in case whatever debian or anyone else ships right now though.

llelectronics commented 6 years ago

Hmm... tested this a few minutes here with changing back and forth. (long video ahead) Finally it did raise the memory a bit but nothing so drastically like you have. Wonder what could cause this. Might be just the thumbnail loading :P https://youtu.be/knukjxGG18c

kbroulik commented 6 years ago

From what I can tell this is a Qt bug, especially noticeable with slideshow where images repeatedly change: https://bugs.kde.org/show_bug.cgi?id=368838

kbroulik commented 6 years ago

I tried slamming a QQuickWindow::releaseResources() call after we unloaded the previous wallpaper with no real effect, though, but I also didn't observe this memory leak here.

notmart commented 6 years ago

could be tried to do a minimal qml that loads in qmlscene continuouly switching the source of an Image to isolate if the problem is actually in Image. (and if there is difference between changing source to an Image which is what the wallpaper is doing now compared to creating and destroying different image components w/releaseresources each time) for the wallpaper in general yeah, I would prefer to wait to work on David's rewrite

notmart commented 6 years ago

important point: memory grows both just hitting apply and keeping always the same dialog open and opening/closing the dialog each time? (this is to exclude something wrong in the dialog, thumbnail loading etc)

notmart commented 6 years ago

this might be related https://bugreports.qt.io/browse/QTBUG-61754 tough my qt build has both patches in but is still somewhat reproducible (about 10 MB every wallpaper switch which is indeed around the size of the final uncompressed image, so somehow Image is still not freeing part of the texture data) the test case attached to that bug report doesn't seem to increase memory a lot while running

notmart commented 6 years ago

the leak seems to be in the blurring happening when the image is smaller then the desktop disabling all that code the leak goes awaty, it may be a leak in the qt GaussianBlur component, but more likely is that regardless that blur fill is used or not, after some switches, 4 images stay always loaded in memory (2 images for the fade anim, and one copy of each of them for the blur fill) that's afaik what David rewrite adresses. interesting to see if keeps climbing or as it seems here, stabilizes after few changes, which would mean.. easy optimizations

kbroulik commented 6 years ago

If the GaussianBlur item leaked, David's rewrite would indeed "fix" that as with the new approach a fresh item will be created and the old one destroyed whereas currently we reuse it

NuLogicSystems commented 6 years ago

Question: Does the rewrite also fix the mouse lag issue in the settings dialog?

star-buck commented 6 years ago

Wrong thread, this has nothing to do with mouse lag, please dont hijack topics that are absolutely unrelated and have todo with libinput and co., thanks.

On Feb 28, 2018 4:06 PM, "James Kittsmiller" notifications@github.com wrote:

Question: Does the rewrite also fix the mouse lag issue in the settings dialog?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/blue-systems/plasma-5.13/issues/125#issuecomment-369267425, or mute the thread https://github.com/notifications/unsubscribe-auth/AAzRhnSp9wRQkdw6_UbG_ZUGgHewM2aDks5tZWr8gaJpZM4SRUIX .

Pointedstick commented 6 years ago

Wasn't this fixed with an upstream Qt change in 5.11?

https://codereview.qt-project.org/#/c/224684/

davidedmundson commented 5 years ago

Since my Qt patch I've stopped seeing this issue in KDE bugzilla. Please let me know if it's an issue with Qt 5.11.2 or higher.

Pointedstick commented 5 years ago

I can confirm the fix myself. Also, we backported the fix to Qt 5.9.5 in Kubuntu 18.04 and users there confirmed the fix too.