Skip to content

Freeze strings and other constants#7638

Merged
mitchellhenke merged 8 commits intomainfrom
mitchellhenke/freezing-strings
Jan 13, 2023
Merged

Freeze strings and other constants#7638
mitchellhenke merged 8 commits intomainfrom
mitchellhenke/freezing-strings

Conversation

@mitchellhenke
Copy link
Contributor

@mitchellhenke mitchellhenke commented Jan 12, 2023

🛠 Summary of changes

This PR intends to freeze strings and other values in some of the warmer paths and also enables config.action_view.frozen_string_literal which will freeze string literals in templates. We could do # frozen_string_literal: true everywhere with Rubocop, but I am hoping to start smaller since we may eventually be able to use RUBYOPT=--enable=frozen-string-literal

Some preliminary testing suggests this patch could reduce memory allocation by 2-3% on a GET / request

@mitchellhenke mitchellhenke marked this pull request as ready for review January 12, 2023 22:41
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM! I'd be in favor of adding that comment to all files via Rubocop

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/freezing-strings branch from 6738d93 to 5ae6c59 Compare January 13, 2023 16:21
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/freezing-strings branch from c63785a to 7509865 Compare January 13, 2023 17:20
@mitchellhenke mitchellhenke merged commit 398b097 into main Jan 13, 2023
@mitchellhenke mitchellhenke deleted the mitchellhenke/freezing-strings branch January 13, 2023 17: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.

2 participants