Skip to content

Commit 25396b1

Browse files
committed
Avoid adding constants to Enumerable
Ref: aws/aws-sdk-ruby#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.
1 parent bd4d97c commit 25396b1

File tree

2 files changed

+35
-9
lines changed

2 files changed

+35
-9
lines changed

activesupport/lib/active_support/core_ext/enumerable.rb

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,30 @@
11
# frozen_string_literal: true
22

3-
module Enumerable
4-
INDEX_WITH_DEFAULT = Object.new
5-
private_constant :INDEX_WITH_DEFAULT
3+
module ActiveSupport
4+
module EnumerableCoreExt # :nodoc:
5+
module Constants
6+
private
7+
def const_missing(name)
8+
if name == :SoleItemExpectedError
9+
::ActiveSupport::EnumerableCoreExt::SoleItemExpectedError
10+
else
11+
super
12+
end
13+
end
14+
end
15+
end
16+
end
617

18+
module Enumerable
719
# Error generated by +sole+ when called on an enumerable that doesn't have
820
# exactly one item.
921
class SoleItemExpectedError < StandardError; end
1022

23+
# HACK: For performance reasons, Enumerable shouldn't have any constants of its own.
24+
# So we move SoleItemExpectedError into ActiveSupport::EnumerableCoreExt.
25+
ActiveSupport::EnumerableCoreExt::SoleItemExpectedError = remove_const(:SoleItemExpectedError)
26+
singleton_class.prepend(ActiveSupport::EnumerableCoreExt::Constants)
27+
1128
# Enumerable#sum was added in Ruby 2.4, but it only works with Numeric elements
1229
# when we omit an identity.
1330

@@ -106,17 +123,17 @@ def index_by
106123
#
107124
# %i( created_at updated_at ).index_with(Time.now)
108125
# # => { created_at: 2020-03-09 22:31:47, updated_at: 2020-03-09 22:31:47 }
109-
def index_with(default = INDEX_WITH_DEFAULT)
126+
def index_with(default = (no_default = true))
110127
if block_given?
111128
result = {}
112129
each { |elem| result[elem] = yield(elem) }
113130
result
114-
elsif default != INDEX_WITH_DEFAULT
131+
elsif no_default
132+
to_enum(:index_with) { size if respond_to?(:size) }
133+
else
115134
result = {}
116135
each { |elem| result[elem] = default }
117136
result
118-
else
119-
to_enum(:index_with) { size if respond_to?(:size) }
120137
end
121138
end
122139

@@ -240,8 +257,8 @@ def in_order_of(key, series)
240257
def sole
241258
case count
242259
when 1 then return first # rubocop:disable Style/RedundantReturn
243-
when 0 then raise SoleItemExpectedError, "no item found"
244-
when 2.. then raise SoleItemExpectedError, "multiple items found"
260+
when 0 then raise ActiveSupport::EnumerableCoreExt::SoleItemExpectedError, "no item found"
261+
when 2.. then raise ActiveSupport::EnumerableCoreExt::SoleItemExpectedError, "multiple items found"
245262
end
246263
end
247264
end

activesupport/test/core_ext/enumerable_test.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,4 +360,13 @@ def test_sole
360360
assert_raise(expected_raise) { GenericEnumerable.new([1, 2]).sole }
361361
assert_raise(expected_raise) { GenericEnumerable.new([1, nil]).sole }
362362
end
363+
364+
def test_doesnt_bust_constant_cache
365+
skip "Only applies to MRI" unless defined?(RubyVM.stat) && RubyVM.stat(:global_constant_state)
366+
367+
object = Object.new
368+
assert_no_difference -> { RubyVM.stat(:global_constant_state) } do
369+
object.extend(Enumerable)
370+
end
371+
end
363372
end

0 commit comments

Comments
 (0)