Merge pull request #2042 from davalillo/perf/search-text-line-lookup-pr

perf(search_text): precompute line offsets for O(log n) line lookup

CHANGELOG.md

@@ -24,6 +24,9 @@ Status of the `main` branch. Changes prior to the next official version change w
     actually used; the unconditional import added seconds to CLI/MCP startup on some machines (#2012)
   - Fix: Parallel agents auto-registering projects could overwrite each other's changes to the global
     project list in `serena_config.yml`
+  - Perf: `search_for_pattern` resolved each match's line number by rescanning the file from the
+    beginning (O(n) per match, O(n*m) total for m matches); coordinates are now resolved via the new
+    `TextCoordinates` abstraction (cached line starts + binary search)
   - Fix: `TextUtils.insert_text_at_position` returned a wrong position when the inserted text merged
     with an adjacent character into a single newline sequence (e.g. a `\n` inserted directly after an
     existing `\r`); the position is now determined from the resulting text

scripts/profile_search_text.py

@@ -0,0 +1,110 @@
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+"""Benchmark for search_text line-coordinate resolution (see PR "perf(search_text): precompute line offsets").
+
+Reproduces the before/after numbers quoted in the PR description:
+
+- "before" resolves each match's line number via ``TextUtils.get_line_from_index``,
+  which walks a TextStepper from index 0 (O(n) per match, O(n*m) for m matches)
+- "after" precomputes line start offsets once (O(n)) and resolves each match via
+  binary search (O(log n) per match)
+
+The content is fully synthetic and generated with a fixed seed: it contains no
+code from any real project. Both paths must produce identical results; the script
+asserts that before timing anything.
+
+Usage: uv run python scripts/profile_search_text.py [num_lines] [num_matches]
+"""
+
+import random
+import re
+import sys
+import time
+
+import solidlsp  # noqa: F401  # imported first: solidlsp must resolve before serena.util.text_utils (known circular-import window)
+from solidlsp.ls_utils import TextCoordinateProvider, TextUtils
+
+
+def generate_synthetic_content(num_lines: int, match_identifier: str, match_frequency: float = 0.15) -> str:
+    """
+    Generates synthetic pseudo-code content with a fixed seed.
+
+    :param num_lines: number of lines to generate
+    :param match_identifier: the identifier the benchmark pattern will search for
+    :param match_frequency: fraction of lines that reference the match identifier
+    :return: the synthetic content as a single string
+    """
+    rng = random.Random(1234)  # fixed seed for reproducibility
+    filler_identifiers = ["var_global_contador", "otra_funcion_aleatoria", "gestion_operaciones", "ajuste_niveles"]
+    lines = []
+    for i in range(num_lines):
+        kind = rng.random()
+        if kind < match_frequency:
+            lines.append(f"    double resultado_{i} = {match_identifier}({rng.randint(1, 999)});")
+        elif kind < match_frequency + 0.15:
+            lines.append(f"    if ({filler_identifiers[rng.randrange(len(filler_identifiers))]} > {rng.randint(0, 100)}) {{")
+        elif kind < match_frequency + 0.25:
+            lines.append("    }")
+        elif kind < match_frequency + 0.40:
+            lines.append(f"// comment line {i} with filler text {rng.randint(1000, 9999)}")
+        else:
+            lines.append(f"    int local_variable_{i} = {rng.randint(0, 5000)};")
+    return "\n".join(lines) + "\n"
+
+
+def resolve_before(content: str, compiled_pattern: re.Pattern) -> list[tuple[int, int]]:
+    """Resolves line coordinates the way upstream currently does (TextStepper from index 0 per match)."""
+    results = []
+    for match in compiled_pattern.finditer(content):
+        start_pos, end_pos = match.start(), match.end()
+        start_line_num = TextUtils.get_line_from_index(content, start_pos)
+        end_line_num = TextUtils.get_line_from_index(content, end_pos)
+        if end_line_num > start_line_num and TextUtils.get_line_col_from_index(content, end_pos)[1] == 0:
+            end_line_num -= 1
+        results.append((start_line_num, end_line_num))
+    return results
+
+
+def resolve_after(content: str, compiled_pattern: re.Pattern) -> list[tuple[int, int]]:
+    """Resolves line coordinates the way this PR proposes (cached line starts + binary search)."""
+    coordinates = TextCoordinateProvider(content)
+    results = []
+    for match in compiled_pattern.finditer(content):
+        start_pos, end_pos = match.start(), match.end()
+        s = coordinates.compute_coordinates(start_pos).line
+        end_loc = coordinates.compute_coordinates(end_pos)
+        e = end_loc.line
+        if e > s and end_loc.col == 0:
+            e -= 1
+        results.append((s, e))
+    return results
+
+
+def main() -> None:
+    num_lines = int(sys.argv[1]) if len(sys.argv) > 1 else 12000
+    match_identifier = "funcion_calcula_parametro"
+    content = generate_synthetic_content(num_lines, match_identifier)
+    compiled_pattern = re.compile(match_identifier)
+
+    num_matches = len(list(compiled_pattern.finditer(content)))
+    print(f"synthetic file: {content.count(chr(10)):,} lines, {len(content):,} chars, {num_matches} matches")
+
+    before = resolve_before(content, compiled_pattern)
+    after = resolve_after(content, compiled_pattern)
+    assert before == after, "implementations disagree; this script only measures identical work"
+    print(f"results identical: OK ({len(before)} matches)")
+
+    for name, fn in [
+        ("before (TextStepper, O(n) per match)", lambda: resolve_before(content, compiled_pattern)),
+        ("after (bisect, O(log n) per match)", lambda: resolve_after(content, compiled_pattern)),
+    ]:
+        times = []
+        for _ in range(5):
+            t0 = time.perf_counter()
+            fn()
+            times.append(time.perf_counter() - t0)
+        print(f"{name}: {min(times) * 1000:.2f} ms (best of 5)")
+
+
+if __name__ == "__main__":
+    main()

src/serena/util/text_utils.py

@@ -14,7 +14,7 @@ from joblib import Parallel, delayed
 from sensai.util.string import ToStringMixin
 
 from serena.util.file_proxy import FileCollection, FileProxy
-from solidlsp.ls_utils import TextUtils
+from solidlsp.ls_utils import TextCoordinateProvider, TextCoordinates, TextUtils
 
 if TYPE_CHECKING:
     from serena.code_editor import CodeEditor
@@ -155,6 +155,10 @@ def search_text(
     lines = TextUtils.split_lines(content)
     total_lines = len(lines)
 
+    # precompute line start offsets once so that each match's coordinates can be resolved via binary search
+    # instead of re-scanning the text from the beginning for every match
+    coordinates = TextCoordinateProvider(content)
+
     # For multiline matches, optionally use DOTALL so '.' matches newlines
     flags = (re.MULTILINE | re.DOTALL) if multiline else 0
     compiled_pattern = re.compile(pattern, flags)
@@ -164,9 +168,10 @@ def search_text(
         end_pos = match.end()
 
         # Find the line numbers for the start and end positions
-        start_line_num = TextUtils.get_line_from_index(content, start_pos)
-        end_line_num = TextUtils.get_line_from_index(content, end_pos)
-        if end_line_num > start_line_num and TextUtils.get_line_col_from_index(content, end_pos)[1] == 0:
+        start_loc = coordinates.compute_coordinates(start_pos)
+        end_loc = coordinates.compute_coordinates(end_pos)
+        start_line_num, end_line_num = start_loc.line, end_loc.line
+        if end_line_num > start_line_num and end_loc.col == 0:
             # `end_pos` is exclusive, so if it is at the start of a line, the match ends with the
             # preceding line's newline and does not extend into the line that `end_pos` points to
             end_line_num -= 1
@@ -907,19 +912,7 @@ class MultiFileReplacement:
         return MultiFileReplacementResult({path: len(occs) for path, occs in occurrences_by_file.items()})
 
 
-@dataclass
-class TextCoords:
-    line: int
-    """
-    0-based line number
-    """
-    col: int
-    """
-    0-based column number
-    """
-
-
-def find_text_coordinates(content: str, regex: str, require_unique: bool = False) -> TextCoords | None:
+def find_text_coordinates(content: str, regex: str, require_unique: bool = False) -> TextCoordinates | None:
     """
     Finds the line and column number of the first match of a regex pattern in the given content.
 
@@ -944,7 +937,7 @@ def find_text_coordinates(content: str, regex: str, require_unique: bool = False
             raise ValueError(f"Regex must contain exactly one group to capture the position, but found {len(match.groups())} groups.")
         index_in_content = match.start(1)
         line, col = TextUtils.get_line_col_from_index(content, index_in_content)
-        return TextCoords(line, col)
+        return TextCoordinates(line, col)
 
 
 class TextOutputUtils:

src/solidlsp/ls_utils.py

@@ -3,6 +3,7 @@ This file contains various utility functions like I/O operations, handling paths
 """
 # SPDX-License-Identifier: MIT
 
+import bisect
 import gzip
 import hashlib
 import logging
@@ -15,6 +16,7 @@ import tempfile
 import uuid
 import zipfile
 from collections.abc import Sequence
+from dataclasses import dataclass
 from enum import Enum
 from pathlib import Path, PurePath
 from typing import Literal, cast
@@ -179,6 +181,70 @@ class TextStepper:
         return lines
 
 
+@dataclass
+class TextCoordinates:
+    """
+    Represents a position in a text as a pair of 0-based line and column numbers.
+    """
+
+    line: int
+    """the 0-based line number"""
+
+    col: int
+    """the 0-based column number"""
+
+
+class TextCoordinateProvider:
+    """
+    Accelerates multiple computations of line/column coordinates in a given text by precomputing the text's line start indices.
+    """
+
+    def __init__(self, text: str):
+        """
+        :param text: the text in which character indices are to be located
+        """
+        self._text = text
+        self._line_starts = self._compute_line_starts()
+
+    def compute_coordinates(self, index: int) -> TextCoordinates:
+        r"""
+        Returns the line/column coordinates corresponding to the given character index.
+
+        An index pointing at the "\n" of a "\r\n" sequence denotes the beginning of the following line
+        (column 0), in the same way as a cursor insertion position between "\r" and "\n" does.
+
+        :param index: the 0-based index in the text; must not exceed the text length
+        :return: the coordinates corresponding to the index
+        :raises InvalidTextLocationError: if the index is negative or greater than the text length
+        """
+        # determine the line containing the index, which is the last line whose start does not exceed the index
+        if index < 0 or index > len(self._text):
+            raise InvalidTextLocationError(f"{index=}")
+        line_num = bisect.bisect_right(self._line_starts, index) - 1
+        line_start = self._line_starts[line_num]
+
+        # an index pointing at the "\n" of a "\r\n" pair maps to the beginning of the following line
+        if index > 0 and self._text[index - 1] == "\r" and self._text[index : index + 1] == "\n":
+            return TextCoordinates(line=line_num + 1, col=0)
+
+        return TextCoordinates(line=line_num, col=index - line_start)
+
+    def _compute_line_starts(self) -> list[int]:
+        """
+        Computes the character offsets at which the lines of the text begin, using a TextStepper to process
+        the text line by line.
+
+        :return: a list where entry i is the 0-based character offset at which line i starts;
+            entry 0 is always 0
+        """
+        line_starts = [0]
+        text_stepper = TextStepper(self._text)
+        while text_stepper.step_line():
+            if text_stepper.is_newline:
+                line_starts.append(text_stepper.line_start_idx)
+        return line_starts
+
+
 class TextUtils:
     """
     Utilities for text operations.

test/serena/test_text_utils.py

@@ -197,6 +197,41 @@ class TestSearchText:
 
         assert len(matches) == 0
 
+    def test_search_text_crlf_line_numbers(self):
+        """Matches in CRLF content report the same line numbers as with LF endings."""
+        crlf = "alpha\r\nbeta\r\ngamma\r\n"
+        lf = "alpha\nbeta\ngamma\n"
+        crlf_matches = search_text("beta", content=crlf)
+        lf_matches = search_text("beta", content=lf)
+        assert len(crlf_matches) == 1
+        assert len(lf_matches) == 1
+        assert crlf_matches[0].start_line == lf_matches[0].start_line == 1
+        assert crlf_matches[0].end_line == lf_matches[0].end_line == 1
+
+    def test_search_text_bare_cr_line_numbers(self):
+        r"""Bare \r line endings produce separate lines, matching TextStepper semantics."""
+        content = "alpha\rbeta\r"
+        matches = search_text("beta", content=content)
+        assert len(matches) == 1
+        assert matches[0].start_line == 1
+        assert matches[0].end_line == 1
+
+    def test_search_text_match_at_boundaries(self):
+        """Matches at the very start and very end of the content resolve to sane line numbers."""
+        content = "first\nmiddle\nlast"
+        first = search_text("first", content=content)
+        assert first[0].start_line == 0
+        last = search_text("last", content=content)
+        assert last[0].start_line == 2
+
+    def test_search_text_multiline_match_line_range(self):
+        """A multiline match spanning several lines reports the full matched range."""
+        content = "a\nTARGET_START\nb\nc\nTARGET_END\nd\n"
+        matches = search_text("TARGET_START[\\s\\S]*?TARGET_END", content=content)
+        assert len(matches) == 1
+        assert matches[0].start_line == 1
+        assert matches[0].end_line == 4
+
 
 # Mock file reader that always returns matching content
 def mock_reader_always_match(file_path: str) -> str:

test/solidlsp/test_text_coordinates.py

@@ -0,0 +1,101 @@
+import pytest
+
+from solidlsp.ls_utils import InvalidTextLocationError, TextCoordinateProvider, TextCoordinates
+
+
+class TestTextCoordinates:
+    """Tests for the public contract of TextCoordinates and LineCol."""
+
+    @pytest.mark.parametrize(
+        "content",
+        [
+            "hello\nworld\n",
+            "hello\r\nworld\r\n",
+            "hello\rworld\r",
+            "a\rb\nc\r\nd\r\n",
+            "",
+            "no newline at end",
+            "\n",
+            "\r",
+            "a\r\n\r\nb",
+        ],
+    )
+    def test_line_col_at_index_end_to_end(self, content):
+        """Every index within the text resolves to a position that is consistent with the line layout."""
+        coordinates = TextCoordinateProvider(content)
+        for index in range(len(content) + 1):
+            loc = coordinates.compute_coordinates(index)
+            assert loc.line >= 0
+            assert loc.col >= 0
+            # a col greater than 0 implies that none of the col characters before the index is a line separator
+            if loc.col > 0:
+                preceding = content[index - loc.col : index]
+                assert "\n" not in preceding and "\r" not in preceding
+
+    def test_line_col_at_index_basic_lf(self):
+        """Coordinates for LF-only content match the manually derived line layout."""
+        coordinates = TextCoordinateProvider("alpha\nbeta\ngamma\n")
+        assert coordinates.compute_coordinates(0) == TextCoordinates(line=0, col=0)
+        assert coordinates.compute_coordinates(3) == TextCoordinates(line=0, col=3)
+        assert coordinates.compute_coordinates(5) == TextCoordinates(line=0, col=5)
+        assert coordinates.compute_coordinates(6) == TextCoordinates(line=1, col=0)
+        assert coordinates.compute_coordinates(8) == TextCoordinates(line=1, col=2)
+        assert coordinates.compute_coordinates(11) == TextCoordinates(line=2, col=0)
+        assert coordinates.compute_coordinates(12) == TextCoordinates(line=2, col=1)
+        assert coordinates.compute_coordinates(16) == TextCoordinates(line=2, col=5)
+        # an index at the end of the text (after the trailing newline) denotes the start of a new line
+        assert coordinates.compute_coordinates(17) == TextCoordinates(line=3, col=0)
+
+    def test_line_col_at_index_crlf(self):
+        r"""CRLF sequences count as a single line ending. An index pointing at the "\n" of a pair denotes
+        the beginning of the following line (column 0), whereas the "\r" itself still belongs to the
+        preceding line.
+        """
+        coordinates = TextCoordinateProvider("alpha\r\nbeta\r\n")
+        assert coordinates.compute_coordinates(0) == TextCoordinates(line=0, col=0)
+        assert coordinates.compute_coordinates(5) == TextCoordinates(line=0, col=5)
+        assert coordinates.compute_coordinates(6) == TextCoordinates(line=1, col=0)
+        assert coordinates.compute_coordinates(7) == TextCoordinates(line=1, col=0)
+        assert coordinates.compute_coordinates(8) == TextCoordinates(line=1, col=1)
+        assert coordinates.compute_coordinates(11) == TextCoordinates(line=1, col=4)
+        assert coordinates.compute_coordinates(12) == TextCoordinates(line=2, col=0)
+        assert coordinates.compute_coordinates(13) == TextCoordinates(line=2, col=0)
+
+    def test_line_col_at_index_bare_cr(self):
+        r"""Bare "\r" characters act as line separators."""
+        coordinates = TextCoordinateProvider("alpha\rbeta\r")
+        assert coordinates.compute_coordinates(5) == TextCoordinates(line=0, col=5)
+        assert coordinates.compute_coordinates(6) == TextCoordinates(line=1, col=0)
+        assert coordinates.compute_coordinates(10) == TextCoordinates(line=1, col=4)
+        assert coordinates.compute_coordinates(11) == TextCoordinates(line=2, col=0)
+
+    def test_line_col_at_index_mixed(self):
+        """Mixed line endings resolve consistently in a single text."""
+        coordinates = TextCoordinateProvider("a\r\nb\rc\nd\r\n")
+        # "\r\n" ends line 0; "\r" ends line 1; "\n" ends line 2; "\r\n" ends line 3
+        assert coordinates.compute_coordinates(0) == TextCoordinates(line=0, col=0)
+        assert coordinates.compute_coordinates(1) == TextCoordinates(line=0, col=1)
+        assert coordinates.compute_coordinates(2) == TextCoordinates(line=1, col=0)
+        assert coordinates.compute_coordinates(3) == TextCoordinates(line=1, col=0)
+        assert coordinates.compute_coordinates(4) == TextCoordinates(line=1, col=1)
+        assert coordinates.compute_coordinates(5) == TextCoordinates(line=2, col=0)
+        assert coordinates.compute_coordinates(6) == TextCoordinates(line=2, col=1)
+        assert coordinates.compute_coordinates(7) == TextCoordinates(line=3, col=0)
+        assert coordinates.compute_coordinates(8) == TextCoordinates(line=3, col=1)
+        assert coordinates.compute_coordinates(9) == TextCoordinates(line=4, col=0)
+        assert coordinates.compute_coordinates(10) == TextCoordinates(line=4, col=0)
+
+    def test_line_col_at_index_empty_content(self):
+        """The only valid index in empty content resolves to the origin."""
+        coordinates = TextCoordinateProvider("")
+        assert coordinates.compute_coordinates(0) == TextCoordinates(line=0, col=0)
+        with pytest.raises(InvalidTextLocationError):
+            coordinates.compute_coordinates(1)
+
+    def test_line_col_at_index_out_of_range(self):
+        """Indices beyond the text length are rejected."""
+        coordinates = TextCoordinateProvider("abc")
+        with pytest.raises(InvalidTextLocationError):
+            coordinates.compute_coordinates(4)
+        with pytest.raises(InvalidTextLocationError):
+            coordinates.compute_coordinates(-1)