-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
GH-130328: pasting in new REPL is slow on Windows #132884
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
Conversation
Good analysis and speedup! 👍 |
if not events.value: | ||
return None | ||
if not block and not self.wait(timeout=0): | ||
return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now we are nicely in sync with unix_console.py
cpython/Lib/_pyrepl/unix_console.py
Lines 400 to 401 in e1c09ff
if not block and not self.wait(timeout=0): | |
return None |
We generally don't backport perf fixes unless they lead to a denial-of-service problem (so usually like an exponential-time algorithm), or result in a serious bug. So I'm removing the backport label. |
On second thought, "slow" is perhaps an understatement. So once this is merged, let's ping Thomas to ask for his permission to backport to 3.13 |
Yeah, in case of the legacy console pasting of the 30 + 3 lines given by the OP takes 15 seconds, and this brings it down to 1.8. Most people will hopefully use a virtual terminal, where the situation is much less severe. After all, the new REPL can be disabled using https://docs.python.org/3/using/cmdline.html#envvar-PYTHON_BASIC_REPL. 3 more lines added by me for easier benchmarking #130328 (comment)
"test value" given in the OP
"""1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
|
If @ambv has not checked this this in a couple of days ping me and I will do a pass and land it. Thanks a lot for looking into this @chris-eibl ❤️ |
in class Reader
This reverts commit bba26c9.
Thanks for the fix, Chris! |
@Yhg1s seeking permission to backport this to 3.13. According to Chris, it takes 15 seconds to paste 58 lines in the new repl in 3.13. Do you think that's slow enough to be worth backporting? |
The reason why pasting is so slow on Windows (especially in the legacy console case where virtual terminal mode - and thus bracketed paste - is disabled):
cpython/Lib/_pyrepl/reader.py
Lines 702 to 706 in 9f5994b
and
cpython/Lib/_pyrepl/windows_console.py
Lines 523 to 532 in 9f5994b
It is interesting, that
msvcrt.kbhit()
returns True in case of pasting, but if that weren't the case, we'd hit the 100ms timeout and would be even way slower. However,msvcrt.kbhit()
is very slow in case of the legacy console, and it is called at least twice as often, because we get a key up and a key down event - but only a key down event in the virtual terminal case. If the pasted input contains upper case letters, we additionally get a "shift key pressed" event - so in the worst case,msvcrt.kbhit()
gets called 3 times more often (and each call is slower).Output of
python.bat -m cProfile -m _pyrepl
when pasting the "test value" given in the OP for a legacy console:Here the relevant line in case of virtual terminal:
The fix is to use WaitForSingleObject
which speeds up especially the legacy console (times in seconds):
Note, see also #132440 (comment):
cmd.exe
. Timings gotten by pastingcommands.py
: