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.

Download Raw Diff

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

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

martinvonz created this revision.Aug 1 2018, 5:57 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptAug 1 2018, 5:57 PM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

martinvonz added a child revision: D4040: shortest: make isrev() a top-level function.Aug 1 2018, 5:57 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.

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.

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

(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

			Path	Packages
M			mercurial/scmutil.py (4 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	9755		Aug 1 2018, 5:57 PM	★	★
Diff 2	9887		Aug 4 2018, 1:26 AM	★	★
Diff 3	9894	rHG3588e41f796dcbb28bb72faf565350ec259fdb73	Apr 17 2018, 2:16 PM	★	★

Commit	Parents	Author	Summary	Date
		Martin von Zweigbergk		Apr 17 2018, 2:16 PM

Status	Author	Revision
Closed	martinvonz	D4042 shortest: use 'x' prefix to disambiguate from revnum if configured
Closed	martinvonz	D4041 revisions: allow "x123" to refer to nodeid prefix "123"
Closed	martinvonz	D4040 shortest: make isrev() a top-level function
Closed	martinvonz	D4039 shortest: cache disambiguation revset
Closed	martinvonz	D4038 scmutil: make shortest() respect disambiguation revset
Closed	martinvonz	D4037 lookup: add option to disambiguate prefix within revset
Closed	martinvonz	D4036 revlog: use specialized exception for ambiguous prefix lookup

Diff 9755

mercurial/scmutil.py

	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 = repo.anyrevs([revset], user=True)			if not util.safehasattr(repo, '_cacheddisambiguationrevs'):
				repo._cacheddisambiguationrevs = repo.anyrevs([revset], user=True)
				revs = repo._cacheddisambiguationrevs
	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]: