Skip to content

Commit 14cd0c4

Browse files
committed
Avoid unnecessary replacements when the node doesn't change
This represents a +2x performance optimization when the replacement logic is based on some condition, and it returns the same unchanged node when it wants to skip it: ```ruby html = <<~HTML <div> #{'<p>ignore me</p>' * 1000} #{'<p>replace me</p>' * 1000} </div> HTML content = content_from_html(html) replacement_example = -> do content.fragment.replace("p") do |node| if node.text =~ /replace me/ "<p>replace me</p>" else node end end end current_implementation = -> do class ActionText::Fragment def replace(selector) update do |source| source.css(selector).each do |node| replacement_node = yield(node) node.replace(replacement_node.to_s) end end end end replacement_example.call end new_implementation = -> do class ActionText::Fragment def replace(selector) update do |source| source.css(selector).each do |node| replacement_node = yield(node) node.replace(replacement_node.to_s) if node != replacement_node end end end end replacement_example.call end Benchmark.ips do |x| x.report "Current implementation", &current_implementation x.report "New implementation", &new_implementation x.compare! end ``` Results: ``` Warming up -------------------------------------- Current implementation 2.000 i/100ms New implementation 5.000 i/100ms Calculating ------------------------------------- Current implementation 32.484 (±30.8%) i/s - 134.000 in 5.036419s New implementation 74.878 (±38.7%) i/s - 250.000 in 5.052168s Comparison: New implementation: 74.9 i/s Current implementation: 32.5 i/s - 2.31x (± 0.00) slower ```
1 parent a3e392f commit 14cd0c4

File tree

2 files changed

+29
-1
lines changed

2 files changed

+29
-1
lines changed

actiontext/lib/action_text/fragment.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ def update
3737
def replace(selector)
3838
update do |source|
3939
source.css(selector).each do |node|
40-
node.replace(yield(node).to_s)
40+
replacement_node = yield(node)
41+
node.replace(replacement_node.to_s) if node != replacement_node
4142
end
4243
end
4344
end

actiontext/test/unit/content_test.rb

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,33 @@ class ActionText::ContentTest < ActiveSupport::TestCase
134134
assert_match %r/\A#{Regexp.escape '<div class="trix-content">'}/, rendered
135135
end
136136

137+
test "replace certain nodes" do
138+
html = <<~HTML
139+
<div>
140+
<p>replace me</p>
141+
<p>ignore me</p>
142+
</div>
143+
HTML
144+
145+
expected_html = <<~HTML
146+
<div>
147+
<p>replaced</p>
148+
<p>ignore me</p>
149+
</div>
150+
HTML
151+
152+
content = content_from_html(html)
153+
replaced_fragment = content.fragment.replace("p") do |node|
154+
if node.text =~ /replace me/
155+
"<p>replaced</p>"
156+
else
157+
node
158+
end
159+
end
160+
161+
assert_equal expected_html.strip, replaced_fragment.to_html
162+
end
163+
137164
private
138165
def content_from_html(html)
139166
ActionText::Content.new(html).tap do |content|

0 commit comments

Comments
 (0)