Skip to content

Commit fd8b751

Browse files
Reintegrate AS::ParameterFilter::CompiledFilter
`ActiveSupport::ParameterFilter::CompiledFilter` was originally extracted in 79e91cc as a performance optimization. Its purpose was to avoid redundantly checking `@filters.empty?`. However, redundant checks can be avoided by using separate `ParameterFilter` methods, rather than allocating a separate `CompiledFilter` object. Therefore, this commit reintegrates `CompiledFilter` back into `ParameterFilter`. Additionally, lazy compilation of filters predates the extraction of `ParameterFilter` from `ActionDispatch::Http::FilterParameters` in e466354. However, since `ActionDispatch::Http::FilterParameters` lazily allocates a `ParameterFilter` instance, there is no benefit to `ParameterFilter` also lazily compiling filters. Therefore, this commit switches from lazy compilation to eager compilation. Together, these changes yield a small performance increase: **Benchmark script** ```ruby # frozen_string_literal: true require "benchmark/ips" require "benchmark/memory" require "active_support" require "active_support/parameter_filter" ootb = [:passw, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn] mixed = [:passw, "secret", /token/, :crypt, "salt", /certificate/, "user.otp", /user\.ssn/, proc {}] params = { "user" => { "name" => :name, "email" => :email, "password" => :password, "ssn" => :ssn, "locations" => [ { "city" => :city, "country" => :country }, { "city" => :city, "country" => :country }, ], } } Benchmark.ips do |x| x.report("ootb") do ActiveSupport::ParameterFilter.new(ootb).filter(params) end x.report("mixed") do ActiveSupport::ParameterFilter.new(mixed).filter(params) end end ``` **Before** ``` Warming up -------------------------------------- ootb 2.032k i/100ms mixed 1.521k i/100ms Calculating ------------------------------------- ootb 20.315k (± 1.3%) i/s - 101.600k in 5.001939s mixed 15.142k (± 1.2%) i/s - 76.050k in 5.023077s ``` **After** ``` Warming up -------------------------------------- ootb 2.163k i/100ms mixed 1.604k i/100ms Calculating ------------------------------------- ootb 21.478k (± 1.2%) i/s - 108.150k in 5.036188s mixed 16.052k (± 0.8%) i/s - 81.804k in 5.096656s ```
1 parent 7b2e0da commit fd8b751

File tree

1 file changed

+53
-67
lines changed

1 file changed

+53
-67
lines changed

activesupport/lib/active_support/parameter_filter.rb

Lines changed: 53 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -40,97 +40,83 @@ class ParameterFilter
4040
#
4141
# * <tt>:mask</tt> - A replaced object when filtered. Defaults to <tt>"[FILTERED]"</tt>.
4242
def initialize(filters = [], mask: FILTERED)
43-
@filters = filters
4443
@mask = mask
44+
compile_filters!(filters)
4545
end
4646

4747
# Mask value of +params+ if key matches one of filters.
4848
def filter(params)
49-
compiled_filter.call(params)
49+
@no_filters ? params.dup : call(params)
5050
end
5151

5252
# Returns filtered value for given key. For +Proc+ filters, third block argument is not populated.
5353
def filter_param(key, value)
54-
@filters.empty? ? value : compiled_filter.value_for_key(key, value)
54+
@no_filters ? value : value_for_key(key, value)
5555
end
5656

