Repository navigation
Conversation
| TOP_LEVEL_DOMAINS_SET = Set.new(TOP_LEVEL_DOMAINS).freeze | ||
|
|
||
| def initialize(content) | ||
| matching_tld = TOP_LEVEL_DOMAINS.any? { |d| content.upcase.split('.').last == d } |
There was a problem hiding this comment.
Probably not the main issue behind the OOM crashes we are having but still has a big memory footprint because of content.upcase.split('.').last returning new strings in the loop
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce per-call allocations/memory usage in RailsValues::PublicDomainSuffix#initialize, and includes a gem version bump consistent with the release.
Changes:
- Bump gem version from
2.0.8to2.0.9. - Optimize TLD validation in
PublicDomainSuffix#initializeby precomputing the last domain part once and using aSetfor membership checks. - Reduce allocations in
EmailAddressList#to_sby joining the list directly (avoiding an intermediatemap(&:to_s)array).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rails_values.gemspec | Bumps gem version to 2.0.9. |
| lib/rails_values/public_domain_suffix.rb | Introduces a Set for faster/leaner TLD membership checks during initialization. |
| lib/rails_values/email_address_list.rb | Simplifies to_s to avoid creating a temporary array of strings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| TOP_LEVEL_DOMAINS_SET = Set.new(TOP_LEVEL_DOMAINS).freeze | ||
|
|
There was a problem hiding this comment.
Set is referenced here but this file doesn’t require the Ruby stdlib set. In environments where Set hasn’t already been loaded, this will raise NameError: uninitialized constant Set. Add require 'set' near the top of the file (or ensure it’s required before this class is loaded).
Summary
Reduce memory footprint for initialize method PublicDomainSuffix
Checklist
rails_values.gemspecTest plan
No change in behaviour tests already present