# HG changeset patch # User Georges Racinet <georges.racinet@octobus.net> # Date 1696260259 -7200 # Mon Oct 02 17:24:19 2023 +0200 # Branch stable # Node ID 830f307161b0530b86b26bf81831a6dfc31f690a # Parent 798ac9c4d639537f8750fd3bfbf6e1f4dc7c91cd ListLastCommitForTree: avoid climbing the entire changelog for each path The new algorithm in `latest_changeset_for_path` was more efficient on average than using the `files` revset predicate on a single path, but it was inefficient to do it over and over for a serious amount of paths in `ListLastCommitsForTree`. We now climb the changelog only once, keeping track of wanted files and subdirectories along the way. Measurements taken on the `tests/` directory of the mercurial-devel repository (in seconds): revset based: 270s parent changeset: 426s this changeset: 19s Figures taken before a small fix on the new implementation that could have improved performance. This is still quite bad, but less catastrophic than it was, and it is amenable to a Rust reimplementation. diff --git a/hgitaly/service/commit.py b/hgitaly/service/commit.py --- a/hgitaly/service/commit.py +++ b/hgitaly/service/commit.py @@ -669,14 +669,15 @@ # subtrees first, then regular files, each one in lexicographical order subtrees, file_paths = manifest.miner(from_ctx).ls_dir(req_path) + changesets = latest_changesets_for_paths(subtrees, file_paths, + from_ctx) all_paths = subtrees all_paths.extend(file_paths) - for chunk in chunked(all_paths[offset:offset + limit]): yield ListLastCommitsForTreeResponse( commits=[ message.commit_for_tree( - latest_changeset_for_path(path, from_ctx), + changesets[path], path ) for path in chunk @@ -1049,6 +1050,45 @@ return changeset +def latest_changesets_for_paths(subdirs, file_paths, seen_from): + """Return latest ancestor of ``seen_from`` that touched the given path. + + :param bytes path: subdir or file + :param seen_from: changectx + """ + dir_prefixes = set(d.rstrip(b'/') + b'/' for d in subdirs) + file_paths = set(file_paths) + + latest_changesets = {} + + for changeset in ancestors_inclusive(seen_from): + if not dir_prefixes and not file_paths: + break + + for cfp in changeset.files(): + # TODO some performance improvement possible with a binary + # search if changeset files are guaranteed to be sorted + # but it'd probably make more of a difference in a Rust impl + for dp in dir_prefixes: + if cfp.startswith(dp): + latest_changesets[dp.rstrip(b'/')] = changeset + dir_prefixes.discard(dp) + break + else: + for fp in file_paths: + if fp == cfp: + latest_changesets[fp] = changeset + break + else: + continue + file_paths.discard(fp) + continue + + dir_prefixes.discard(dp) + + return latest_changesets + + def blamelines(repo, ctx, file): """Yield blame lines of a file. """ diff --git a/hgitaly/service/tests/test_commit.py b/hgitaly/service/tests/test_commit.py --- a/hgitaly/service/tests/test_commit.py +++ b/hgitaly/service/tests/test_commit.py @@ -1140,6 +1140,13 @@ (b'foo', sha3), ] + # case when we don't have to walk the entire changelog + (wrapper.path / 'more').mkdir() + sha5 = wrapper.commit_file('more/foo').hex().decode() + assert do_rpc(sha5, b'more/') == [ + (b'more/foo', sha5), + ] + def test_raw_blame(grpc_channel, server_repos_root): grpc_stub = CommitServiceStub(grpc_channel)