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

JupyterKernel uses 100% CPU when idle #16

Closed
lassoan opened this issue Sep 18, 2018 · 12 comments
Closed

JupyterKernel uses 100% CPU when idle #16

lassoan opened this issue Sep 18, 2018 · 12 comments

Comments

@lassoan
Copy link
Contributor

lassoan commented Sep 18, 2018

When Jupyter kernel is connected, Slicer uses 100% CPU usage (at least on Windows). This does not impact performance (it only uses idle CPU cycles) but certainly not nice.

It is due to how QSocketNotifier is set up.

If we put a delay in the poll, as it was done earlier then CPU usage is low but GUI becomes much less responsive (slice browsing is slow, etc.).

It may be possible that the problem can be fixed by initializing the socket differently and making sure all available data is read as described here.

@pieper
Copy link
Member

pieper commented Sep 18, 2018

The socket should be set to be in blocking mode and there's be no need for polling or delays. The QSocketNotifier will signal when data is ready. The slot should be set up to buffer whatever is available to read.

@jcfr
Copy link
Member

jcfr commented Sep 18, 2018

I think this is what this PR was intended to do: https://github.com/Slicer/SlicerJupyter/pull/4/files

@pieper
Copy link
Member

pieper commented Sep 19, 2018

If QSocketNotifier isn't working then it looks like the zmq socket is in a non-blocking mode. It should be blocking so it can be cleanly integrated with the event loop.

@lassoan
Copy link
Contributor Author

lassoan commented Sep 19, 2018

According to Qt documentation: "Note for Windows users: The socket passed to QSocketNotifier will become non-blocking, even if it was created as a blocking socket."

In Qt4, there was also a note: "Notes for Windows Users: The socket passed to QSocketNotifier will become non-blocking, even if it was created as a blocking socket. The activated() signal is sometimes triggered by high general activity on the host, even if there is nothing to read. A subsequent read from the socket can then fail, the error indicating that there is no data available (e.g., WSAEWOULDBLOCK). This is an operating system limitation, and not a bug in QSocketNotifier." Fortunately, this comment has been removed for Qt5, so probably they figured out how to handle this properly.

I've added logs to print which socket produces the the notification events continuously, and it is the "stdin" handle. All the other sockets behave correctly.

If I simply remove the socket notifier for stdin then unfortunately Slicer stops responding to Jupyter messages.

Why do we need to add a notifier to stdin?

Is it possible that Jupyter notebook really constantly sends data to stdin socket? It is unlikely that others wouldn't have noticed this, even if it only happens on Windows. I've tried latest jupyterlab and the behavior is the same.

Should we switch to plain timer-based polling on Windows?

@pieper
Copy link
Member

pieper commented Sep 19, 2018

It does sound like some kind of polling and timer is required on windows, and that's really too bad. It'll need to fire somewhat often or latency will be bad, but this will mean the application will never really sleep, which is bad for things like battery life on laptops.

There must be some windows-friendly way to integrate sockets into the event loop. I haven't done that level of programming on windows for a long time but ultimately the app is event driven.

@lassoan
Copy link
Contributor Author

lassoan commented Sep 19, 2018

Polling with a timer is completely fine and it is done already in many places (e.g., the CLI network and processing queue, OpenIGTLink, virtual reality, etc). Since latency is not critical at all (if the user receives response within 1/10 second it is completely fine), we can call the timer at every 50-100ms.

There must be some windows-friendly way to integrate sockets into the event loop

Socket notifiers work well for all the other sockets (shell, controller, controller_sub, publisher_sub), they only emit event when something is received. There is something wrong with stdin socket, which is for allowing the frontend to send keyboard input to the kernel. I think this stdin feature is not used at all, so I don't really understand why disabling notification for it blocks notification.

It is not only about windows, because the original user report was for Linux.

lassoan added a commit that referenced this issue Sep 19, 2018
CPU use on Windows was 100% due to stdin socket generating events continuously.
Solved this by polling at regular intervals (every 50ms).
@lassoan
Copy link
Contributor Author

lassoan commented Sep 19, 2018

I've commited a fix that makes the kernel use a timer instead of socket notifier to trigger event processing on Windows.

The problem may exist on non-Windows platforms as well - see user report here. It might be a bug in Jupyter or Qt.

@dmichalskii
Copy link

dmichalskii commented Sep 20, 2018

Hi,

I have Linux Mint 18.3 and get 100% on one process with Slicer built through Slicer/SlicerBuildEnvironment#8 with this extension. Qt 5.11.0

This problem doesn't exist on my own built Slicer same revision as above but Qt 5.11.1

Edit. I checked it a moment ago and despite compiling with version 5.11.1 it does not work correctly. But working version is built with this instruction https://www.slicer.org/wiki/Documentation/Nightly/Developers/Build_Instructions.

@ihnorton
Copy link
Member

Also seeing high resting CPU on binder, but it seems inconsistent.

@lassoan
Copy link
Contributor Author

lassoan commented Oct 17, 2018

Does it solve the high CPU usage issue if you modify xSlicerServer.h to always use #define JUPYTER_POLL_USING_TIMER ?

@dmichalskii
Copy link

I test this module and there is no more 100% CPU usage.

@lassoan
Copy link
Contributor Author

lassoan commented Dec 11, 2018

Thanks for testing.

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

No branches or pull requests

5 participants