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

Skip _select! call unless needed for preloader #21033

Merged
merged 1 commit into from Oct 5, 2015

Conversation

Projects
None yet
3 participants
@dgynn
Contributor

dgynn commented Jul 26, 2015

This PR skips the _select! call during build_scope for preloads unless it is needed. The performance gain depends on the number of columns in the preload table. For a simple preload with ~20 columns the _select! call accounts for 40% of the time (of build_scope) and isn't needed. It also allocates ~180 strings.

The root of the issue is that _select! is being called with table[Arel.*] which is an Arel::Node::As and not just a string or symbol. When _select! attempts to do attribute_alias lookups it ends up calling inspect on the Node to convert it to a string. And that causes the model object to be inspected as well. This happens twice since _select! is called again during the scope merge.

Here is a benchmark comparison of doing a query that involves a preload. It does not completely isolate the build_scope method but does show an overall performance gain and reduction in objects created.

Before

Calculating -------------------------------------
posts with preload(:comments)
                       208.000  i/100ms
-------------------------------------------------
posts with preload(:comments)
                          2.047k (± 1.6%) i/s -     10.400k
Total objects allocated: 697

After

Calculating -------------------------------------
posts with preload(:comments)
                       256.000  i/100ms
-------------------------------------------------
posts with preload(:comments)
                          2.524k (± 1.9%) i/s -     12.800k
Total objects allocated: 516

Benchmark script

#!/usr/bin/env ruby

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 'rails', path: '.' # github: 'rails/rails', :branch => 'master'
  gem 'arel', github: 'rails/arel', :branch => 'master'
  gem 'sqlite3'
  gem 'memory_profiler'
  gem 'benchmark-ips'
end

require 'active_record'

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')

ActiveRecord::Schema.define do
  create_table :posts, force: true  do |t|
    t.string  :post_col1
  end

  create_table :comments, force: true  do |t|
    t.integer :post_id
    20.times do |n|
      t.string  "comment_col#{n}"
    end
  end
end

class Post < ActiveRecord::Base
  has_many :comments
end

class Comment < ActiveRecord::Base
  belongs_to :post
end

# Create single Post with no Comments
Post.create!
# Query once so everything is loaded
Post.all.preload(:comments).to_a

GC.disable
Benchmark.ips do |x|
  x.report('posts with preload(:comments)') { Post.all.preload(:comments).to_a }
end
GC.enable

results = MemoryProfiler.report { Post.all.preload(:comments).to_a }

puts "Total objects allocated: #{results.total_allocated}"
skip _select! call unless :select values are specified
the default scope will select all fields. removing this
improves performance and reduces String creation.

arthurnn pushed a commit that referenced this pull request Oct 5, 2015

Arthur Nogueira Neves
Merge pull request #21033 from dgynn/preloader_build_scope_tuning
Skip _select! call unless needed for preloader

@arthurnn arthurnn merged commit f854be5 into rails:master Oct 5, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@arthurnn

This comment has been minimized.

Member

arthurnn commented Oct 5, 2015

Looking good..
thanks! ❤️

@igorkasyanchuk

This comment has been minimized.

Contributor

igorkasyanchuk commented Oct 12, 2015

Please check my https://github.com/igorkasyanchuk/benchmark_methods has some connection to profiling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment