Skip to content

Commit f45267b

Browse files
committed
ActiveRecord#respond_to? No longer allocates strings
This is an alternative to #34195 The active record `respond_to?` method needs to do two things if `super` does not say that the method exists. It has to see if the "name" being passed in represents a column in the table. If it does then it needs to pass it to `has_attribute?` to see if the key exists in the current object. The reason why this is slow is that `has_attribute?` needs a string and most (almost all) objects passed in are symbols. The only time we need to allocate a string in this method is if the column does exist in the database, and since these are a limited number of strings (since column names are a finite set) then we can pre-generate all of them and use the same string. We generate a list hash of column names and convert them to symbols, and store the value as the string name. This allows us to both check if the "name" exists as a column, but also provides us with a string object we can use for the `has_attribute?` call. I then ran the test suite and found there was only one case where we're intentionally passing in a string and changed it to a symbol. (However there are tests where we are using a string key, but they don't ship with rails). As re-written this method should never allocate unless the user passes in a string key, which is fairly uncommon with `respond_to?`. This also eliminates the need to special case every common item that might come through the method via the `case` that was originally added in f80aa59 (by me) and then with an attempt to extend in #34195. As a bonus this reduces 6,300 comparisons (in the CodeTriage app homepage) to 450 as we also no longer need to loop through the column array to check for an `include?`.
1 parent 134dab4 commit f45267b

File tree

3 files changed

+11
-12
lines changed

3 files changed

+11
-12
lines changed

activerecord/lib/active_record/attribute_methods.rb

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -261,21 +261,14 @@ def column_for_attribute(name)
261261
def respond_to?(name, include_private = false)
262262
return false unless super
263263

264-
case name
265-
when :to_partial_path
266-
name = "to_partial_path"
267-
when :to_model
268-
name = "to_model"
269-
else
270-
name = name.to_s
271-
end
272-
273264
# If the result is true then check for the select case.
274265
# For queries selecting a subset of columns, return false for unselected columns.
275266
# We check defined?(@attributes) not to issue warnings if called on objects that
276267
# have been allocated but not yet initialized.
277-
if defined?(@attributes) && self.class.column_names.include?(name)
278-
return has_attribute?(name)
268+
if defined?(@attributes)
269+
if name = self.class.symbol_column_to_string(name.to_sym)
270+
return has_attribute?(name)
271+
end
279272
end
280273

281274
true

activerecord/lib/active_record/model_schema.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,11 @@ def column_names
388388
@column_names ||= columns.map(&:name)
389389
end
390390

391+
def symbol_column_to_string(name_symbol) # :nodoc:
392+
@symbol_column_to_string_name_hash ||= column_names.index_by(&:to_sym)
393+
@symbol_column_to_string_name_hash[name_symbol]
394+
end
395+
391396
# Returns an array of column objects where the primary id, all columns ending in "_id" or "_count",
392397
# and columns used for single table inheritance have been removed.
393398
def content_columns
@@ -477,6 +482,7 @@ def load_schema!
477482
def reload_schema_from_cache
478483
@arel_table = nil
479484
@column_names = nil
485+
@symbol_column_to_string_name_hash = nil
480486
@attribute_types = nil
481487
@content_columns = nil
482488
@default_attributes = nil

activerecord/lib/active_record/nested_attributes.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ def assign_nested_attributes_for_one_to_one_association(association_name, attrib
426426
existing_record.assign_attributes(assignable_attributes)
427427
association(association_name).initialize_attributes(existing_record)
428428
else
429-
method = "build_#{association_name}"
429+
method = :"build_#{association_name}"
430430
if respond_to?(method)
431431
send(method, assignable_attributes)
432432
else

0 commit comments

Comments
 (0)