Faster check for any default scope with all_queries

[nvasilevski]

Summary

Faster & presumably cleaner way of checking for default scopes with true all_queries attribute

It might look like a cosmetic change but on the other hand might be worth implementing for a better readability and performance at scale. Thanks!

Other Information

I'm not sure what is the size of the default_scopes array on average but it seems to be a bit faster even with an empty array, here is some benchmarks:

Benchmark code:

DEFAULT_SCOPES_NUM = 1

Obj = Struct.new(:all_queries)
data = Array.new(DEFAULT_SCOPES_NUM) { Obj.new(rand < 0.5) }
puts data.inspect
Benchmark.ips do |x|
  x.report('map.include') { data.map(&:all_queries).include?(true) }
  x.report('any?') { data.any?(&:all_queries) }
  x.compare!
end

DEFAULT_SCOPES_NUM = 0

Calculating -------------------------------------
         map.include      5.803M (± 4.8%) i/s -     29.436M in   5.084265s
                any?     10.324M (± 3.6%) i/s -     52.006M in   5.045096s

Comparison:
                any?: 10324011.1 i/s
         map.include:  5802745.5 i/s - 1.78x  (± 0.00) slower

DEFAULT_SCOPES_NUM = 1

=> [#<struct Obj all_queries=true>]
Calculating -------------------------------------
         map.include      4.349M (± 8.0%) i/s -     21.841M in   5.056497s
                any?      7.178M (± 4.8%) i/s -     36.253M in   5.064650s

Comparison:
                any?:  7178478.4 i/s
         map.include:  4349401.5 i/s - 1.65x  (± 0.00) slower

DEFAULT_SCOPES_NUM = 5

=> [#<struct Obj all_queries=true>, #<struct Obj all_queries=false>, #<struct Obj all_queries=true>, #<struct Obj all_queries=false>, #<struct Obj all_queries=true>]
Calculating -------------------------------------
         map.include      1.958M (±11.8%) i/s -      9.781M in   5.066797s
                any?      7.133M (± 1.4%) i/s -     35.796M in   5.019214s

Comparison:
                any?:  7133324.7 i/s
         map.include:  1957590.5 i/s - 3.64x  (± 0.00) slower

[zzak]

Nice work @nvasilevski! Running CI to see if this might have any unintended side-effects.

[nvasilevski]

I just realized that technically it will work a little bit differently because .include?(true) explicitly checks for a true object, when .any? will look for any "truthy" value, i.e.

Obj = Struct.new(:all_queries)
data = [Obj.new("I'm just a truthy value"), Obj.new(false)]
data.any?(&:all_queries) #  => true
data.map(&:all_queries).include?(true) # => false

But I believe we are fine because it seems like by design the all_queries attribute is supposed to be either boolean or nil
We can do self.default_scopes.any? { |s| s.all_queries == true } to keep the exact behaviour but I feel like it would be an overkill

[zzak]

So I was just curious and it seems default_scopes? was deprecated in #10113 and removed in 4.1? What I haven't figured out yet was when it was added back, and why 🤔

[nvasilevski]

I think it was added as part of the all_queries scoping feature here - https://github.com/rails/rails/pull/40805/files#diff-f63f081b640fcc6bc7c782ac997566b88a440dba9e06e434db99de94a9d2c8f5R58

[zzak]

@nvasilevski Ok yeah that makes sense, thanks for finding it! Let's see if Eileen can provide some valuable feedback here 🙏

[eileencodes]

Thanks for the change and benchmarks! I don't think we default scope stacks are usually that large, but I'd rather it be faster and easier to read regardless.