Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce memory usage by keeping main window out of memory #207

Closed
stefanobartoletti opened this issue Feb 23, 2019 · 22 comments
Closed

Reduce memory usage by keeping main window out of memory #207

stefanobartoletti opened this issue Feb 23, 2019 · 22 comments
Assignees

Comments

@stefanobartoletti
Copy link

I'm reporting this from Linux (openSUSE Tumbleweed to be precise, if it matters).

On my system, Vorta uses about 215~220 MB of RAM when running, even when not performing any action and merely residing idle on the system tray.

This seems to be a bit high.

For comparison, KeepassXC uses about 25~30 kB; it is a different software, with a different purpose, but it behaves in a similar way: it resides on the tray when idle, and like Vorta it uses Qt.

@m3nu
Copy link
Contributor

m3nu commented Feb 23, 2019

KeepassXC is a Qt C++ application. Vorta is PyQt (Python + Qt), which introduces some overhead.

I did a quick memory profiling and most of it is allocated when loading the different Qt UIs. So one way to optimize would be to avoid creating/loading the main window unless it's actually opened. I tried this just now and memory usage went to 50MB. So that's surely an option.

To reach the memory usage of C++, it would need rewriting in that or maybe Go. Can be done, but a big project and will lock many people out of contributing.

On macOS I also noticed that RAM usage goes down a lot after starting it. I guess the OS moves some data to swap or compresses it.

Filename: /Users/manu/Workspace/vorta/src/vorta/views/main_window.py

Line #    Mem usage    Increment   Line Contents
================================================
    23   90.453 MiB   90.453 MiB       @profile
    24                                 def __init__(self, parent=None):
    25   90.523 MiB    0.070 MiB           super().__init__()
    26  128.906 MiB   38.383 MiB           self.setupUi(self)
    27  128.906 MiB    0.000 MiB           self.setWindowTitle('Vorta for Borg Backup')
    28  128.906 MiB    0.000 MiB           self.app = parent
    29  128.914 MiB    0.008 MiB           self.current_profile = BackupProfileModel.select().order_by('id').first()
    30  128.918 MiB    0.004 MiB           self.setWindowFlags(QtCore.Qt.WindowCloseButtonHint | QtCore.Qt.WindowMinimizeButtonHint)
    31
    32                                     # Load tab models
    33  158.242 MiB   29.324 MiB           self.repoTab = RepoTab(self.repoTabSlot)
    34  160.418 MiB    2.176 MiB           self.sourceTab = SourceTab(self.sourceTabSlot)
    35  250.922 MiB   90.504 MiB           self.archiveTab = ArchiveTab(self.archiveTabSlot)
    36  304.477 MiB   53.555 MiB           self.scheduleTab = ScheduleTab(self.scheduleTabSlot)

@m3nu m3nu added the help wanted This issue is available, comment if you want to fix it label Feb 23, 2019
@ThomasWaldmann
Copy link
Collaborator

How much of this is virt(ual), how much res(ident)? See columns in top.

@m3nu
Copy link
Contributor

m3nu commented Feb 23, 2019

They call RES "Real Memory". I'm assuming VIRT is normal memory. There is also compressed memory.

Columns in top are the same, it's different from Linux.

Looking at some other processes, we're not too terrible. Could still be lower if we manage to fully remove the main window from memory when it's not used.

screen shot 2019-02-23 at 22 21 13

@ThomasWaldmann
Copy link
Collaborator

ThomasWaldmann commented Feb 23, 2019

Can you try calling the gc after setupUI and after each tab is initialized?

Part of the "problem" might be Python fetching memory from the OS, but being rather reluctant to give it back, but rather keeping free memory in its own memory management.

As long as the memory is not actively used, it is maybe not as bad as it looks though.

@m3nu
Copy link
Contributor

m3nu commented Feb 23, 2019

Like this? Not much difference.

I think the main window including all references need to be fully removed when the window is closed.

Line #    Mem usage    Increment   Line Contents
================================================
    22   90.281 MiB   90.281 MiB       @profile
    23                                 def __init__(self, parent=None):
    24   90.348 MiB    0.066 MiB           super().__init__()
    25  128.676 MiB   38.328 MiB           self.setupUi(self)
    26  128.676 MiB    0.000 MiB           self.setWindowTitle('Vorta for Borg Backup')
    27  128.676 MiB    0.000 MiB           self.app = parent
    28  128.688 MiB    0.012 MiB           self.current_profile = BackupProfileModel.select().order_by('id').first()
    29  128.691 MiB    0.004 MiB           self.setWindowFlags(QtCore.Qt.WindowCloseButtonHint | QtCore.Qt.WindowMinimizeButtonHint)
    30
    31                                     # Load tab models
    32  157.914 MiB   29.223 MiB           self.repoTab = RepoTab(self.repoTabSlot)
    33  157.914 MiB    0.000 MiB           gc.collect()
    34  160.066 MiB    2.152 MiB           self.sourceTab = SourceTab(self.sourceTabSlot)
    35  160.066 MiB    0.000 MiB           gc.collect()
    36  250.496 MiB   90.430 MiB           self.archiveTab = ArchiveTab(self.archiveTabSlot)
    37  250.496 MiB    0.000 MiB           gc.collect()
    38  299.992 MiB   49.496 MiB           self.scheduleTab = ScheduleTab(self.scheduleTabSlot)
    39  299.992 MiB    0.000 MiB           gc.collect()
    40  300.023 MiB    0.031 MiB           self.miscTabSlot = MiscTab(self.miscTabSlot)
    41  300.023 MiB    0.000 MiB           gc.collect()
    42  300.125 MiB    0.102 MiB           self.tabWidget.setCurrentIndex(0)

