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

Increase stdin chunksize from 16 to 1024 chars #771

Open
wants to merge 1 commit into
base: latestw_all
Choose a base branch
from

Conversation

lhecker
Copy link

@lhecker lhecker commented Dec 20, 2024

PR Summary

This bumps the chunk size for stdin from 16 to 1024 chars.

PR Context

It would be very unusual to read() from a file with a 16-byte buffer.
The same is true for stdin, but counter-intuitively it's actually
a lot "more true" for stdin than it would be for any file:
Since the stdin pipe is unbuffered and synchronous, the round-trip
overhead for reading from it is exorbitantly high. To put it into
perspective, on Windows the call overhead alone is roughly the same
as subsequentially reading in the order of 100K characters.

The increased buffer size has an immediately noticeable effect on
the interactive shells I've tested. Pasting content into a prompt now
appears immediately and atomically, while previously it would only
progressively arrive and get syntax highlighted repeatedly (= flicker).

Another nice benefit is that some shells have trouble determining
whether an escape character is a sequence inducer or a lone escape
character. Increasing the buffer size reduces the chance for hitting
this edge case from 1/16th (6%) to 1/1024th (0.1%) and makes it
very unlikely to occur at all for inputs <1024 characters.
This issue is what prompted me to make this modification.

@tgauth
Copy link
Collaborator

tgauth commented Dec 20, 2024

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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.

3 participants