Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Allocate the "ruby" and "\0" Strings once. #3997

Merged
merged 1 commit into from Sep 24, 2015

Conversation

jrafanie
Copy link
Contributor

Index#search is called many times so this change is worth it.

With a single gem in your Gemfile, this change drops the string
allocations by nearly 30,000.

Script:

require 'allocation_tracer'

ObjectSpace::AllocationTracer.setup(%i{path line type})

result = ObjectSpace::AllocationTracer.trace do
  begin
    require 'bundler/inline'
  rescue LoadError => e
    $stderr.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
    raise e
  end

  gemfile(true) do
    source 'https://rubygems.org'
    gem 'benchmark-ips'
  end
end

result.sort_by{|k, v| k}.each{|k, v|
  puts ([v[0]]+k).join("\t")
}

String allocations before:

$ruby benchmark_allocations.rb | grep T_STRING | sort -nr | head -n 10
65461 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb  54  T_STRING
64982 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  163 T_STRING
29986 /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 71  T_STRING
22142 /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/spec_set.rb  111 T_STRING
20457 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  225 T_STRING
5280  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 87  T_STRING
4848  /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/requirement.rb  92  T_STRING
4708  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/lockfile_parser.rb 54  T_STRING
4608  /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  282 T_STRING
4561  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 19  T_STRING

String allocations after:

$ ruby benchmark_allocations.rb | grep T_STRING | sort -nr | head -n 10
65479 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb  54  T_STRING
64143 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  163 T_STRING
22142 /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/spec_set.rb  111 T_STRING
20407 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  225 T_STRING
5280  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 90  T_STRING
4916  /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/requirement.rb  92  T_STRING
4730  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/lockfile_parser.rb 54  T_STRING
4608  /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  282 T_STRING
4561  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 22  T_STRING
3334  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/spec_set.rb  134 T_STRING

@@ -13,6 +13,9 @@ def self.build
attr_reader :specs, :all_specs, :sources
protected :specs, :all_specs

RUBY = "ruby"
Copy link
Member

Choose a reason for hiding this comment

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

Instead of a constant, could we just freeze the string literals?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes @segiddins, thanks for the review!

We can do that but only ruby 2.1, 2.2+ will benefit since they're the only rubies that will use the same single shared frozen string: http://tmm1.net/ruby21-fstrings/

The constant way will work with older and newer rubies AFAIK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no attachment to using constants. I thought bundler favored backward compatible changes and in this case, I didn't think the "new shiny" way was that much better to warrant excluding ruby 2.0 and below from the "fun". 😉

@segiddins
Copy link
Member

Ah, had no idea the interned frozen strings were new! In that case, frozen constants sounds good :)

Index#search is called many times so this change is worth it.

With a single gem in your Gemfile, this change drops the string
allocations by nearly 30,000.

Script:

```ruby
require 'allocation_tracer'

ObjectSpace::AllocationTracer.setup(%i{path line type})

result = ObjectSpace::AllocationTracer.trace do
  begin
    require 'bundler/inline'
  rescue LoadError => e
    $stderr.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
    raise e
  end

  gemfile(true) do
    source 'https://rubygems.org'
    gem 'benchmark-ips'
  end
end

result.sort_by{|k, v| k}.each{|k, v|
  puts ([v[0]]+k).join("\t")
}
```

**String allocations before**:

```
$ruby benchmark_allocations.rb | grep T_STRING | sort -nr | head -n 10
65461 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb  54  T_STRING
64982 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  163 T_STRING
29986 /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 71  T_STRING
22142 /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/spec_set.rb  111 T_STRING
20457 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  225 T_STRING
5280  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 87  T_STRING
4848  /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/requirement.rb  92  T_STRING
4708  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/lockfile_parser.rb 54  T_STRING
4608  /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  282 T_STRING
4561  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 19  T_STRING
```

**String allocations after**:

```
$ ruby benchmark_allocations.rb | grep T_STRING | sort -nr | head -n 10
65479 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb  54  T_STRING
64143 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  163 T_STRING
22142 /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/spec_set.rb  111 T_STRING
20407 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  225 T_STRING
5280  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 90  T_STRING
4916  /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/requirement.rb  92  T_STRING
4730  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/lockfile_parser.rb 54  T_STRING
4608  /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  282 T_STRING
4561  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 22  T_STRING
3334  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/spec_set.rb  134 T_STRING
```
@jrafanie
Copy link
Contributor Author

Thanks @segiddins, updated.

@segiddins
Copy link
Member

@homu r+

@homu
Copy link
Contributor

homu commented Sep 24, 2015

📌 Commit e20d286 has been approved by segiddins

@homu
Copy link
Contributor

homu commented Sep 24, 2015

⌛ Testing commit e20d286 with merge b0eab63...

homu added a commit that referenced this pull request Sep 24, 2015
Allocate the "ruby" and "\0" Strings once.

Index#search is called many times so this change is worth it.

With a single gem in your Gemfile, this change drops the string
allocations by nearly 30,000.

Script:

```ruby
require 'allocation_tracer'

ObjectSpace::AllocationTracer.setup(%i{path line type})

result = ObjectSpace::AllocationTracer.trace do
  begin
    require 'bundler/inline'
  rescue LoadError => e
    $stderr.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
    raise e
  end

  gemfile(true) do
    source 'https://rubygems.org'
    gem 'benchmark-ips'
  end
end

result.sort_by{|k, v| k}.each{|k, v|
  puts ([v[0]]+k).join("\t")
}
```

**String allocations before**:

```
$ruby benchmark_allocations.rb | grep T_STRING | sort -nr | head -n 10
65461 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb  54  T_STRING
64982 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  163 T_STRING
29986 /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 71  T_STRING
22142 /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/spec_set.rb  111 T_STRING
20457 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  225 T_STRING
5280  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 87  T_STRING
4848  /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/requirement.rb  92  T_STRING
4708  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/lockfile_parser.rb 54  T_STRING
4608  /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  282 T_STRING
4561  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 19  T_STRING
```

**String allocations after**:

```
$ ruby benchmark_allocations.rb | grep T_STRING | sort -nr | head -n 10
65479 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb  54  T_STRING
64143 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  163 T_STRING
22142 /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/spec_set.rb  111 T_STRING
20407 /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  225 T_STRING
5280  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 90  T_STRING
4916  /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/requirement.rb  92  T_STRING
4730  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/lockfile_parser.rb 54  T_STRING
4608  /Users/joerafaniello/.rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/version.rb  282 T_STRING
4561  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/index.rb 22  T_STRING
3334  /Users/joerafaniello/.gem/ruby/2.2.3/gems/bundler-1.10.6/lib/bundler/spec_set.rb  134 T_STRING
```
@homu
Copy link
Contributor

homu commented Sep 24, 2015

☀️ Test successful - status

@homu homu merged commit e20d286 into rubygems:master Sep 24, 2015
@coilysiren coilysiren modified the milestone: Release Archive Oct 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants