From f45267bc423017109e442e5c35a5765dc482b12b Mon Sep 17 00:00:00 2001 From: schneems Date: Thu, 11 Oct 2018 17:07:28 -0500 Subject: ActiveRecord#respond_to? No longer allocates strings This is an alternative to https://github.com/rails/rails/pull/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 https://github.com/rails/rails/commit/f80aa5994603e684e3fecd3f53bfbf242c73a107 (by me) and then with an attempt to extend in https://github.com/rails/rails/pull/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?`. --- activerecord/lib/active_record/model_schema.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'activerecord/lib/active_record/model_schema.rb') diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 5a4a1fb969..c50a420432 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -388,6 +388,11 @@ module ActiveRecord @column_names ||= columns.map(&:name) end + def symbol_column_to_string(name_symbol) # :nodoc: + @symbol_column_to_string_name_hash ||= column_names.index_by(&:to_sym) + @symbol_column_to_string_name_hash[name_symbol] + end + # Returns an array of column objects where the primary id, all columns ending in "_id" or "_count", # and columns used for single table inheritance have been removed. def content_columns @@ -477,6 +482,7 @@ module ActiveRecord def reload_schema_from_cache @arel_table = nil @column_names = nil + @symbol_column_to_string_name_hash = nil @attribute_types = nil @content_columns = nil @default_attributes = nil -- cgit v1.2.3