-
Notifications
You must be signed in to change notification settings - Fork 66
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
Use frozen string literals #144
Conversation
Below is the relevant bits from a
BeforeTotal allocated: 5_363_399_946 bytes (45_328_676 objects) allocated memory by file96815592 /Users/josh.nichols/.rbenv/versions/3.2.2/lib/ruby/3.2.0/net/http/header.rb allocated objects by file1675464 /Users/josh.nichols/.rbenv/versions/3.2.2/lib/ruby/3.2.0/net/http/header.rb Allocated String Report
AfterTotal allocated: 4_795_299_370 bytes (42_124_716 objects) allocated memory by file68355240 /Users/josh.nichols/workspace/net-http/lib/net/http/header.rb allocated objects by file1038079 /Users/josh.nichols/workspace/net-http/lib/net/http/header.rb Allocated String Report
|
@natematykiewicz's suggestion to avoid a string allocation Co-authored-by: Nate Matykiewicz <natematykiewicz@gmail.com>
@natematykiewicz's suggestion to avoid another string allocation Co-authored-by: Nate Matykiewicz <natematykiewicz@gmail.com>
Thank you! |
Thanks for marking me as a co-author @technicalpickles! |
I've been using memory_profiler on an extremely slow Rails system spec, and was surprised to see net/http came up in it. This particular test does an absolute ton of HTTP requests though.
I was looking through this code, and realized there's a lot of string literals used in some of the hot code paths, but
frozen_string_literal
had been explicitly set to false. There was only a few small changes after turning frozen_string_literal on to get the suite passing.I tested my app against this, and it shaved off some 23s, from 2m15s to 1m52s 😱
I'll post before/after of the memory_profiler reports shortly.