@m3nu
Copy link
Contributor

m3nu commented Feb 23, 2019

Related: https://stackoverflow.com/questions/20164015/is-deletelater-necessary-in-pyqt-pyside

Should be possible to recreate the window instead of hiding it when it's closed.

I just worry about signals not being passed through. Will need some case-by-case adjustments.

@stefanobartoletti
Copy link
Author

KeepassXC is a Qt C++ application. Vorta is PyQt (Python + Qt), which introduces some overhead.

I did a quick memory profiling and most of it is allocated when loading the different Qt UIs. So one way to optimize would be to avoid creating/loading the main window unless it's actually opened. I tried this just now and memory usage went to 50MB. So that's surely an option.

To reach the memory usage of C++, it would need rewriting in that or maybe Go. Can be done, but a big project and will lock many people out of contributing.

I mentioned KeepassXC just as an example, just because it was the first thing that hit me when looking at what app on my tray I could refer to.

I was not suggesting a complete rewrite; I think that preventing the main window to load when not used, or running the program in a way similar to a daemon, could be a viable solution.

@m3nu
Copy link
Contributor

m3nu commented Feb 26, 2019

Yeah. I also think cleaning up the main window from memory is a good step.

I may look into rewriting it in Go at some point. Many things, like the UI files should be directly reusable anyways.

@m3nu m3nu removed the help wanted This issue is available, comment if you want to fix it label Feb 26, 2019
@m3nu m3nu self-assigned this Feb 26, 2019
@m3nu m3nu changed the title Reduce memory usage Reduce memory usage by keeping main window out of memory Feb 26, 2019
@m3nu
Copy link
Contributor

m3nu commented Feb 26, 2019

Here a fairly small PR #208 that removes the main window from memory when closing it. With it, memory usage is about 70-80MB on macOS (10MB compressed memory) and Linux. With the window open, it increases to about 300MB, but goes down again on closing.

@m3nu
Copy link
Contributor

m3nu commented Feb 27, 2019

If there aren't any complaints, I'll go ahead and merge this soon. So far I tested it under macOS and Ubuntu. No issues. The main window was fairly isolated except for a single call to refresh the time of the next scheduled backup.

@m3nu
Copy link
Contributor

m3nu commented Mar 4, 2019

Merged this, since nobody complained. Please report any bugs that may appear in the master-branch.

@ymage
Copy link

ymage commented Mar 25, 2019

I have an issue with this fix. main_window attribute is not created when launched in background.
So when clicked in tray icon, it crashes with

File "/usr/lib/python3.7/site-packages/vorta/tray_menu.py", line 28, in on_activation self.app.toggle_main_window_visibility() File "/usr/lib/python3.7/site-packages/vorta/application.py", line 86, in toggle_main_window_visibility if self.main_window.isVisible(): AttributeError: 'VortaApp' object has no attribute 'main_window'

@m3nu
Copy link
Contributor

m3nu commented Mar 25, 2019

Probably true. The function toggle_main_window_visibility(self) needs to be updated.

@m3nu m3nu reopened this Mar 25, 2019
@m3nu
Copy link
Contributor

m3nu commented Mar 25, 2019

Here a slightly changed function. Does this work? (I don't have KDE, so can't test it). The main window could be hidden OR closed.

    def open_main_window_action(self):  # loads the main window into memory and brings it to the top.
        self.main_window = MainWindow(self)
        self.main_window.show()
        self.main_window.raise_()

    def toggle_main_window_visibility(self):  # used in KDE only
        if hasattr(self, 'main_window') and self.main_window.isVisible():  # changed part
            self.main_window.close()
        else:
            self.open_main_window_action()

@ymage
Copy link

ymage commented Mar 25, 2019

Almost ok.

  1. Launched in background.
  2. First click to get it foreground is ok
  3. Close the window destroys the MainWindow object
  4. Secund click to get it foreground is ko : RuntimeError: wrapped C/C++ object of type MainWindow has been deleted

@m3nu
Copy link
Contributor

m3nu commented Mar 25, 2019

I see. You have the full stacktrace? Should be a quick fix too.

Also thanks for testing this.

@ymage
Copy link

ymage commented Mar 25, 2019

The full stacktrace :

File "/usr/lib/python3.7/site-packages/vorta/tray_menu.py", line 28, in on_activation
    self.app.toggle_main_window_visibility()
File "/usr/lib/python3.7/site-packages/vorta/application.py", line 87, in toggle_main_window_visibility
    if hasattr(self, 'main_window') and self.main_window.isVisible():  # changed part
RuntimeError: wrapped C/C++ object of type MainWindow has been deleted

@m3nu
Copy link
Contributor

m3nu commented Mar 25, 2019

Here a new snippet:

    def toggle_main_window_visibility(self):
        import sip
        main_window_open = hasattr(self, 'main_window') and not sip.isdeleted(self.main_window)
        if main_window_open:
            self.main_window.close()
        else:
            self.open_main_window_action()

@ymage
Copy link

ymage commented Mar 25, 2019

My previous workflow seems to work.

@m3nu
Copy link
Contributor

m3nu commented Mar 25, 2019

Great. Since this crashes it, I'll probably do another hotfix. Seems we had too many risky changes in the last release and people don't bother testing the master branch.

@m3nu
Copy link
Contributor

m3nu commented Mar 25, 2019

Released as v0.6.15.

@ymage
Copy link

ymage commented Mar 27, 2019

Thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants