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

Prevent qtconsole frontend freeze on lots of output. #3409

Merged
merged 1 commit into from Jul 18, 2013

Conversation

pankajp
Copy link
Contributor

@pankajp pankajp commented Jun 8, 2013

The output from the kernel is now clipped to last buffer_size
before displaying and a timer is used to flush the pending output
text instead of attempting to display text on every stream
output from kernel. The timer interval is adjusted based on
actual time taken to append a screenful of text to widget.
This throttles the widget repaints and avoids choking the Qt
event loop leaving time to handle other Qt events.

Test cases:
In [1]: for i in xrange(1000000): print i
In [2]: range(100000)

Without this commit the first input causes the qtconsole frontend
to freeze, not responding to Ctrl+C.

@ellisonbg
Copy link
Member

I have tested this and it works well. The code also looks good. But I wouldn't mind someone else having a look as I know relatively little about Qt.

@ccordoba12
Copy link
Member

Thanks a lot for tackling this one, it's been bugging me for some time.

This is a very good start but with the current buffer_size default (500 lines), there are still some usability issues (at least on my Linux machine):

  1. A Ctrl+C interruption takes a long time to be processed (like 2-3 secs), making believe the user that the frontend didn't really process it.
  2. The frontend takes a lot of time to show the Kernel menu, which makes very hard to trigger an interruption from there.
  3. There is no visual feedback suggesting that the frontend is printing an ongoing output.

From the user perspective, all these things would suggest that the console froze (although now it's not the case).

Right now I'm trying to come up with a solution to improve this situation.

@pankajp
Copy link
Contributor Author

pankajp commented Jun 17, 2013

@ccordoba12 This is very responsive on my machine, without much perceptible delay. I had actually written a very elaborate timer based mechanism to display output, but ended up with this as it very well for me with much simpler code too.
I can try the timer based approach again if you can describe what platform/machine you are experiencing the slowness. I suspect it may be a graphics card issue because i also noticed the slowness a few times, when my nvidia graphics card slows down after wake from sleep (on linux), slowing down all applications with it.

@ccordoba12
Copy link
Member

@pankajp: I'm on Kubuntu 13.04, with an ATI Wrestler (Radeon 6320) card and an AMD E-450 processor of two cores. This is not a powerful machine, just a netbook.

What do you see if you increase buffer_size to 2000 or 5000 lines? I suspect the slowness comes from the fact that the widget is trying to insert too much text at once.

So if the text could be broken up and inserted in chunks of, say, 100 lines, that could help to improve responsiveness in machines like mine. I made some simple tests and that worked for me.

@ccordoba12
Copy link
Member

@pankajp: I was wrong: printing buffer_size lines in chunks of 100 didn't turn out to be of much help. The real solution for me is to reduce buffer_size to 100 lines (or even 200), then I have very good interactivity on the widget.

But that seems to be a too small size. What if buffer_size is reduced automatically to 100 after trying to clip the incoming text a certain number of times?

@pankajp
Copy link
Contributor Author

pankajp commented Jun 21, 2013

@ccordoba12 I like the idea of automatically reducing the buffer_size, but it needs more bookkeeping to revert it to original setting after execution is completed, keeping the previous output.
Meanwhile, can your try my another branch https://github.com/pankajp/ipython/tree/qt-console-freeze-on-output-clip-timer where i've attempted to use a timer based approach to throttle stream writes.

@ccordoba12
Copy link
Member

@pankajp I just tested your timer branch and qtconsole is working the same as with my suggestion of reducing buffer_size. But after reading your commit with care, I think your solution is better because the timer interval is auto-adjusted according to the time needed for the widget to process the amount of text it is about to print every time.

I think this will deal much better with the case of varying response times per computer architecture and output size.

@minrk
Copy link
Member

minrk commented Jul 16, 2013

If this gets a rebase, I think we can merge soon.

The output from the kernel is now clipped to last `buffer_size`
before displaying and a timer is used to flush the pending output
text instead of attempting to display text on every stream
output from kernel. The timer interval is adjusted based on
actual time taken to append a screenful of text to widget.
This throttles the widget repaints and avoids choking the Qt
event loop leaving time to handle other Qt events.

Test cases:
  In [1]: for i in xrange(1000000): print i
  In [2]: range(100000)

Without this commit the first input causes the qtconsole frontend
to freeze, not responding to `Ctrl+C`.
@pankajp
Copy link
Contributor Author

pankajp commented Jul 18, 2013

@ccordoba12 Thanks for testing, I have moved the timer based approach here, and rebased on top of ipython master.
@minrk I hope that is what you meant.

@ccordoba12
Copy link
Member

A big +1 from me. @minrk it's very important for us at Spyder (and I imagine Enthought guys too) to have this in 1.0, because right now long or infinite print's block an app embedding qtconsole completely, and the user needs to hard kill it.

@minrk
Copy link
Member

minrk commented Jul 18, 2013

thanks, will give it a look over and hopefully merge later today.

@minrk
Copy link
Member

minrk commented Jul 18, 2013

Works in my tests. I'll defer to @ccordoba12 as you guys use the QtConsole much more aggressively than most.

Merging.

minrk added a commit that referenced this pull request Jul 18, 2013
Prevent qtconsole frontend freeze on lots of output.

The output from the kernel is now clipped to last buffer_size
before displaying and a timer is used to flush the pending output
text instead of attempting to display text on every stream
output from kernel. The timer interval is adjusted based on
actual time taken to append a screenful of text to widget.
This throttles the widget repaints and avoids choking the Qt
event loop leaving time to handle other Qt events.
@minrk minrk merged commit 0397c02 into ipython:master Jul 18, 2013
@ccordoba12
Copy link
Member

Thanks!

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
…put-clip

Prevent qtconsole frontend freeze on lots of output.

The output from the kernel is now clipped to last buffer_size
before displaying and a timer is used to flush the pending output
text instead of attempting to display text on every stream
output from kernel. The timer interval is adjusted based on
actual time taken to append a screenful of text to widget.
This throttles the widget repaints and avoids choking the Qt
event loop leaving time to handle other Qt events.
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