diff options
author | Ryuta Kamizono <kamipo@gmail.com> | 2019-04-22 14:41:00 +0900 |
---|---|---|
committer | Ryuta Kamizono <kamipo@gmail.com> | 2019-04-22 14:41:00 +0900 |
commit | b6828fc91531ae0cc0a0f216705dd19112596301 (patch) | |
tree | ccff8b37db7f15d86fe128085640827269ba8eea | |
parent | 02b5b8cb4357fe0eda3a5cc0afcfe3ff9f300806 (diff) | |
download | rails-b6828fc91531ae0cc0a0f216705dd19112596301.tar.gz rails-b6828fc91531ae0cc0a0f216705dd19112596301.tar.bz2 rails-b6828fc91531ae0cc0a0f216705dd19112596301.zip |
PERF: 20% faster pk attribute access
I've realized that `user.id` is 20% slower than `user.name` in the
benchmark (https://github.com/rails/rails/pull/35987#issuecomment-483882480).
The reason that performance difference is that `self.class.primary_key`
method call is a bit slow.
Avoiding that method call will make almost attribute access faster and
`user.id` will be completely the same performance with `user.name`.
Before (02b5b8cb):
```
Warming up --------------------------------------
user.id 140.535k i/100ms
user['id'] 96.549k i/100ms
user.name 158.110k i/100ms
user['name'] 94.507k i/100ms
user.changed? 19.003k i/100ms
user.saved_changes? 25.404k i/100ms
Calculating -------------------------------------
user.id 2.231M (± 0.9%) i/s - 11.243M in 5.040066s
user['id'] 1.310M (± 1.3%) i/s - 6.565M in 5.012607s
user.name 2.683M (± 1.2%) i/s - 13.439M in 5.009392s
user['name'] 1.322M (± 0.9%) i/s - 6.615M in 5.003239s
user.changed? 201.999k (±10.9%) i/s - 1.007M in 5.091195s
user.saved_changes? 258.214k (±17.1%) i/s - 1.245M in 5.007421s
```
After (this change):
```
Warming up --------------------------------------
user.id 158.364k i/100ms
user['id'] 106.412k i/100ms
user.name 158.644k i/100ms
user['name'] 107.518k i/100ms
user.changed? 19.082k i/100ms
user.saved_changes? 24.886k i/100ms
Calculating -------------------------------------
user.id 2.768M (± 1.1%) i/s - 13.936M in 5.034957s
user['id'] 1.507M (± 2.1%) i/s - 7.555M in 5.017211s
user.name 2.727M (± 1.5%) i/s - 13.643M in 5.004766s
user['name'] 1.521M (± 1.3%) i/s - 7.634M in 5.018321s
user.changed? 200.865k (±11.1%) i/s - 992.264k in 5.044868s
user.saved_changes? 269.652k (±10.5%) i/s - 1.344M in 5.077972s
```
9 files changed, 30 insertions, 26 deletions
diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 331813f338..220043c061 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -465,7 +465,7 @@ module ActiveRecord end def pk_attribute?(name) - name == self.class.primary_key + name == @primary_key end end end diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index feaef72a30..b4f5e6e75a 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -16,34 +16,32 @@ module ActiveRecord # Returns the primary key column's value. def id - primary_key = self.class.primary_key - _read_attribute(primary_key) if primary_key + _read_attribute(@primary_key) end # Sets the primary key column's value. def id=(value) - primary_key = self.class.primary_key - _write_attribute(primary_key, value) if primary_key + _write_attribute(@primary_key, value) end # Queries the primary key column's value. def id? - query_attribute(self.class.primary_key) + query_attribute(@primary_key) end # Returns the primary key column's value before type cast. def id_before_type_cast - read_attribute_before_type_cast(self.class.primary_key) + read_attribute_before_type_cast(@primary_key) end # Returns the primary key column's previous value. def id_was - attribute_was(self.class.primary_key) + attribute_was(@primary_key) end # Returns the primary key column's value from the database. def id_in_database - attribute_in_database(self.class.primary_key) + attribute_in_database(@primary_key) end private @@ -116,7 +114,7 @@ module ActiveRecord # # Project.primary_key # => "foo_id" def primary_key=(value) - @primary_key = value && value.to_s + @primary_key = value && -value.to_s @quoted_primary_key = nil @attributes_builder = nil end diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 409a150e56..c787fb6a94 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -31,8 +31,7 @@ module ActiveRecord name = self.class.attribute_alias(name) end - primary_key = self.class.primary_key - name = primary_key if name == "id" && primary_key + name = @primary_key if name == "id" && @primary_key _read_attribute(name, &block) end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index 6a21643884..6a54bd2fc2 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -35,8 +35,7 @@ module ActiveRecord name = self.class.attribute_alias(name) end - primary_key = self.class.primary_key - name = primary_key if name == "id" && primary_key + name = @primary_key if name == "id" && @primary_key _write_attribute(name, value) end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index b9046fcc1a..068ebf3c09 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -174,8 +174,7 @@ module ActiveRecord record = statement.execute([id], connection)&.first unless record - raise RecordNotFound.new("Couldn't find #{name} with '#{primary_key}'=#{id}", - name, primary_key, id) + raise RecordNotFound.new("Couldn't find #{name} with '#{key}'=#{id}", name, key, id) end record end @@ -398,7 +397,7 @@ module ActiveRecord ## def initialize_dup(other) # :nodoc: @attributes = @attributes.deep_dup - @attributes.reset(self.class.primary_key) + @attributes.reset(@primary_key) _run_initialize_callbacks @@ -570,6 +569,7 @@ module ActiveRecord end def init_internals + @primary_key = self.class.primary_key @readonly = false @destroyed = false @marked_for_destruction = false diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index b7eecda59e..6711ee9bf4 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -87,7 +87,7 @@ module ActiveRecord affected_rows = self.class._update_record( attributes_with_values(attribute_names), - self.class.primary_key => id_in_database, + @primary_key => id_in_database, locking_column => previous_lock_value ) @@ -110,7 +110,7 @@ module ActiveRecord locking_column = self.class.locking_column affected_rows = self.class._delete_record( - self.class.primary_key => id_in_database, + @primary_key => id_in_database, locking_column => read_attribute_before_type_cast(locking_column) ) diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index bd572486c8..1ff7ee4d89 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -353,6 +353,7 @@ module ActiveRecord end def _insert_record(values) # :nodoc: + primary_key = self.primary_key primary_key_value = nil if primary_key && Hash === values @@ -674,7 +675,7 @@ module ActiveRecord affected_rows = self.class._update_record( attributes, - self.class.primary_key => id_in_database + @primary_key => id_in_database ) affected_rows == 1 @@ -874,7 +875,7 @@ module ActiveRecord end def _delete_row - self.class._delete_record(self.class.primary_key => id_in_database) + self.class._delete_record(@primary_key => id_in_database) end def _touch_row(attribute_names, time) @@ -890,7 +891,7 @@ module ActiveRecord def _update_row(attribute_names, attempted_action = "update") self.class._update_record( attributes_with_values(attribute_names), - self.class.primary_key => id_in_database + @primary_key => id_in_database ) end @@ -928,7 +929,7 @@ module ActiveRecord attributes_with_values(attribute_names) ) - self.id ||= new_id if self.class.primary_key + self.id ||= new_id if @primary_key @new_record = false diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 9f52734e00..49a8206c84 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -432,9 +432,8 @@ module ActiveRecord end @mutations_from_database = nil @mutations_before_last_save = nil - pk = self.class.primary_key - if pk && @attributes.fetch_value(pk) != restore_state[:id] - @attributes.write_from_user(pk, restore_state[:id]) + if @attributes.fetch_value(@primary_key) != restore_state[:id] + @attributes.write_from_user(@primary_key, restore_state[:id]) end freeze if restore_state[:frozen?] end diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index 4759d3b6b2..511d7fc982 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -203,6 +203,14 @@ class PrimaryKeysTest < ActiveRecord::TestCase assert_queries(3, ignore_none: true) { klass.create! } end + def test_assign_id_raises_error_if_primary_key_doesnt_exist + klass = Class.new(ActiveRecord::Base) do + self.table_name = "dashboards" + end + dashboard = klass.new + assert_raises(ActiveModel::MissingAttributeError) { dashboard.id = "1" } + end + if current_adapter?(:PostgreSQLAdapter) def test_serial_with_quoted_sequence_name column = MixedCaseMonkey.columns_hash[MixedCaseMonkey.primary_key] |