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.

Sign up for GitHub

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

Jump to bottom

Reduce allocations from just loading the gem #1104

Merged
merged 1 commit into from
May 27, 2019
Merged

Reduce allocations from just loading the gem #1104

merged 1 commit into from
May 27, 2019

Conversation

ashmaroli
Copy link
Contributor

Pathname generates a lot of intermediate strings during operations on its instances. This change swaps Pathname objects with regular String objects.

To compare with master, use below script:

# frozen_string_literal: true

require 'memory_profiler'
require 'bundler/setup'

MemoryProfiler.report(trace: [String]) do
  require 'rouge'
end.pretty_print(to_file: '.memprof', scale_bytes: true)

@dblessing
Copy link
Collaborator

Thanks @ashmaroli I like these type of PRs.

Tests for older versions of Ruby are failing - I think due to the removal of the bundler specification in the Gemfile. Can you take a look at that, please?

@ashmaroli
Copy link
Contributor Author

due to the removal of the bundler specification in the Gemfile.

That was a superfluous change actually. The tests will fail even if I add bundler back into the Gemfile. The builds fail way before the bundle install command is even invoked..

Please see #1062 for probable solution.

@ashmaroli ashmaroli force-pushed the optimize-gem-require branch from 10e9c37 to 2a55297 Compare April 22, 2019 17:28
@pyrmont pyrmont mentioned this pull request May 10, 2019
Merged
pyrmont referenced this pull request in pyrmont/rouge May 10, 2019
@pyrmont
Reduce allocations from just loading the gem (#3)
c746dd2 
Originally jneen#1104.
@ashmaroli ashmaroli force-pushed the optimize-gem-require branch from 2a55297 to dd2f734 Compare May 27, 2019 16:25
@pyrmont pyrmont merged commit 6835163 into rouge-ruby:master May 27, 2019
@ashmaroli ashmaroli deleted the optimize-gem-require branch June 19, 2019 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ashmaroli @dblessing @pyrmont