Skip to content

Commit

Permalink
Remove useless set_value / get_value helper methods
Browse files Browse the repository at this point in the history
Those helper methods makes relation values access 15% slower.

https://gist.github.com/kamipo/e64439f7a206e1c5b5c69d92d982828e

Before (02b5b8c):

```
Warming up --------------------------------------
        #limit_value   237.074k i/100ms
    #limit_value = 1   222.052k i/100ms
Calculating -------------------------------------
        #limit_value      6.477M (± 2.9%) i/s -     32.479M in   5.019475s
    #limit_value = 1      5.297M (± 4.3%) i/s -     26.424M in   4.999933s
```

After (this change):

```
Warming up --------------------------------------
        #limit_value   261.109k i/100ms
    #limit_value = 1   239.646k i/100ms
Calculating -------------------------------------
        #limit_value      7.412M (± 1.6%) i/s -     37.077M in   5.003345s
    #limit_value = 1      6.134M (± 1.0%) i/s -     30.675M in   5.000908s
```
  • Loading branch information
kamipo committed Apr 22, 2019
1 parent 02b5b8c commit 89b8664
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 17 deletions.
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/relation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -553,8 +553,8 @@ def destroy_all
# # => ActiveRecord::ActiveRecordError: delete_all doesn't support distinct
def delete_all
invalid_methods = INVALID_METHODS_FOR_DELETE_ALL.select do |method|
value = get_value(method)
SINGLE_VALUE_METHODS.include?(method) ? value : value.any?
value = @values[method]
method == :distinct ? value : value&.any?
end
if invalid_methods.any?
raise ActiveRecordError.new("delete_all doesn't support #{invalid_methods.join(', ')}")
Expand Down
23 changes: 8 additions & 15 deletions activerecord/lib/active_record/relation/query_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,13 @@ def not(opts, *rest)
end
class_eval <<-CODE, __FILE__, __LINE__ + 1
def #{method_name} # def includes_values
get_value(#{name.inspect}) # get_value(:includes)
default = DEFAULT_VALUES[:#{name}] # default = DEFAULT_VALUES[:includes]
@values.fetch(:#{name}, default) # @values.fetch(:includes, default)
end # end
def #{method_name}=(value) # def includes_values=(value)
set_value(#{name.inspect}, value) # set_value(:includes, value)
assert_mutability! # assert_mutability!
@values[:#{name}] = value # @values[:includes] = value
end # end
CODE
end
Expand Down Expand Up @@ -417,7 +419,8 @@ def unscope!(*args) # :nodoc:
if !VALID_UNSCOPING_VALUES.include?(scope)
raise ArgumentError, "Called unscope() with invalid unscoping argument ':#{scope}'. Valid arguments are :#{VALID_UNSCOPING_VALUES.to_a.join(", :")}."
end
set_value(scope, DEFAULT_VALUES[scope])
assert_mutability!
@values[scope] = DEFAULT_VALUES[scope]
when Hash
scope.each do |key, target_value|
if key != :where
Expand Down Expand Up @@ -1005,17 +1008,6 @@ def build_subquery(subquery_alias, select_value) # :nodoc:
end

private
# Returns a relation value with a given name
def get_value(name)
@values.fetch(name, DEFAULT_VALUES[name])
end

# Sets the relation value with the given name
def set_value(name, value)
assert_mutability!
@values[name] = value
end

def assert_mutability!
raise ImmutableRelation if @loaded
raise ImmutableRelation if defined?(@arel) && @arel
Expand Down Expand Up @@ -1316,7 +1308,8 @@ def check_if_method_has_arguments!(method_name, args)
def structurally_incompatible_values_for_or(other)
values = other.values
STRUCTURAL_OR_METHODS.reject do |method|
get_value(method) == values.fetch(method, DEFAULT_VALUES[method])
default = DEFAULT_VALUES[method]
@values.fetch(method, default) == values.fetch(method, default)
end
end

Expand Down

0 comments on commit 89b8664

Please sign in to comment.