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

Optimize select_value, select_values, select_rows in Postgresql adapter. #14790

Merged
merged 1 commit into from Apr 18, 2014

Conversation

@krisselden
Copy link
Contributor

krisselden commented Apr 17, 2014

Reduces creating unused objects in select_value, select_values, and select_rows. The most dramatic reduction is in select_values which used to map(&:first) an array of single element arrays.

Dry up checking whether to exec with cache for and clear the result.

… whether to exec with cache for Postgresql adapter

Reduces creating unused objects, with the most dramatic reduction in select_values which used to map(&:first) an array of single element arrays.
@rafaelfranca

This comment has been minimized.

Copy link
Member

rafaelfranca commented Apr 17, 2014

Cool!!!!!!!!!!!!

Do you have any number to show how much it was improved?

@krisselden

This comment has been minimized.

Copy link
Contributor Author

krisselden commented Apr 18, 2014

It is more about not creating needless garbage. The performance is always better because it is doing the same thing without steps that weren't being used, but how much greatly depends on the query.

In select_rows, the performance is the same because most the overhead is in the pg gem and postgres. The new version removes the construction of the ActiveRecord::Result instance and just returns result.values. The old version created ActiveRecord::Result with result.values and returned .rows (which is result.values) thus immediately throwing out the ActiveRecord::Result and returning the argument it passed into the constructor. But removing one constructor isn't going to be noticed performance wise compared with building a decent sized result set.

In select_value, it is also avoiding creating the ActiveRecord::Result and also values array, and just reading the first value of the first tuple directly off the PG::Result, modest gain of around 8% ips (before: 4729.1 (±3.1%) i/s after: 5139.9 (±3.3%) i/s) on a fast query (select value by PK), but again less garbage.

In select_values, it was doing ActiveRecord::Result.new(...).rows.map(&:first) now it just using the column_values of PG::Result which is a decent gain of about 18% on a select of ids (before 2085.1 (±4.3%) i/s after 2476.3 (±4.2%) i/s) on a smallish table (I did 365 rows). The more rows you have the more arrays it used to create.

To summarize, the biggest performance gain is select_values, it was also the most wasteful GC wise, but the performance benefit is dependent on the sql query. These methods were fast, now they are a little faster, but garbage tends to add up, so why create it when you don't need to?

And lastly, I DRYed up this pattern that was being repeated and I didn't want to add more repeats of it:

def execute_and_clear(sql, name, binds)
  result = without_prepared_statement?(binds) ? exec_no_cache(sql, name, binds) :
                                                exec_cache(sql, name, binds)
  ret = yield result
  result.clear
  ret
end
@rafaelfranca

This comment has been minimized.

Copy link
Member

rafaelfranca commented Apr 18, 2014

Thank you so much for you detailed explanation. I asked more about number just for future reference.

Your patch is great, I'm merging.

Many thanks again.

rafaelfranca added a commit that referenced this pull request Apr 18, 2014
Optimize select_value, select_values, select_rows in Postgresql adapter.
@rafaelfranca rafaelfranca merged commit 1aeb5e7 into rails:master Apr 18, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
@joevandyk

This comment has been minimized.

Copy link
Contributor

joevandyk commented Jun 6, 2014

I've been noticing some problems with leaking memory when creating lots of AR objects in a loop. Would this fix that problem? Using Rails 4.0.5 on Ruby 2.1 if it matters.

Basically, I'm doing something like: 250000.times { SomeModel.create!(options) } and I see memory go up and up and up, past the gigabyte mark.

This is what ObjectSpace.dump_all reports:

5145 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/core.rb:171:HASH
5145 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/sequel-3.48.0/lib/sequel/adapters/postgres.rb:598:STRING
5146 /mnt/data/tanga/current/shared/gems/promotion_generator/lib/promotion_generator.rb:17:STRING
5146 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activesupport-4.0.5/lib/active_support/hash_with_indifferent_access.rb:81:HASH
5220 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/attribute_methods.rb:50:DATA
5296 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/arel-4.0.2/lib/arel/table.rb:18:ARRAY
10001 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/connection_adapters/postgresql/database_statements.rb:148:OBJECT
10290 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/connection_adapters/column.rb:252:DATA
11000 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/persistence.rb:52:OBJECT
11000 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/result.rb:66:HASH
11147 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/core.rb:443:HASH
11290 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/persistence.rb:51:HASH
16000 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/core.rb:440:HASH
16000 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/core.rb:441:HASH
16000 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/core.rb:442:HASH
16000 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/core.rb:444:HASH
16000 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/core.rb:451:HASH
16000 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/core.rb:453:ARRAY
40000 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/autosave_association.rb:152:ARRAY
55000 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activesupport-4.0.5/lib/active_support/hash_with_indifferent_access.rb:261:ARRAY
130002 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/result.rb:60:STRING
130441 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/activerecord-4.0.5/lib/active_record/connection_adapters/postgresql/database_statements.rb:146:STRING
257117 /usr/local/stow/ruby-2.1.1-gh-2014.3.18/lib/ruby/gems/2.1.0/gems/pg-0.17.1/lib/pg/result.rb:10:STRING
@thedarkone

This comment has been minimized.

Copy link
Contributor

thedarkone commented Jun 6, 2014

@joevandyk this PR was not about fixing a memory leak!

If you experience memory leaks with ActiveRecord you should try to reproduce your issue with an self-contained script, see: Rails Guides and open a new issue here on GitHub.

Otherwise: is 250000.times { SomeModel.create!(options) } in a transaction and does your model have transaction callbacks (after_rollback, after_commit)? If so, you have to remove them, otherwise AR has to hold on onto your records until the current transaction commits or aborts (it is impossible to implement after_rollback or after_commit callbacks otherwise).

@joevandyk

This comment has been minimized.

Copy link
Contributor

joevandyk commented Jun 6, 2014

#15549 shows what I'm seeing. I asked on this ticket because the code creating the retained objects were touched by this code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.