Skip to content

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

Closed
martinkirch wants to merge 2 commits into
python:mainfrom
martinkirch:46740-faster-telnet-buffers
Closed

bpo-46740: speedup telnetlib's buffer transfers#31449
martinkirch wants to merge 2 commits into
python:mainfrom
martinkirch:46740-faster-telnet-buffers

Conversation

@martinkirch

@martinkirch martinkirch commented Feb 20, 2022

Copy link
Copy Markdown

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.
Comment thread Lib/telnetlib.py
else:
self.iacseq += c
c = self.rawq_getchar()
if c == theNULL:

Copy link
Copy Markdown
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
Copy Markdown
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

hugovk commented Apr 12, 2022

Copy link
Copy Markdown
Member

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