Improve scan of regexp handling

[tompng]

Scanning regexp handling that I implemented in #1559 was not performant.

1. Run string.scan for each regexp handling once
2. Collect matched locations
3. Scan collected location from the beginning

[matzbot]

🚀 Preview deployment available at: https://b15e5470.rdoc-6cd.pages.dev (commit: 5561522)

[st0012]

What's the difference between char[pos...beg_pos] and text[pos...beg_pos]? That is, do we need to create this character array and join the characters to get the substring later?

[tompng]

Updated to text[pos...beg_pos]. There's no reason to use char array.

Here's the wrong reason that I used chars[range]:
Worst case, extremely long multibyte string, text[range] may not be performant.

s = 'a'*1000000+'α'
byte_range = range = 1000000-20..1000000-10
10000.times{s[range]} #=> 1.615413s
10000.times{chars[range].join} #=> 0.005913s
10000.times{s.byteslice(byte_range)} #=> 0.002450s

But... MatchData#begin was already slow in this case.

s.scan(/a/){Regexp.last_match} #=> 0.220335s
s.scan(/a/){Regexp.last_match.byteoffset 0}; #=> 0.252146s, requires ruby >= 3.2
s.scan(/a/){Regexp.last_match.begin 0}; #=> 18.810536s

In normal case: relatively short ascii-only text, it won't be a bottleneck.

[Copilot]

Pull request overview

This PR optimizes RDoc::Markup::Formatter#apply_regexp_handling by changing it from an incremental “find next earliest match” loop into a two-phase approach that scans each registered regexp once, collects match locations, then replays matches in order to build the final [text, converted?] segments.

Changes:

• Scan text once per regexp handling, collecting match ranges and metadata.
• Sort collected matches by start position (and registration priority) before emitting output segments.
• Emit handled/unhandled segments in a single forward pass through the sorted match list.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

lib/rdoc/markup/formatter.rb:95

• Using text.scan(pattern) enumerates non-overlapping matches for each regexp. The previous implementation used pattern.match(text, start) and could still find a later match that starts inside a previously found match for the same pattern if another regexp handling advanced start into that region (e.g., when two handlings begin at the same position and the earlier-registered one consumes fewer characters). With scan, those overlapping candidates are never collected, which can change output for some regexp handling patterns (including user-added ones, per RDoc::Markup#add_regexp_handling docs). Consider generating matches with a match loop (advancing by the capture end you actually consume) or otherwise ensuring overlapping matches are discoverable when the global pos advances into a previously-matched region.

    @markup.regexp_handlings.each_with_index do |(pattern, name), priority|
      text.scan(pattern) do
        m = Regexp.last_match
        idx = m[1] ? 1 : 0
        matched << [m.begin(idx), m.end(idx), m[idx], name, priority]
      end
    end

lib/rdoc/markup/formatter.rb:109

• This implementation collects all matches for all regexp handlings into matched and then sorts them. This increases peak memory (O(total_matches)) and adds an O(total_matches log total_matches) sort, which can be expensive for inputs with many small matches (e.g., quote handling matching every '/"`" character). If the goal is performance, consider a streaming merge (e.g., maintain the next match for each pattern and repeatedly pick the leftmost using a small priority queue) so you don’t have to materialize and sort the full match list.

    matched = []
    @markup.regexp_handlings.each_with_index do |(pattern, name), priority|
      text.scan(pattern) do
        m = Regexp.last_match
        idx = m[1] ? 1 : 0
        matched << [m.begin(idx), m.end(idx), m[idx], name, priority]
      end
    end
    # If the start positions are the same, prefer the earlier-registered one
    # (lower numeric priority from each_with_index).
    matched.sort_by! {|beg_pos, _, _, _, priority| [beg_pos, priority] }

    pos = 0
    output = []
    matched.each do |beg_pos, end_pos, s, name|
      next if beg_pos < pos

      output << [text[pos...beg_pos], false] if beg_pos != pos
      handled = public_send(:"handle_regexp_#{name}", s)
      output << [handled, true]
      pos = end_pos
    end