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

Cop idea: prefer String#bytesize over String#bytes#size #475

Closed
viralpraxis opened this issue Oct 14, 2024 · 3 comments
Closed

Cop idea: prefer String#bytesize over String#bytes#size #475

viralpraxis opened this issue Oct 14, 2024 · 3 comments

Comments

@viralpraxis
Copy link
Contributor

I've seen some developers using some_string.bytes.size (or count/length) instead of the constant-time String#bytesize method.

In MRI there is no copy-on-write optimization for string/array conversion, so for large strings, the unnecessary array allocation could result in performance issues

require "objspace"

s = "a" * 1_000_000
initial = ObjectSpace.count_objects_size[:T_ARRAY]
b = s.bytes.count
p (ObjectSpace.count_objects_size[:T_ARRAY] - initial) / (1 << 10)

> 7812

I've opened #474 which will be finished if the idea accepted

@dvandersluis
Copy link
Member

This makes sense as a cop to me. The link you included in the PR (https://github.com/fastruby/fast-ruby#remove-extra-spaces-or-other-contiguous-characters-code) doesn't mention bytesize though, is there another source?

@viralpraxis
Copy link
Contributor Author

I copy-pasted it from another config entry, that Reference key is obsolete in this case

viralpraxis added a commit to viralpraxis/rubocop-performance that referenced this issue Oct 22, 2024
@viralpraxis
Copy link
Contributor Author

The PR is ready for review

viralpraxis added a commit to viralpraxis/rubocop-performance that referenced this issue Oct 22, 2024
viralpraxis added a commit to viralpraxis/rubocop-performance that referenced this issue Oct 22, 2024
@koic koic closed this as completed in d3637be Nov 2, 2024
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

No branches or pull requests

2 participants