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

Minor cleanup and simplification of readString #376

Merged
merged 2 commits into from
Aug 23, 2024
Merged

Conversation

qwwdfsad
Copy link
Collaborator

  • Request Long.MAX_VALUE from the source at once (note: no need for a loop as we are unable to build the string of such size anyway)
  • Get rid of redundant checks in commonReadUtf8

Ticks one of the boxes in #342

@qwwdfsad qwwdfsad requested a review from fzhinkin August 21, 2024 12:19
Copy link
Collaborator

@fzhinkin fzhinkin left a comment

Choose a reason for hiding this comment

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

@qwwdfsad thanks for the change! Everything looks good in general, I have only a few nits regarding the benchmark.

* Request Long.MAX_VALUE from the source at once (note: no need for a loop as we are unable to build the string of such size anyway)
* Get rid of redundant checks in commonReadUtf8

Ticks one of the boxes in #342
@fzhinkin fzhinkin self-requested a review August 23, 2024 14:31
Copy link
Collaborator

@fzhinkin fzhinkin left a comment

Choose a reason for hiding this comment

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

LGTM

@qwwdfsad qwwdfsad merged commit 92e0214 into develop Aug 23, 2024
@qwwdfsad qwwdfsad deleted the read-string-opto branch August 23, 2024 14:34
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.

2 participants