Avoid adding constants to Enumerable

Ref: https://github.com/aws/aws-sdk-ruby/pull/2670

Some gems like aws-sdk-core use `Object#extend(Enumerable)`.
It's not a very good pattern, but it's somehwat handled ok by Ruby.

However if Enumerable has constants, then any time the module is
extended, the global constant cache is flushed and this has a very
negative impact on performance for the virtual machine, and even
worse for JITs.
This commit is contained in:
Jean Boussier 2022-03-04 12:50:31 +01:00
parent bd4d97ce11
commit 25396b1c2a
2 changed files with 35 additions and 9 deletions

View File

@ -1,13 +1,30 @@
# frozen_string_literal: true
module Enumerable
INDEX_WITH_DEFAULT = Object.new
private_constant :INDEX_WITH_DEFAULT
module ActiveSupport
module EnumerableCoreExt # :nodoc:
module Constants
private
def const_missing(name)
if name == :SoleItemExpectedError
::ActiveSupport::EnumerableCoreExt::SoleItemExpectedError
else
super
end
end
end
end
end
module Enumerable
# Error generated by +sole+ when called on an enumerable that doesn't have
# exactly one item.
class SoleItemExpectedError < StandardError; end
# HACK: For performance reasons, Enumerable shouldn't have any constants of its own.
# So we move SoleItemExpectedError into ActiveSupport::EnumerableCoreExt.
ActiveSupport::EnumerableCoreExt::SoleItemExpectedError = remove_const(:SoleItemExpectedError)
singleton_class.prepend(ActiveSupport::EnumerableCoreExt::Constants)
# Enumerable#sum was added in Ruby 2.4, but it only works with Numeric elements
# when we omit an identity.
@ -106,17 +123,17 @@ module Enumerable
#
# %i( created_at updated_at ).index_with(Time.now)
# # => { created_at: 2020-03-09 22:31:47, updated_at: 2020-03-09 22:31:47 }
def index_with(default = INDEX_WITH_DEFAULT)
def index_with(default = (no_default = true))
if block_given?
result = {}
each { |elem| result[elem] = yield(elem) }
result
elsif default != INDEX_WITH_DEFAULT
elsif no_default
to_enum(:index_with) { size if respond_to?(:size) }
else
result = {}
each { |elem| result[elem] = default }
result
else
to_enum(:index_with) { size if respond_to?(:size) }
end
end
@ -240,8 +257,8 @@ module Enumerable
def sole
case count
when 1 then return first # rubocop:disable Style/RedundantReturn
when 0 then raise SoleItemExpectedError, "no item found"
when 2.. then raise SoleItemExpectedError, "multiple items found"
when 0 then raise ActiveSupport::EnumerableCoreExt::SoleItemExpectedError, "no item found"
when 2.. then raise ActiveSupport::EnumerableCoreExt::SoleItemExpectedError, "multiple items found"
end
end
end

View File

@ -360,4 +360,13 @@ class EnumerableTests < ActiveSupport::TestCase
assert_raise(expected_raise) { GenericEnumerable.new([1, 2]).sole }
assert_raise(expected_raise) { GenericEnumerable.new([1, nil]).sole }
end
def test_doesnt_bust_constant_cache
skip "Only applies to MRI" unless defined?(RubyVM.stat) && RubyVM.stat(:global_constant_state)
object = Object.new
assert_no_difference -> { RubyVM.stat(:global_constant_state) } do
object.extend(Enumerable)
end
end
end