Skip to content

Commit 7a9a537

Browse files
committed
Remove respond_to? in assign_attribute happy path
Kernel#respond_to? is generally slow, and so it should be avoided especially in hot loops. In this case, assign_attributes ends up calling respond_to? for each attribute being assigned. The purpose of the check here is to raise UnknownAttributeError when the attribute setter method doesn't exist. This commit moves the respond_to? inside a rescue. For a single attribute being assigned, the performance is about the same. However, the performance is ~10% better for 10 attributes being assigned and ~30% better for 100 attributes. There is a tradeoff here since using rescue for control flow is generally not good for performance, however in this case the ~10% decrease in performance only applies to the exceptional case when attempting to assign an unknown attribute. This also partially reverts a225d4b. The commit message doesn't have much info so it's unclear to me why this was changed. Benchmark: ``` require "active_record" require "benchmark/ips" require "logger" ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:") ActiveRecord::Base.logger = Logger.new(STDOUT) ActiveRecord::Schema.define do create_table :posts, force: true do |t| 100.times do |i| t.text :"body_#{i}" end end end class Post < ActiveRecord::Base end one_attribute = { "body_1" => "1" } ten_attributes = {} hundered_attributes = {} invalid_attribute = { "body__1" => "1" } 100.times do |i| ten_attributes["body_#{i}"] = i.to_s if i < 11 hundered_attributes["body_#{i}"] = i.to_s end Benchmark.ips do |x| x.report("1 attribute") { Post.new(one_attribute) } x.report("10 attribute") { Post.new(ten_attributes) } x.report("100 attribute") { Post.new(hundered_attributes) } x.report("invalid attribute") do Post.new(invalid_attribute) rescue ActiveModel::UnknownAttributeError end end ``` Before: ``` Warming up -------------------------------------- 1 attribute 1.090k i/100ms 10 attribute 805.000 i/100ms 100 attribute 251.000 i/100ms invalid attribute 966.000 i/100ms Calculating ------------------------------------- 1 attribute 10.882k (± 1.3%) i/s - 54.500k in 5.009085s 10 attribute 8.070k (± 1.2%) i/s - 41.055k in 5.088110s 100 attribute 2.548k (± 1.6%) i/s - 12.801k in 5.026321s invalid attribute 9.687k (± 2.5%) i/s - 49.266k in 5.089246s ``` After: ``` Warming up -------------------------------------- 1 attribute 1.098k i/100ms 10 attribute 884.000 i/100ms 100 attribute 341.000 i/100ms invalid attribute 868.000 i/100ms Calculating ------------------------------------- 1 attribute 11.004k (± 1.1%) i/s - 55.998k in 5.089635s 10 attribute 8.817k (± 1.8%) i/s - 44.200k in 5.014699s 100 attribute 3.410k (± 0.4%) i/s - 17.391k in 5.100166s invalid attribute 8.695k (± 0.9%) i/s - 44.268k in 5.091846s ```
1 parent ba7204e commit 7a9a537

File tree

1 file changed

+3
-1
lines changed

1 file changed

+3
-1
lines changed

activemodel/lib/active_model/attribute_assignment.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,10 @@ def _assign_attributes(attributes)
4545

4646
def _assign_attribute(k, v)
4747
setter = :"#{k}="
48+
public_send(setter, v)
49+
rescue NoMethodError
4850
if respond_to?(setter)
49-
public_send(setter, v)
51+
raise
5052
else
5153
raise UnknownAttributeError.new(self, k.to_s)
5254
end

0 commit comments

Comments
 (0)