Skip to content

Commit bf33510

Browse files
committed
Optimize CurrentAttributes method generation
The bulk of the optimization is to generate code rather than use `define_method` with a closure. ``` Warming up -------------------------------------- original 207.468k i/100ms code-generator 340.849k i/100ms Calculating ------------------------------------- original 2.127M (± 1.1%) i/s - 10.788M in 5.073860s code-generator 3.426M (± 0.9%) i/s - 17.383M in 5.073965s Comparison: code-generator: 3426241.0 i/s original: 2126539.2 i/s - 1.61x (± 0.00) slower ``` ```ruby require 'benchmark/ips' require 'active_support/all' class Original < ActiveSupport::CurrentAttributes attribute :foo end class CodeGen < ActiveSupport::CurrentAttributes class << self def attribute(*names) ActiveSupport::CodeGenerator.batch(generated_attribute_methods, __FILE__, __LINE__) do |owner| names.each do |name| owner.define_cached_method(name, namespace: :current_attributes) do |batch| batch << "def #{name}" << "attributes[:#{name}]" << "end" end owner.define_cached_method("#{name}=", namespace: :current_attributes) do |batch| batch << "def #{name}=(value)" << "attributes[:#{name}] = value" << "end" end end end ActiveSupport::CodeGenerator.batch(singleton_class, __FILE__, __LINE__) do |owner| names.each do |name| owner.define_cached_method(name, namespace: :current_attributes_delegation) do |batch| batch << "def #{name}" << "instance.#{name}" << "end" end owner.define_cached_method("#{name}=", namespace: :current_attributes_delegation) do |batch| batch << "def #{name}=(value)" << "instance.#{name} = value" << "end" end end end end end attribute :foo end Benchmark.ips do |x| x.report('original') { Original.foo } x.report('code-generator') { CodeGen.foo } x.compare! end ```
1 parent f314bae commit bf33510

File tree

4 files changed

+94
-79
lines changed

4 files changed

+94
-79
lines changed

activemodel/lib/active_model/attribute_methods.rb

Lines changed: 3 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ def attribute_method_affix(*affixes)
208208
# person.nickname_short? # => true
209209
def alias_attribute(new_name, old_name)
210210
self.attribute_aliases = attribute_aliases.merge(new_name.to_s => old_name.to_s)
211-
CodeGenerator.batch(self, __FILE__, __LINE__) do |code_generator|
211+
ActiveSupport::CodeGenerator.batch(self, __FILE__, __LINE__) do |code_generator|
212212
attribute_method_matchers.each do |matcher|
213213
method_name = matcher.method_name(new_name).to_s
214214
target_name = matcher.method_name(old_name).to_s
@@ -274,7 +274,7 @@ def attribute_alias(name)
274274
# end
275275
# end
276276
def define_attribute_methods(*attr_names)
277-
CodeGenerator.batch(generated_attribute_methods, __FILE__, __LINE__) do |owner|
277+
ActiveSupport::CodeGenerator.batch(generated_attribute_methods, __FILE__, __LINE__) do |owner|
278278
attr_names.flatten.each { |attr_name| define_attribute_method(attr_name, _owner: owner) }
279279
end
280280
end
@@ -309,7 +309,7 @@ def define_attribute_methods(*attr_names)
309309
# person.name # => "Bob"
310310
# person.name_short? # => true
311311
def define_attribute_method(attr_name, _owner: generated_attribute_methods)
312-
CodeGenerator.batch(_owner, __FILE__, __LINE__) do |owner|
312+
ActiveSupport::CodeGenerator.batch(_owner, __FILE__, __LINE__) do |owner|
313313
attribute_method_matchers.each do |matcher|
314314
method_name = matcher.method_name(attr_name)
315315

@@ -358,69 +358,6 @@ def undefine_attribute_methods
358358
end
359359

360360
private
361-
class CodeGenerator # :nodoc:
362-
class MethodSet
363-
METHOD_CACHES = Hash.new { |h, k| h[k] = Module.new }
364-
365-
def initialize(namespace)
366-
@cache = METHOD_CACHES[namespace]
367-
@sources = []
368-
@methods = {}
369-
end
370-
371-
def define_cached_method(name, as: name)
372-
name = name.to_sym
373-
as = as.to_sym
374-
@methods.fetch(name) do
375-
unless @cache.method_defined?(as)
376-
yield @sources
377-
end
378-
@methods[name] = as
379-
end
380-
end
381-
382-
def apply(owner, path, line)
383-
unless @sources.empty?
384-
@cache.module_eval("# frozen_string_literal: true\n" + @sources.join(";"), path, line)
385-
end
386-
@methods.each do |name, as|
387-
owner.define_method(name, @cache.instance_method(as))
388-
end
389-
end
390-
end
391-
392-
class << self
393-
def batch(owner, path, line)
394-
if owner.is_a?(CodeGenerator)
395-
yield owner
396-
else
397-
instance = new(owner, path, line)
398-
result = yield instance
399-
instance.execute
400-
result
401-
end
402-
end
403-
end
404-
405-
def initialize(owner, path, line)
406-
@owner = owner
407-
@path = path
408-
@line = line
409-
@namespaces = Hash.new { |h, k| h[k] = MethodSet.new(k) }
410-
end
411-
412-
def define_cached_method(name, namespace:, as: name, &block)
413-
@namespaces[namespace].define_cached_method(name, as: as, &block)
414-
end
415-
416-
def execute
417-
@namespaces.each_value do |method_set|
418-
method_set.apply(@owner, @path, @line - 1)
419-
end
420-
end
421-
end
422-
private_constant :CodeGenerator
423-
424361
def generated_attribute_methods
425362
@generated_attribute_methods ||= Module.new.tap { |mod| include mod }
426363
end

