This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4039

shortest: cache disambiguation revset
ClosedPublic

Authored by martinvonz on Aug 1 2018, 5:57 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG3588e41f796d: shortest: cache disambiguation revset
Summary

This makes it actually useful. In compared the time in my hg repo with
69.6k revisions and with a disambiguation revset of "not public()"
that matches 563 visible revisions. I ran "time hg log -T
'{shortest(node1,)}' -r 0:1000" (no revisions within the revset in
that revision range). Before this patch, it took 57s and after it took
0.7s.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Aug 1 2018, 5:57 PM
yuja added a subscriber: yuja.Aug 3 2018, 11:10 PM
This doesn't seem like the right way to cache it. Suggestions?

Suppose the cache exists mainly for templating, templatefuncs.shortest()
can pass in a cached revset to resolvehexnodeidprefix(), and we don't have
to take care of cache invalidation.

If we want to cache a prefix tree, maybe it could be turned into a "resolver"
object which the templater creates at the first shortest() call.

martinvonz added a comment.Aug 4 2018, 12:26 AM
In D4039#63460, @yuja wrote:
This doesn't seem like the right way to cache it. Suggestions?

Suppose the cache exists mainly for templating, templatefuncs.shortest()
can pass in a cached revset to resolvehexnodeidprefix(), and we don't have
to take care of cache invalidation.

Yep, it's pretty much only for templating, so that's a good suggestion. I'll start working on that. Thanks!

If we want to cache a prefix tree, maybe it could be turned into a "resolver"
object which the templater creates at the first shortest() call.

I have a stack of changes that make the nodetree struct a Python type that can be used for this. I might clean that up and send it another day. I'll try to follow your advice here then (I don't know what a resolver is yet, but I'll figure that out later).

martinvonz edited the summary of this revision. (Show Details)Aug 4 2018, 1:26 AM
martinvonz retitled this revision from [RFC] shortest: cache disambiguation revset to shortest: cache disambiguation revset.
martinvonz updated this revision to Diff 9887.
martinvonz added a comment.Aug 4 2018, 1:27 AM

I've added support for passing in a cache, so this is ready for review again, thanks.

yuja added a comment.Aug 4 2018, 2:42 AM

(I don't know what a resolver is yet, but I'll figure that out later).

Perhaps I mistook a function name. I just meant shortesthexnodeidprefix()
could be turned into a closure having node cache.

Closed by commit rHG3588e41f796d: shortest: cache disambiguation revset (authored by martinvonz). · Explain WhyAug 4 2018, 2:43 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/scmutil.py (16 lines)
Mmercurial/templatefuncs.py (5 lines)

Diff 9894

mercurial/scmutil.py

if len(matches) == 1: if len(matches) == 1:
return matches[0] return matches[0]
raise raise
if node is None: if node is None:
return return
repo.changelog.rev(node) # make sure node isn't filtered repo.changelog.rev(node) # make sure node isn't filtered
return node return node
def shortesthexnodeidprefix(repo, node, minlength=1): def shortesthexnodeidprefix(repo, node, minlength=1, cache=None):
"""Find the shortest unambiguous prefix that matches hexnode.""" """Find the shortest unambiguous prefix that matches hexnode.
If "cache" is not None, it must be a dictionary that can be used for
caching between calls to this method.
"""
# _partialmatch() of filtered changelog could take O(len(repo)) time, # _partialmatch() of filtered changelog could take O(len(repo)) time,
# which would be unacceptably slow. so we look for hash collision in # which would be unacceptably slow. so we look for hash collision in
# unfiltered space, which means some hashes may be slightly longer. # unfiltered space, which means some hashes may be slightly longer.
cl = repo.unfiltered().changelog cl = repo.unfiltered().changelog
def isrev(prefix): def isrev(prefix):
try: try:
i = int(prefix) i = int(prefix)
hexnode = hex(node) hexnode = hex(node)
for length in range(len(prefix), len(hexnode) + 1): for length in range(len(prefix), len(hexnode) + 1):
prefix = hexnode[:length] prefix = hexnode[:length]
if not isrev(prefix): if not isrev(prefix):
return prefix return prefix
revset = repo.ui.config('experimental', 'revisions.disambiguatewithin') revset = repo.ui.config('experimental', 'revisions.disambiguatewithin')
if revset: if revset:
revs = None
if cache is not None:
revs = cache.get('disambiguationrevset')
if revs is None:
revs = repo.anyrevs([revset], user=True) revs = repo.anyrevs([revset], user=True)
if cache is not None:
cache['disambiguationrevset'] = revs
if cl.rev(node) in revs: if cl.rev(node) in revs:
hexnode = hex(node) hexnode = hex(node)
for length in range(minlength, len(hexnode) + 1): for length in range(minlength, len(hexnode) + 1):
matches = [] matches = []
prefix = hexnode[:length] prefix = hexnode[:length]
for rev in revs: for rev in revs:
otherhexnode = repo[rev].hex() otherhexnode = repo[rev].hex()
if prefix == otherhexnode[:length]: if prefix == otherhexnode[:length]:

mercurial/templatefuncs.py

if not argstr: if not argstr:
continue continue
if first: if first:
first = False first = False
else: else:
yield sep yield sep
yield argstr yield argstr
@templatefunc('shortest(node, minlength=4)', requires={'repo'}) @templatefunc('shortest(node, minlength=4)', requires={'repo', 'cache'})
def shortest(context, mapping, args): def shortest(context, mapping, args):
"""Obtain the shortest representation of """Obtain the shortest representation of
a node.""" a node."""
if not (1 <= len(args) <= 2): if not (1 <= len(args) <= 2):
# i18n: "shortest" is a keyword # i18n: "shortest" is a keyword
raise error.ParseError(_("shortest() expects one or two arguments")) raise error.ParseError(_("shortest() expects one or two arguments"))
hexnode = evalstring(context, mapping, args[0]) hexnode = evalstring(context, mapping, args[0])
try: try:
node = scmutil.resolvehexnodeidprefix(repo, hexnode) node = scmutil.resolvehexnodeidprefix(repo, hexnode)
except error.WdirUnsupported: except error.WdirUnsupported:
node = wdirid node = wdirid
except error.LookupError: except error.LookupError:
return hexnode return hexnode
if not node: if not node:
return hexnode return hexnode
cache = context.resource(mapping, 'cache')
try: try:
return scmutil.shortesthexnodeidprefix(repo, node, minlength) return scmutil.shortesthexnodeidprefix(repo, node, minlength, cache)
except error.RepoLookupError: except error.RepoLookupError:
return hexnode return hexnode
@templatefunc('strip(text[, chars])') @templatefunc('strip(text[, chars])')
def strip(context, mapping, args): def strip(context, mapping, args):
"""Strip characters from a string. By default, """Strip characters from a string. By default,
strips all leading and trailing whitespace.""" strips all leading and trailing whitespace."""
if not (1 <= len(args) <= 2): if not (1 <= len(args) <= 2):