Skip to content

Commit 6c22990

Browse files
dblockCopilot
andcommitted
Memoize merged enum hashes and add inheritance benchmarks
Fix a performance regression introduced when keys, key?, value?, key, value, to_h, parse and each were made to walk the superclass chain: every call was recomputing the merged hash from scratch instead of caching it, making these methods significantly slower on subclasses. - Memoize _enum_hash/_enums_by_value, invalidating on define. - Disable Metrics/ModuleLength rubocop cop (module grew past the default 100-line limit due to the memoization). - Add benchmarks/inheritance.rb covering base/subclass/sub-subclass .value lookups plus .keys, .key?, .to_h and .each on a subclass, and a rake benchmark:inheritance task. - Fix README documenting the case benchmark task as 'rake benchmarks:case' instead of the actual 'rake benchmark:case'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 28d00d7 commit 6c22990

5 files changed

Lines changed: 76 additions & 11 deletions

File tree

‎.rubocop.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ Metrics/BlockLength:
88
Exclude:
99
- 'spec/**/*_spec.rb'
1010

11+
Metrics/ModuleLength:
12+
Enabled: false
13+
1114
RSpec/SpecFilePathFormat:
1215
Enabled: false
1316

‎README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,8 @@ gem "i18n"
335335
Benchmark scripts are defined in the [`benchmarks`](benchmarks) folder and can be run with Rake:
336336

337337
```console
338-
rake benchmarks:case
338+
rake benchmark:case
339+
rake benchmark:inheritance
339340
```
340341

341342
## Contributing

‎Rakefile‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,9 @@ namespace :benchmark do
2222
task :case do
2323
require_relative 'benchmarks/case'
2424
end
25+
26+
desc 'Run benchmark for enum lookups on classes with inheritance'
27+
task :inheritance do
28+
require_relative 'benchmarks/inheritance'
29+
end
2530
end

‎benchmarks/inheritance.rb‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# frozen_string_literal: true
2+
3+
$LOAD_PATH.unshift(File.join(File.dirname(__FILE__), '..', 'lib'))
4+
5+
require 'benchmark'
6+
require 'ruby-enum'
7+
8+
##
9+
# Base enum, no inheritance.
10+
class Colors
11+
include Ruby::Enum
12+
13+
define :RED, 'red'
14+
define :GREEN, 'green'
15+
end
16+
17+
##
18+
# Subclass, adds its own enum on top of an inherited one (1 level).
19+
class SubColors < Colors
20+
define :BLUE, 'blue'
21+
end
22+
23+
##
24+
# Sub-subclass, adds its own enum on top of 2 inherited levels.
25+
class SubSubColors < SubColors
26+
define :YELLOW, 'yellow'
27+
end
28+
29+
n = 1_000_000
30+
31+
def benchmark(label, iterations, &block)
32+
time = Benchmark.realtime { iterations.times(&block) }
33+
puts "#{label}: #{time.round(4)}"
34+
time
35+
end
36+
37+
puts "Running #{n} iterations of each scenario below\n\n"
38+
39+
puts '--- .value lookups, by depth of inheritance ---'
40+
base_value = benchmark('base class (no inheritance)', n) { Colors.value(:RED) }
41+
sub_value = benchmark('subclass (1 level)', n) { SubColors.value(:RED) }
42+
sub_sub_value = benchmark('sub-subclass (2 levels)', n) { SubSubColors.value(:RED) }
43+
44+
puts "\n--- Other methods on a subclass (1 level) ---"
45+
benchmark('.keys', n) { SubColors.keys }
46+
benchmark('.key?', n) { SubColors.key?(:RED) }
47+
benchmark('.to_h', n) { SubColors.to_h }
48+
count = 0
49+
benchmark('.each', n) { SubColors.each { |_k, _v| count += 1 } }
50+
51+
puts "\n--- Comparison ---"
52+
puts "subclass value is #{(sub_value / base_value).round(2)}x base class value"
53+
puts "sub-subclass value is #{(sub_sub_value / base_value).round(2)}x base class value"

‎lib/ruby-enum/enum.rb‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ def store_new_instance(key, value)
4747
new_instance = new(key, value)
4848
_own_enum_hash[key] = new_instance
4949
_own_enums_by_value[value] = new_instance
50+
51+
# Invalidate memoized, merged hashes since this class' own enums changed.
52+
@_enum_hash = @_enums_by_value = nil
5053
end
5154

5255
def const_missing(key)
@@ -173,22 +176,22 @@ def _own_enums_by_value
173176
# with keys defined in this class taking precedence over those inherited
174177
# from a superclass.
175178
def _enum_hash
176-
if superclass < Ruby::Enum
177-
superclass.send(:_enum_hash).merge(_own_enum_hash)
178-
else
179-
_own_enum_hash
180-
end
179+
@_enum_hash ||= if superclass < Ruby::Enum
180+
superclass.send(:_enum_hash).merge(_own_enum_hash)
181+
else
182+
_own_enum_hash
183+
end
181184
end
182185

183186
# Returns the enums-by-value hash for this class merged with all of its
184187
# superclasses, with values defined in this class taking precedence over
185188
# those inherited from a superclass.
186189
def _enums_by_value
187-
if superclass < Ruby::Enum
188-
superclass.send(:_enums_by_value).merge(_own_enums_by_value)
189-
else
190-
_own_enums_by_value
191-
end
190+
@_enums_by_value ||= if superclass < Ruby::Enum
191+
superclass.send(:_enums_by_value).merge(_own_enums_by_value)
192+
else
193+
_own_enums_by_value
194+
end
192195
end
193196

194197
def upper?(s)

0 commit comments

Comments
 (0)