Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PERF: 20% faster pk attribute access #36052

Merged
merged 1 commit into from Apr 22, 2019

Conversation

Projects
None yet
2 participants
@kamipo
Copy link
Member

commented Apr 22, 2019

I've realized that user.id is 20% slower than user.name in the
benchmark (#35987 (comment)).

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 (02b5b8c):

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
PERF: 20% faster pk attribute access
I've realized that `user.id` is 20% slower than `user.name` in the
benchmark (#35987 (comment)).

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 (02b5b8c):

```
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
```

@rails-bot rails-bot bot added the activerecord label Apr 22, 2019

@@ -116,7 +114,7 @@ def get_primary_key(base_name) #:nodoc:
#
# Project.primary_key # => "foo_id"
def primary_key=(value)
@primary_key = value && value.to_s
@primary_key = value && -value.to_s

This comment has been minimized.

Copy link
@DamirSvrtan

DamirSvrtan Apr 22, 2019

What does the minus do here?

This comment has been minimized.

Copy link
@kamipo

kamipo Apr 22, 2019

Author Member

Ruby 2.5 and higher has a string deduplication (fstring table), String#-@ registers the string to fstring table and fetches the deduplicated instance.
The most @primary_keys are "id", the deduplication will reduce memory usage.

@@ -16,34 +16,32 @@ def to_key

# 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)

This comment has been minimized.

Copy link
@kamipo

kamipo Apr 22, 2019

Author Member

This if primary_key is redundant, since _read_attribute(nil) returns nil.

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)

This comment has been minimized.

Copy link
@kamipo

kamipo Apr 22, 2019

Author Member

Previously #id= behaves as no-op if primary key doesn't exist.
But I think that calling #id= on no-pk object is already something wrong, raising MissingAttributeError is preferable than ignore any error to me.

And also, since most models has pk, this guard is almost redundant.
E.g. persistence.rb#L932 checks if @primary_key three times as before, now only once.

new_id = self.class._insert_record(
attributes_with_values(attribute_names)
)
self.id ||= new_id if @primary_key

@kamipo kamipo merged commit ada56f8 into rails:master Apr 22, 2019

2 checks passed

buildkite/rails Build #60659 passed (10 minutes, 13 seconds)
Details
codeclimate All good!
Details

@kamipo kamipo deleted the kamipo:fast_id branch Apr 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.