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

PR: Disable AppNap #441

Merged
merged 5 commits into from
Oct 13, 2019
Merged

PR: Disable AppNap #441

merged 5 commits into from
Oct 13, 2019

Conversation

impact27
Copy link
Contributor

@impact27 impact27 commented Sep 21, 2019

@ccordoba12
Copy link
Member

This could be done only at the event loops level and not for the entire application. At least that's how it was done in old IPython versions:

https://github.com/ipython/ipython/blob/a2685d78f2403f84e6cb915ae11a4f6033ccc5f6/IPython/lib/inputhook.py

@impact27
Copy link
Contributor Author

impact27 commented Sep 22, 2019

I disagree, the problem here is not limited to the back ends. If the user creates a gui app (qt5 / cocoa/ ...), macos will reclassify the kernel as a GUI application. Therefore, If the kernel (which is not a GUI application intrinsically) shows any window at any point, then macos will start to slow it down. Therefore AppNap should not apply to ipykernel. (As AppNap uses window usage as a way to know if the app is used, which is not at all a good indicator in the case of ipykernel.)

@impact27
Copy link
Contributor Author

I am not sure what the error is. I see the error intermittently in spyder-kernels tests as well.
in with setup_kernel(cmd) as client:, somehow the connection file is empty? json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

@impact27
Copy link
Contributor Author

Retriggering the tests

@impact27 impact27 closed this Sep 25, 2019
@impact27 impact27 reopened this Sep 25, 2019
@ccordoba12
Copy link
Member

@impact27, you should really avoid your practice of mixing PRs because it makes reviewing them a nightmare.

So please remove your last commit here and in PRs #438 and #439, and instead please try to make PR #445 pass, so it can be merged first.

@impact27
Copy link
Contributor Author

impact27 commented Oct 5, 2019

@impact27, you should really avoid your practice of mixing PRs because it makes reviewing them a nightmare.

So please remove your last commit here and in PRs #438 and #439, and instead please try to make PR #445 pass, so it can be merged first.

I am sorry that this makes it hard for you to review. It is my fault for merging the PRs before #445 was ready.

I will mark all the PR that depends on another PR as [Waiting depencency merge] so you know that they must not be reviewed before the dependent PR is merged.

@impact27 impact27 changed the title Disable AppNap [Waiting depencency merge] PR: Disable AppNap Oct 5, 2019
@impact27
Copy link
Contributor Author

impact27 commented Oct 5, 2019

The reason why I need to merge #445in the other PR is that AppVeyor gets stuck if #445 is not merged, so instead of having test suceeding in 10 min, there is failing tests in 3h.

@impact27 impact27 changed the title [Waiting depencency merge] PR: Disable AppNap [Depends on #445] PR: Disable AppNap Oct 5, 2019
@impact27 impact27 changed the title [Depends on #445] PR: Disable AppNap PR: Disable AppNap Oct 9, 2019
@impact27 impact27 closed this Oct 9, 2019
@impact27 impact27 reopened this Oct 9, 2019
@ccordoba12
Copy link
Member

If the user creates a gui app (qt5 / cocoa/ ...), macos will reclassify the kernel as a GUI application

Even if there you haven't run %gui qt5 before creating the app?

@impact27
Copy link
Contributor Author

I was able to trigger the problem purely with pyqt5

@impact27
Copy link
Contributor Author

impact27 commented Oct 10, 2019

#!/usr/bin/env python3

import time
# Uncomment either
from PySide2 import QtWidgets, QtCore
# from PyQt5 import QtWidgets, QtCore

app = QtWidgets.QApplication([''])
timer = QtCore.QTimer(app)
timer.setSingleShot(True)
timer.timeout.connect(lambda: app.quit())
timer.start(35000)
app.exec_()
start = time.time()
time.sleep(0.0005)
print('dt = ', time.time() - start)

@ccordoba12
Copy link
Member

I was able to trigger the problem purely with pyqt5

Ok, if that's the case then this should be merged as it is.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @impact27 for finding the cause of this very annoying bug!

@blink1073, I think this one is ready.

@blink1073
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

4 participants