5757
private
58-
def compiled_filter
59-
@compiled_filter ||= CompiledFilter.compile(@filters, mask: @mask)
60-
end
61-
62-
class CompiledFilter # :nodoc:
63-
def self.compile(filters, mask:)
64-
return lambda { |params| params.dup } if filters.empty?
65-
66-
strings, regexps, blocks, deep_regexps, deep_strings = [], [], [], nil, nil
67-
68-
filters.each do |item|
69-
case item
70-
when Proc
71-
blocks << item
72-
when Regexp
73-
if item.to_s.include?("\\.")
74-
(deep_regexps ||= []) << item
75-
else
76-
regexps << item
77-
end
58+
def compile_filters!(filters)
59+
@no_filters = filters.empty?
60+
return if @no_filters
61+
62+
@regexps, strings = [], []
63+
@deep_regexps, deep_strings = nil, nil
64+
@blocks = nil
65+
66+
filters.each do |item|
67+
case item
68+
when Proc
69+
(@blocks ||= []) << item
70+
when Regexp
71+
if item.to_s.include?("\\.")
72+
(@deep_regexps ||= []) << item
7873
else
79-
s = Regexp.escape(item.to_s)
80-
if s.include?("\\.")
81-
(deep_strings ||= []) << s
82-
else
83-
strings << s
84-
end
74+
@regexps << item
75+
end
76+
else
77+
s = Regexp.escape(item.to_s)
78+
if s.include?("\\.")
79+
(deep_strings ||= []) << s
80+
else
81+
strings << s
8582
end
8683
end
87-
88-
regexps << Regexp.new(strings.join("|"), true) unless strings.empty?
89-
(deep_regexps ||= []) << Regexp.new(deep_strings.join("|"), true) if deep_strings&.any?
90-
91-
new regexps, deep_regexps, blocks, mask: mask
92-
end
93-
94-
attr_reader :regexps, :deep_regexps, :blocks
95-
96-
def initialize(regexps, deep_regexps, blocks, mask:)
97-
@regexps = regexps
98-
@deep_regexps = deep_regexps&.any? ? deep_regexps : nil
99-
@blocks = blocks
100-
@mask = mask
10184
end
10285

103-
def call(params, full_parent_key = nil, original_params = params)
104-
filtered_params = params.class.new
86+
@regexps << Regexp.new(strings.join("|"), true) unless strings.empty?
87+
(@deep_regexps ||= []) << Regexp.new(deep_strings.join("|"), true) if deep_strings
88+
end
10589

106-
params.each do |key, value|
107-
filtered_params[key] = value_for_key(key, value, full_parent_key, original_params)
108-
end
90+
def call(params, full_parent_key = nil, original_params = params)
91+
filtered_params = params.class.new
10992

110-
filtered_params
93+
params.each do |key, value|
94+
filtered_params[key] = value_for_key(key, value, full_parent_key, original_params)
11195
end
11296

113-
def value_for_key(key, value, full_parent_key = nil, original_params = nil)
114-
if deep_regexps
115-
full_key = full_parent_key ? "#{full_parent_key}.#{key}" : key.to_s
116-
end
97+
filtered_params
98+
end
11799

118-
if regexps.any? { |r| r.match?(key.to_s) }
119-
value = @mask
120-
elsif deep_regexps&.any? { |r| r.match?(full_key) }
121-
value = @mask
122-
elsif value.is_a?(Hash)
123-
value = call(value, full_key, original_params)
124-
elsif value.is_a?(Array)
125-
value = value.map { |v| value_for_key(key, v, full_parent_key, original_params) }
126-
elsif blocks.any?
127-
key = key.dup if key.duplicable?
128-
value = value.dup if value.duplicable?
129-
blocks.each { |b| b.arity == 2 ? b.call(key, value) : b.call(key, value, original_params) }
130-
end
100+
def value_for_key(key, value, full_parent_key = nil, original_params = nil)
101+
if @deep_regexps
102+
full_key = full_parent_key ? "#{full_parent_key}.#{key}" : key.to_s
103+
end
131104

132-
value
105+
if @regexps.any? { |r| r.match?(key.to_s) }
106+
value = @mask
107+
elsif @deep_regexps&.any? { |r| r.match?(full_key) }
108+
value = @mask
109+
elsif value.is_a?(Hash)
110+
value = call(value, full_key, original_params)
111+
elsif value.is_a?(Array)
112+
value = value.map { |v| value_for_key(key, v, full_parent_key, original_params) }
113+
elsif @blocks
114+
key = key.dup if key.duplicable?
115+
value = value.dup if value.duplicable?
116+
@blocks.each { |b| b.arity == 2 ? b.call(key, value) : b.call(key, value, original_params) }
133117
end
118+
119+
value
134120
end
135121
end
136122
end

0 commit comments

Comments
 (0)