Improve Qt object management and performance#14700
Merged
Carreau merged 3 commits intoipython:mainfrom Feb 2, 2025
Merged
Conversation
Make the Qt timer shorter in line with the Tk one. Reduces perceived lag when the inputhook is engaged. ipython#14581
Instead or recreating an eventloop and timer 100 times a second, use a single instance throughout. ipython#14240 introduced garbage collection in the past, but we don't need to garbage collect if we reuse the objects
Contributor
Author
|
|
Member
|
Ok ,let's try it. |
Member
|
thanks ! |
meeseeksmachine
pushed a commit
to meeseeksmachine/ipython
that referenced
this pull request
Feb 2, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR tries to address perceived lag in the IPython console when the
%gui qteventloop is enabled (#14581) as well as wider management of Qt objects in said eventloop, initially addressed in #14240.The lag is resolved by setting the timer for exiting the Qt eventloop to 10ms instead of 50ms. This is consistent with the Tk implementation and removes the observable lag on Windows.
I also took this opportunity to restructure the handling of Qt objects - the old implementation created a new
QEventLoopandQTimereach time the inputhook was called. Sincecontext.input_is_ready()pretty much always returns True after 10ms in my tests, this meant creating these objects anew 20 times per second (or 100 with this proposed timer patch). #14240 adds garbage collection for theQEventLoop, but not for theQTimer(which is a feature on Windows only).I propose creating one instance of
QEventLoopandQTimerinitially, and then just reusing them. In my testing this doesn't improve performance (Qt is pretty quick at making new objects it seems), but it does avoid garbage collection issues.Note that a
QSocketNotifieris still created every iteration on POSIX platforms. I'm not sure if that is garbage collected or not. The value ofcontext.fileno()could conceivably change, so we may need to recreate it as it is done now instead of reusing. I'm not sure, so I leave it as is.Let me know if you would rather just change the timer to 10ms to fix #14581 and avoid the larger restructure in this PR. In that case we could only apply this commit: jdranczewski@e65cd11