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

Optimize IO#read_string(0) #13732

Merged

Conversation

jgaskins
Copy link
Contributor

@jgaskins jgaskins commented Aug 7, 2023

When fetching an empty string from a remote server, we can avoid a heap allocation by returning an empty string as an optimized special case.

I also checked and IO#read_fully(slice : Slice) does not need this optimization. It already short-circuits if there is nothing to read.

@jgaskins
Copy link
Contributor Author

jgaskins commented Aug 8, 2023

Looks like the tests failing in CI have been failing on previous commits on the master branch, as well. Are they just flaky?

@jgaskins jgaskins force-pushed the optimize-io-read-string-empty branch from 355ff5b to bff4ebc Compare August 8, 2023 22:26
@beta-ziliani
Copy link
Member

yup, interpreter std tests are flaky indeed.

Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

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

makes sense to me 👍

@straight-shoota straight-shoota added this to the 1.10.0 milestone Aug 16, 2023
@straight-shoota straight-shoota merged commit bfc7e20 into crystal-lang:master Aug 18, 2023
50 of 53 checks passed
@jgaskins jgaskins deleted the optimize-io-read-string-empty branch August 19, 2023 03:46
Blacksmoke16 pushed a commit to Blacksmoke16/crystal that referenced this pull request Dec 11, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants