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

Freeze all the strings! #1649

Merged
merged 1 commit into from Oct 17, 2018
Merged

Freeze all the strings! #1649

merged 1 commit into from Oct 17, 2018

Conversation

schneems
Copy link
Contributor

Reduces runtime allocation by freezing string literals by default.

We could also remove a ton of manual .freeze calls, however the ruby supported version is 2.2 and the magic comment only targets 2.3+.

Reduces runtime allocation by freezing string literals by default.

We could also remove a ton of manual `.freeze` calls, however the ruby supported version is 2.2 and the magic comment only targets 2.3+.
@MSP-Greg
Copy link
Member

I tested the patch for this in my fork, Appveyor & Travis passed, but Travis failed on both Ruby trunk jobs, although probably not due to this PR.

@nijikon
Copy link

nijikon commented Oct 17, 2018

@schneems Support for ruby 2.2 ended on March 31, 2018. I think that we should utilize new features of the language, people who use 2.2 could still use puma but to a particular version which I think it's fine.

@schneems schneems merged commit 2668597 into master Oct 17, 2018
@tmikoss
Copy link

tmikoss commented Apr 4, 2019

After deploying version 3.12.1 in production, I'm getting regular "can't modify frozen strings" errors when trying to establish actioncable websocket connections, pointing at https://github.com/faye/websocket-driver-ruby/blob/0.7.0/lib/websocket/driver/draft76.rb#L11, without any application code in the backtrace.

Could it be linked to this? Running on ruby 2.5.3 / rails 5.2.2.1. Would a complete backtrace help?

@schneems
Copy link
Contributor Author

schneems commented Apr 5, 2019 via email

@nateberkopec nateberkopec deleted the schneems/frozen branch March 14, 2020 21:53
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.

None yet

4 participants