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

ActiveRecord initialization optimizations #29215

Merged
merged 4 commits into from May 25, 2017

Conversation

Projects
None yet
3 participants
@lovitt
Contributor

lovitt commented May 24, 2017

Summary

ActiveRecord model initialization got a lot slower between Rails 4.2 and Rails 5 — 2-5x slower or more, depending on the specific circumstances.

This PR includes optimizations that speed up model initialization by approximately 2x for STI models and 1.3x for non-STI models, as compared to vanilla 5.1, returning performance somewhat closer (but by no means entirely) to 4.2 levels.

We have been running these optimizations on our high-traffic Rails 5.0.2 app in production for the last few days without issue. We are seeing an average response time speedup of just over 4%, compared to vanilla Rails 5.0.2, with certain actions showing speedup of 5-15%. (Our app depends heavily on STI and uses a caching scheme that results in many model initializations even on cache hits; YMMV.)

Other Information

Benchmarks

Highlights:

================ Instantiate STI base class without attributes =================

      v5.1.1 patched:    20641.2 i/s
              v5.1.1:     9916.0 i/s - 2.08x  slower

================= Instantiate Non-STI class without attributes =================

      v5.1.1 patched:    27071.0 i/s
              v5.1.1:    20723.3 i/s - 1.31x  slower

Details:

Running the benchmarks

These benchmarks were produced by this benchmarking script using Ruby 2.3.3 on OS X. To run them yourself, clone the gist, then:

# benchmark Rails 4.2.8 vanilla only
RAILS_VERSION=4.2.8 ruby ar_init_optimizations.rb  

# compare Rails 5.1.1 vanilla to Rails 5.1.1 patched
RAILS_VERSION=5.1.1 ruby ar_init_optimizations.rb

Potential future work

With these patches in place, the remaining ActiveRecord initialization slowness introduced in Rails 5.0 seems to be related to changes in ActiveRecord::Core#initialize — in particular, in calls to AttributeSet#deep_dup and Scoping#initialize_internals_callback.

I'm no expert in this area of the Rails codebase, and I could not find a relevant PR that describes the rationale behind these changes, but I think any further optimizations would be much more involved than the straightforward changes made here.


Credit to @jamiemccarthy and @thenonsequitur for the investigative and profiling work behind this PR.

@maclover7

This comment has been minimized.

Member

maclover7 commented May 24, 2017

If there are other possible performance slowdowns, feel free to open up an issue to identify them, and then we can open them up to community contributions :)

lovitt added some commits May 18, 2017

Performance optimization for AttributeSet#deep_dup
Skip the call to #dup, since it does a shallow copy of attributes,
which is wasted effort, since #deep_dup then replaces that
shallow copy with a #deep_dup of the given attributes.

This change addresses slowness in ActiveRecord initialization
introduced starting in Rails 5.0.
Performance optimization for ActiveRecord#subclass_from_attributes
This change addresses slowness in ActiveRecord initialization
introduced starting in Rails 5.0.
Performance optimization for ActiveRecord#column_defaults
Memoize the #column_defaults class property, as ActiveRecord does
for other properties in this module.

This change addresses slowness in ActiveRecord initialization
introduced starting in Rails 5.0. This method's performance has not
changed with Rails 5, but it is now called much more frequently than
before: every time an STI model is instantiated.

@lovitt lovitt force-pushed the voxmedia:ar_optimizations branch to 63dd12b May 24, 2017

@@ -217,7 +217,7 @@ def type_condition(table = arel_table)
def subclass_from_attributes(attrs)
attrs = attrs.to_h if attrs.respond_to?(:permitted?)
if attrs.is_a?(Hash)
subclass_name = attrs.with_indifferent_access[inheritance_column]
subclass_name = attrs[inheritance_column] || attrs[inheritance_column.to_sym]

This comment has been minimized.

@matthewd

matthewd May 25, 2017

Member

Is inheritance_column guaranteed to be a string here?

This comment has been minimized.

@lovitt

lovitt May 25, 2017

Contributor

Yes, the default value of inheritance_column is "type", and when the default is overridden, the setter ensures the custom value is a string.

dup.tap do |copy|
copy.instance_variable_set(:@attributes, attributes.deep_dup)
end
self.class.new(attributes.deep_dup)

This comment has been minimized.

@matthewd

matthewd May 25, 2017

Member

This is avoiding the wasted shallow dup in initialize_dup below, right?

Does dup.tap do |copy| copy.attributes.transform_values!(&:deep_dup) end bench similarly?

My only hesitation about using new is that while it currently saves an allocation, it seems more likely to become the costlier of the two at some point in the future.

This comment has been minimized.

@lovitt

lovitt May 25, 2017

Contributor

Yes, this change is to avoid the wasted shallow dup in initialize_dup.

I think using Class#allocate may be the best approach:

def deep_dup
  self.class.allocate.tap do |copy|
    copy.instance_variable_set(:@attributes, attributes.deep_dup)
  end
end

That seems to do exactly what we want: create a new instance of AttributeSet without calling initialize, so that we can initialize @attributes (and any future instance variables) ourselves within deep_dup and avoid wasted shallow duplication. (This is effectively what Object#dup does under the hood.)

As you would expect, this variation provides the same speedup as the current optimization.

This comment has been minimized.

@lovitt

lovitt May 25, 2017

Contributor

@matthewd I went ahead and pushed that change. Let me know what you think.

Make #deep_dup use #allocate instead of #new
This change preserves the speedup made in a24912c (by avoiding
the wasted shallow dup of @attributes) while ensuring that the
performance of #deep_dup won't be tied to the performance of #initialize

@matthewd matthewd merged commit 289cd37 into rails:master May 25, 2017

2 checks passed

codeclimate no new or fixed issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@matthewd

This comment has been minimized.

Member

matthewd commented May 25, 2017

🚀

@lovitt lovitt deleted the voxmedia:ar_optimizations branch May 30, 2017

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