From d4d49bc0628698b65d73801c58e68df16f249253 Mon Sep 17 00:00:00 2001 From: Pablo Brasero Date: Wed, 24 Jun 2026 13:23:42 +0100 Subject: [PATCH] Keep linkification regexps in memory MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit I was playing with [derailed_benchmarks](https://github.com/zombocom/derailed_benchmarks) today and noticed that history pages were spending a long time in `Regexp#initialize`. Example: ``` $ PATH_TO_HIT=/node/29/history be derailed exec perf:stackprof Booting: production (...) Method: GET Endpoint: "/node/29/history" (...) ================================== Mode: cpu(1000) Samples: 3716 (15.58% miss rate) GC: 343 (9.23%) ================================== TOTAL (pct) SAMPLES (pct) FRAME 529 (14.2%) 529 (14.2%) Regexp#initialize 230 (6.2%) 230 (6.2%) (marking) 165 (4.4%) 165 (4.4%) Dir.glob 163 (4.4%) 163 (4.4%) Zlib::Inflate#inflate 113 (3.0%) 113 (3.0%) (sweeping) 88 (2.4%) 78 (2.1%) String#gsub (...) ``` Note that this is measured while running the app in production mode. I went poking and saw that the [18 patterns (as of these lines)](https://github.com/openstreetmap/openstreetmap-website/blob/5053e2e76b82031b54cba71944d50128929459b7/config/settings.yml#L146-L182) used to detect text to linkify are being used to instantiate `Regexp` every time that linkification takes place. Looks like this takes a significant amount of time. This PR moves the instantiation to a class method where the result is memoized. This is the result, for comparison: ``` $ PATH_TO_HIT=/node/29/history be derailed exec perf:stackprof Booting: production (...) Method: GET Endpoint: "/node/29/history" (...) ================================== Mode: cpu(1000) Samples: 3521 (19.50% miss rate) GC: 329 (9.34%) ================================== TOTAL (pct) SAMPLES (pct) FRAME 226 (6.4%) 226 (6.4%) (marking) 184 (5.2%) 184 (5.2%) Zlib::Inflate#inflate 131 (3.7%) 119 (3.4%) String#gsub 102 (2.9%) 102 (2.9%) (sweeping) 79 (2.2%) 67 (1.9%) Set#initialize 60 (1.7%) 60 (1.7%) PG::Connection#exec_prepared ``` Or in terms of requests per second, this is the code currently in master: ``` $ PATH_TO_HIT=/node/29/history bundle exec derailed exec perf:ips Booting: production (...) Database 'openstreetmap' already exists Method: GET Endpoint: "/node/29/history" ruby 4.0.5 (2026-05-20 revision 64336ffd0e) +YJIT +PRISM [x86_64-linux] Warming up -------------------------------------- ips 1.000 i/100ms Calculating ------------------------------------- ips 23.713 (±25.3%) i/s (42.17 ms/i) - 119.000 in 5.018360s ``` And this is this PR: ``` > PATH_TO_HIT=/node/29/history bundle exec derailed exec perf:ips Booting: production (...) Method: GET Endpoint: "/node/29/history" ruby 4.0.5 (2026-05-20 revision 64336ffd0e) +YJIT +PRISM [x86_64-linux] Warming up -------------------------------------- ips 1.000 i/100ms Calculating ------------------------------------- ips 30.870 (±25.9%) i/s (32.39 ms/i) - 155.000 in 5.021047s ``` So that's from ~24 ips to ~30 ips, or 25% more requests per second. Caveat: the tests for `RichText` set up custom patterns, assuming no memoization. To work around this, I have added a `RichText.reset_state` method that is run before each relevant test. --- lib/rich_text.rb | 32 +++++++++++++++++++------------- test/lib/rich_text_test.rb | 9 ++++++++- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/lib/rich_text.rb b/lib/rich_text.rb index bee6da923..d09fce5ca 100644 --- a/lib/rich_text.rb +++ b/lib/rich_text.rb @@ -13,6 +13,23 @@ module RichText end end + def self.reset_state + @gsub_pairs_for_linkify_detection = nil + end + + def self.gsub_pairs_for_linkify_detection + @gsub_pairs_for_linkify_detection ||= + Array + .wrap(Settings.linkify&.detection_rules) + .select { |rule| rule.path_template && rule.patterns.is_a?(Array) } + .flat_map do |rule| + expanded_path = "#{rule.host || "#{Settings.server_protocol}://#{Settings.server_url}"}/#{rule.path_template}" + rule.patterns + .grep(String) + .map { |pattern| [Regexp.new("(?<=^|#{URL_UNSAFE_CHARS})#{pattern}", Regexp::IGNORECASE, :timeout => 1), expanded_path] } + end + end + class SimpleFormat include ActionView::Helpers::TextHelper include ActionView::Helpers::OutputSafetyHelper @@ -104,20 +121,9 @@ module RichText private - def gsub_pairs_for_linkify_detection - Array - .wrap(Settings.linkify&.detection_rules) - .select { |rule| rule.path_template && rule.patterns.is_a?(Array) } - .flat_map do |rule| - expanded_path = "#{rule.host || "#{Settings.server_protocol}://#{Settings.server_url}"}/#{rule.path_template}" - rule.patterns - .grep(String) - .map { |pattern| [Regexp.new("(?<=^|#{URL_UNSAFE_CHARS})#{pattern}", Regexp::IGNORECASE, :timeout => 1), expanded_path] } - end - end - def expand_link_shorthands(text) - gsub_pairs_for_linkify_detection + RichText + .gsub_pairs_for_linkify_detection .reduce(text) { |text, (pattern, replacement)| text.gsub(pattern, replacement) } end diff --git a/test/lib/rich_text_test.rb b/test/lib/rich_text_test.rb index d1199b3b6..2505ddb8c 100644 --- a/test/lib/rich_text_test.rb +++ b/test/lib/rich_text_test.rb @@ -5,6 +5,10 @@ require "test_helper" class RichTextTest < ActiveSupport::TestCase include Rails::Dom::Testing::Assertions::SelectorAssertions + def setup + RichText.reset_state + end + def test_html_to_html r = RichText.new("html", "foo http://example.com/ bar") assert_html r do @@ -368,7 +372,7 @@ class RichTextTest < ActiveSupport::TestCase end end - def test_text_to_html_linkify_recognize_wiki_path + def test_text_to_html_linkify_recognize_wiki_key_value_path with_settings(:linkify => { :detection_rules => [{ :patterns => ["(?[^\"?#<>/\\s]+)=(?[^\"?#<>\\s]+)"], :path_template => "Tag:\\k=\\k", :host => "http://example.wiki" }] }) do r = RichText.new("text", "foo surface=metal bar") assert_html r do @@ -378,6 +382,9 @@ class RichTextTest < ActiveSupport::TestCase end end end + end + + def test_text_to_html_linkify_recognize_wiki_key_path with_settings(:linkify => { :detection_rules => [{ :patterns => ["(?[^\"?#<>/\\s]+)=\\*?"], :path_template => "Key:\\k", :host => "http://example.wiki" }] }) do r = RichText.new("text", "foo surface=* bar") assert_html r do -- 2.47.3