activesupport/lib/active_support.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ module ActiveSupport
3434
extend ActiveSupport::Autoload
3535

3636
autoload :Concern
37+
autoload :CodeGenerator
3738
autoload :ActionableError
3839
autoload :ConfigurationFile
3940
autoload :CurrentAttributes
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# frozen_string_literal: true
2+
3+
module ActiveSupport
4+
class CodeGenerator # :nodoc:
5+
class MethodSet
6+
METHOD_CACHES = Hash.new { |h, k| h[k] = Module.new }
7+
8+
def initialize(namespace)
9+
@cache = METHOD_CACHES[namespace]
10+
@sources = []
11+
@methods = {}
12+
end
13+
14+
def define_cached_method(name, as: name)
15+
name = name.to_sym
16+
as = as.to_sym
17+
@methods.fetch(name) do
18+
unless @cache.method_defined?(as)
19+
yield @sources
20+
end
21+
@methods[name] = as
22+
end
23+
end
24+
25+
def apply(owner, path, line)
26+
unless @sources.empty?
27+
@cache.module_eval("# frozen_string_literal: true\n" + @sources.join(";"), path, line)
28+
end
29+
@methods.each do |name, as|
30+
owner.define_method(name, @cache.instance_method(as))
31+
end
32+
end
33+
end
34+
35+
class << self
36+
def batch(owner, path, line)
37+
if owner.is_a?(CodeGenerator)
38+
yield owner
39+
else
40+
instance = new(owner, path, line)
41+
result = yield instance
42+
instance.execute
43+
result
44+
end
45+
end
46+
end
47+
48+
def initialize(owner, path, line)
49+
@owner = owner
50+
@path = path
51+
@line = line
52+
@namespaces = Hash.new { |h, k| h[k] = MethodSet.new(k) }
53+
end
54+
55+
def define_cached_method(name, namespace:, as: name, &block)
56+
@namespaces[namespace].define_cached_method(name, as: as, &block)
57+
end
58+
59+
def execute
60+
@namespaces.each_value do |method_set|
61+
method_set.apply(@owner, @path, @line - 1)
62+
end
63+
end
64+
end
65+
end

activesupport/lib/active_support/current_attributes.rb

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -98,25 +98,37 @@ def instance
9898

9999
# Declares one or more attributes that will be given both class and instance accessor methods.
100100
def attribute(*names)
101-
generated_attribute_methods.module_eval do
101+
ActiveSupport::CodeGenerator.batch(generated_attribute_methods, __FILE__, __LINE__) do |owner|
102102
names.each do |name|
103-
define_method(name) do
104-
attributes[name.to_sym]
103+
owner.define_cached_method(name, namespace: :current_attributes) do |batch|
104+
batch <<
105+
"def #{name}" <<
106+
"attributes[:#{name}]" <<
107+
"end"
105108
end
106-
107-
define_method("#{name}=") do |attribute|
108-
attributes[name.to_sym] = attribute
109+
owner.define_cached_method("#{name}=", namespace: :current_attributes) do |batch|
110+
batch <<
111+
"def #{name}=(value)" <<
112+
"attributes[:#{name}] = value" <<
113+
"end"
109114
end
110115
end
111116
end
112117

113-
names.each do |name|
114-
define_singleton_method(name) do
115-
instance.public_send(name)
116-
end
117-
118-
define_singleton_method("#{name}=") do |attribute|
119-
instance.public_send("#{name}=", attribute)
118+
ActiveSupport::CodeGenerator.batch(singleton_class, __FILE__, __LINE__) do |owner|
119+
names.each do |name|
120+
owner.define_cached_method(name, namespace: :current_attributes_delegation) do |batch|
121+
batch <<
122+
"def #{name}" <<
123+
"instance.#{name}" <<
124+
"end"
125+
end
126+
owner.define_cached_method("#{name}=", namespace: :current_attributes_delegation) do |batch|
127+
batch <<
128+
"def #{name}=(value)" <<
129+
"instance.#{name} = value" <<
130+
"end"
131+
end
120132
end
121133
end
122134
end

0 commit comments

Comments
 (0)