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

Memory usage grow on task switching #62

Closed
pohmelie opened this issue Apr 1, 2016 · 6 comments
Closed

Memory usage grow on task switching #62

pohmelie opened this issue Apr 1, 2016 · 6 comments

Comments

@pohmelie
Copy link

pohmelie commented Apr 1, 2016

import asyncio

from PyQt5 import QtWidgets
from quamash import QEventLoop


@asyncio.coroutine
def Switcher():

    while True:

        yield from asyncio.sleep(0.001)


if __name__ == "__main__":

    import sys

    app = QtWidgets.QApplication(sys.argv)
    loop = QEventLoop(app)
    asyncio.set_event_loop(loop)

    with loop:

        loop.run_until_complete(Switcher())

Usage grow proportinally to switching speed. I made some big app with awesome 👍 quamash, and saw slow growing for hours of work.

Tested with:

  • Ubuntu 14.04 / Python 3.4.3 / PyQt 5.5
  • Windows 7 / Python 3.5.0 / PyQt 5.5.1
  • Windows xp / Python 3.4.4 / PyQt 5.5
@harvimt
Copy link
Owner

harvimt commented Apr 4, 2016

Looks like the QApplication object keeps a reference to the QTimer objects.

I'm not sure what to do about this. We could re-user the QTimer objects, but it still seems like a good idea to be able to destroy them.

There's some discussion on these issues here: http://enki-editor.org/2014/08/23/Pyqt_mem_mgmt.html

I may have to subclass or wrap QTimer in some way so we're not using a closure as a slot. (or we may have to use QBasicTimer instead of QTimer

@harvimt
Copy link
Owner

harvimt commented Apr 4, 2016

I looks like migrating to QBasicTimer is probably a good idea anyway, but it'll require some pretty serious refactoring.

@harvimt
Copy link
Owner

harvimt commented Apr 4, 2016

I've pushed https://github.com/harvimt/quamash/tree/gh62 which has pretty much the minimum code needed to fix this, but it seems kind of fugly. (like _SimpleTimer and cancellable should be changed, and maybe simple timer should be a factory function that takes in a QtCore like make_signaller does?)

@pohmelie
Copy link
Author

pohmelie commented Apr 4, 2016

I'm glad that you find the problem. Your patch works just fine in place of memory usage. That's great! There is some problem, when I make KeyboardInterrupt (ctrl-c) the test code above. I have infinite logging:

Traceback (most recent call last):
  File "/home/broomrider/tmp/quamash/quamash/__init__.py", line 191, in timerEvent
    self.callback()
  File "/home/broomrider/tmp/quamash/quamash/__init__.py", line 332, in upon_timeout
    assert timer in self.__timers, 'Timer {} not among {}'.format(timer, self.__timers)
AssertionError: Timer None not among []

But that is minor problem, hope for master tree update soon 👏

@harvimt
Copy link
Owner

harvimt commented Apr 4, 2016

try now, I changed that from an assertion to a warning.

@pohmelie
Copy link
Author

pohmelie commented Apr 4, 2016

I think this can be closed since now 👍

@pohmelie pohmelie closed this as completed Apr 4, 2016
harvimt added a commit that referenced this issue Apr 4, 2016
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

2 participants