This is an archive of the discontinued Mercurial Phabricator instance.

shortest: cache disambiguation revset
ClosedPublic

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

Details

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
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.

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.

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.

This revision was automatically updated to reflect the committed changes.