Skip to content

bpo-46740: speedup telnetlib's buffer transfers #31449

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

Closed

Conversation

martinkirch
Copy link

@martinkirch martinkirch commented Feb 20, 2022

This patch leverages the very low frequency of IAC (control) characters: instead of reading/interpreting/appending per character, it's much faster to search them linearly (if any), then copy the data (non-IAC) slice to the cooked queue. rawq index handling in _next_nonIAC_slice is almost copied from rawq_getchar.

Also calls socket.recv with the currently recommended buffer size, 4096.

Patching process_rawq patching was inspired by point 2) in this very old Python-dev topic
https://python-dev.python.narkive.com/A86fs3PG/patch-to-telnetlib-py
and yields a 5x speedup too.

https://bugs.python.org/issue46740

This change leverages the very low frequency of IAC (control) characters:
instead of reading/interpreting/appending per character, it's much faster to
search them linearly (if any), then copy the data slice to the cooked queue.
This was inspired by point 2) in this very old Python-dev topic
https://python-dev.python.narkive.com/A86fs3PG/patch-to-telnetlib-py
and yields a 5x speedup too.

Also calling `socket.recv` with the currently recommended buffer size, 4096.
else:
self.iacseq += c
c = self.rawq_getchar()
if c == theNULL:
Copy link
Member

Choose a reason for hiding this comment

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

Can theNull be replaced with b'\0'?

Copy link
Author

Choose a reason for hiding this comment

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

I think theNull should be replaced by NOOPT (they're equal), but it's another topic. This comparison comes from the initial implementation, I've only reindented.

@AlexWaygood AlexWaygood added the performance Performance or resource usage label Feb 21, 2022
@hugovk
Copy link
Member

hugovk commented Apr 12, 2022

Note the telnetlib module is deprecated in 3.11 and set for removal in 3.13.

See PEP 594 – Removing dead batteries from the standard library and #91217.

@martinkirch martinkirch mannequin mentioned this pull request Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review performance Performance or resource usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants