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

Extract UNSAFE regex in utilities uri to reduce memory usage #1706

Merged
merged 1 commit into from Aug 18, 2015

Conversation

simonprev
Copy link
Contributor

While benchmarking my app (using derailed), I noticed a an unusual memory usage in the Carrierwave::Utilities::Uri module. The action benchmarked is serializing 60 records each containing a field with an uploader that has 3 versions. The memory usage is not critical and this is not a huge gain in performance so merge it if you think the code respect the guideline of the repository 😄

Here is my benchmarks results:

allocated memory by gem

Before

2399180 carrierwave/lib
1470695 activesupport-4.2.0
1444700 activerecord-4.2.0

After

1708780 carrierwave/lib
1470695 activesupport-4.2.0
1444700 activerecord-4.2.0

allocated memory by file

Before

989475 /Users/simonprev/.rbenv/versions/2.2.2/.../active_support/json/encoding.rb
817070 /Users/simonprev/Code/carrierwave/lib/carrierwave/utilities/uri.rb <-- This is unusual
509070 /Users/simonprev/Code/carrierwave/lib/carrierwave/uploader/url.rb
507460 /Users/simonprev/.rbenv/versions/2.2.2/.../json/common.rb

After
989475 /Users/simonprev/.rbenv/versions/2.2.2/.../active_support/json/encoding.rb
509070 /Users/simonprev/Code/carrierwave/lib/carrierwave/uploader/url.rb
507460 /Users/simonprev/.rbenv/versions/2.2.2/.../json/common.rb
...
126670 /Users/simonprev/Code/carrierwave/lib/carrierwave/utilities/uri.rb <-- Ahh

bensie added a commit that referenced this pull request Aug 18, 2015
Extract UNSAFE regex in utilities uri to reduce memory usage
@bensie bensie merged commit 8116f01 into carrierwaveuploader:master Aug 18, 2015
@bensie
Copy link
Member

bensie commented Aug 18, 2015

Thanks!

@bensheldon
Copy link

@simonprev just wanted to send my thanks because I noticed this exact issue too when trying to track down profligate memory usage! 👍

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

3 participants