Use Enumerator#all? and Enumerator#any? with classes instead of iterations

These methods have changed in Ruby 2.5 to be more akin to grep:

https://bugs.ruby-lang.org/issues/11286

Using classes seems to be faster (and a bit more expressive) than iterating over
the collection items:

```
Warming up --------------------------------------
    #all? with class   504.000  i/100ms
     #all? with proc   189.000  i/100ms
Calculating -------------------------------------
    #all? with class      4.960k (± 1.6%) i/s -     25.200k in   5.082049s
     #all? with proc      1.874k (± 2.8%) i/s -      9.450k in   5.047866s

Comparison:
    #all? with class:     4959.9 i/s
     #all? with proc:     1873.8 i/s - 2.65x  (± 0.00) slower
```

Benchmark script:

```ruby
require "minitest/autorun"
require "benchmark/ips"

class BugTest < Minitest::Test
  def test_enumerators_with_classes
    arr = (1..10000).to_a << nil

    assert_equal arr.all?(Integer), arr.all? { |v| v.is_a?(Integer) }

    Benchmark.ips do |x|
      x.report("#all? with class") do
        arr.all?(Integer)
      end

      x.report("#all? with proc") do
        arr.all? { |v| v.is_a?(Integer) }
      end

      x.compare!
    end
  end
end
```
This commit is contained in:
Ricardo Díaz 2021-02-06 23:08:19 -05:00
parent 39e49edaf9
commit 93cbc30f34
12 changed files with 12 additions and 15 deletions

@ -102,7 +102,7 @@ def formats
def variant=(variant)
variant = Array(variant)
if variant.all? { |v| v.is_a?(Symbol) }
if variant.all?(Symbol)
@variant = ActiveSupport::ArrayInquirer.new(variant)
else
raise ArgumentError, "request.variant must be set to a Symbol or an Array of Symbols."

@ -392,9 +392,7 @@ class ParametersAccessorsTest < ActiveSupport::TestCase
test "#dig converts hashes to parameters" do
assert_kind_of ActionController::Parameters, @params.dig(:person)
assert_kind_of ActionController::Parameters, @params.dig(:person, :addresses, 0)
assert @params.dig(:person, :addresses).all? do |value|
value.is_a?(ActionController::Parameters)
end
assert @params.dig(:person, :addresses).all?(ActionController::Parameters)
end
test "mutating #dig return value mutates underlying parameters" do

@ -152,7 +152,7 @@ def validates_each(*attr_names, &block)
def validate(*args, &block)
options = args.extract_options!
if args.all? { |arg| arg.is_a?(Symbol) }
if args.all?(Symbol)
options.each_key do |k|
unless VALID_OPTIONS_FOR_VALIDATE.include?(k)
raise ArgumentError.new("Unknown key: #{k.inspect}. Valid keys are: #{VALID_OPTIONS_FOR_VALIDATE.map(&:inspect).join(', ')}. Perhaps you meant to call `validates` instead of `validate`?")

@ -75,8 +75,7 @@ def check_validity!
validator = Minitest::Mock.new
validator.expect(:new, validator, [{ foo: :bar, if: :condition_is_true, class: Topic }])
validator.expect(:validate, nil, [topic])
validator.expect(:is_a?, false, [Symbol])
validator.expect(:is_a?, false, [String])
validator.expect(:is_a?, false, [String]) # Call run by ActiveSupport::Callbacks::Callback.build
Topic.validates_with(validator, if: :condition_is_true, foo: :bar)
assert_predicate topic, :valid?

@ -264,7 +264,7 @@ def writer_method(name, class_name, mapping, allow_nil, converter)
end
hash_from_multiparameter_assignment = part.is_a?(Hash) &&
part.each_key.all? { |k| k.is_a?(Integer) }
part.keys.all?(Integer)
if hash_from_multiparameter_assignment
raise ArgumentError unless part.size == part.each_key.max
part = klass.new(*part.sort.map(&:last))

@ -46,7 +46,7 @@ def assign_multiparameter_attributes(pairs)
def execute_callstack_for_multiparameter_attributes(callstack)
errors = []
callstack.each do |name, values_with_empty_parameters|
if values_with_empty_parameters.each_value.all?(&:nil?)
if values_with_empty_parameters.each_value.all?(NilClass)
values = nil
else
values = values_with_empty_parameters

@ -166,7 +166,7 @@ def build_configs(configs)
return configs if configs.is_a?(Array)
db_configs = configs.flat_map do |env_name, config|
if config.is_a?(Hash) && config.all? { |_, v| v.is_a?(Hash) }
if config.is_a?(Hash) && config.values.all?(Hash)
walk_configs(env_name.to_s, config)
else
build_db_config_from_raw_config(env_name.to_s, "primary", config)

@ -264,7 +264,7 @@ def _enum_methods_module
end
def assert_valid_enum_definition_values(values)
unless values.is_a?(Hash) || values.all? { |v| v.is_a?(Symbol) } || values.all? { |v| v.is_a?(String) }
unless values.is_a?(Hash) || values.all?(Symbol) || values.all?(String)
error_message = <<~MSG
Enum values #{values} must be either a hash, an array of symbols, or an array of strings.
MSG

@ -260,7 +260,7 @@ def test_reflection_should_not_raise_error_when_compared_to_other_object
end
def test_reflections_should_return_keys_as_strings
assert Category.reflections.keys.all? { |key| key.is_a? String }, "Model.reflections is expected to return string for keys"
assert Category.reflections.keys.all?(String), "Model.reflections is expected to return string for keys"
end
def test_has_and_belongs_to_many_reflection

@ -356,7 +356,7 @@ def check_conditionals(conditionals)
return EMPTY_ARRAY if conditionals.blank?
conditionals = Array(conditionals)
if conditionals.any? { |c| c.is_a?(String) }
if conditionals.any?(String)
raise ArgumentError, <<-MSG.squish
Passing string to be evaluated in :if and :unless conditional
options is not supported. Pass a symbol for an instance method,

@ -187,7 +187,7 @@ def to_xml(options = {})
options[:indent] ||= 2
options[:builder] ||= Builder::XmlMarkup.new(indent: options[:indent])
options[:root] ||= \
if first.class != Hash && all? { |e| e.is_a?(first.class) }
if first.class != Hash && all?(first.class)
underscored = ActiveSupport::Inflector.underscore(first.class.name)
ActiveSupport::Inflector.pluralize(underscored).tr("/", "_")
else

@ -20,7 +20,7 @@ def assert_called(object, method_name, message = nil, times: 1, returns: nil)
def assert_called_with(object, method_name, args, returns: nil)
mock = Minitest::Mock.new
if args.all? { |arg| arg.is_a?(Array) }
if args.all?(Array)
args.each { |arg| mock.expect(:call, returns, arg) }
else
mock.expect(:call, returns, args)