Commit 3dab45d3 authored by Georges Racinet's avatar Georges Racinet 🦑
Browse files

Markdown renderings: lookup for Mercurial short nodes

The `CommitReferenceFilter` class is the one responsible for
the rendering.

We're *adding* resolution of truncated Mercurial Node ids for
the case of the `hg_git` VCS type, relying on the existing
map of full Mercurial SHA (Node id) to Git SHA.

This is very inefficient, and proper calls with low startup
overhead to a persistent Mercurial nodemap would do much better
above a certain repository size (persistent nodemap is provided
by Rust native extensions, and low overhead would be achieved only
by a fastpath without Python or a long-running process, such as
HGitaly)

While the `Git::Commit.batch_by_oid` method
doesn't seem to be called except from the `#commits_by` method of the
`Repository` model, the latter is used in several places
of the application, with risks of breakage and/or severe performance
degradation. For `hg_git` VCS type, most callers don't need the Mercurial
resolution. That's why we're executing the new lookup only if
the new `hg_full_resolution` argument is set to `true`.

Preliminary performance analysis (non scientific, on my workstation)
shows that in the 100k changesets range (pypy repo) this naive lookup takes
about 30ms, whereas `hg log -T` is in the 100-200ms ballpark.
Around 500k changesets (mozilla-central repo), times have the same order
of magnitude (around 100-200ms again). To insist, this is with the hg startup
overhead and without the Rust persistent nodemap (would take less than 1ms).
We fully expect the current hg<->git maps to be an unbereable performance
problem in Heptapod anyway around 500k to 1M changesets anyway, only solved
by HGitaly.

All in all, the performance question seems to be acceptable in the current
inefficient context *for Note rendering*.
HGitaly would provide an efficient RPC call anyway.

--HG--
branch : heptapod-stable
parent 0368ff9b95c8
Pipeline #10833 skipped with stages
......@@ -55,8 +55,8 @@ def commit_by(oid:)
repository.commit_by(oid: oid)
end
def commits_by(oids:)
repository.commits_by(oids: oids)
def commits_by(oids:, **options)
repository.commits_by(oids: oids, **options)
end
def repository
......
......@@ -137,10 +137,18 @@ def commit_by(oid:)
@commit_cache[oid] = find_commit(oid)
end
def commits_by(oids:)
# The hg_full_resolution triggers lookup by Mercurial SHA as well as Git SHAs
# It is meaningful for `hg_git` VCS types and will be ignored both for
# `git` and `hg` (native) VCS types.
def commits_by(oids:, hg_full_resolution: false)
return [] unless oids.present?
# necessary to keep unconditionally because we have so many old
# references in notes Markdown and the like there
commits = Gitlab::Git::Commit.batch_by_oid(raw_repository, oids)
if hg_full_resolution && vcs_type == 'hg_git'
commits.concat(Gitlab::Git::Commit.hg_batch_by_node_id(raw_repository, oids))
end
if commits.present?
Commit.decorate(commits, container)
......
......@@ -21,7 +21,11 @@ def self.references_in(text, pattern = Commit.reference_pattern)
def find_object(project, id)
return unless project.is_a?(Project) && project.valid_repo?
_, record = records_per_parent[project].detect { |k, _v| Gitlab::Git.shas_eql?(k, id) }
is_hg = project.mercurial?
_, record = records_per_parent[project].detect do |k, v|
Gitlab::Git.shas_eql?(k, id) || (is_hg && v.hgid.start_with?(id))
end
record
end
......@@ -71,7 +75,7 @@ def object_link_text_extras(object, matches)
private
def parent_records(parent, ids)
parent.commits_by(oids: ids.to_a)
parent.commits_by(oids: ids.to_a, hg_full_resolution: true)
end
def noteable
......
......@@ -156,6 +156,11 @@ def batch_by_oid(repo, oids)
end
end
# Mercurial equivalent of batch_by_oid
def hg_batch_by_node_id(repo, node_ids)
batch_by_oid(repo, repo.batch_by_hgsha(node_ids))
end
def extract_signature_lazily(repository, commit_id)
BatchLoader.for(commit_id).batch(key: repository) do |commit_ids, loader, args|
batch_signature_extraction(args[:key], commit_ids).each do |commit_id, signature_data|
......
......@@ -290,6 +290,21 @@ def hgsha_from_sha(sha)
hg_sha_map[sha]
end
def batch_by_hgsha(sha_prefixes)
# TODO stupid algorithm. Above a certain size, it's more efficient
# to call hg with a the `sha_prefixes.join('+')` revset but then, we
# actually need a specific command so that ambiguous (too short)
# prefixes don't turn in hard errors that make just one ambiguous
# among many break the lookup of all.
res = []
git_sha_map.each_pair do |hgsha, gitsha|
sha_prefixes.each do |pref|
res << gitsha if hgsha.start_with?(pref)
end
end
res
end
def hg_git_invalidate_maps!
@git_sha_map = nil
@hg_sha_map = nil
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment