diff --git a/hgitaly/service/repository.py b/hgitaly/service/repository.py index e05bb0fcaae90f2505e89f44256bf3726b3199d8_aGdpdGFseS9zZXJ2aWNlL3JlcG9zaXRvcnkucHk=..e40148cc11fe22c70883f27aeb0645f979f37768_aGdpdGFseS9zZXJ2aWNlL3JlcG9zaXRvcnkucHk= 100644 --- a/hgitaly/service/repository.py +++ b/hgitaly/service/repository.py @@ -11,6 +11,7 @@ import logging import os from pathlib import Path +from collections import deque import re import shutil import tempfile @@ -437,27 +438,14 @@ # (surely deprecated) and the Rails client does not read it # TODO cheaper iteration on a window of lines (note that # splitlines() takes care of `\r\n` and even `\r` EOLs.) - lines = content.splitlines() - latest_upper_bound = None - for lineno, line in enumerate(lines): - if rx.search(line): - if latest_upper_bound is None: - lower_bound = lineno - 2 - else: - if latest_upper_bound < lineno - 2: - yield from iter_sfbc_resps(match_data) - lower_bound = max(latest_upper_bound, lineno - 2) - latest_upper_bound = render_git_grep_matches( - match_data, - ref, path, - lower_bound, lineno + 2, - lines) - - match_len = match_data.tell() - if not match_len: - continue - - yield from iter_sfbc_resps(match_data) + for matching_lines in grep_file(rx, content): + render_git_grep_matches( + match_data, + ref, path, + matching_lines) + yield from iter_sfbc_resps(match_data) + match_data.truncate(0) + match_data.seek(0) def set_custom_hooks(self, request, context): def load_repo(req, context): @@ -768,6 +756,6 @@ raise RepositoryCreationError(repository) -def render_git_grep_matches(buf, ref, path, start_line, end_line, lines): +def render_git_grep_matches(buf, ref, path, enum_lines): """Render a slice of lines as git grep does. @@ -772,5 +760,20 @@ """Render a slice of lines as git grep does. - :param:`start_line` and param:`end_line`: follow the indexing convention: - counting from 0. + :param enum_lines: iterable of pairs `(line_no, line)` + """ + for lineno, line in enum_lines: + buf.write(b'%s:%s\x00%d\x00%s\n' % (ref, path, lineno, line)) + + +SPLITLINES_RX = re.compile(br'(.*?)(\r\n?|\n|\Z)', re.MULTILINE) + + +def grep_file(rx, data, context_width=2): + """Iterator yielding matches the given regular expression with context + + This implementation avoids duplicating the data in memory. + + :param int context_width: number of lines before and after to include + if possible (same as `grep -C`) + :returns: pairs `(line_no, line)` """ @@ -776,8 +779,49 @@ """ - upper_bound = min(end_line + 1, len(lines)) # exclusive - for i in range(max(start_line, 0), upper_bound): - buf.write(b'%s:%s\x00%d\x00%s\n' % (ref, path, i + 1, lines[i])) - return upper_bound + prev_match_line_no = None + current_window = deque() # current line and up to 2 lines before + match_lines = [] + # According the the best ranked answer for + # https://stackoverflow.com/questions/3054604, + # this regexp-based splitting for lines can be a bit slower than + # `iter(splitlines)` (things may have changes since then, though). + # Yet, our biggest concern is to avoid exhausting HGitaly's RAM budget + # if this happens to run on large files. + # Unfortunately, we already have to load the entire data in RAM because + # it is typically Mercurial file content, we don't want to do it once + # more in the form of lines. In both cases a bytes string + # is allocated for each line, but that is harder to prevent and they + # should be deallocated at each iteration (no cycles). + # Unless there is a bug in the current approach, it is not worth the + # effort to try and further improve memory efficiency: implementing + # in RHGitaly would be the way to go. + for line_idx, line_m in enumerate(SPLITLINES_RX.finditer(data)): + line = line_m.group(1) + if not line: + continue + line_no = line_idx + 1 + current_window.append((line_no, line)) + if rx.search(line): + if ( + prev_match_line_no is None + or line_no - prev_match_line_no > 2 * context_width + ): + match_lines = list(current_window) + else: + match_lines.append((line_no, line)) + prev_match_line_no = line_no + elif ( + prev_match_line_no is not None + and line_no <= prev_match_line_no + context_width + ): + match_lines.append((line_no, line)) + elif match_lines: + yield match_lines + match_lines = [] + + if len(current_window) > context_width: + current_window.popleft() + if match_lines: + yield match_lines def iter_sfbc_resps(match_data: BytesIO): diff --git a/hgitaly/service/tests/test_repository_service.py b/hgitaly/service/tests/test_repository_service.py index e05bb0fcaae90f2505e89f44256bf3726b3199d8_aGdpdGFseS9zZXJ2aWNlL3Rlc3RzL3Rlc3RfcmVwb3NpdG9yeV9zZXJ2aWNlLnB5..e40148cc11fe22c70883f27aeb0645f979f37768_aGdpdGFseS9zZXJ2aWNlL3Rlc3RzL3Rlc3RfcmVwb3NpdG9yeV9zZXJ2aWNlLnB5 100644 --- a/hgitaly/service/tests/test_repository_service.py +++ b/hgitaly/service/tests/test_repository_service.py @@ -644,7 +644,8 @@ (sub / 'bar').write_text('line1\nline2\nbar content\nline4\nline5') (sub / 'ba2').write_text('line1\nba2 content') (sub / 'ba3').write_text('ba3 content\nline2') - (sub / 'ba4').write_text('l1\nl2\nl3\nba4 content\nl5') + # this one has Windows and MacOS Classic line endings: + (sub / 'ba4').write_text('l1\nl2\r\nl3\rba4 content\nl5') (sub / 'ba5').write_text('m1\nm2\nm3\nm4\nm5\nm6\nba5 content\n') long_content = 'very large content\n' * 3000 shutil.copy2(TEST_DATA_DIR / 'backup_additional_no_git.tar', @@ -668,7 +669,8 @@ b'branch/default:sub/bar\x003\x00bar content', b'branch/default:sub/bar\x004\x00line4', b'branch/default:sub/bar\x005\x00line5', - b''], + b'', + ], END_OF_MATCH, ] @@ -676,9 +678,10 @@ assert search(query="^ba2.c") == [ [b'branch/default:sub/ba2\x001\x00line1', b'branch/default:sub/ba2\x002\x00ba2 content', - b''], + b'', + ], END_OF_MATCH, ] assert search(query="^ba3.c") == [ [b'branch/default:sub/ba3\x001\x00ba3 content', b'branch/default:sub/ba3\x002\x00line2', @@ -680,9 +683,10 @@ END_OF_MATCH, ] assert search(query="^ba3.c") == [ [b'branch/default:sub/ba3\x001\x00ba3 content', b'branch/default:sub/ba3\x002\x00line2', - b''], + b'', + ], END_OF_MATCH, ] @@ -692,7 +696,8 @@ b'branch/default:sub/ba4\x003\x00l3', b'branch/default:sub/ba4\x004\x00ba4 content', b'branch/default:sub/ba4\x005\x00l5', - b''], + b'', + ], END_OF_MATCH, ] @@ -703,7 +708,8 @@ b'branch/default:sub/ba4\x003\x00l3', b'branch/default:sub/ba4\x004\x00ba4 content', b'branch/default:sub/ba4\x005\x00l5', - b''], + b'', + ], END_OF_MATCH, ] assert search(query="^m1|ba5.c") == [ diff --git a/tests_with_gitaly/test_repository_service.py b/tests_with_gitaly/test_repository_service.py index e05bb0fcaae90f2505e89f44256bf3726b3199d8_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9yZXBvc2l0b3J5X3NlcnZpY2UucHk=..e40148cc11fe22c70883f27aeb0645f979f37768_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9yZXBvc2l0b3J5X3NlcnZpY2UucHk= 100644 --- a/tests_with_gitaly/test_repository_service.py +++ b/tests_with_gitaly/test_repository_service.py @@ -637,7 +637,11 @@ (sub / 'bar').write_text('line1\nline2\nbar content\nline4\nline5') (sub / 'ba2').write_text('line1\nba2 content') (sub / 'ba3').write_text('ba3 content\nline2') - (sub / 'ba4').write_text('l1\nl2\nl3\nba4 content\nline6') + # This one has a Windows endings, and exhibits that `git grep` normalizes + # to `\n`. Also Git does not interpret the MacOS classic line ending + # '\r' and we do. In that case, we can claim our response to be more + # correct and we will not compare it. + (sub / 'ba4').write_text('l1\r\nl2\nl3\nba4 content\nline6') (sub / 'ba5').write_text('m1\nm2\nm3\nm4\nm5\nm6\nba5 content\n') (wrapper.path / 'large').write_text('very large content\n' * 3000) # TODO OS indep for paths (actually TODO make wrapper.commit easier to