aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2019-04-22 14:41:00 +0900
committerRyuta Kamizono <kamipo@gmail.com>2019-04-22 14:41:00 +0900
commitb6828fc91531ae0cc0a0f216705dd19112596301 (patch)
treeccff8b37db7f15d86fe128085640827269ba8eea
parent02b5b8cb4357fe0eda3a5cc0afcfe3ff9f300806 (diff)
downloadrails-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 ```
-rw-r--r--activerecord/lib/active_record/attribute_methods.rb2
-rw-r--r--activerecord/lib/active_record/attribute_methods/primary_key.rb16
-rw-r--r--activerecord/lib/active_record/attribute_methods/read.rb3
-rw-r--r--activerecord/lib/active_record/attribute_methods/write.rb3
-rw-r--r--activerecord/lib/active_record/core.rb6
-rw-r--r--activerecord/lib/active_record/locking/optimistic.rb4
-rw-r--r--activerecord/lib/active_record/persistence.rb9
-rw-r--r--activerecord/lib/active_record/transactions.rb5
-rw-r--r--activerecord/test/cases/primary_keys_test.rb8
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]